From 70bed02bc9d990b107a470c1e1c3b66ef665b90b Mon Sep 17 00:00:00 2001 From: yhirose Date: Thu, 22 Oct 2026 10:59:57 -0400 Subject: [PATCH] Bound how much of an unread request body the server skips After answering, with a status below 400, a request whose body the handler left unread, the server read past the body to reuse the connection up to the payload limit (100MB by default), with only each read bounded by the read timeout, so a slow sender could hold a worker for a long time. It now skips at most CPPHTTPLIB_UNREAD_PAYLOAD_MAX_LENGTH (256KB, as Go's net/http does) within the read timeout as a whole, and closes the connection otherwise. An error response already closed the connection. --- README.md | 2 +- httplib.h | 19 +++++++++++++++---- test/test.cc | 50 ++++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 66 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 8d9c001a..fcfcf379 100644 --- a/README.md +++ b/README.md @@ -936,7 +936,7 @@ svr.set_read_timeout(std::chrono::seconds(5)); svr.set_keep_alive_timeout(std::chrono::seconds(10)); ``` -The read timeout bounds the request line and headers as a whole, and each read of a request body. +The read timeout bounds the request line and headers as a whole, and each read of a request body. An error response closes the connection. Any other response to a request whose body was left unread, such as a redirect from a pre-routing handler, keeps the connection only when the rest of the body is at most `CPPHTTPLIB_UNREAD_PAYLOAD_MAX_LENGTH` (256KB, `0` for no limit) and arrives within the read timeout, since the server has to read past it first. ### Set maximum payload length for reading a request body diff --git a/httplib.h b/httplib.h index b78c9463..32e551bb 100644 --- a/httplib.h +++ b/httplib.h @@ -142,6 +142,12 @@ #define CPPHTTPLIB_PAYLOAD_MAX_LENGTH (100 * 1024 * 1024) // 100MB #endif +// How much of an unread request body the server skips to reuse the connection, +// 0 for no limit +#ifndef CPPHTTPLIB_UNREAD_PAYLOAD_MAX_LENGTH +#define CPPHTTPLIB_UNREAD_PAYLOAD_MAX_LENGTH (256 * 1024) // 256KB +#endif + #ifndef CPPHTTPLIB_SSE_MAX_EVENT_LENGTH #define CPPHTTPLIB_SSE_MAX_EVENT_LENGTH (16 * 1024 * 1024) // 16MB #endif @@ -15038,7 +15044,8 @@ Server::process_request(Stream &strm, const std::string &remote_addr, // The read timeout bounds the request line and headers as a whole, so a // client sending them a byte at a time cannot hold the worker any longer. - strm.set_max_timeout(read_timeout_sec_ * 1000 + read_timeout_usec_ / 1000); + auto read_timeout_msec = read_timeout_sec_ * 1000 + read_timeout_usec_ / 1000; + strm.set_max_timeout(read_timeout_msec); // Connection has been closed on client if (!line_reader.getline()) { return false; } @@ -15422,14 +15429,18 @@ Server::process_request(Stream &strm, const std::string &remote_addr, // keep-alive. Without framing there is no body to drain — reading would // consume the next request (issue #2450). If the response has committed the // connection to close, there is no next request to protect. A body read in - // part cannot be drained from where it stopped, so the connection closes. + // part cannot be drained from where it stopped, so the connection closes, + // as it does when the body is longer than the drain limit or does not + // arrive within the read timeout. if (!req.body_consumed_ && detail::has_framed_body(req) && !detail::has_header_token(res.headers, "Connection", "close")) { + strm.set_max_timeout(read_timeout_msec); int dummy_status; if (req.body_read_started_ || !detail::read_content( - strm, req, payload_max_length_, dummy_status, nullptr, - [](const char *, size_t, size_t, size_t) { return true; }, false)) { + strm, req, CPPHTTPLIB_UNREAD_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 d1d0fec2..78888d1e 100644 --- a/test/test.cc +++ b/test/test.cc @@ -29977,6 +29977,56 @@ TEST(RequestSmugglingTest, AbortedContentReaderClosesConnection) { } } +// An error response closes the connection anyway, so the body is left unread +// with a redirect, such as one to a login page. +static void serve_redirects(Server &svr, + const std::function &client) { + svr.set_pre_routing_handler([](const Request &, Response &res) { + res.set_redirect("/login"); + return Server::HandlerResponse::Handled; + }); + + 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(); + + client(port); +} + +TEST(UnreadBodyDrainTest, LongerThanTheLimitClosesTheConnection) { + Server svr; + serve_redirects(svr, [](int port) { + auto body = std::string(CPPHTTPLIB_UNREAD_PAYLOAD_MAX_LENGTH + 1, 'x'); + auto req = "POST / HTTP/1.1\r\nHost: localhost\r\nContent-Length: " + + std::to_string(body.size()) + "\r\n\r\n" + body + + "GET / HTTP/1.1\r\nHost: localhost\r\n\r\n"; + + std::string resp; + ASSERT_TRUE(send_request(1, req, &resp, port)); + ASSERT_EQ(0u, resp.find("HTTP/1.1 302")); + EXPECT_EQ(std::string::npos, resp.find("HTTP/1.1", 1)); + }); +} + +TEST(UnreadBodyDrainTest, SlowerThanTheReadTimeoutClosesTheConnection) { + Server svr; + svr.set_read_timeout(0, 200000); + serve_redirects(svr, [](int port) { + // A byte every 100 ms keeps each read within the read timeout, but not the + // body as a whole, so the server closes and the bytes sent after that fail. + std::vector parts{"POST / HTTP/1.1\r\nHost: localhost\r\n" + "Content-Length: 10\r\n\r\n"}; + parts.resize(11, "x"); + EXPECT_FALSE(send_request_in_parts(1, parts, nullptr, port)); + }); +} + TEST(RequestSmugglingTest, ContentLengthAndTransferEncodingRejected) { // RFC 9112 §6.3: A request with both Content-Length and Transfer-Encoding // must be rejected with 400 Bad Request.