From 1efd736d5ae911fe12eda502564c9d9c61ecd0e6 Mon Sep 17 00:00:00 2001 From: yhirose Date: Sat, 18 Jul 2026 16:50:14 -0400 Subject: [PATCH] Match chunked as the final transfer coding, order-independently is_chunked_transfer_encoding compared the whole Transfer-Encoding field value against "chunked", so a message whose final coding is chunked but which names another coding first ("gzip, chunked", valid under RFC 9112 6.1) was read as unframed. On a keep-alive server the body was then left in the socket and parsed as a smuggled request; open_stream repeated the check case-sensitively with a raw ==, desyncing the client stream the same way. Rework the helper to match the last coding token case-insensitively and route open_stream through it so both paths agree. The codings may also be split across multiple Transfer-Encoding lines (RFC 9110 5.3); since Headers is an unordered_multimap whose duplicate-key iteration order is not portable, the final coding of a multi-line field cannot be determined reliably, so treat any such message that names chunked as chunked (fail safe: a mis-parse only closes the connection, whereas the opposite error enables smuggling). A unit test covers the helper, including order-independent multi-line cases. Based on #2487 by @metsw24-max. --- httplib.h | 47 ++++++++++++++++++++++++++++++++++++++++++----- test/test.cc | 42 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 84 insertions(+), 5 deletions(-) diff --git a/httplib.h b/httplib.h index 7f9813e7f9..6063d77887 100644 --- a/httplib.h +++ b/httplib.h @@ -7386,8 +7386,46 @@ inline ReadContentResult read_content_chunked(Stream &strm, T &x, } inline bool is_chunked_transfer_encoding(const Headers &headers) { - return case_ignore::equal( - get_header_value(headers, "Transfer-Encoding", "", 0), "chunked"); + // 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". + // + // 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 is an unordered_multimap whose iteration order for duplicate keys + // is not portable, so when there is more than one Transfer-Encoding line we + // cannot tell which coding is truly final. In that ambiguous case we fail + // safe by treating the message as chunked (a mis-parse just closes the + // connection, whereas the opposite error enables smuggling). + auto rng = headers.equal_range("Transfer-Encoding"); + + size_t line_count = 0; + bool chunked_present = false; + bool last_line_ends_with_chunked = false; + + 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; + 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"); + } + + if (line_count == 0) { return false; } + if (line_count == 1) { return last_line_ends_with_chunked; } + return chunked_present; } template @@ -13226,9 +13264,8 @@ ClientImpl::open_stream(const std::string &method, const std::string &path, handle.body_reader_.content_length = content_length; } - auto transfer_encoding = - handle.response->get_header_value("Transfer-Encoding"); - handle.body_reader_.chunked = (transfer_encoding == "chunked"); + handle.body_reader_.chunked = + detail::is_chunked_transfer_encoding(handle.response->headers); auto content_encoding = handle.response->get_header_value("Content-Encoding"); if (!content_encoding.empty()) { diff --git a/test/test.cc b/test/test.cc index b1d4847622..0a718f314a 100644 --- a/test/test.cc +++ b/test/test.cc @@ -420,6 +420,48 @@ TEST(SanitizeFilenameTest, VariousPatterns) { EXPECT_EQ("", httplib::sanitize_filename(" ")); } +// Forward declaration (see the base64_encode note below): split builds move the +// definition into httplib.cc, so re-declaring it here lets the test link. +namespace httplib { +namespace detail { +bool is_chunked_transfer_encoding(const Headers &headers); +} // namespace detail +} // namespace httplib + +TEST(ChunkedTransferEncodingTest, DetectsChunkedAsFinalCoding) { + // Build a Headers with the given Transfer-Encoding lines (nullptr = none). + auto make = [](std::initializer_list lines) { + Headers h; + for (auto te : lines) { + if (te) { h.emplace("Transfer-Encoding", te); } + } + return h; + }; + + // Sole coding, matched case-insensitively. + EXPECT_TRUE(detail::is_chunked_transfer_encoding(make({"chunked"}))); + EXPECT_TRUE(detail::is_chunked_transfer_encoding(make({"Chunked"}))); + EXPECT_TRUE(detail::is_chunked_transfer_encoding(make({"CHUNKED"}))); + + // RFC 9112 6.1: chunked as the final coding of a list still frames the body. + EXPECT_TRUE(detail::is_chunked_transfer_encoding(make({"gzip, chunked"}))); + EXPECT_TRUE(detail::is_chunked_transfer_encoding(make({"gzip,chunked"}))); + EXPECT_TRUE(detail::is_chunked_transfer_encoding(make({"gzip, Chunked "}))); + + // Single line: chunked absent, or not the final coding -> not chunk-framed. + EXPECT_FALSE(detail::is_chunked_transfer_encoding(make({"gzip"}))); + EXPECT_FALSE(detail::is_chunked_transfer_encoding(make({"chunked, gzip"}))); + 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. + 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({"gzip", "deflate"}))); +} + // Forward declaration: in split builds split.py strips `inline` and moves the // definition into httplib.cc, so detail::base64_encode is not visible from the // public httplib.h. Re-declaring it here lets the tests link against the symbol