mirror of
https://github.com/yhirose/cpp-httplib.git
synced 2026-10-01 13:12:40 +07:00
Don't wait for a response to a request rejected before sending
process_request reads the response even after write_request fails, so that an early response (e.g. 413/414) sent while the body is still being uploaded is not lost. A request line or header rejected while the request is being built in memory never reaches the socket, though, so no response will come and the client blocked until the read timeout (or until the server closed the idle connection). write_request now reports such a local rejection, and process_request returns immediately in that case. Socket write failures still read the response as before.
This commit is contained in:
@@ -2986,7 +2986,7 @@ private:
|
||||
bool read_response_line(Stream &strm, const Request &req, Response &res,
|
||||
bool skip_100_continue = true) const;
|
||||
bool write_request(Stream &strm, Request &req, bool close_connection,
|
||||
Error &error, bool skip_body = false);
|
||||
Error &error, bool skip_body, bool &rejected_locally);
|
||||
bool write_request_body(Stream &strm, Request &req, Error &error);
|
||||
void prepare_default_headers(Request &r, bool for_stream,
|
||||
const std::string &ct);
|
||||
@@ -15681,7 +15681,9 @@ inline bool ClientImpl::write_content_with_provider(Stream &strm,
|
||||
|
||||
inline bool ClientImpl::write_request(Stream &strm, Request &req,
|
||||
bool close_connection, Error &error,
|
||||
bool skip_body) {
|
||||
bool skip_body, bool &rejected_locally) {
|
||||
rejected_locally = false;
|
||||
|
||||
// Prepare additional headers
|
||||
if (close_connection) {
|
||||
if (!req.has_header("Connection")) {
|
||||
@@ -15775,11 +15777,13 @@ inline bool ClientImpl::write_request(Stream &strm, Request &req,
|
||||
// set_path_encode(false)) must fail the request cleanly instead of
|
||||
// emitting a request-line-less, header-injecting request.
|
||||
error = Error::Write;
|
||||
rejected_locally = true;
|
||||
output_error_log(error, &req);
|
||||
return false;
|
||||
}
|
||||
if (!detail::check_and_write_headers(bstrm, req.headers, header_writer_,
|
||||
error)) {
|
||||
rejected_locally = true;
|
||||
output_error_log(error, &req);
|
||||
return false;
|
||||
}
|
||||
@@ -16048,8 +16052,16 @@ inline bool ClientImpl::process_request(Stream &strm, Request &req,
|
||||
detail::has_header_token(req.headers, "Expect", "100-continue");
|
||||
|
||||
// Send request (skip body if using Expect: 100-continue)
|
||||
auto rejected_locally = false;
|
||||
auto write_request_success =
|
||||
write_request(strm, req, close_connection, error, expect_100_continue);
|
||||
write_request(strm, req, close_connection, error, expect_100_continue,
|
||||
rejected_locally);
|
||||
|
||||
// A failed write normally still reads the response below, since the server
|
||||
// may have answered early (e.g. 413/414) and closed while the body was being
|
||||
// sent. A request rejected before any byte reached the socket gets no such
|
||||
// response, and waiting for one would block until the read timeout.
|
||||
if (rejected_locally) { return false; }
|
||||
|
||||
#ifdef CPPHTTPLIB_SSL_ENABLED
|
||||
if (is_ssl() && !expect_100_continue) {
|
||||
|
||||
+48
-3
@@ -10196,9 +10196,6 @@ TEST(RequestLineInjectionTest, ClientRejectsNonTokenMethodEndToEnd) {
|
||||
|
||||
{
|
||||
Client cli(HOST, port);
|
||||
// Nothing is written, so shorten the read timeout the connection would
|
||||
// otherwise sit in.
|
||||
cli.set_read_timeout(1, 0);
|
||||
|
||||
const std::string evil_methods[] = {
|
||||
"GET /smuggled HTTP/1.1\r\nHost: x\r\n\r\nGET",
|
||||
@@ -17415,6 +17412,54 @@ TEST(VulnerabilityTest, CRLFInjectionInHeaders) {
|
||||
server_thread.join();
|
||||
}
|
||||
|
||||
// A request rejected before any byte reaches the socket must fail right away
|
||||
// instead of waiting for a response the server will never send.
|
||||
TEST(ClientRejectedRequestTest, DoesNotWaitForResponse) {
|
||||
// The kernel completes the TCP handshake from the listen backlog, so the
|
||||
// client connects, but nothing ever reads, responds or closes.
|
||||
auto srv = ::socket(AF_INET, SOCK_STREAM, 0);
|
||||
default_socket_options(srv);
|
||||
|
||||
sockaddr_in addr{};
|
||||
addr.sin_family = AF_INET;
|
||||
addr.sin_port = htons(static_cast<uint16_t>(PORT + 1));
|
||||
::inet_pton(AF_INET, "127.0.0.1", &addr.sin_addr);
|
||||
ASSERT_EQ(0, ::bind(srv, reinterpret_cast<sockaddr *>(&addr), sizeof(addr)));
|
||||
ASSERT_EQ(0, ::listen(srv, 8));
|
||||
|
||||
auto cli = Client("127.0.0.1", PORT + 1);
|
||||
cli.set_read_timeout(10, 0);
|
||||
|
||||
auto elapsed_ms = [](std::chrono::steady_clock::time_point start) {
|
||||
return std::chrono::duration_cast<std::chrono::milliseconds>(
|
||||
std::chrono::steady_clock::now() - start)
|
||||
.count();
|
||||
};
|
||||
|
||||
{
|
||||
Request req;
|
||||
req.method = "GE T";
|
||||
req.path = "/";
|
||||
auto start = std::chrono::steady_clock::now();
|
||||
auto res = cli.send(req);
|
||||
EXPECT_FALSE(res);
|
||||
EXPECT_EQ(Error::Write, res.error());
|
||||
EXPECT_LT(elapsed_ms(start), 1000);
|
||||
}
|
||||
|
||||
{
|
||||
auto start = std::chrono::steady_clock::now();
|
||||
auto res = cli.Get("/", Headers{{"A", "B\r\nEvil: 1"}});
|
||||
EXPECT_FALSE(res);
|
||||
EXPECT_EQ(Error::InvalidHeaders, res.error());
|
||||
EXPECT_LT(elapsed_ms(start), 1000);
|
||||
}
|
||||
|
||||
EXPECT_FALSE(cli.is_socket_open());
|
||||
|
||||
detail::close_socket(srv);
|
||||
}
|
||||
|
||||
TEST(PathParamsTest, StaticMatch) {
|
||||
const auto pattern = "/users/all";
|
||||
detail::PathParamsMatcher matcher(pattern);
|
||||
|
||||
Reference in New Issue
Block a user