From f3e9a4d88742a5c332918f1be5203942aab70c50 Mon Sep 17 00:00:00 2001 From: Emre Ay Date: Mon, 20 Jul 2026 20:36:06 +0300 Subject: [PATCH] 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. --- httplib.h | 15 ++++++++++----- test/test.cc | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 5 deletions(-) diff --git a/httplib.h b/httplib.h index 261790bf..3f69d76d 100644 --- a/httplib.h +++ b/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; + } } } diff --git a/test/test.cc b/test/test.cc index e5dd95e5..f83bc452 100644 --- a/test/test.cc +++ b/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(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(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:, listen thread spawned by listen(),