From 40efd422e4a8e3c191c060b701bcf0e89bfc09f1 Mon Sep 17 00:00:00 2001 From: Ziya Kapoor Date: Mon, 2 Nov 2026 22:00:34 +0530 Subject: [PATCH] Drop Content-Length when HTTP1xCodec sends Transfer-Encoding: chunked 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. --- proxygen/lib/http/codec/HTTP1xCodec.cpp | 8 ++- .../lib/http/codec/test/HTTP1xCodecTest.cpp | 70 +++++++++++++++++++ .../test/HTTPDownstreamSessionTest.cpp | 23 +++--- 3 files changed, 91 insertions(+), 10 deletions(-) diff --git a/proxygen/lib/http/codec/HTTP1xCodec.cpp b/proxygen/lib/http/codec/HTTP1xCodec.cpp index 343ab52404..6a4bf5d771 100644 --- a/proxygen/lib/http/codec/HTTP1xCodec.cpp +++ b/proxygen/lib/http/codec/HTTP1xCodec.cpp @@ -766,7 +766,13 @@ void HTTP1xCodec::generateHeader( appendLiteral(writeBuf, len, "\r\n"); } - if (!deferredContentLength.empty()) { + // A message sent with Transfer-Encoding must not also carry Content-Length + // (RFC 9112 section 6.1): the body goes out chunked, so a peer framing it + // by Content-Length instead would read the chunk headers as body bytes and + // parse whatever follows as another message. + const bool sentTransferEncodingChunked = + hasTransferEncodingChunked && mayChunkEgress_; + if (!deferredContentLength.empty() && !sentTransferEncodingChunked) { appendLiteral(writeBuf, len, "Content-Length: "); appendString(writeBuf, len, deferredContentLength); appendString(writeBuf, len, CRLF); diff --git a/proxygen/lib/http/codec/test/HTTP1xCodecTest.cpp b/proxygen/lib/http/codec/test/HTTP1xCodecTest.cpp index acfb437560..f21e3bb5c3 100644 --- a/proxygen/lib/http/codec/test/HTTP1xCodecTest.cpp +++ b/proxygen/lib/http/codec/test/HTTP1xCodecTest.cpp @@ -810,6 +810,76 @@ TEST(HTTP1xCodecTest, TestBadTransferEncodingHeader) { "www.facebook.com"); } +// A request that arrives with both Transfer-Encoding: chunked and +// Content-Length is framed by the former on ingress and re-chunked on egress, +// so the Content-Length it carried must not go out with it (RFC 9112 section +// 6.1): a backend that framed the body by Content-Length instead would parse +// the tail of the chunked body as a second request. +TEST(HTTP1xCodecTest, TestChunkedRequestWithContentLengthNotForwarded) { + HTTP1xCodec downstream(TransportDirection::DOWNSTREAM); + HTTP1xCodecCallback callbacks; + downstream.setCallback(&callbacks); + + auto reqBuf = folly::IOBuf::copyBuffer( + "POST /upload HTTP/1.1\r\n" + "Host: www.facebook.com\r\n" + "Transfer-Encoding: chunked\r\n" + "Content-Length: 5\r\n" + "\r\n" + "5\r\nhello\r\n0\r\n\r\n"); + downstream.onIngress(*reqBuf); + EXPECT_EQ(callbacks.errors, 0); + EXPECT_EQ(callbacks.headersComplete, 1); + EXPECT_EQ(callbacks.messageComplete, 1); + EXPECT_EQ(callbacks.bodyLen, 5); + ASSERT_NE(callbacks.msg_, nullptr); + EXPECT_TRUE(callbacks.msg_->getIsChunked()); + + HTTP1xCodec upstream(TransportDirection::UPSTREAM); + folly::IOBufQueue writeBuf(folly::IOBufQueue::cacheChainLength()); + upstream.generateHeader(writeBuf, upstream.createStream(), *callbacks.msg_); + auto forwarded = writeBuf.move()->moveToFbString().toStdString(); + EXPECT_THAT(forwarded, HasSubstr("Transfer-Encoding: chunked\r\n")); + EXPECT_THAT(forwarded, Not(HasSubstr("Content-Length"))); +} + +// Same for a response relayed to an HTTP/1.1 client. +TEST(HTTP1xCodecTest, TestChunkedResponseWithContentLengthNotForwarded) { + HTTP1xCodec upstream(TransportDirection::UPSTREAM); + HTTP1xCodecCallback callbacks; + upstream.setCallback(&callbacks); + folly::IOBufQueue writeBuf(folly::IOBufQueue::cacheChainLength()); + upstream.generateHeader( + writeBuf, upstream.createStream(), getGetRequest("/foo")); + + auto respBuf = folly::IOBuf::copyBuffer( + "HTTP/1.1 200 OK\r\n" + "Transfer-Encoding: chunked\r\n" + "Content-Length: 5\r\n" + "\r\n" + "5\r\nhello\r\n0\r\n\r\n"); + upstream.onIngress(*respBuf); + EXPECT_EQ(callbacks.errors, 0); + EXPECT_EQ(callbacks.headersComplete, 1); + EXPECT_EQ(callbacks.messageComplete, 1); + EXPECT_EQ(callbacks.bodyLen, 5); + ASSERT_NE(callbacks.msg_, nullptr); + + // The client-facing codec has to have seen an HTTP/1.1 request to be allowed + // to send a chunked response. + HTTP1xCodec downstream(TransportDirection::DOWNSTREAM); + HTTP1xCodecCallback downstreamCallbacks; + downstream.setCallback(&downstreamCallbacks); + downstream.onIngress(*getSimpleRequestData()); + EXPECT_EQ(downstreamCallbacks.headersComplete, 1); + + writeBuf.move(); + downstream.generateHeader(writeBuf, 1, *callbacks.msg_); + auto relayed = writeBuf.move()->moveToFbString().toStdString(); + EXPECT_THAT(relayed, HasSubstr("Transfer-Encoding: chunked\r\n")); + EXPECT_THAT(relayed, Not(HasSubstr("Content-Length"))); +} + // url_ accumulates per message, so a rejection on a pipelined request must // report that request's URL rather than one carried over from its predecessor. TEST(HTTP1xCodecTest, TestPartialMsgBackfilledForPipelinedRequest) { diff --git a/proxygen/lib/http/session/test/HTTPDownstreamSessionTest.cpp b/proxygen/lib/http/session/test/HTTPDownstreamSessionTest.cpp index 1e9c20489c..b796b64705 100644 --- a/proxygen/lib/http/session/test/HTTPDownstreamSessionTest.cpp +++ b/proxygen/lib/http/session/test/HTTPDownstreamSessionTest.cpp @@ -3513,15 +3513,20 @@ TEST_F(HTTP2DownstreamSessionTest, Test304ContentLength) { // chunked with wrong content-length TEST_F(HTTPDownstreamSessionTest, HttpShortContentLength) { InSequence enforceOrder; - auto req = getPostRequest(10); - req.setIsChunked(true); - req.getHeaders().add(HTTP_HEADER_TRANSFER_ENCODING, "chunked"); - auto streamID = sendRequest(req, false); - clientCodec_->generateChunkHeader(requests_, streamID, 20); - clientCodec_->generateBody( - requests_, streamID, makeBuf(20), HTTPCodec::NoPadding, false); - clientCodec_->generateChunkTerminator(requests_, streamID); - clientCodec_->generateEOM(requests_, streamID); + // Written out by hand: HTTP1xCodec does not send Content-Length alongside + // Transfer-Encoding: chunked, so clientCodec_ cannot produce this request. + string req = folly::to( + "POST / HTTP/1.1\r\n" + "Host: www.foo.com\r\n" + "Content-Length: 10\r\n" + "Transfer-Encoding: chunked\r\n" + "\r\n" + "14\r\n", + string(20, 'a'), + "\r\n" + "0\r\n" + "\r\n"); + requests_.append(req.data(), req.size()); auto handler1 = addSimpleStrictHandler(); handler1->expectHeaders();