Summary: This is a violation of the spec. Reported by Apple
Test Plan: New unit test
Reviewed By: xiaosophiawang@fb.com
Subscribers: xiaosophiawang, noamler, bmatheny
FB internal diff: D2160494
Tasks: 7431234
Summary: fix is just to match on the non-specific part of the output
Test Plan: ran the test with SAN
Reviewed By: afrind@fb.com
Subscribers: bmatheny
FB internal diff: D2156375
Tasks: 7410379
Signature: t1:2156375:1434475204:6d1e1d92e1abffc5f2906b53f790785af0dc4b15
Summary:
- Write an HTTPCodecPrinter class that inherits from the filter, and move print*
methods from SPDYCodec to HTTPCodecPrinter
- Make the interface in HTTPCodecPrinter generic so that other HTTP variants
(HTTP/2) can use it. This includes creating callbacks onFrameHeader() and
onHeadersCompleteSPDYInfo() in HTTPCodec::Callback. The latter one is
specific to SPDY, I create it because I want it to handle onSynStream() and
onSynReply() without changing the interface of onHeadersComplete()
- Carefully remove the callsites of printer_.print* in SPDYCodec, and add callsites of
HTTPCodecPrinter callbacks, to make sure that they are equivalent.
- The printed messages are not exactly the same as before (e.g., when there is an
exception, error.what() is printed instead of spdy status code;
onSynStream/onSynReply info are printed by two callbacks instead of one). But
the printed info is the same.
- Haven't tested out the printer for HTTP/2, will attach another diff to
address that.
Test Plan:
- Capture a pcap from prospect using spdy31
- Convert packets to raw bytes in wireshark
- Run spdyprint to print out info for the raw bytes and make sure info is
printed out
Reviewed By: afrind@fb.com
Subscribers: trunkagent, bmatheny
FB internal diff: D2150844
Tasks: 3102118
Signature: t1:2150844:1434151970:e233c46e39a2e55138868525900238e3aecaadbc
Summary: useful in debugging; as a bonus I switched sprintf() to the safer snprintf()
Test Plan: compiled and ran it locally
Reviewed By: afrind@fb.com
Subscribers: trunkagent, cgheorghe, doug, bmatheny
FB internal diff: D2064032
Signature: t1:2064032:1434151759:2486da1840418c8e44e334da065a8f9eba6818ed
Summary:
SPDYCodec had spearate logic for this case which didn't get migrated to HTTP2Codec
Sending a duplicate HTTP/2 HEADERS frame will crash proxygen. This is a packet-of-death. At present I'm tempted not to hotfix unless we see a massive influx of crashes.
Test Plan: unit test
Reviewed By: xiaosophiawang@fb.com
Subscribers: bmatheny
FB internal diff: D2145681
Tasks: 7302126
Signature: t1:2145681:1433986710:86ee18465d7268eb27e557d85a3b83216df37152
Summary:
add arg options (-j and --jobs) to deps.sh to configure the number of
jobs for compilation
Test Plan:
tested under OSS linux server (fogvm001.07.ash3.facebook.com) to
make sure that
- the specified number N results in `make -jN`
- not specifying a number results in `make -j8`
- specifying other invalid arguments prints out the USAGE
Question: is there anyway to observe the number of threads being used in the
process of compilation?
Reviewed By: afrind@fb.com
Subscribers: bmatheny
FB internal diff: D2142643
Tasks: 6327334
Signature: t1:2142643:1433965490:e2d5d6b40463a30ca5de29dadb46f68f353ff760
Summary:
This reverts commit e10646ea4139b5dabcbbaa61b8e920e140d0c8ec.
It broke load tests. I'm not sure why but we're on a compressed schedule this week.
Test Plan: Load Tests
Reviewed By: viswanath@fb.com, noamler@fb.com
Subscribers: noamler, bmatheny
FB internal diff: D2136629
Tasks: 7347571
Summary: @afrind found out the decompression error in upstream SPDY is related to my zlib optimization. I revert this first and work on the fix in this week.
Test Plan: run all test cases
Reviewed By: afrind@fb.com
Subscribers: trunkagent, bmatheny, cgheorghe, afrind
FB internal diff: D2135598
Tasks: 7316261
Signature: t1:2135598:1433792802:2f752fb450d1bf8d5d71c7b846c4620eda2f178a
Blame Revision: D1852092
Summary:
Only GzipHeaderCodec allowed 128kb, and it turns out that HTTP1xCodec allows up to 80KB + 1 read buffer = 84KB. So when using spdy-hpack e2o, I was getting frame size errors.
Test Plan: Unit tests, e2o with spdy-hpack
Reviewed By: cgheorghe@fb.com
Subscribers: doug, bmatheny, cgheorghe
FB internal diff: D2133468
Signature: t1:2133468:1433550727:420ca8c0175d9cf0227fbf79620a6e661e09dc7d
Summary: At first I was pretty excited to see that 60% of HTTP/2 requests were sending 0 bytes of headers...
Test Plan: Will canary in latest
Reviewed By: noamler@fb.com
Subscribers: doug, bmatheny
FB internal diff: D2131180
Summary:
proxygen/facebook/httpserver/filter/StatsFilter expects the threadlocal stats_ variable to be initialized before any requests, and destroyed on the same thread.
There were two issues:
1) addObserver was being called after bind(), so it was possible to get some requests before the stats_ variable was initialized. Moved up the observer, should fix it (although I don't have a repro for this)
2) on server stop(), sometimes the observer onServerStop() callback was called, but the eventBase had already stopped. The IOThreadPoolExecutor eventually deleted the eventBase, causing consumeUntilDrained to be called, but in the main thread, and not in the eventBase's thread. I'm not as sure about this fix, hopefully @jsedgwick can chime in: I explicitly deleted the eventBase on the eventBase thread before join. Here's the stack for this issue:
P19866015
Test Plan:
fbconfig -r haystack/src2
~/fbcode/_bin/haystack/src2/photos_haystackd -workload=server -allow_root -base_path=/tmp -nthreads=10 -lvids=100 -port 8181
Repro from @shiva, although I never could get either to repro for me.
Reviewed By: shiva@fb.com
Subscribers: doug, fugalh, bmatheny, folly-diffs@, yfeldblum, chalfant, shiva, jsedgwick
FB internal diff: D2132208
Signature: t1:2132208:1433537787:4379ba50a908ca6f19924c2339876afaf663364c
Summary:
I broke this in D2132775, and for some reason it wasn't caught there. There's
still some work to get this project building with clang 3.6, so bandaid around
it for now.
Test Plan: fbconfig -r proxygen/facebook/httpclient && fbmake dbg
Reviewed By: seanc@fb.com
Subscribers: bmatheny, mathieubaudet
FB internal diff: D2134611
Tasks: 7335753
Signature: t1:2134611:1433712837:3fac1237cd3b3b78d3671e1bbcf0fc42e700d9bc
Summary:
Most of the changes are adding override keywords as appropriate, and
then a few selectively-placed pragmas for bigger issues I didn't want to deal
with right now.
Test Plan:
automated tests
Reviewed By: bill@fb.com
Subscribers: bill, yfeldblum, trunkagent, hphp-diffs@, marcelo, ckwalsh, mcduff, hitesh, mshneer, bmatheny, mathieubaudet
FB internal diff: D2132775
Tasks: 7321012
Signature: t1:2132775:1433554315:de92e39faca84167a742b62b16586976a93e1b6a
Summary: ERROR fires all the time and annoys people. Changing to INFO and investigating.
Test Plan: built the lib
Reviewed By: mzlee@fb.com
Subscribers: mzlee, bmatheny, seanc
FB internal diff: D2129081
Tasks: 5266254
Signature: t1:2129081:1433458253:ba1f57f6e523bb587ea74ca74980e0386aa7d7ee
Summary:
HTTP/2 padding bytes are flow controlled, but both the session and the stream flow control windows are maintained outside of the codec. So pass the number of flow controlled pad bytes to the callback, and have the window maintainers immediately add them to toAck (since the bytes have been discarded). Also (primarily for testing) allow padding to be passed to generateBody.
One drawback of the current approach is that we immediately free the padding and add to toAck_, but we cannot generate a window update until the next time an API with a writeBuf is called. In the pathological case of the test, we don't ack the padding until some read data gets passed to the handler.
Test Plan: New unit test
Reviewed By: xiaosophiawang@fb.com
Subscribers: doug, bmatheny
FB internal diff: D2121370
Tasks: 6902645
Signature: t1:2121370:1433354568:7f35841c0192cdb6f89d5a70a273205ab5add6ef
Summary:
* Added a new field and getter to HTTPSession.h to store the maximum number of transactions that session had in its history. Its value is calculated in createTransaction method in HTTPSession.cpp
Test Plan:
* fbmake runtests
Reviewed By: shikong@fb.com
Subscribers: trunkagent, #csti, bmatheny
FB internal diff: D2119468
Tasks: 6966442
Signature: t1:2119468:1433282789:ba093c71bbbd6b624d0dec716aa5fc624dfcaf82
Summary:
For zero delta WINDOW_UPDATE, do the following two things as per spec suggest (https://tools.ietf.org/html/draft-ietf-httpbis-http2-17#section-6.9)
- Generate a stream error of type PROTOCOL_ERROR
- Send an RST_STREAM
Test Plan:
unittests
- Make sure that a stream error of type PROTOCOL_ERROR is generated
Reviewed By: afrind@fb.com
Subscribers: bmatheny, moa
FB internal diff: D2104782
Tasks: 6049829
Signature: t1:2104782:1433282001:546a4c5b5fc37d1d31555e95d09f4482e221848f
Summary:
If a header was set multiple times into HTTPHeaders, and one or more of the values had zero length, then GzipHeaderCodec was emitting an illegal header block. The SPDY spec says that the value cannot start or end with a \0, and all intermediate values must have non-zero length.
It is legal to serialize a header with an empty value, however. So skip any duplicate headers with zero length, and only append the \0 if the first header had non-zero length, otherwise overwrite it.
Test Plan: New unit test
Reviewed By: viswanath@fb.com
Subscribers: viswanath, subodh, doug, bmatheny, cgheorghe, seanc, pgriess
FB internal diff: D2108278
Tasks: 7240576
Summary:
I noticed that for certain HTTPTransaction detected error conditions, the handler was only seeing detachTransaction and not onError
Test Plan: Unit tests
Reviewed By: noamler@fb.com
Subscribers: doug, bmatheny
FB internal diff: D2101350
Summary:
Create a streaming API for decoding http2 headers. Use a switch kHTTP2CodecUseStreaming to turn on/off (off by default).
- Created two sets of callbacks. One from HPACKCodec to HPACKDecoder, and the other from HTTP2Codec to HPACKCodec. This is useful for logging stats and errors in HPACKCodec.
- Left most legacy code there except 1) Changed interfaces in HPACKDecoder directly; 2) Changed HTTP2Codec::parseHeadersImpl directly. Legacy code should be removed later if other parts of the task are completed and if streaming API works ok.
- Moved HTTPRequestVerifier to a separate class because it is used in callbacks and is thus stored in HTTP2Codec::HeaderDecodeInfo.
Test Plan:
Passed the following unittest(s) when turning on and off streaming decoding
- proxygen/lib/http/codec/experimental/test/CodecTests
Reviewed By: afrind@fb.com
Subscribers: trunkagent, sammerat, bmatheny, cgheorghe, moa
FB internal diff: D2081287
Tasks: 6513634
Signature: t1:2081287:1432162140:144f99ca8cd652a1d2194820cdc9d9fd99ae2259
Summary:
Seems like we'll need this in a number of places :)
Note: Chrome 464748 was fixed in Chrome 43, so bump down the version number with the fix.
Test Plan: Unit tests
Reviewed By: noamler@fb.com
Subscribers: doug, bmatheny
FB internal diff: D2087921
Summary:
These errors could be caused by the application, and so are in some ways less serious than pure HPACK decode errors.
Test Plan: unit tests
Reviewed By: cgheorghe@fb.com
Subscribers: trunkagent, doug, bmatheny, cgheorghe
FB internal diff: D2058967
Signature: t1:2058967:1431137241:d84e43eda1f261686d6fc3f7965a44558d024d02
Summary:
We used to do a silly thing by mashaling all out
events for cert verification into json and then
unmarshalling them.
If json parsing failed for some reason we would
get inconsistent data.
This changes it so that we use the trace events
framework to flatten all the events related to
cert verification into the metadata, thus avoiding
the need for json parsing.
This also helps us get rid of the dep on jackson.
Test Plan: Unit tests
Reviewed By: mzlee@fb.com
Subscribers: trunkagent, bmatheny, ssl-diffs@, linshung, seanc
FB internal diff: D2042243
Tasks: 6943340
Summary: Pushed folly 0.39 to github, will be pushing proxygen now
Test Plan: Building on ubuntu VM
Reviewed By: noamler@fb.com
Subscribers: bmatheny
FB internal diff: D2088027
Signature: t1:2088027:1432145194:b9a2f82a5b7936e6aa9cc3c26d83c20a22df41f7
Summary:
We like to run some debug tiers at VLOG(4) but these are really hurting on event loop time. Lower the pri on some messages
^
Test Plan: Ran a canary and observed loop time back in the hundreds of usec
Reviewed By: praveenr@fb.com
Subscribers: doug, bmatheny, cgheorghe
FB internal diff: D2084510
Summary: PROTOCOL_ERROR was the default error for any downstream transaction sending an abort without a specific code. This makes it hard to separate out legitimate protocol errors coming from the codec vs a handler calling sendAbort. Make INTERNAL_ERROR the default code. The special case is ingress timeouts waiting for WINDOW_UPDATE seem like a legit protocol error, so reworked onIngressTimeout to handle this.
Test Plan: Unit tests
Reviewed By: praveenr@fb.com
Subscribers: doug, bmatheny
FB internal diff: D1940267
Signature: t1:1940267:1431391549:efbbadfe86b2f0ac563945f2420a777026292fac
Summary:
First cut at implementing the h2 priority tree.
For HTTP servers, we'll want to expose the priority info (via HTTPMessage?). It's unclear if we need/want to expose the entire tree.
The general plan within proxygen is to use priorities to determin which transactions can produce egress.
We need a common interface for the SPDY priority queue and H2 priority tree. The current spdy interface is just top(), and HTTPTransaction juggles priorities within a band to RR across txns.
I had previously thought about attempting to hint to the handler just how much egress a given transaction might be allowed to produce, multiplying the relative weight by the buffer limit. This might be an invasive API change.
Test Plan: Unit tests
Reviewed By: ranjeeth@fb.com
Subscribers: doug, bmatheny
FB internal diff: D2048426
Signature: t1:2048426:1431127832:1eb2e8b4cedc4191c6d48b4c168af2766d13da2c
Summary: I think this should get all but "unused variable" warnings for ScopeGuards.
Test Plan: unit tests
Reviewed By: noamler@fb.com
Subscribers: doug, bmatheny, cgheorghe
FB internal diff: D2053665
Tasks: 6676433
Signature: t1:2053665:1431024937:8bf535ceed8d81bcced1d158c7231a990c05a303
Summary: 1.6 is throwing gobs of warnings
Test Plan: Ran the tests on fogvm
Reviewed By: viswanath@fb.com
Subscribers: doug, bmatheny
FB internal diff: D2051227
Tasks: 6676433
Signature: t1:2051227:1430952264:eeff50ef4227f9d8025cc4be449346283cf69b5f
Summary:
Adding gzip response compression to proxygen HTTPServer:
1. Moved the ZlibStreamDecompressor out of private code.
2. Added a ZlibServerFilter to httpserver and associated tests.
The compression is off by default.
Test Plan:
1. Unit tests (in filter/tests and lib/utils/test)
2. Running the filter with EchoServer
3. Using address sanitizer
Reviewed By: woo@fb.com
Subscribers: trunkagent, bmatheny
FB internal diff: D1980318
Tasks: 5811867
Signature: t1:1980318:1430426916:6e36e1a0ed8fbd7768d231798294f7eecb76a441
Summary:
This is step 2 of removing folly::dynamic from our dependency to reduce the number of crashes with vague dump info. I refactored TraceEvent a bit:
- The internal data type is a boost::variant<int64_t, std::string>. All intergral types goes into int64_t and string types goes into std::string
- Accessing data could either through hasTraceField/getTraceFieldDataAs interface or through an iterator
- Flexible type conversion is provided via templated interface and the real conversion work is done by folly::to
Test Plan: All passed.
Reviewed By: shikong@fb.com
Subscribers: mzlee, bmatheny, seanc, yzhan, #csti
FB internal diff: D2033227
Signature: t1:2033227:1430776656:881e88c8443960ba83b6808f588d8574e5229215
Summary:
This time, as `awk` -- yeah, seriously. It's still pretty
succinct; I don't think going all the way out to python is necessarily
worth it here (especially given how annoying random python scripts can
be in fbcode). It also works on OS X, and is (very slightly) more
readable than what was there before IMO.
Fixes https://github.com/facebook/proxygen/issues/45
Test Plan:
`make hphp_proxygen` works on OS X and Debian 7, to build
proxygen as part of the HHVM build.
`fbconfig -r proxygen/lib/http proxygen/facebook/lib/http/ && fbmake dbg && fbmake runtests`
still works.
Reviewed By: noamler@fb.com
Subscribers: bmatheny
FB internal diff: D2041084
Tasks: 6929817
Signature: t1:2041084:1430764314:e4f4c3bc4eab9bec7d286c2be502c5881d44ef0e
Summary: latestActive_ should be updated properly, no matter infocallback is set or not.
Test Plan: compiling
Reviewed By: noamler@fb.com
Subscribers: bmatheny
FB internal diff: D2043079
Signature: t1:2043079:1430763298:7787ee70127173195a501355a21648b47e83ebc6
Summary:
Address another Android initialization crash by delaying
initialization of the header name structures until first use.
Test Plan: existing tests
Reviewed By: pgriess@fb.com
Subscribers: trunkagent, bmatheny, #csti
FB internal diff: D2033265
Tasks: 6915463
Signature: t1:2033265:1430407244:5f0ef3836a76f5c356ef3b4945b25d37086bd07d
