diff --git a/httplib.h b/httplib.h index c739ee7..b675b68 100644 --- a/httplib.h +++ b/httplib.h @@ -11311,29 +11311,18 @@ inline bool Server::read_content_core( size_t /*len*/) { return receiver(buf, n); }; } - // RFC 7230 Section 3.3.3: If this is a request message and none of the above - // are true (no Transfer-Encoding and no Content-Length), then the message - // body length is zero (no message body is present). - // - // For non-SSL builds, detect clients that send a body without a - // Content-Length header (raw HTTP over TCP). Check both the stream's - // internal read buffer (data already read from the socket during header - // parsing) and the socket itself for pending data. If data is found and - // exceeds the configured payload limit, reject with 413. - // For SSL builds we cannot reliably peek the decrypted application bytes, - // so keep the original behaviour. + // RFC 9112 §6: no Transfer-Encoding and no Content-Length means no body. + // For non-SSL builds we still scan non-persistent connections for stray + // body bytes so the payload limit is enforced (413). On keep-alive, + // pending bytes may be the next request (issue #2450), so skip. #if !defined(CPPHTTPLIB_SSL_ENABLED) if (!req.has_header("Content-Length") && !detail::is_chunked_transfer_encoding(req.headers)) { - // Only check if payload_max_length is set to a finite value - if (payload_max_length_ > 0 && + if (!detail::is_connection_persistent(req) && payload_max_length_ > 0 && payload_max_length_ < (std::numeric_limits::max)()) { - // Check if there is data already buffered in the stream (read during - // header parsing) or pending on the socket. Use a non-blocking socket - // check to avoid deadlock when the client sends no body. - bool has_data = strm.is_readable(); + auto has_data = strm.is_readable(); if (!has_data) { - socket_t s = strm.socket(); + auto s = strm.socket(); if (s != INVALID_SOCKET) { has_data = detail::select_read(s, 0, 0) > 0; } @@ -12193,15 +12182,14 @@ Server::process_request(Stream &strm, const std::string &remote_addr, ret = write_response(strm, close_connection, req, res); } - // Drain any unconsumed request body to prevent request smuggling on - // keep-alive connections. - if (!req.body_consumed_ && detail::expect_content(req)) { - int drain_status = 200; // required by read_content signature + // 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). + if (!req.body_consumed_ && detail::has_framed_body(req)) { + int dummy_status; if (!detail::read_content( - strm, req, payload_max_length_, drain_status, nullptr, + strm, req, payload_max_length_, dummy_status, nullptr, [](const char *, size_t, size_t, size_t) { return true; }, false)) { - // Body exceeds payload limit or read error — close the connection - // to prevent leftover bytes from being misinterpreted. connection_closed = true; } } diff --git a/test/test.cc b/test/test.cc index 3f1a4cd..f09bde9 100644 --- a/test/test.cc +++ b/test/test.cc @@ -18204,3 +18204,63 @@ TEST(RequestSmugglingTest, ContentLengthAndTransferEncodingRejected) { response.substr(0, response.find("\r\n"))); } } + +// Regression for issue #2450: a DELETE without Content-Length on a +// keep-alive connection must not let the post-response drain consume the +// next request's bytes. +TEST(KeepAliveTest, DeleteWithoutContentLengthDoesNotEatNextRequest) { + Server svr; + + std::atomic delete_count(0); + svr.Delete("/items/:id", [&](const Request &, Response &res) { + delete_count++; + res.status = StatusCode::NoContent_204; + }); + + 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=*/2, 0, + /*write_timeout_sec=*/2, 0, std::string(), error); + ASSERT_NE(INVALID_SOCKET, sock); + auto sock_se = detail::scope_exit([&] { detail::close_socket(sock); }); + + auto send_request_and_read_response = [&](const std::string &req, + std::string &out) -> bool { + auto sent = send(sock, req.data(), req.size(), 0); + if (sent != static_cast(req.size())) { return false; } + char buf[4096]; + for (;;) { + auto n = recv(sock, buf, sizeof(buf), 0); + if (n <= 0) { return !out.empty(); } + out.append(buf, static_cast(n)); + if (out.find("\r\n\r\n") != std::string::npos) { return true; } + } + }; + + std::string req1 = "DELETE /items/1 HTTP/1.1\r\n" + "Host: localhost\r\n" + "\r\n"; + std::string resp1; + ASSERT_TRUE(send_request_and_read_response(req1, resp1)); + EXPECT_NE(std::string::npos, resp1.find("HTTP/1.1 204")); + + std::string req2 = "DELETE /items/2 HTTP/1.1\r\n" + "Host: localhost\r\n" + "Connection: close\r\n" + "\r\n"; + std::string resp2; + ASSERT_TRUE(send_request_and_read_response(req2, resp2)); + EXPECT_NE(std::string::npos, resp2.find("HTTP/1.1 204")); + + EXPECT_EQ(2, delete_count.load()); +}