Repository navigation
Conversation
generateHeader appended the deferred Content-Length even after writing Transfer-Encoding: chunked, so a relayed message that arrived with both went out with both while generateBody re-chunked the body. A peer that frames by Content-Length then reads the chunk framing as body and parses the rest as another message. RFC 9112 section 6.1 forbids sending the two together. HttpShortContentLength built its TE+CL request through the client codec; write the raw bytes instead so it keeps covering the server-side check.
Contributor
|
@afrind has imported this pull request. If you are a Meta employee, you can view this in D129465359. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
HTTP1xCodec::generateHeader writes every header it is given and appends the deferred Content-Length last, so a message carrying both
Transfer-Encoding: chunkedandContent-Lengthgoes out with both while generateBody chunk-encodes the body. Ingress hands applications exactly such a message: http_parser frames TE+CL by the chunking, and HTTPTransaction only checks that the dechunked body adds up to the Content-Length. A relay that forwards it without calling stripPerHopHeaders (the bundled sample proxy is one) sends both headers upstream, and since egress re-chunks every body piece, the forwarded Content-Length no longer matches the bytes on the wire. A backend that frames by Content-Length (TE.CL desync, CWE-444) then parses the tail of the client's body as its own request. Example: 50 one-byte chunks followed by anfechunk holdingGET /admin HTTP/1.1 ..., sent withContent-Length: 304(the dechunked size). proxygen forwardsContent-Length: 304, the 304th forwarded body byte is the end offe\r\n, and the backend's next request isGET /admin. Responses relayed to a client take the same path.Skip the deferred Content-Length once
Transfer-Encoding: chunkedhas actually been written. RFC 9112 section 6.1 says a sender MUST NOT send Content-Length in a message that has Transfer-Encoding, and doing it in the serializer covers relays and application-built messages alike; the HTTP/1.0 dechunk path is unchanged because no Transfer-Encoding is written there. HttpShortContentLength built its TE+CL request through the client codec, so it now writes those bytes directly and keeps covering the server-side Content-Length/body mismatch check. The two new HTTP1xCodecTest cases fail before the change and pass after it, and the full ctest run (1820 tests) passes.