diff --git a/httplib.h b/httplib.h index 062059a3..80d90a12 100644 --- a/httplib.h +++ b/httplib.h @@ -7792,44 +7792,33 @@ inline ReadContentResult read_content_chunked(Stream &strm, T &x, inline bool is_chunked_transfer_encoding(const Headers &headers) { // RFC 9112 6.1: a message is framed with the chunked coding when "chunked" // is the final transfer coding. A single field value may list several - // codings ("gzip, chunked"), and the list may be split across multiple - // Transfer-Encoding header lines (RFC 9110 5.3). Match the last coding token - // case-insensitively rather than comparing the whole value against "chunked". + // codings ("gzip, chunked"), and RFC 9110 5.3 lets that list be split across + // several Transfer-Encoding lines, which combine into one comma-separated + // list in the order the lines were received. Headers preserves that order, + // so the final coding is the last token of the last line. Match it + // case-insensitively rather than comparing the whole value against + // "chunked". // // Security: reading a chunked message as unframed leaves its body in the // socket, where a keep-alive connection parses it as a smuggled request. - // Headers now preserves the order the lines were received in, so the final - // coding could be identified even when it is split across lines, but we - // keep failing safe and treat any message carrying more than one - // Transfer-Encoding line as chunked (a mis-parse just closes the - // connection, whereas the opposite error enables smuggling). + // Server::process_request() answers 400 and closes when the final coding is + // not chunked, so a request whose framing cannot be determined never + // reaches the "no body" path. auto rng = headers.equal_range("Transfer-Encoding"); + if (rng.first == rng.second) { return false; } - size_t line_count = 0; - bool chunked_present = false; - bool last_line_ends_with_chunked = false; + // Cleared per line, so a trailing line carrying no coding at all leaves the + // combined list ending in nothing rather than inheriting the line before it. + std::string last_coding; for (auto it = rng.first; it != rng.second; ++it) { - line_count++; const auto &value = it->second; - - std::string last_coding; - bool line_has_chunked = false; + last_coding.clear(); split(value.data(), value.data() + value.size(), ',', - [&](const char *b, const char *e) { - last_coding.assign(b, e); - if (case_ignore::equal(last_coding, "chunked")) { - line_has_chunked = true; - } - }); - - if (line_has_chunked) { chunked_present = true; } - last_line_ends_with_chunked = case_ignore::equal(last_coding, "chunked"); + [&](const char *b, const char *e) { last_coding.assign(b, e); }); } - if (line_count == 0) { return false; } - if (line_count == 1) { return last_line_ends_with_chunked; } - return chunked_present; + return case_ignore::equal(last_coding, "chunked"); } template diff --git a/test/test.cc b/test/test.cc index 52c8d722..c641f6c1 100644 --- a/test/test.cc +++ b/test/test.cc @@ -462,12 +462,23 @@ TEST(ChunkedTransferEncodingTest, DetectsChunkedAsFinalCoding) { EXPECT_FALSE(detail::is_chunked_transfer_encoding(make({""}))); EXPECT_FALSE(detail::is_chunked_transfer_encoding(make({nullptr}))); - // Multiple Transfer-Encoding lines: iteration order for duplicate keys is not - // portable, so any line naming chunked is treated as chunked (fail safe). - // The result must not depend on the order the lines were added. + // RFC 9110 5.3: multiple Transfer-Encoding lines combine, in the order they + // were received, into one list. Headers preserves that order, so the answer + // is decided by the last coding of the last line and the order the lines + // arrived in is significant. EXPECT_TRUE(detail::is_chunked_transfer_encoding(make({"gzip", "chunked"}))); - EXPECT_TRUE(detail::is_chunked_transfer_encoding(make({"chunked", "gzip"}))); + EXPECT_FALSE(detail::is_chunked_transfer_encoding(make({"chunked", "gzip"}))); EXPECT_FALSE(detail::is_chunked_transfer_encoding(make({"gzip", "deflate"}))); + + // The split can fall anywhere in the list. + EXPECT_TRUE( + detail::is_chunked_transfer_encoding(make({"deflate", "gzip, chunked"}))); + EXPECT_FALSE( + detail::is_chunked_transfer_encoding(make({"gzip, chunked", "deflate"}))); + + // A trailing line naming no coding leaves the list ending in nothing, so it + // must not inherit the chunked from the line before it. + EXPECT_FALSE(detail::is_chunked_transfer_encoding(make({"chunked", ""}))); } // Forward declaration: in split builds split.py strips `inline` and moves the @@ -20951,6 +20962,24 @@ TEST(RequestSmugglingTest, NonFinalChunkedTransferEncodingRejected) { << transfer_encoding; } + // RFC 9110 5.3: the codings may also be split across several + // Transfer-Encoding lines, which combine in the order they were received. + // "chunked" followed by "gzip" therefore ends in gzip and must be rejected + // just like the single-line "chunked, gzip" above. + { + auto req = "POST /test HTTP/1.1\r\n" + "Host: localhost\r\n" + "Transfer-Encoding: chunked\r\n" + "Transfer-Encoding: gzip\r\n" + "\r\n" + "0\r\n\r\n"; + + std::string response; + ASSERT_TRUE(send_request(1, req, &response)); + 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" @@ -20962,6 +20991,20 @@ TEST(RequestSmugglingTest, NonFinalChunkedTransferEncodingRejected) { std::string response; ASSERT_TRUE(send_request(1, req, &response)); EXPECT_EQ("HTTP/1.1 200 OK", response.substr(0, response.find("\r\n"))); + + // ...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"; + + 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"))); } // Regression for issue #2450: a DELETE without Content-Length on a