Determine the final transfer coding across Transfer-Encoding lines (#2522)

RFC 9110 5.3 lets a coding list be split across several Transfer-Encoding
lines, which combine, in the order the lines were received, into one
comma-separated list. RFC 9112 6.1 then frames the message as chunked
only when chunked is the final coding of that combined list.

is_chunked_transfer_encoding() could not apply that rule while Headers
was an unordered_multimap, since the order of the lines was not
recoverable, so it fell back to reporting any message naming chunked on
any line as chunked. Headers now preserves the order the lines were
received in, so read the final coding directly: the last token of the
last line.

The fallback erred toward reporting chunked because mis-reading a
chunked message as unframed leaves its body in the socket, where a
keep-alive connection parses it as a smuggled request. That direction is
no longer needed. A request whose combined list ends in something other
than chunked is now reported as not chunked, and process_request()
answers 400 and closes rather than letting it reach the "no body" path,
which is what it already did for the single-line "chunked, gzip" form.
So `Transfer-Encoding: chunked` followed by `Transfer-Encoding: gzip` is
rejected instead of being read as chunked, and `gzip` followed by
`chunked` is still accepted.

A trailing line carrying no coding at all now leaves the combined list
ending in nothing rather than inheriting the coding from the line
before it.

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 <typename T, typename U>

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