From fbd3e2cc9bdbf2a1a78f23f0456f8eebf215ba06 Mon Sep 17 00:00:00 2001 From: yhirose Date: Thu, 22 Oct 2026 07:58:28 -0400 Subject: [PATCH] Answer 501 to a request with a transfer coding besides chunked A request whose Transfer-Encoding ended in chunked but listed more (another coding before chunked, or chunked twice) was read with only the chunked framing removed, so the handler got a body still in the other coding. RFC 9112 6.1 lets a server answer 501 to a transfer coding it does not implement. The server now accepts only a single Transfer-Encoding field whose value is chunked, as Go and nginx do, and answers any other list ending in chunked with 501 and closes the connection. A list whose final coding is not chunked still gets 400. --- httplib.h | 10 ++++++++++ test/test.cc | 51 +++++++++++++++++++++++++++++---------------------- 2 files changed, 39 insertions(+), 22 deletions(-) diff --git a/httplib.h b/httplib.h index b23bfa0a..44e3d1a1 100644 --- a/httplib.h +++ b/httplib.h @@ -15135,6 +15135,16 @@ Server::process_request(Stream &strm, const std::string &remote_addr, return write_response(strm, close_connection, req, res); } + // RFC 9112 6.1: a lone "chunked" is the only transfer coding implemented + if (req.has_header("Transfer-Encoding") && + (req.get_header_value_count("Transfer-Encoding") != 1 || + !detail::case_ignore::equal(req.get_header_value("Transfer-Encoding"), + "chunked"))) { + connection_closed = true; + res.status = StatusCode::NotImplemented_501; + return write_response(strm, close_connection, req, res); + } + // RFC 9112 ยง6.1: a Transfer-Encoding alongside a Content-Length, or in an // HTTP/1.0 request, is honoured, but the connection closes after the // response. diff --git a/test/test.cc b/test/test.cc index 32c2089c..fb87a2f9 100644 --- a/test/test.cc +++ b/test/test.cc @@ -30108,32 +30108,39 @@ TEST(RequestSmugglingTest, NonFinalChunkedTransferEncodingRejected) { EXPECT_EQ("HTTP/1.1 400 Bad Request", response.substr(0, response.find("\r\n"))); } +} - // A sequence ending in chunked stays valid. - auto req = "POST /test HTTP/1.1\r\n" - "Host: localhost\r\n" - "Transfer-Encoding: gzip, chunked\r\n" - "Connection: close\r\n" - "\r\n" - "0\r\n\r\n"; +TEST(RequestSmugglingTest, TransferEncodingOtherThanChunkedNotImplemented) { + Server svr; + svr.Post("/test", [&](const Request &, Response &res) { + res.set_content("ok", "text/plain"); + }); - std::string response; - ASSERT_TRUE(send_request(1, req, &response)); - EXPECT_EQ("HTTP/1.1 200 OK", response.substr(0, response.find("\r\n"))); + auto port = svr.bind_to_any_port(HOST); + ASSERT_GT(port, 0); + thread t = thread([&] { svr.listen_after_bind(); }); + auto se = detail::scope_exit([&] { + svr.stop(); + t.join(); + ASSERT_FALSE(svr.is_running()); + }); + svr.wait_until_ready(); - // ...including when it is spread over several lines. - auto split_req = "POST /test HTTP/1.1\r\n" - "Host: localhost\r\n" - "Transfer-Encoding: gzip\r\n" - "Transfer-Encoding: chunked\r\n" - "Connection: close\r\n" - "\r\n" - "0\r\n\r\n"; + for (auto transfer_encoding : + {"Transfer-Encoding: gzip, chunked\r\n", + "Transfer-Encoding: gzip\r\nTransfer-Encoding: chunked\r\n", + "Transfer-Encoding: chunked\r\nTransfer-Encoding: chunked\r\n"}) { + auto req = std::string("POST /test HTTP/1.1\r\nHost: localhost\r\n") + + transfer_encoding + "\r\n0\r\n\r\n"; - std::string split_response; - ASSERT_TRUE(send_request(1, split_req, &split_response)); - EXPECT_EQ("HTTP/1.1 200 OK", - split_response.substr(0, split_response.find("\r\n"))); + std::string response; + ASSERT_TRUE(send_request(1, req, &response, port)) << transfer_encoding; + EXPECT_EQ("HTTP/1.1 501 Not Implemented", + response.substr(0, response.find("\r\n"))) + << transfer_encoding; + EXPECT_NE(std::string::npos, response.find("Connection: close")) + << transfer_encoding; + } } // Regression for issue #2450: a DELETE without Content-Length on a