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.

Author: yhirose

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.

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