Răsfoiți Sursa

Skip request body drain when connection will close

The post-response drain exists to keep unread framed body bytes from being parsed as a subsequent request on a persistent connection. Once response generation has committed the connection to close, there can be no subsequent request, so draining no longer provides that protection.

Continuing to drain is especially harmful when a ContentReader aborts an unterminated chunked upload: the server can wait indefinitely for the terminal chunk even after sending Connection: close. This delays the transport close that tells an in-flight uploader to stop and leaves a worker occupied consuming discarded data.

Use the finalized response Connection header as the single source of truth for whether to skip the drain. write_response_core already sets this header for keep-alive exhaustion, request-directed closure, handler-directed closure, and error responses. Marking connection_closed then terminates the keep-alive loop and closes the socket.

Add a raw-socket regression test whose ContentReader rejects the first chunk of an unterminated upload. The test verifies that the 409 response announces Connection: close and that the peer observes EOF rather than timing out while the server drains.
Emre Ay 2 săptămâni în urmă
părinte
comite
f3e9a4d887
2 a modificat fișierele cu 62 adăugiri și 5 ștergeri
  1. 10 5
      httplib.h
  2. 52 0
      test/test.cc

+ 10 - 5
httplib.h

@@ -12715,13 +12715,18 @@ Server::process_request(Stream &strm, const std::string &remote_addr,
 
   // Drain any unconsumed framed body to prevent request smuggling on
   // keep-alive. Without framing there is no body to drain — reading would
-  // consume the next request (issue #2450).
+  // consume the next request (issue #2450). If the response has committed the
+  // connection to close, there is no next request to protect.
   if (!req.body_consumed_ && detail::has_framed_body(req)) {
-    int dummy_status;
-    if (!detail::read_content(
-            strm, req, payload_max_length_, dummy_status, nullptr,
-            [](const char *, size_t, size_t, size_t) { return true; }, false)) {
+    if (res.get_header_value("Connection") == "close") {
       connection_closed = true;
+    } else {
+      int dummy_status;
+      if (!detail::read_content(
+              strm, req, payload_max_length_, dummy_status, nullptr,
+              [](const char *, size_t, size_t, size_t) { return true; }, false)) {
+        connection_closed = true;
+      }
     }
   }
 

+ 52 - 0
test/test.cc

@@ -19917,6 +19917,58 @@ TEST(KeepAliveTest, DeleteWithoutContentLengthDoesNotEatNextRequest) {
   EXPECT_EQ(2, delete_count.load());
 }
 
+TEST(KeepAliveTest, UnconsumedChunkedBodyIsNotDrainedWhenResponseCloses) {
+  Server svr;
+  svr.Post("/ingest", [&](const Request &, Response &res,
+                          const ContentReader &content_reader) {
+    auto consumed =
+        content_reader([](const char *, size_t) { return false; });
+    EXPECT_FALSE(consumed);
+    res.status = StatusCode::Conflict_409;
+    res.set_header("Connection", "close");
+  });
+
+  auto port = svr.bind_to_any_port(HOST);
+  thread t = thread([&] { svr.listen_after_bind(); });
+  auto se = detail::scope_exit([&] {
+    svr.stop();
+    t.join();
+  });
+  svr.wait_until_ready();
+
+  auto error = Error::Success;
+  auto sock = detail::create_client_socket(
+      HOST, "", port, AF_UNSPEC, false, false, nullptr,
+      /*connection_timeout_sec=*/2, 0,
+      /*read_timeout_sec=*/1, 0,
+      /*write_timeout_sec=*/2, 0, std::string(), error);
+  ASSERT_NE(INVALID_SOCKET, sock);
+  auto sock_se = detail::scope_exit([&] { detail::close_socket(sock); });
+
+  std::string request = "POST /ingest HTTP/1.1\r\n"
+                        "Host: localhost\r\n"
+                        "Transfer-Encoding: chunked\r\n"
+                        "\r\n"
+                        "4\r\n"
+                        "data\r\n";
+  auto sent = send(sock, request.data(), request.size(), 0);
+  ASSERT_EQ(static_cast<ssize_t>(request.size()), sent);
+
+  std::string response;
+  ssize_t received = 0;
+  do {
+    char buf[4096];
+    received = recv(sock, buf, sizeof(buf), 0);
+    if (received > 0) {
+      response.append(buf, static_cast<size_t>(received));
+    }
+  } while (received > 0);
+
+  EXPECT_NE(std::string::npos, response.find("HTTP/1.1 409 Conflict"));
+  EXPECT_NE(std::string::npos, response.find("Connection: close"));
+  EXPECT_EQ(0, received);
+}
+
 namespace no_proxy_test {
 
 // Server bound to 127.0.0.1:<dynamic>, listen thread spawned by listen(),