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.