mirror of
https://github.com/yhirose/cpp-httplib.git
synced 2026-10-22 11:46:13 +00:00
Fix keep-alive corruption on requests without framed body (#2450)
This commit is contained in:
1 parent
d755c43d58
commit
91271c062d
2 files changed
+73
-25
No files matched your search
@@ -11311,29 +11311,18 @@ inline bool Server::read_content_core(
|
|||||||
size_t /*len*/) { return receiver(buf, n); };
|
size_t /*len*/) { return receiver(buf, n); };
|
||||||
}
|
}
|
||||||
|
|
||||||
// RFC 7230 Section 3.3.3: If this is a request message and none of the above
|
// RFC 9112 §6: no Transfer-Encoding and no Content-Length means no body.
|
||||||
// are true (no Transfer-Encoding and no Content-Length), then the message
|
// For non-SSL builds we still scan non-persistent connections for stray
|
||||||
// body length is zero (no message body is present).
|
// body bytes so the payload limit is enforced (413). On keep-alive,
|
||||||
//
|
// pending bytes may be the next request (issue #2450), so skip.
|
||||||
// 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.
|
|
||||||
#if !defined(CPPHTTPLIB_SSL_ENABLED)
|
#if !defined(CPPHTTPLIB_SSL_ENABLED)
|
||||||
if (!req.has_header("Content-Length") &&
|
if (!req.has_header("Content-Length") &&
|
||||||
!detail::is_chunked_transfer_encoding(req.headers)) {
|
!detail::is_chunked_transfer_encoding(req.headers)) {
|
||||||
// Only check if payload_max_length is set to a finite value
|
if (!detail::is_connection_persistent(req) && payload_max_length_ > 0 &&
|
||||||
if (payload_max_length_ > 0 &&
|
|
||||||
payload_max_length_ < (std::numeric_limits<size_t>::max)()) {
|
payload_max_length_ < (std::numeric_limits<size_t>::max)()) {
|
||||||
// Check if there is data already buffered in the stream (read during
|
auto has_data = strm.is_readable();
|
||||||
// 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();
|
|
||||||
if (!has_data) {
|
if (!has_data) {
|
||||||
socket_t s = strm.socket();
|
auto s = strm.socket();
|
||||||
if (s != INVALID_SOCKET) {
|
if (s != INVALID_SOCKET) {
|
||||||
has_data = detail::select_read(s, 0, 0) > 0;
|
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);
|
ret = write_response(strm, close_connection, req, res);
|
||||||
}
|
}
|
||||||
|
|
||||||
// Drain any unconsumed request body to prevent request smuggling on
|
// Drain any unconsumed framed body to prevent request smuggling on
|
||||||
// keep-alive connections.
|
// keep-alive. Without framing there is no body to drain — reading would
|
||||||
if (!req.body_consumed_ && detail::expect_content(req)) {
|
// consume the next request (issue #2450).
|
||||||
int drain_status = 200; // required by read_content signature
|
if (!req.body_consumed_ && detail::has_framed_body(req)) {
|
||||||
|
int dummy_status;
|
||||||
if (!detail::read_content(
|
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)) {
|
[](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;
|
connection_closed = true;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -18204,3 +18204,63 @@ TEST(RequestSmugglingTest, ContentLengthAndTransferEncodingRejected) {
|
|||||||
response.substr(0, response.find("\r\n")));
|
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<int> 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<ssize_t>(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<size_t>(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());
|
||||||
|
}
|
||||||
Reference in new issue
Block a user