From fa21b7fa1613eb6b0481d06030a1ea717da4857d Mon Sep 17 00:00:00 2001 From: "Anton N. Petrov" Date: Tue, 1 Sep 2026 02:08:12 +0300 Subject: [PATCH 01/14] Add a request body size limit that closes over-limit connections Content-Length is checked at headers-complete, before 100 Continue. Chunked bodies are counted in on_body. Either way the client gets 413 and the connection closes; the handler does not run and leftover bytes are not parsed as the next request. The cap applies to 404, 405, and slash-redirects as well as matched routes. UINT64_MAX remains unlimited. Addresses the unbounded-body half of #1064. --- docs/guides/app.md | 19 ++ docs/guides/routes.md | 1 + include/crow/app.h | 25 +++ include/crow/common.h | 2 +- include/crow/http_connection.h | 43 ++++- include/crow/parser.h | 30 ++- include/crow/routing.h | 29 +++ tests/CMakeLists.txt | 1 + tests/max_body_size_tests.cpp | 344 +++++++++++++++++++++++++++++++++ 9 files changed, 485 insertions(+), 9 deletions(-) create mode 100644 tests/max_body_size_tests.cpp diff --git a/docs/guides/app.md b/docs/guides/app.md index daa17217a2..654137da42 100644 --- a/docs/guides/app.md +++ b/docs/guides/app.md @@ -52,6 +52,25 @@ app.tcp_nodelay(true) .run(); ``` +## Request body size +[:octicons-feed-tag-16: master](https://github.com/CrowCpp/Crow) + +By default Crow accepts a request body of any size. `#!cpp app.max_body_size(bytes)` sets an app-wide limit (`UINT64_MAX` is unlimited). A route can override it with `#!cpp .max_body_size(bytes)`. + +The advertised `Content-Length` is checked when headers complete, before `100 Continue`. Chunked bodies are counted as they arrive. An over-limit request is answered `413 Payload Too Large` and the connection is closed; the body is not read or drained, and the route handler does not run. The same cap applies to 404, 405, and slash-redirects. + +```cpp +crow::SimpleApp app; +app.max_body_size(64ull * 1024 * 1024); + +CROW_ROUTE(app, "/upload") + .methods(crow::HTTPMethod::Post) + .max_body_size(8ull * 1024 * 1024) + ([](const crow::request& req) { + return crow::response(200); + }); +``` +

For more info on middlewares, check out [this page](middleware.md).

diff --git a/docs/guides/routes.md b/docs/guides/routes.md index 4552905fac..781cfc6c4f 100644 --- a/docs/guides/routes.md +++ b/docs/guides/routes.md @@ -82,6 +82,7 @@ Handlers can also use information from the request by adding it as a parameter ` You can also access the URL parameters in the handler using `#!cpp req.url_params.get("param_name");`. If the parameter doesn't exist, `nullptr` is returned.

+To cap the request body for one route, use `#!cpp .max_body_size(bytes)` (see [Request body size](app.md#request-body-size)). Without a per-route value, `#!cpp app.max_body_size()` applies, including on 404 and 405. !!! note "Note      [:octicons-feed-tag-16: master](https://github.com/CrowCpp/Crow)" diff --git a/include/crow/app.h b/include/crow/app.h index 7aa60522db..d21cd800a3 100644 --- a/include/crow/app.h +++ b/include/crow/app.h @@ -349,6 +349,30 @@ namespace crow return max_payload_; } + /// \brief Set the maximum request body size in bytes (`UINT64_MAX` = unlimited, the default). + /// + /// Applied to every request, including 404, 405, and slash-redirects. + /// Advertised `Content-Length` is checked at headers-complete, before `100 Continue`. + /// Chunked bodies are counted as they arrive. An over-limit request is answered 413 + /// and the connection is closed without reading (or draining) the body. + self_t& max_body_size(uint64_t bytes) + { + max_body_size_ = bytes; + return *this; + } + + /// \brief Get the app-wide request body size limit + uint64_t max_body_size() const + { + return max_body_size_; + } + + /// \brief Effective body size limit for a routing result (used by the connection). + uint64_t effective_max_body_size(const routing_handle_result& found) const + { + return router_.effective_max_body_size(found, max_body_size_); + } + self_t& signal_clear() { signals_.clear(); @@ -914,6 +938,7 @@ namespace crow unsigned int concurrency_ = 2; std::atomic_bool is_bound_ = false; uint64_t max_payload_{UINT64_MAX}; + uint64_t max_body_size_{UINT64_MAX}; std::string server_name_ = std::string("Crow/") + VERSION; std::string bindaddr_ = "0.0.0.0"; bool use_unix_ = false; diff --git a/include/crow/common.h b/include/crow/common.h index a8e58f4abc..facd98dd12 100644 --- a/include/crow/common.h +++ b/include/crow/common.h @@ -291,7 +291,7 @@ namespace crow size_t rule_index; std::vector blueprint_indices; routing_params r_params; - HTTPMethod method; + HTTPMethod method{}; routing_handle_result() {} diff --git a/include/crow/http_connection.h b/include/crow/http_connection.h index 2ebf72a956..045201ec4a 100644 --- a/include/crow/http_connection.h +++ b/include/crow/http_connection.h @@ -121,8 +121,24 @@ namespace crow } } - void handle_header() + /// Apply `max_body_size` before 100-continue. Return 1 to skip the body (F_SKIPBODY). + int handle_header() { + payload_too_large_ = false; + const uint64_t limit = handler_->effective_max_body_size(*routing_handle_result_); + parser_.set_max_body_size(limit); + + if (parser_.content_length != CROW_ULLONG_MAX) + { + if (limit != UINT64_MAX && parser_.content_length > limit) + { + payload_too_large_ = true; + return 1; + } + if (parser_.content_length <= req_.body.max_size()) + req_.body.reserve(static_cast(parser_.content_length)); + } + // HTTP 1.1 Expect: 100-continue if (req_.http_ver_major == 1 && req_.http_ver_minor == 1 && get_header_value(req_.headers, "expect") == "100-continue") { @@ -142,6 +158,18 @@ namespace crow need_to_call_after_handlers_ = true; complete_request(); } + return 0; + } + + void reject_payload_too_large() + { + payload_too_large_ = true; + handle(); + } + + bool parser_should_abort() const + { + return payload_too_large_; } void handle() @@ -160,7 +188,15 @@ namespace crow add_keep_alive_ = req_.keep_alive; close_connection_ = req_.close_connection; - if (req_.check_version(1, 1)) // HTTP/1.1 + if (payload_too_large_) + { + res = response(status::PAYLOAD_TOO_LARGE); + res.set_header("Connection", "close"); + res.end(); + close_connection_ = true; + add_keep_alive_ = false; + } + else if (req_.check_version(1, 1)) // HTTP/1.1 { if (!req_.headers.count("host")) { @@ -214,6 +250,8 @@ namespace crow } else { + if (payload_too_large_) + need_to_call_after_handlers_ = true; complete_request(); } } @@ -536,6 +574,7 @@ namespace crow bool need_to_call_after_handlers_{}; bool need_to_start_read_after_complete_{}; bool add_keep_alive_{}; + bool payload_too_large_{false}; std::tuple* middlewares_; detail::context ctx_; diff --git a/include/crow/parser.h b/include/crow/parser.h index 1417d6cb60..acf253d634 100644 --- a/include/crow/parser.h +++ b/include/crow/parser.h @@ -1,8 +1,9 @@ #pragma once +#include +#include #include #include -#include #include "crow/http_request.h" #include "crow/http_parser_merged.h" @@ -82,13 +83,20 @@ namespace crow self->set_connection_parameters(); - self->process_header(); - return 0; + return self->process_header(); } static int on_body(http_parser* self_, const char* at, size_t length) { HTTPParser* self = static_cast(self_); + if (self->max_body_size_ != UINT64_MAX && + (length > self->max_body_size_ || + self->body_bytes_ > self->max_body_size_ - length)) + { + self->handler_->reject_payload_too_large(); + return 1; + } self->req.body.insert(self->req.body.end(), at, at + length); + self->body_bytes_ += length; return 0; } static int on_message_complete(http_parser* self_) @@ -97,7 +105,8 @@ namespace crow self->message_complete = true; self->process_message(); - return 0; + // Stop so leftover skipped-body bytes are not parsed as the next request. + return self->handler_->parser_should_abort() ? 1 : 0; } HTTPParser(Handler* handler): http_parser(), @@ -145,17 +154,24 @@ namespace crow header_building_state = 0; qs_point = 0; message_complete = false; + body_bytes_ = 0; + max_body_size_ = UINT64_MAX; state = CROW_NEW_MESSAGE(); } + void set_max_body_size(uint64_t bytes) + { + max_body_size_ = bytes; + } + inline void process_url() { handler_->handle_url(); } - inline void process_header() + inline int process_header() { - handler_->handle_header(); + return handler_->handle_header(); } inline void process_message() @@ -190,6 +206,8 @@ namespace crow private: int header_building_state = 0; bool message_complete = false; + uint64_t body_bytes_{0}; + uint64_t max_body_size_{UINT64_MAX}; std::string header_field; std::string header_value; diff --git a/include/crow/routing.h b/include/crow/routing.h index 42ac0a78f9..770ac22596 100644 --- a/include/crow/routing.h +++ b/include/crow/routing.h @@ -160,6 +160,8 @@ namespace crow // NOTE: Already documented in "crow/app.h" std::string rule_; std::string name_; bool added_{false}; + uint64_t max_body_size_{UINT64_MAX}; + bool max_body_size_override_{false}; std::unique_ptr rule_to_upgrade_; @@ -618,6 +620,14 @@ namespace crow // NOTE: Already documented in "crow/app.h" static_cast(this)->mw_indices_.template push(); return static_cast(*this); } + + /// Override the app-wide request body size limit for this route. + self_t& max_body_size(uint64_t bytes) + { + static_cast(this)->max_body_size_ = bytes; + static_cast(this)->max_body_size_override_ = true; + return static_cast(*this); + } }; /// A rule that can change its parameters during runtime. @@ -1842,6 +1852,25 @@ namespace crow // NOTE: Already documented in "crow/app.h" return blueprints_; } + /// Effective request body limit for a `handle_initial()` result. + /// + /// 404, 405, slash-redirect, and unmatched OPTIONS use `app_default`. + /// A matched rule uses its override when set, otherwise `app_default`. + uint64_t effective_max_body_size(const routing_handle_result& found, uint64_t app_default) const + { + if (found.catch_all || found.rule_index <= RULE_SPECIAL_REDIRECT_SLASH) + return app_default; + if (found.method >= HTTPMethod::InternalMethodCount) + return app_default; + const auto& rules = per_methods_[static_cast(found.method)].rules; + if (found.rule_index >= rules.size()) + return app_default; + const BaseRule* rule = rules[found.rule_index]; + if (!rule || !rule->max_body_size_override_) + return app_default; + return rule->max_body_size_; + } + std::function& exception_handler() { return exception_handler_; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 00db73a7be..6773e80e74 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -22,6 +22,7 @@ enable_testing() # list the test sources set(TEST_SRCS unittest.cpp + max_body_size_tests.cpp query_string_tests.cpp unit_tests/test_http_response.cpp unit_tests/test_json.cpp diff --git a/tests/max_body_size_tests.cpp b/tests/max_body_size_tests.cpp new file mode 100644 index 0000000000..46b5429f81 --- /dev/null +++ b/tests/max_body_size_tests.cpp @@ -0,0 +1,344 @@ +#define CROW_ENABLE_DEBUG +#define CROW_LOG_LEVEL 0 + +#include +#include +#include +#include +#include +#include + +#include "catch2/catch_all.hpp" +#include "crow.h" +#include "crow/middlewares/cors.h" + +using namespace crow; + +#ifdef CROW_USE_BOOST +namespace asio = boost::asio; +using asio_error_code = boost::system::error_code; +#else +using asio_error_code = asio::error_code; +#endif + +#define LOCALHOST_ADDRESS "127.0.0.1" + +namespace +{ + bool response_complete(const std::string& data) + { + auto search_from = std::size_t{0}; + while (true) + { + const auto status_line = data.find("HTTP/1.1 ", search_from); + if (status_line == std::string::npos) + return false; + const auto code = std::atoi(data.c_str() + status_line + 9); + const auto header_end = data.find("\r\n\r\n", status_line); + if (header_end == std::string::npos) + return false; + if (code >= 100 && code < 200) + { + search_from = header_end + 4; + continue; + } + const auto length_pos = data.find("Content-Length:", status_line); + if (length_pos == std::string::npos || length_pos > header_end) + return true; + const auto length = static_cast(std::stoul(data.substr(length_pos + 15))); + return data.size() >= header_end + 4 + length; + } + } + + class TestClient + { + public: + TestClient(uint16_t port): + socket_(io_context_) + { + socket_.connect(asio::ip::tcp::endpoint(asio::ip::make_address(LOCALHOST_ADDRESS), port)); + } + + void send(const std::string& data) + { + asio::write(socket_, asio::buffer(data)); + } + + std::string receive() + { + std::string response; + std::array buffer{}; + const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(5); + while (!response_complete(response)) + { + REQUIRE(std::chrono::steady_clock::now() < deadline); + asio_error_code ec; + const auto n = socket_.read_some(asio::buffer(buffer), ec); + if (ec) + break; + response.append(buffer.data(), n); + } + return response; + } + + private: + asio::io_context io_context_{}; + asio::ip::tcp::socket socket_; + }; + + int status_of(const std::string& response) + { + auto search_from = std::size_t{0}; + while (true) + { + const auto status_line = response.find("HTTP/1.1 ", search_from); + if (status_line == std::string::npos) + return 0; + const auto code = std::atoi(response.c_str() + status_line + 9); + if (code >= 100 && code < 200) + { + search_from = status_line + 9; + continue; + } + return code; + } + } +} // namespace + +TEST_CASE("max_body_size advertised length", "[http][max_body_size]") +{ + std::atomic handler_ran{false}; + SimpleApp app; + app.max_body_size(8); + + CROW_ROUTE(app, "/upload") + .methods("POST"_method)([&handler_ran](const request& req) { + handler_ran = true; + return req.body; + }); + + auto server = app.bindaddr(LOCALHOST_ADDRESS).port(0).run_async(); + app.wait_for_server_start(); + const auto port = app.port(); + + auto post = [&](const std::string& target, const std::string& payload, + const std::string& extra = {}) { + TestClient client(port); + client.send( + "POST " + target + + " HTTP/1.1\r\n" + "Host: localhost\r\n" + + extra + + "Content-Length: " + std::to_string(payload.size()) + + "\r\n" + "\r\n" + + payload); + return client.receive(); + }; + + SECTION("body of exactly max is accepted") + { + const auto resp = post("/upload", "12345678"); + CHECK(status_of(resp) == 200); + CHECK(resp.find("12345678") != std::string::npos); + CHECK(handler_ran); + } + + SECTION("Content-Length above the cap is 413 at headers and does not run the handler") + { + handler_ran = false; + const auto resp = post("/upload", "123456789"); + CHECK(status_of(resp) == 413); + CHECK(resp.find("Connection: close") != std::string::npos); + CHECK_FALSE(handler_ran); + } + + SECTION("Expect 100-continue is not sent for an over-limit body") + { + handler_ran = false; + TestClient client(port); + client.send( + "POST /upload HTTP/1.1\r\n" + "Host: localhost\r\n" + "Expect: 100-continue\r\n" + "Content-Length: 5000000\r\n" + "\r\n"); + const auto resp = client.receive(); + CHECK(resp.find("100 Continue") == std::string::npos); + CHECK(status_of(resp) == 413); + CHECK_FALSE(handler_ran); + } + + SECTION("unmatched URL still applies the app limit") + { + handler_ran = false; + const auto resp = post("/missing", std::string(100, 'x')); + CHECK(status_of(resp) == 413); + CHECK_FALSE(handler_ran); + } + + SECTION("wrong method still applies the app limit") + { + handler_ran = false; + TestClient client(port); + client.send( + "PUT /upload HTTP/1.1\r\n" + "Host: localhost\r\n" + "Content-Length: 100\r\n" + "\r\n" + + std::string(100, 'x')); + const auto resp = client.receive(); + CHECK(status_of(resp) == 413); + CHECK_FALSE(handler_ran); + } + + app.stop(); +} + +TEST_CASE("max_body_size slash redirect", "[http][max_body_size]") +{ + std::atomic handler_ran{false}; + SimpleApp app; + app.max_body_size(8); + + CROW_ROUTE(app, "/probe/") + .methods("POST"_method)([&handler_ran] { + handler_ran = true; + return "ok"; + }); + + auto server = app.bindaddr(LOCALHOST_ADDRESS).port(0).run_async(); + app.wait_for_server_start(); + + TestClient client(app.port()); + client.send( + "POST /probe HTTP/1.1\r\n" + "Host: localhost\r\n" + "Content-Length: 100\r\n" + "\r\n" + + std::string(100, 'x')); + const auto resp = client.receive(); + CHECK(status_of(resp) == 413); + CHECK_FALSE(handler_ran); + + app.stop(); +} + +TEST_CASE("max_body_size chunked accumulate", "[http][max_body_size]") +{ + std::atomic handler_ran{false}; + SimpleApp app; + app.max_body_size(8); + + CROW_ROUTE(app, "/upload") + .methods("POST"_method)([&handler_ran](const request&) { + handler_ran = true; + return "ok"; + }); + + auto server = app.bindaddr(LOCALHOST_ADDRESS).port(0).run_async(); + app.wait_for_server_start(); + + TestClient client(app.port()); + client.send( + "POST /upload HTTP/1.1\r\n" + "Host: localhost\r\n" + "Transfer-Encoding: chunked\r\n" + "\r\n" + "5\r\nhello\r\n" + "5\r\nworld\r\n" + "0\r\n" + "\r\n"); + const auto resp = client.receive(); + CHECK(status_of(resp) == 413); + CHECK_FALSE(handler_ran); + + app.stop(); +} + +TEST_CASE("max_body_size per-route override", "[http][max_body_size]") +{ + SimpleApp app; + app.max_body_size(8); + + CROW_ROUTE(app, "/small") + .methods("POST"_method)([](const request& req) { + return req.body; + }); + CROW_ROUTE(app, "/large") + .methods("POST"_method) + .max_body_size(32)([](const request& req) { + return req.body; + }); + + auto server = app.bindaddr(LOCALHOST_ADDRESS).port(0).run_async(); + app.wait_for_server_start(); + const auto port = app.port(); + + const std::string payload(16, 'A'); + { + TestClient client(port); + client.send( + "POST /small HTTP/1.1\r\nHost: localhost\r\nContent-Length: 16\r\n\r\n" + payload); + CHECK(status_of(client.receive()) == 413); + } + { + TestClient client(port); + client.send( + "POST /large HTTP/1.1\r\nHost: localhost\r\nContent-Length: 16\r\n\r\n" + payload); + CHECK(status_of(client.receive()) == 200); + } + + app.stop(); +} + +TEST_CASE("max_body_size default unlimited", "[http][max_body_size]") +{ + SimpleApp app; + CROW_ROUTE(app, "/upload") + .methods("POST"_method)([](const request& req) { + return std::to_string(req.body.size()); + }); + + auto server = app.bindaddr(LOCALHOST_ADDRESS).port(0).run_async(); + app.wait_for_server_start(); + + const std::string payload(64 * 1024, 'B'); + TestClient client(app.port()); + client.send( + "POST /upload HTTP/1.1\r\nHost: localhost\r\nContent-Length: " + + std::to_string(payload.size()) + "\r\n\r\n" + payload); + const auto resp = client.receive(); + CHECK(status_of(resp) == 200); + CHECK(resp.find("65536") != std::string::npos); + + app.stop(); +} + +TEST_CASE("max_body_size 413 runs after-handlers", "[http][max_body_size]") +{ + App app; + app.max_body_size(8); + + CROW_ROUTE(app, "/upload") + .methods("POST"_method)([] { + return "ok"; + }); + + auto server = app.bindaddr(LOCALHOST_ADDRESS).port(0).run_async(); + app.wait_for_server_start(); + + TestClient client(app.port()); + client.send( + "POST /upload HTTP/1.1\r\n" + "Host: localhost\r\n" + "Origin: https://example.test\r\n" + "Content-Length: 100\r\n" + "\r\n" + + std::string(100, 'x')); + const auto resp = client.receive(); + CHECK(status_of(resp) == 413); + CHECK(resp.find("Access-Control-Allow-Origin: *") != std::string::npos); + + app.stop(); +} From 87adfe644baef5453761a58dbe701cbfebaf13ef Mon Sep 17 00:00:00 2001 From: "Anton N. Petrov" Date: Tue, 1 Sep 2026 02:19:11 +0300 Subject: [PATCH 02/14] Add a request body sink, with body_file() as a file-backed wrapper .body_sink() installs a per-request destination at headers-complete; the parser writes each on_body span into it. .body_file() is that sink with an exclusive fd kept open until finish(), 0600 on POSIX, and unlink unless take_body_file() transfers ownership. The file is created only when a body is coming. Write/close failures are 500 with Connection: close. Size limits come from max_body_size (the previous commit), not a file-only knob. Answers gittiver on CrowCpp/Crow#1233: the primitive is a stream/sink so targets without a filesystem can still accept large bodies. --- README.md | 1 + docs/guides/body-file.md | 83 +++++++ docs/guides/routes.md | 2 + examples/CMakeLists.txt | 4 + examples/example_body_file.cpp | 29 +++ include/crow.h | 1 + include/crow/app.h | 33 +++ include/crow/body_sink.h | 184 ++++++++++++++ include/crow/http_connection.h | 36 ++- include/crow/http_request.h | 23 +- include/crow/parser.h | 62 ++++- include/crow/routing.h | 70 ++++++ mkdocs.yml | 1 + tests/CMakeLists.txt | 1 + tests/body_file_tests.cpp | 427 +++++++++++++++++++++++++++++++++ 15 files changed, 945 insertions(+), 12 deletions(-) create mode 100644 docs/guides/body-file.md create mode 100644 examples/example_body_file.cpp create mode 100644 include/crow/body_sink.h create mode 100644 tests/body_file_tests.cpp diff --git a/README.md b/README.md index 8edcf2909f..a44ebc74a1 100644 --- a/README.md +++ b/README.md @@ -23,6 +23,7 @@ Crow is a C++ framework for creating HTTP or Websocket web services. It uses rou - Middleware support for extensions. - HTTP/1.1 and Websocket support. - Multi-part request and response support. + - Request body size limit (`max_body_size`) and optional body-to-file / body-sink uploads. - Uses modern C++ (11/14) ### Still in development diff --git a/docs/guides/body-file.md b/docs/guides/body-file.md new file mode 100644 index 0000000000..cdc5aae1de --- /dev/null +++ b/docs/guides/body-file.md @@ -0,0 +1,83 @@ +[:octicons-feed-tag-16: master](https://github.com/CrowCpp/Crow) + +By default Crow stores a request body in `req.body` (`std::string`). That is +the right default for JSON and form fields, but a large upload then occupies +the same amount of RAM as the file. + +A route can divert body bytes **while they arrive** to a sink. The built-in +sink is a unique file; you can also supply your own (flash, SD, a bounded +buffer) when there is no writable filesystem. + +The handler still runs only after the full body has been received. Writes run +synchronously on the connection's io thread. + +This is the request-side counterpart of returning a file with +`response.set_static_file_info()`: the payload is never held as one string. + +Size limits are `app.max_body_size()` / `.max_body_size()` — see +[Request body size](app.md#request-body-size). Over-limit requests are 413 +and the connection closes; no file is left behind. + +## File convenience + +```cpp +CROW_ROUTE(app, "/upload") + .methods(crow::HTTPMethod::Put, crow::HTTPMethod::Post) + .body_file()([](const crow::request& req) { + if (!req.has_body_file()) + return crow::response(500); + // req.body is empty; the bytes are in req.body_file_path + std::ifstream in(req.body_file_path, std::ios::binary); + // ... + return crow::response(200); + }); +``` + +Pass a directory to `.body_file()` to override the app default for +that route. Crow always generates a unique file name, so concurrent requests +do not share a path. The descriptor is kept open until the body is complete; +the handler then reads the path. + +```cpp +app.body_file_directory("uploads"); // default: system temp directory +app.max_body_size(64ull * 1024 * 1024); +``` + +The file is deleted after the response is sent. Call `req.take_body_file()` +if the application will use the file after the handler returns (for example +after renaming it into place). + +A client that disconnects before the body is complete never reaches the +handler; the partial file is removed. Bodyless GET/HEAD (and `Content-Length: 0`) +do not create a file. + +## Custom sink + +```cpp +struct FlashSink : crow::BodySink { + bool write(const char* data, std::size_t length) override; + bool finish() override; +}; + +CROW_ROUTE(app, "/upload") + .methods(crow::HTTPMethod::Post) + .body_sink([](const crow::request& req) { + return std::make_unique(req); + }) + ([](const crow::request&) { + return crow::response(200); + }); +``` + +The factory runs after headers, before the body. Each request gets its own +sink. `req.body` stays empty. + +## What this is not + +`.body_file()` / `.body_sink()` store the **raw** request body. +`multipart/form-data` is still parsed from `req.body` by +`crow::multipart::message`. Saving individual multipart parts as they arrive +is a separate feature. + +The route handler is not invoked per chunk. Incremental processing belongs in +`BodySink::write`. diff --git a/docs/guides/routes.md b/docs/guides/routes.md index 781cfc6c4f..68a2dcb0af 100644 --- a/docs/guides/routes.md +++ b/docs/guides/routes.md @@ -84,6 +84,8 @@ You can also access the URL parameters in the handler using `#!cpp req.url_param To cap the request body for one route, use `#!cpp .max_body_size(bytes)` (see [Request body size](app.md#request-body-size)). Without a per-route value, `#!cpp app.max_body_size()` applies, including on 404 and 405. +To write a large request body to a file (or a custom sink) while it is received instead of filling `req.body`, use `#!cpp .body_file()` or `#!cpp .body_sink(...)`. See [Request body files](body-file.md). + !!! note "Note      [:octicons-feed-tag-16: master](https://github.com/CrowCpp/Crow)" parameters inside the body can be parsed using `#!cpp req.get_body_params();`. which is useful for requests of type `application/x-www-form-urlencoded`. Its format is similar to `url_params`. diff --git a/examples/CMakeLists.txt b/examples/CMakeLists.txt index f564ae5323..0d38aca4ad 100644 --- a/examples/CMakeLists.txt +++ b/examples/CMakeLists.txt @@ -93,6 +93,10 @@ add_executable(example_file_upload example_file_upload.cpp) add_warnings_optimizations(example_file_upload) target_link_libraries(example_file_upload PUBLIC Crow::Crow) +add_executable(example_body_file example_body_file.cpp) +add_warnings_optimizations(example_body_file) +target_link_libraries(example_body_file PUBLIC Crow::Crow) + add_executable(example_unix_socket example_unix_socket.cpp) add_warnings_optimizations(example_unix_socket) target_link_libraries(example_unix_socket PUBLIC Crow::Crow) diff --git a/examples/example_body_file.cpp b/examples/example_body_file.cpp new file mode 100644 index 0000000000..0a3d763935 --- /dev/null +++ b/examples/example_body_file.cpp @@ -0,0 +1,29 @@ +#include "crow.h" + +#include +#include +#include + +int main() +{ + crow::SimpleApp app; + app.body_file_directory("uploads").max_body_size(64ull * 1024 * 1024); + + // PUT or POST the raw body; Crow writes it to disk as the bytes arrive. + CROW_ROUTE(app, "/upload") + .methods(crow::HTTPMethod::Put, crow::HTTPMethod::Post) + .body_file()([](const crow::request& req) { + if (!req.has_body_file()) + return crow::response(500); + + std::ifstream in(req.body_file_path, std::ios::binary); + const std::string contents((std::istreambuf_iterator(in)), std::istreambuf_iterator()); + + crow::json::wvalue reply; + reply["bytes"] = contents.size(); + reply["preview"] = contents.substr(0, 32); + return crow::response(reply); + }); + + app.port(18080).multithreaded().run(); +} diff --git a/include/crow.h b/include/crow.h index cb3720cff3..bd07f52baf 100644 --- a/include/crow.h +++ b/include/crow.h @@ -13,6 +13,7 @@ #include "crow/utility.h" #include "crow/common.h" #include "crow/http_request.h" +#include "crow/body_sink.h" #include "crow/websocket.h" #include "crow/parser.h" #include "crow/http_response.h" diff --git a/include/crow/app.h b/include/crow/app.h index d21cd800a3..69eb48875d 100644 --- a/include/crow/app.h +++ b/include/crow/app.h @@ -23,6 +23,8 @@ #include #include #include +#include +#include #include #include #include @@ -544,6 +546,36 @@ namespace crow return res_stream_threshold_; } + /// \brief Directory used for request body files created by routes that call `body_file()`. + /// + /// Empty (the default) uses the system temporary directory. Unique names are generated per request. + /// The directory is created when this is set. + self_t& body_file_directory(std::string directory) + { + body_file_directory_ = std::move(directory); + if (!body_file_directory_.empty()) + { + std::error_code ec; + std::filesystem::create_directories(body_file_directory_, ec); + } + return *this; + } + + const std::string& body_file_directory() const + { + return body_file_directory_; + } + + /// \brief Create a body sink for the matched route, or nullptr for in-memory `req.body`. + std::unique_ptr make_body_sink(const routing_handle_result& found, const request& req) const + { + return router_.make_body_sink(found, req, body_file_directory_); + } + + bool uses_body_sink(const routing_handle_result& found) const + { + return router_.uses_body_sink(found); + } self_t& register_blueprint(Blueprint& blueprint) { @@ -945,6 +977,7 @@ namespace crow detail::socket::tcp_socket_options tcp_socket_options_{}; detail::socket::tcp_socket_options websocket_tcp_socket_options_{}; size_t res_stream_threshold_ = 1048576; + std::string body_file_directory_; Router router_; bool static_routes_added_{false}; diff --git a/include/crow/body_sink.h b/include/crow/body_sink.h new file mode 100644 index 0000000000..b3c2f14fd9 --- /dev/null +++ b/include/crow/body_sink.h @@ -0,0 +1,184 @@ +#pragma once + +#include +#include +#include +#include +#include +#include +#include +#include + +#ifndef _WIN32 +#include +#include +#include +#else +#ifndef WIN32_LEAN_AND_MEAN +#define WIN32_LEAN_AND_MEAN +#endif +#include +#endif + +#include "crow/http_request.h" +#include "crow/utility.h" + +namespace crow +{ + /// Destination for request body bytes as they arrive from the parser. + struct BodySink + { + virtual ~BodySink() = default; + virtual bool write(const char* data, std::size_t length) = 0; + virtual bool finish() = 0; + }; + + using BodySinkFactory = std::function(const request&)>; + + /// File-backed sink used by `.body_file()`. Owns the descriptor until `finish()`. + class FileBodySink : public BodySink + { + public: + static std::unique_ptr create(const std::string& directory) + { + std::error_code ec; + std::filesystem::path dir = directory.empty() ? std::filesystem::temp_directory_path(ec) : + std::filesystem::path(directory); + if (ec) + return nullptr; + + for (int attempt = 0; attempt < 128; ++attempt) + { + std::string name; + try + { + name = "crow-body-" + utility::random_alphanum(16); + } + catch (...) + { + return nullptr; + } + const auto path = dir / name; +#ifndef _WIN32 + const int fd = ::open(path.string().c_str(), O_CREAT | O_EXCL | O_WRONLY | O_CLOEXEC, 0600); + if (fd >= 0) + return std::unique_ptr(new FileBodySink(path.string(), fd)); + if (errno != EEXIST) + return nullptr; +#else + SECURITY_ATTRIBUTES sa{}; + sa.nLength = sizeof(sa); + sa.bInheritHandle = FALSE; + HANDLE handle = CreateFileA(path.string().c_str(), GENERIC_WRITE, 0, &sa, CREATE_NEW, + FILE_ATTRIBUTE_NORMAL, nullptr); + if (handle != INVALID_HANDLE_VALUE) + return std::unique_ptr(new FileBodySink(path.string(), handle)); + if (GetLastError() != ERROR_FILE_EXISTS) + return nullptr; +#endif + } + return nullptr; + } + + FileBodySink(const FileBodySink&) = delete; + FileBodySink& operator=(const FileBodySink&) = delete; + + ~FileBodySink() override + { + close_handle(); + if (!persist_ && !path_.empty()) + { + std::error_code ec; + std::filesystem::remove(path_, ec); + } + } + + bool write(const char* data, std::size_t length) override + { +#ifndef _WIN32 + if (fd_ < 0) + return false; + std::size_t written = 0; + while (written < length) + { + const ssize_t n = ::write(fd_, data + written, length - written); + if (n < 0) + { + if (errno == EINTR) + continue; + return false; + } + if (n == 0) + return false; + written += static_cast(n); + } + return true; +#else + if (handle_ == INVALID_HANDLE_VALUE) + return false; + std::size_t written = 0; + while (written < length) + { + DWORD n = 0; + const DWORD chunk = static_cast( + std::min(length - written, static_cast(MAXDWORD))); + if (!WriteFile(handle_, data + written, chunk, &n, nullptr) || n == 0) + return false; + written += n; + } + return true; +#endif + } + + bool finish() override + { + return close_handle(); + } + + const std::string& path() const + { + return path_; + } + + void persist() + { + persist_ = true; + } + + private: +#ifndef _WIN32 + FileBodySink(std::string path, int fd): + path_(std::move(path)), fd_(fd) + {} +#else + FileBodySink(std::string path, HANDLE handle): + path_(std::move(path)), handle_(handle) + {} +#endif + + bool close_handle() + { +#ifndef _WIN32 + if (fd_ < 0) + return true; + const int fd = fd_; + fd_ = -1; + return ::close(fd) == 0; +#else + if (handle_ == INVALID_HANDLE_VALUE) + return true; + const HANDLE handle = handle_; + handle_ = INVALID_HANDLE_VALUE; + return CloseHandle(handle) != 0; +#endif + } + + std::string path_; + bool persist_{false}; +#ifndef _WIN32 + int fd_{-1}; +#else + HANDLE handle_{INVALID_HANDLE_VALUE}; +#endif + }; +} // namespace crow diff --git a/include/crow/http_connection.h b/include/crow/http_connection.h index 045201ec4a..0e2d8c2ee2 100644 --- a/include/crow/http_connection.h +++ b/include/crow/http_connection.h @@ -124,7 +124,7 @@ namespace crow /// Apply `max_body_size` before 100-continue. Return 1 to skip the body (F_SKIPBODY). int handle_header() { - payload_too_large_ = false; + body_error_status_ = 0; const uint64_t limit = handler_->effective_max_body_size(*routing_handle_result_); parser_.set_max_body_size(limit); @@ -132,13 +132,30 @@ namespace crow { if (limit != UINT64_MAX && parser_.content_length > limit) { - payload_too_large_ = true; + body_error_status_ = status::PAYLOAD_TOO_LARGE; return 1; } if (parser_.content_length <= req_.body.max_size()) req_.body.reserve(static_cast(parser_.content_length)); } + if (parser_.has_incoming_body() && handler_->uses_body_sink(*routing_handle_result_)) + { + try + { + if (!parser_.open_body_sink(handler_->make_body_sink(*routing_handle_result_, req_))) + { + body_error_status_ = status::INTERNAL_SERVER_ERROR; + return 1; + } + } + catch (...) + { + body_error_status_ = status::INTERNAL_SERVER_ERROR; + return 1; + } + } + // HTTP 1.1 Expect: 100-continue if (req_.http_ver_major == 1 && req_.http_ver_minor == 1 && get_header_value(req_.headers, "expect") == "100-continue") { @@ -161,15 +178,16 @@ namespace crow return 0; } - void reject_payload_too_large() + void reject_body(int code) { - payload_too_large_ = true; + if (body_error_status_ == 0) + body_error_status_ = code; handle(); } bool parser_should_abort() const { - return payload_too_large_; + return body_error_status_ != 0; } void handle() @@ -188,9 +206,9 @@ namespace crow add_keep_alive_ = req_.keep_alive; close_connection_ = req_.close_connection; - if (payload_too_large_) + if (body_error_status_) { - res = response(status::PAYLOAD_TOO_LARGE); + res = response(body_error_status_); res.set_header("Connection", "close"); res.end(); close_connection_ = true; @@ -250,7 +268,7 @@ namespace crow } else { - if (payload_too_large_) + if (body_error_status_) need_to_call_after_handlers_ = true; complete_request(); } @@ -574,7 +592,7 @@ namespace crow bool need_to_call_after_handlers_{}; bool need_to_start_read_after_complete_{}; bool add_keep_alive_{}; - bool payload_too_large_{false}; + int body_error_status_{0}; std::tuple* middlewares_; detail::context ctx_; diff --git a/include/crow/http_request.h b/include/crow/http_request.h index ab526f3eef..7c736ebc84 100644 --- a/include/crow/http_request.h +++ b/include/crow/http_request.h @@ -51,6 +51,7 @@ namespace crow // NOTE: Already documented in "crow/app.h" query_string url_params; ///< The parameters associated with the request. (everything after the `?` in the URL) ci_map headers; std::string body; + std::string body_file_path; ///< Path of the file that received the body, if the route used `body_file()`. Empty when the body is in `body`. std::string remote_ip_address; ///< The IP address from which the request was sent. unsigned char http_ver_major, http_ver_minor; bool keep_alive, ///< Whether or not the server should send a `connection: Keep-Alive` header to the client. @@ -61,6 +62,9 @@ namespace crow // NOTE: Already documented in "crow/app.h" void* middleware_container{}; asio::io_context* io_context{}; + template + friend struct HTTPParser; + /// Construct an empty request. (sets the method to `GET`) request(): method(HTTPMethod::Get) @@ -91,12 +95,26 @@ namespace crow // NOTE: Already documented in "crow/app.h" /// Get the body as parameters in QS format. /// - /// This is meant to be used with requests of type "application/x-www-form-urlencoded" + /// This is meant to be used with requests of type "application/x-www-form-urlencoded". + /// It reads `body`; it does not open `body_file_path`. const query_string get_body_params() const { return query_string(body, false); } + /// True when the request body was written to `body_file_path` instead of `body`. + bool has_body_file() const + { + return !body_file_path.empty(); + } + + /// Keep the body file and return its path. The parser will not delete it after the response. + std::string take_body_file() const + { + persist_body_file_ = true; + return body_file_path; + } + /// Send data to whoever made this request with a completion handler and return immediately. template void post(CompletionHandler handler) @@ -110,5 +128,8 @@ namespace crow // NOTE: Already documented in "crow/app.h" { asio::dispatch(io_context, handler); } + + private: + mutable bool persist_body_file_{false}; }; } // namespace crow diff --git a/include/crow/parser.h b/include/crow/parser.h index acf253d634..c537a27e8f 100644 --- a/include/crow/parser.h +++ b/include/crow/parser.h @@ -2,9 +2,12 @@ #include #include +#include #include #include +#include "crow/body_sink.h" +#include "crow/common.h" #include "crow/http_request.h" #include "crow/http_parser_merged.h" @@ -92,10 +95,21 @@ namespace crow (length > self->max_body_size_ || self->body_bytes_ > self->max_body_size_ - length)) { - self->handler_->reject_payload_too_large(); + self->handler_->reject_body(status::PAYLOAD_TOO_LARGE); return 1; } - self->req.body.insert(self->req.body.end(), at, at + length); + if (self->body_sink_) + { + if (!self->body_sink_->write(at, length)) + { + self->handler_->reject_body(status::INTERNAL_SERVER_ERROR); + return 1; + } + } + else + { + self->req.body.insert(self->req.body.end(), at, at + length); + } self->body_bytes_ += length; return 0; } @@ -104,6 +118,17 @@ namespace crow HTTPParser* self = static_cast(self_); self->message_complete = true; + if (self->body_sink_) + { + const bool ok = self->body_sink_->finish(); + if (auto* file = dynamic_cast(self->body_sink_.get())) + self->req.body_file_path = file->path(); + if (!ok) + { + self->handler_->reject_body(status::INTERNAL_SERVER_ERROR); + return 1; + } + } self->process_message(); // Stop so leftover skipped-body bytes are not parsed as the next request. return self->handler_->parser_should_abort() ? 1 : 0; @@ -115,6 +140,11 @@ namespace crow http_parser_init(this); } + ~HTTPParser() + { + cleanup_body_sink(); + } + // return false on error /// Parse a buffer into the different sections of an HTTP request. bool feed(const char* buffer, int length) @@ -148,6 +178,7 @@ namespace crow void clear() { + cleanup_body_sink(); req = crow::request(); header_field.clear(); header_value.clear(); @@ -164,6 +195,22 @@ namespace crow max_body_size_ = bytes; } + bool open_body_sink(std::unique_ptr sink) + { + cleanup_body_sink(); + if (!sink) + return false; + body_sink_ = std::move(sink); + return true; + } + + bool has_incoming_body() const + { + if (flags & F_CHUNKED) + return true; + return content_length != CROW_ULLONG_MAX && content_length > 0; + } + inline void process_url() { handler_->handle_url(); @@ -204,10 +251,21 @@ namespace crow request req; private: + void cleanup_body_sink() + { + if (auto* file = dynamic_cast(body_sink_.get())) + { + if (req.persist_body_file_) + file->persist(); + } + body_sink_.reset(); + } + int header_building_state = 0; bool message_complete = false; uint64_t body_bytes_{0}; uint64_t max_body_size_{UINT64_MAX}; + std::unique_ptr body_sink_; std::string header_field; std::string header_value; diff --git a/include/crow/routing.h b/include/crow/routing.h index 770ac22596..bc727a8c20 100644 --- a/include/crow/routing.h +++ b/include/crow/routing.h @@ -8,12 +8,15 @@ #include #include #include +#include +#include #include #include #include "crow/common.h" #include "crow/http_response.h" #include "crow/http_request.h" +#include "crow/body_sink.h" #include "crow/utility.h" #include "crow/logging.h" #include "crow/exceptions.h" @@ -162,6 +165,9 @@ namespace crow // NOTE: Already documented in "crow/app.h" bool added_{false}; uint64_t max_body_size_{UINT64_MAX}; bool max_body_size_override_{false}; + bool body_file_{false}; + std::string body_file_directory_; + BodySinkFactory body_sink_factory_; std::unique_ptr rule_to_upgrade_; @@ -628,6 +634,34 @@ namespace crow // NOTE: Already documented in "crow/app.h" static_cast(this)->max_body_size_override_ = true; return static_cast(*this); } + + /// Write the request body to a user-provided sink while it is received. + /// + /// The factory runs after headers, before the body. The route handler still + /// runs only after the full body has been received. `req.body` stays empty. + self_t& body_sink(BodySinkFactory factory) + { + static_cast(this)->body_sink_factory_ = std::move(factory); + return static_cast(*this); + } + + /// Write the request body to a unique file while it is received. + /// + /// `req.body` stays empty. The handler reads `req.body_file_path`. + /// An empty `directory` uses `app.body_file_directory()`, or the system + /// temporary directory. The file is deleted after the response unless the + /// handler calls `req.take_body_file()`. + self_t& body_file(std::string directory = {}) + { + static_cast(this)->body_file_ = true; + static_cast(this)->body_file_directory_ = std::move(directory); + if (!static_cast(this)->body_file_directory_.empty()) + { + std::error_code ec; + std::filesystem::create_directories(static_cast(this)->body_file_directory_, ec); + } + return static_cast(*this); + } }; /// A rule that can change its parameters during runtime. @@ -1871,6 +1905,42 @@ namespace crow // NOTE: Already documented in "crow/app.h" return rule->max_body_size_; } + bool uses_body_sink(const routing_handle_result& found) const + { + if (found.catch_all || found.rule_index <= RULE_SPECIAL_REDIRECT_SLASH) + return false; + if (found.method >= HTTPMethod::InternalMethodCount) + return false; + const auto& rules = per_methods_[static_cast(found.method)].rules; + if (found.rule_index >= rules.size()) + return false; + const BaseRule* rule = rules[found.rule_index]; + return rule && (rule->body_file_ || static_cast(rule->body_sink_factory_)); + } + + std::unique_ptr make_body_sink(const routing_handle_result& found, const request& req, + const std::string& app_directory) const + { + if (found.catch_all || found.rule_index <= RULE_SPECIAL_REDIRECT_SLASH) + return nullptr; + if (found.method >= HTTPMethod::InternalMethodCount) + return nullptr; + const auto& rules = per_methods_[static_cast(found.method)].rules; + if (found.rule_index >= rules.size()) + return nullptr; + const BaseRule* rule = rules[found.rule_index]; + if (!rule) + return nullptr; + if (rule->body_sink_factory_) + return rule->body_sink_factory_(req); + if (rule->body_file_) + { + const std::string& dir = rule->body_file_directory_.empty() ? app_directory : rule->body_file_directory_; + return FileBodySink::create(dir); + } + return nullptr; + } + std::function& exception_handler() { return exception_handler_; diff --git a/mkdocs.yml b/mkdocs.yml index e5013dbb99..eae6f97c38 100644 --- a/mkdocs.yml +++ b/mkdocs.yml @@ -70,6 +70,7 @@ nav: - SBOM Generation: guides/sbom.md - SSL: guides/ssl.md - Static Files: guides/static.md + - Request body files: guides/body-file.md - Blueprints: guides/blueprints.md - Compression: guides/compression.md - Websockets: guides/websockets.md diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 6773e80e74..e01edeff2c 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -23,6 +23,7 @@ enable_testing() set(TEST_SRCS unittest.cpp max_body_size_tests.cpp + body_file_tests.cpp query_string_tests.cpp unit_tests/test_http_response.cpp unit_tests/test_json.cpp diff --git a/tests/body_file_tests.cpp b/tests/body_file_tests.cpp new file mode 100644 index 0000000000..e5ccfa0f7d --- /dev/null +++ b/tests/body_file_tests.cpp @@ -0,0 +1,427 @@ +#define CROW_ENABLE_DEBUG +#define CROW_LOG_LEVEL 0 + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#ifndef _WIN32 +#include +#endif + +#include "catch2/catch_all.hpp" +#include "crow.h" + +using namespace crow; + +#ifdef CROW_USE_BOOST +namespace asio = boost::asio; +using asio_error_code = boost::system::error_code; +#else +using asio_error_code = asio::error_code; +#endif + +#define LOCALHOST_ADDRESS "127.0.0.1" + +namespace +{ + std::string read_all(const std::string& path) + { + std::ifstream in(path, std::ios::binary); + return std::string((std::istreambuf_iterator(in)), std::istreambuf_iterator()); + } + + bool directory_is_empty(const std::filesystem::path& path) + { + std::error_code ec; + const auto it = std::filesystem::directory_iterator(path, ec); + if (ec) + return false; + return it == std::filesystem::directory_iterator(); + } + + bool response_complete(const std::string& data) + { + auto search_from = std::size_t{0}; + while (true) + { + const auto status_line = data.find("HTTP/1.1 ", search_from); + if (status_line == std::string::npos) + return false; + const auto code = std::atoi(data.c_str() + status_line + 9); + const auto header_end = data.find("\r\n\r\n", status_line); + if (header_end == std::string::npos) + return false; + if (code >= 100 && code < 200) + { + search_from = header_end + 4; + continue; + } + const auto length_pos = data.find("Content-Length:", status_line); + if (length_pos == std::string::npos || length_pos > header_end) + return true; + const auto length = static_cast(std::stoul(data.substr(length_pos + 15))); + return data.size() >= header_end + 4 + length; + } + } + + std::string http_body(const std::string& response) + { + auto search_from = std::size_t{0}; + while (true) + { + const auto status_line = response.find("HTTP/1.1 ", search_from); + if (status_line == std::string::npos) + return {}; + const auto code = std::atoi(response.c_str() + status_line + 9); + const auto header_end = response.find("\r\n\r\n", status_line); + if (header_end == std::string::npos) + return {}; + if (code >= 100 && code < 200) + { + search_from = header_end + 4; + continue; + } + return response.substr(header_end + 4); + } + } + + class TestClient + { + public: + TestClient(uint16_t port): + socket_(io_context_) + { + socket_.connect(asio::ip::tcp::endpoint(asio::ip::make_address(LOCALHOST_ADDRESS), port)); + } + + void send(const std::string& data) + { + asio::write(socket_, asio::buffer(data)); + } + + void send(const char* data, std::size_t size) + { + asio::write(socket_, asio::buffer(data, size)); + } + + std::string receive() + { + std::string response; + std::array buffer{}; + const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(5); + while (!response_complete(response)) + { + REQUIRE(std::chrono::steady_clock::now() < deadline); + asio_error_code ec; + const auto n = socket_.read_some(asio::buffer(buffer), ec); + if (ec) + break; + response.append(buffer.data(), n); + } + return response; + } + + private: + asio::io_context io_context_{}; + asio::ip::tcp::socket socket_; + }; + + struct TempDir + { + std::filesystem::path path; + + TempDir(): + path(std::filesystem::temp_directory_path() / ("crow-body-file-tests-" + utility::random_alphanum(12))) + { + std::filesystem::create_directories(path); + } + + ~TempDir() + { + std::error_code ec; + std::filesystem::remove_all(path, ec); + } + }; + + struct VectorSink : BodySink + { + std::shared_ptr buf; + bool write(const char* data, std::size_t length) override + { + buf->append(data, length); + return true; + } + bool finish() override { return true; } + }; + + std::string trim_crlf(std::string s) + { + while (!s.empty() && (s.back() == '\n' || s.back() == '\r')) + s.pop_back(); + return s; + } +} // namespace + +TEST_CASE("request_body_file", "[http][body_file]") +{ + TempDir dir; + SimpleApp app; + app.body_file_directory(dir.path.string()).max_body_size(1024 * 1024); + + CROW_ROUTE(app, "/memory") + .methods("POST"_method)([](const request& req) { + return std::string(req.has_body_file() ? "file" : "memory") + ':' + req.body; + }); + + CROW_ROUTE(app, "/upload") + .methods("POST"_method) + .body_file()([](const request& req) { + if (!req.has_body_file()) + return std::string("nobody:") + req.body; + return std::string("file:") + read_all(req.body_file_path); + }); + + CROW_ROUTE(app, "/keep") + .methods("POST"_method) + .body_file()([](const request& req) { + return req.take_body_file(); + }); + + CROW_ROUTE(app, "/custom") + .methods("POST"_method) + .body_file((dir.path / "route-dir").string())([](const request& req) { + return req.body_file_path; + }); + + CROW_ROUTE(app, "/getfile") + .body_file()([](const request& req) { + return req.has_body_file() ? "file" : "nobody"; + }); + + auto server = app.bindaddr(LOCALHOST_ADDRESS).port(0).run_async(); + app.wait_for_server_start(); + const auto port = app.port(); + + auto post = [&](const std::string& target, const std::string& payload, + const std::string& extra_headers = {}) { + TestClient client(port); + client.send( + "POST " + target + + " HTTP/1.1\r\n" + "Host: localhost\r\n" + + extra_headers + + "Content-Length: " + std::to_string(payload.size()) + + "\r\n" + "\r\n"); + if (!payload.empty()) + client.send(payload.data(), payload.size()); + return client.receive(); + }; + + const auto memory = post("/memory", "hello"); + CHECK(memory.find("HTTP/1.1 200") != std::string::npos); + CHECK(http_body(memory) == "memory:hello"); + + const std::string payload(64 * 1024, 'A'); + const auto uploaded = post("/upload", payload); + REQUIRE(uploaded.find("HTTP/1.1 200") != std::string::npos); + CHECK(http_body(uploaded) == "file:" + payload); + + const std::string binary(std::string("a") + '\0' + "b" + '\xff' + "c"); + const auto binary_response = post("/upload", binary); + REQUIRE(binary_response.find("HTTP/1.1 200") != std::string::npos); + CHECK(http_body(binary_response) == "file:" + binary); + + const auto empty = post("/upload", ""); + REQUIRE(empty.find("HTTP/1.1 200") != std::string::npos); + CHECK(http_body(empty) == "nobody:"); + + { + TestClient client(port); + client.send( + "GET /getfile HTTP/1.1\r\n" + "Host: localhost\r\n" + "\r\n"); + CHECK(http_body(client.receive()) == "nobody"); + } + + { + TestClient client(port); + client.send( + "POST /upload HTTP/1.1\r\n" + "Host: localhost\r\n" + "Transfer-Encoding: chunked\r\n" + "\r\n" + "5\r\nhello\r\n" + "6\r\n world\r\n" + "0\r\n\r\n"); + const auto response = client.receive(); + REQUIRE(response.find("HTTP/1.1 200") != std::string::npos); + CHECK(http_body(response) == "file:hello world"); + } + + const auto continued = post("/upload", "ping", "Expect: 100-continue\r\n"); + REQUIRE(continued.find("100 Continue") != std::string::npos); + REQUIRE(continued.find("HTTP/1.1 200") != std::string::npos); + CHECK(http_body(continued) == "file:ping"); + + { + const auto response = post("/keep", "abc"); + const auto kept = trim_crlf(http_body(response)); + REQUIRE(std::filesystem::exists(kept)); + CHECK(read_all(kept) == "abc"); +#ifndef _WIN32 + struct stat st {}; + REQUIRE(::stat(kept.c_str(), &st) == 0); + CHECK((st.st_mode & 0777) == 0600); +#endif + std::filesystem::remove(kept); + } + + { + const auto response = post("/custom", "Z"); + const auto custom_path = trim_crlf(http_body(response)); + CHECK(custom_path.find((dir.path / "route-dir").string()) != std::string::npos); + } + + { + TestClient client(port); + client.send( + "POST /upload HTTP/1.1\r\n" + "Host: localhost\r\n" + "Content-Length: 4\r\n" + "\r\n" + "one!"); + const auto first = client.receive(); + CHECK(http_body(first) == "file:one!"); + client.send( + "POST /upload HTTP/1.1\r\n" + "Host: localhost\r\n" + "Content-Length: 5\r\n" + "\r\n" + "two!!"); + const auto second = client.receive(); + CHECK(http_body(second) == "file:two!!"); + } + + const auto upload_path_resp = post("/keep", payload); + const auto first_path = trim_crlf(http_body(upload_path_resp)); + const auto second_keep = post("/keep", "x"); + const auto second_path = trim_crlf(http_body(second_keep)); + CHECK(first_path != second_path); + std::filesystem::remove(first_path); + std::filesystem::remove(second_path); + + app.stop(); +} + +TEST_CASE("request_body_file_too_large", "[http][body_file]") +{ + TempDir dir; + std::atomic handler_ran{false}; + SimpleApp app; + app.body_file_directory(dir.path.string()).max_body_size(8); + + CROW_ROUTE(app, "/upload") + .methods("POST"_method) + .body_file()([&handler_ran](const request&) { + handler_ran = true; + return "ran"; + }); + + auto server = app.bindaddr(LOCALHOST_ADDRESS).port(0).run_async(); + app.wait_for_server_start(); + + TestClient client(app.port()); + client.send( + "POST /upload HTTP/1.1\r\n" + "Host: localhost\r\n" + "Content-Length: 9\r\n" + "\r\n" + "123456789"); + const auto response = client.receive(); + CHECK(response.find("HTTP/1.1 413") != std::string::npos); + CHECK_FALSE(handler_ran.load()); + CHECK(directory_is_empty(dir.path)); + + app.stop(); +} + +TEST_CASE("request_body_file_disconnect_cleans_up", "[http][body_file]") +{ + TempDir dir; + std::atomic handler_ran{false}; + SimpleApp app; + app.body_file_directory(dir.path.string()); + + CROW_ROUTE(app, "/upload") + .methods("POST"_method) + .body_file()([&handler_ran](const request&) { + handler_ran = true; + return "ran"; + }); + + auto server = app.bindaddr(LOCALHOST_ADDRESS).port(0).run_async(); + app.wait_for_server_start(); + + { + TestClient client(app.port()); + client.send( + "POST /upload HTTP/1.1\r\n" + "Host: localhost\r\n" + "Content-Length: 64\r\n" + "\r\n" + "partial"); + } + + const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(2); + bool empty = false; + while (std::chrono::steady_clock::now() < deadline) + { + empty = directory_is_empty(dir.path); + if (empty) + break; + std::this_thread::sleep_for(std::chrono::milliseconds(20)); + } + CHECK(empty); + CHECK_FALSE(handler_ran.load()); + + app.stop(); +} + +TEST_CASE("request_body_sink", "[http][body_file]") +{ + auto buf = std::make_shared(); + SimpleApp app; + app.max_body_size(1024 * 1024); + + CROW_ROUTE(app, "/sink") + .methods("POST"_method) + .body_sink([buf](const request&) { + auto sink = std::make_unique(); + sink->buf = buf; + return sink; + })([buf](const request& req) { + return req.body.empty() ? *buf : req.body; + }); + + auto server = app.bindaddr(LOCALHOST_ADDRESS).port(0).run_async(); + app.wait_for_server_start(); + + const std::string payload(32 * 1024, 'S'); + TestClient client(app.port()); + client.send( + "POST /sink HTTP/1.1\r\nHost: localhost\r\nContent-Length: " + + std::to_string(payload.size()) + "\r\n\r\n" + payload); + CHECK(http_body(client.receive()) == payload); + + app.stop(); +} From 225034f8e72629cbff6f6e71079f40b928019001 Mon Sep 17 00:00:00 2001 From: "Anton N. Petrov" Date: Tue, 1 Sep 2026 18:37:35 +0300 Subject: [PATCH 03/14] Don't allocate from untrusted Content-Length when body size is unlimited Reserve req.body only when a finite max_body_size is in effect and the advertised length is within that cap. The unlimited default grows the buffer as bytes arrive, so a huge Content-Length cannot OOM a worker. Complete 413 through after-handlers only, skipping before-handlers, to match unmatched-route and auto-OPTIONS early rejects. --- include/crow/http_connection.h | 28 ++++++---- tests/max_body_size_tests.cpp | 99 ++++++++++++++++++++++++++++++++++ 2 files changed, 116 insertions(+), 11 deletions(-) diff --git a/include/crow/http_connection.h b/include/crow/http_connection.h index 045201ec4a..2cb99754f2 100644 --- a/include/crow/http_connection.h +++ b/include/crow/http_connection.h @@ -128,15 +128,20 @@ namespace crow const uint64_t limit = handler_->effective_max_body_size(*routing_handle_result_); parser_.set_max_body_size(limit); - if (parser_.content_length != CROW_ULLONG_MAX) + if (parser_.content_length != CROW_ULLONG_MAX && + limit != UINT64_MAX && parser_.content_length > limit) { - if (limit != UINT64_MAX && parser_.content_length > limit) - { - payload_too_large_ = true; - return 1; - } - if (parser_.content_length <= req_.body.max_size()) - req_.body.reserve(static_cast(parser_.content_length)); + payload_too_large_ = true; + return 1; + } + + // Finite cap only: never allocate from an untrusted Content-Length + // when the default unlimited path is in effect. + if (limit != UINT64_MAX && + parser_.content_length != CROW_ULLONG_MAX && + parser_.content_length <= req_.body.max_size()) + { + req_.body.reserve(static_cast(parser_.content_length)); } // HTTP 1.1 Expect: 100-continue @@ -195,8 +200,11 @@ namespace crow res.end(); close_connection_ = true; add_keep_alive_ = false; + need_to_call_after_handlers_ = true; + complete_request(); + return; } - else if (req_.check_version(1, 1)) // HTTP/1.1 + if (req_.check_version(1, 1)) // HTTP/1.1 { if (!req_.headers.count("host")) { @@ -250,8 +258,6 @@ namespace crow } else { - if (payload_too_large_) - need_to_call_after_handlers_ = true; complete_request(); } } diff --git a/tests/max_body_size_tests.cpp b/tests/max_body_size_tests.cpp index 46b5429f81..31425da508 100644 --- a/tests/max_body_size_tests.cpp +++ b/tests/max_body_size_tests.cpp @@ -342,3 +342,102 @@ TEST_CASE("max_body_size 413 runs after-handlers", "[http][max_body_size]") app.stop(); } + +TEST_CASE("max_body_size unlimited does not allocate advertised Content-Length", "[http][max_body_size]") +{ + SimpleApp app; + CROW_ROUTE(app, "/upload") + .methods("POST"_method)([](const request& req) { + return std::to_string(req.body.size()); + }); + + auto server = app.bindaddr(LOCALHOST_ADDRESS).port(0).run_async(); + app.wait_for_server_start(); + const auto port = app.port(); + + // Keep this socket open so handle_header actually sees the huge advertised + // length. Closing immediately can hide a reserve() that only runs after + // the server reads the headers. + TestClient attacker(port); + attacker.send( + "POST /upload HTTP/1.1\r\n" + "Host: localhost\r\n" + "Content-Length: 1125899906842624\r\n" + "\r\n"); + std::this_thread::sleep_for(std::chrono::milliseconds(200)); + + TestClient client(port); + client.send( + "POST /upload HTTP/1.1\r\n" + "Host: localhost\r\n" + "Content-Length: 4\r\n" + "\r\n" + "ping"); + const auto resp = client.receive(); + CHECK(status_of(resp) == 200); + CHECK(resp.find("4") != std::string::npos); + + app.stop(); +} + +TEST_CASE("max_body_size 413 skips before-handlers", "[http][max_body_size]") +{ + struct ProbeMiddleware + { + std::atomic before{false}; + std::atomic after{false}; + + struct context + {}; + + void before_handle(request& /*req*/, response& /*res*/, context& /*ctx*/) + { + before = true; + } + + void after_handle(request& /*req*/, response& /*res*/, context& /*ctx*/) + { + after = true; + } + }; + + App app; + app.max_body_size(8); + + CROW_ROUTE(app, "/upload") + .methods("POST"_method)([] { + return "ok"; + }); + + auto server = app.bindaddr(LOCALHOST_ADDRESS).port(0).run_async(); + app.wait_for_server_start(); + + TestClient client(app.port()); + client.send( + "POST /upload HTTP/1.1\r\n" + "Host: localhost\r\n" + "Content-Length: 100\r\n" + "\r\n" + + std::string(100, 'x')); + const auto resp = client.receive(); + CHECK(status_of(resp) == 413); + + auto& probe = app.get_middleware(); + CHECK_FALSE(probe.before.load()); + CHECK(probe.after.load()); + + probe.before = false; + probe.after = false; + TestClient ok_client(app.port()); + ok_client.send( + "POST /upload HTTP/1.1\r\n" + "Host: localhost\r\n" + "Content-Length: 4\r\n" + "\r\n" + "abcd"); + CHECK(status_of(ok_client.receive()) == 200); + CHECK(probe.before.load()); + CHECK(probe.after.load()); + + app.stop(); +} From 7767c4ca1070d0373df571bb2def8537a147879f Mon Sep 17 00:00:00 2001 From: "Anton N. Petrov" Date: Tue, 1 Sep 2026 22:57:53 +0300 Subject: [PATCH 04/14] Make body_file/body_sink last-call-wins and cover sink failure paths Setting both on one route keeps only the last of .body_file() and .body_sink(). Catch2 drives a real Crow app over TCP for no-reserve on sink/file routes, write/finish/open 500 with Connection: close, over-limit 413 leftover cleanup, concurrent unique files, and GET/Content-Length: 0 creating no file. --- docs/guides/body-file.md | 3 + include/crow/routing.h | 4 + tests/body_file_tests.cpp | 375 +++++++++++++++++++++++++++++++++++++- 3 files changed, 381 insertions(+), 1 deletion(-) diff --git a/docs/guides/body-file.md b/docs/guides/body-file.md index cdc5aae1de..c7275f1247 100644 --- a/docs/guides/body-file.md +++ b/docs/guides/body-file.md @@ -72,6 +72,9 @@ CROW_ROUTE(app, "/upload") The factory runs after headers, before the body. Each request gets its own sink. `req.body` stays empty. +Do not set both `.body_file()` and `.body_sink()` on the same route; the last +call wins. + ## What this is not `.body_file()` / `.body_sink()` store the **raw** request body. diff --git a/include/crow/routing.h b/include/crow/routing.h index bc727a8c20..6e69fd0d1c 100644 --- a/include/crow/routing.h +++ b/include/crow/routing.h @@ -639,9 +639,11 @@ namespace crow // NOTE: Already documented in "crow/app.h" /// /// The factory runs after headers, before the body. The route handler still /// runs only after the full body has been received. `req.body` stays empty. + /// Replaces `.body_file()` if both are set on the same route. self_t& body_sink(BodySinkFactory factory) { static_cast(this)->body_sink_factory_ = std::move(factory); + static_cast(this)->body_file_ = false; return static_cast(*this); } @@ -651,9 +653,11 @@ namespace crow // NOTE: Already documented in "crow/app.h" /// An empty `directory` uses `app.body_file_directory()`, or the system /// temporary directory. The file is deleted after the response unless the /// handler calls `req.take_body_file()`. + /// Replaces `.body_sink()` if both are set on the same route. self_t& body_file(std::string directory = {}) { static_cast(this)->body_file_ = true; + static_cast(this)->body_sink_factory_ = {}; static_cast(this)->body_file_directory_ = std::move(directory); if (!static_cast(this)->body_file_directory_.empty()) { diff --git a/tests/body_file_tests.cpp b/tests/body_file_tests.cpp index e5ccfa0f7d..db03c83103 100644 --- a/tests/body_file_tests.cpp +++ b/tests/body_file_tests.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include #include #include @@ -47,6 +48,20 @@ namespace return it == std::filesystem::directory_iterator(); } + bool no_crow_body_files(const std::filesystem::path& path) + { + std::error_code ec; + for (const auto& entry : std::filesystem::recursive_directory_iterator(path, ec)) + { + if (ec) + return false; + if (entry.is_regular_file() && + entry.path().filename().string().rfind("crow-body-", 0) == 0) + return false; + } + return true; + } + bool response_complete(const std::string& data) { auto search_from = std::size_t{0}; @@ -162,6 +177,18 @@ namespace bool finish() override { return true; } }; + struct FailingWriteSink : BodySink + { + bool write(const char*, std::size_t) override { return false; } + bool finish() override { return true; } + }; + + struct FailingFinishSink : BodySink + { + bool write(const char*, std::size_t) override { return true; } + bool finish() override { return false; } + }; + std::string trim_crlf(std::string s) { while (!s.empty() && (s.back() == '\n' || s.back() == '\r')) @@ -251,6 +278,18 @@ TEST_CASE("request_body_file", "[http][body_file]") "Host: localhost\r\n" "\r\n"); CHECK(http_body(client.receive()) == "nobody"); + CHECK(no_crow_body_files(dir.path)); + } + + { + TestClient client(port); + client.send( + "GET /getfile HTTP/1.1\r\n" + "Host: localhost\r\n" + "Content-Length: 0\r\n" + "\r\n"); + CHECK(http_body(client.receive()) == "nobody"); + CHECK(no_crow_body_files(dir.path)); } { @@ -349,6 +388,7 @@ TEST_CASE("request_body_file_too_large", "[http][body_file]") "123456789"); const auto response = client.receive(); CHECK(response.find("HTTP/1.1 413") != std::string::npos); + CHECK(response.find("Connection: close") != std::string::npos); CHECK_FALSE(handler_ran.load()); CHECK(directory_is_empty(dir.path)); @@ -400,6 +440,9 @@ TEST_CASE("request_body_file_disconnect_cleans_up", "[http][body_file]") TEST_CASE("request_body_sink", "[http][body_file]") { auto buf = std::make_shared(); + std::atomic body_size{999}; + std::atomic body_capacity{999}; + std::atomic had_file{true}; SimpleApp app; app.max_body_size(1024 * 1024); @@ -409,7 +452,10 @@ TEST_CASE("request_body_sink", "[http][body_file]") auto sink = std::make_unique(); sink->buf = buf; return sink; - })([buf](const request& req) { + })([buf, &body_size, &body_capacity, &had_file](const request& req) { + body_size = req.body.size(); + body_capacity = req.body.capacity(); + had_file = req.has_body_file(); return req.body.empty() ? *buf : req.body; }); @@ -422,6 +468,333 @@ TEST_CASE("request_body_sink", "[http][body_file]") "POST /sink HTTP/1.1\r\nHost: localhost\r\nContent-Length: " + std::to_string(payload.size()) + "\r\n\r\n" + payload); CHECK(http_body(client.receive()) == payload); + CHECK(body_size.load() == 0); + CHECK(body_capacity.load() < payload.size()); + CHECK_FALSE(had_file.load()); + + app.stop(); +} + +TEST_CASE("request_body_file does not reserve req.body", "[http][body_file]") +{ + TempDir dir; + std::atomic body_size{999}; + std::atomic body_capacity{999}; + SimpleApp app; + app.body_file_directory(dir.path.string()).max_body_size(1024 * 1024); + + CROW_ROUTE(app, "/upload") + .methods("POST"_method) + .body_file()([&](const request& req) { + body_size = req.body.size(); + body_capacity = req.body.capacity(); + return "ok"; + }); + + auto server = app.bindaddr(LOCALHOST_ADDRESS).port(0).run_async(); + app.wait_for_server_start(); + + const std::string payload(64 * 1024, 'A'); + TestClient client(app.port()); + client.send( + "POST /upload HTTP/1.1\r\n" + "Host: localhost\r\n" + "Content-Length: " + + std::to_string(payload.size()) + + "\r\n" + "\r\n"); + client.send(payload.data(), payload.size()); + const auto response = client.receive(); + REQUIRE(response.find("HTTP/1.1 200") != std::string::npos); + CHECK(body_size.load() == 0); + CHECK(body_capacity.load() < payload.size()); + + app.stop(); +} + +TEST_CASE("request_body_sink write failure is 500", "[http][body_file]") +{ + std::atomic handler_ran{false}; + SimpleApp app; + app.max_body_size(1024 * 1024); + + CROW_ROUTE(app, "/sink") + .methods("POST"_method) + .body_sink([](const request&) { + return std::make_unique(); + })([&handler_ran](const request&) { + handler_ran = true; + return "ran"; + }); + + auto server = app.bindaddr(LOCALHOST_ADDRESS).port(0).run_async(); + app.wait_for_server_start(); + + TestClient client(app.port()); + client.send( + "POST /sink HTTP/1.1\r\n" + "Host: localhost\r\n" + "Content-Length: 4\r\n" + "\r\n" + "fail"); + const auto response = client.receive(); + CHECK(response.find("HTTP/1.1 500") != std::string::npos); + CHECK(response.find("Connection: close") != std::string::npos); + CHECK_FALSE(handler_ran.load()); + + app.stop(); +} + +TEST_CASE("request_body_sink finish failure is 500", "[http][body_file]") +{ + std::atomic handler_ran{false}; + SimpleApp app; + app.max_body_size(1024 * 1024); + + CROW_ROUTE(app, "/sink") + .methods("POST"_method) + .body_sink([](const request&) { + return std::make_unique(); + })([&handler_ran](const request&) { + handler_ran = true; + return "ran"; + }); + + auto server = app.bindaddr(LOCALHOST_ADDRESS).port(0).run_async(); + app.wait_for_server_start(); + + TestClient client(app.port()); + client.send( + "POST /sink HTTP/1.1\r\n" + "Host: localhost\r\n" + "Content-Length: 4\r\n" + "\r\n" + "fail"); + const auto response = client.receive(); + CHECK(response.find("HTTP/1.1 500") != std::string::npos); + CHECK(response.find("Connection: close") != std::string::npos); + CHECK_FALSE(handler_ran.load()); + + app.stop(); +} + +TEST_CASE("request_body_sink last call wins over body_file", "[http][body_file]") +{ + TempDir dir; + auto buf = std::make_shared(); + SimpleApp app; + app.body_file_directory(dir.path.string()).max_body_size(1024 * 1024); + + CROW_ROUTE(app, "/sink-last") + .methods("POST"_method) + .body_file() + .body_sink([buf](const request&) { + auto sink = std::make_unique(); + sink->buf = buf; + return sink; + })([buf](const request& req) { + return req.has_body_file() ? std::string("file") : *buf; + }); + + CROW_ROUTE(app, "/file-last") + .methods("POST"_method) + .body_sink([buf](const request&) { + auto sink = std::make_unique(); + sink->buf = buf; + return sink; + }) + .body_file()([](const request& req) { + return req.has_body_file() ? std::string("file:") + read_all(req.body_file_path) : std::string("sink"); + }); + + auto server = app.bindaddr(LOCALHOST_ADDRESS).port(0).run_async(); + app.wait_for_server_start(); + const auto port = app.port(); + + { + TestClient client(port); + client.send( + "POST /sink-last HTTP/1.1\r\n" + "Host: localhost\r\n" + "Content-Length: 3\r\n" + "\r\n" + "abc"); + CHECK(http_body(client.receive()) == "abc"); + CHECK(directory_is_empty(dir.path)); + } + + { + TestClient client(port); + client.send( + "POST /file-last HTTP/1.1\r\n" + "Host: localhost\r\n" + "Content-Length: 3\r\n" + "\r\n" + "xyz"); + CHECK(http_body(client.receive()) == "file:xyz"); + } + + app.stop(); +} + +TEST_CASE("request_body_sink open failure is 500", "[http][body_file]") +{ + std::atomic handler_ran{false}; + SimpleApp app; + app.max_body_size(1024 * 1024); + + CROW_ROUTE(app, "/null") + .methods("POST"_method) + .body_sink([](const request&) -> std::unique_ptr { + return nullptr; + })([&handler_ran](const request&) { + handler_ran = true; + return "ran"; + }); + + CROW_ROUTE(app, "/throw") + .methods("POST"_method) + .body_sink([](const request&) -> std::unique_ptr { + throw std::runtime_error("sink open"); + })([&handler_ran](const request&) { + handler_ran = true; + return "ran"; + }); + + auto server = app.bindaddr(LOCALHOST_ADDRESS).port(0).run_async(); + app.wait_for_server_start(); + const auto port = app.port(); + + auto post_fail = [&](const std::string& target) { + TestClient client(port); + client.send( + "POST " + target + + " HTTP/1.1\r\n" + "Host: localhost\r\n" + "Content-Length: 4\r\n" + "\r\n" + "fail"); + return client.receive(); + }; + + { + const auto response = post_fail("/null"); + CHECK(response.find("HTTP/1.1 500") != std::string::npos); + CHECK(response.find("Connection: close") != std::string::npos); + CHECK_FALSE(handler_ran.load()); + } + + handler_ran = false; + { + const auto response = post_fail("/throw"); + CHECK(response.find("HTTP/1.1 500") != std::string::npos); + CHECK(response.find("Connection: close") != std::string::npos); + CHECK_FALSE(handler_ran.load()); + } + + app.stop(); +} + +TEST_CASE("request_body_file open failure is 500 and leaves no file", "[http][body_file]") +{ + TempDir dir; + const auto not_a_dir = dir.path / "not-a-dir"; + { + std::ofstream out(not_a_dir); + out << "x"; + } + std::atomic handler_ran{false}; + SimpleApp app; + app.max_body_size(1024 * 1024); + + CROW_ROUTE(app, "/upload") + .methods("POST"_method) + .body_file(not_a_dir.string())([&handler_ran](const request&) { + handler_ran = true; + return "ran"; + }); + + auto server = app.bindaddr(LOCALHOST_ADDRESS).port(0).run_async(); + app.wait_for_server_start(); + + TestClient client(app.port()); + client.send( + "POST /upload HTTP/1.1\r\n" + "Host: localhost\r\n" + "Content-Length: 4\r\n" + "\r\n" + "fail"); + const auto response = client.receive(); + CHECK(response.find("HTTP/1.1 500") != std::string::npos); + CHECK(response.find("Connection: close") != std::string::npos); + CHECK_FALSE(handler_ran.load()); + + std::error_code ec; + for (const auto& entry : std::filesystem::directory_iterator(dir.path, ec)) + { + CHECK(entry.path().filename() == "not-a-dir"); + } + + app.stop(); +} + +TEST_CASE("request_body_file concurrent uploads get distinct paths", "[http][body_file]") +{ + TempDir dir; + SimpleApp app; + app.body_file_directory(dir.path.string()).max_body_size(1024 * 1024); + + CROW_ROUTE(app, "/keep") + .methods("POST"_method) + .body_file()([](const request& req) { + return req.take_body_file(); + }); + + auto server = app.bindaddr(LOCALHOST_ADDRESS).port(0).run_async(); + app.wait_for_server_start(); + const auto port = app.port(); + + TestClient a(port); + TestClient b(port); + a.send( + "POST /keep HTTP/1.1\r\n" + "Host: localhost\r\n" + "Content-Length: 8\r\n" + "\r\n" + "AAAA"); + b.send( + "POST /keep HTTP/1.1\r\n" + "Host: localhost\r\n" + "Content-Length: 8\r\n" + "\r\n" + "BBBB"); + + const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(2); + std::size_t nfiles = 0; + while (std::chrono::steady_clock::now() < deadline) + { + nfiles = 0; + std::error_code ec; + for (const auto& entry : std::filesystem::directory_iterator(dir.path, ec)) + { + if (!ec && entry.is_regular_file()) + ++nfiles; + } + if (nfiles >= 2) + break; + std::this_thread::sleep_for(std::chrono::milliseconds(20)); + } + REQUIRE(nfiles >= 2); + + a.send("aaaa"); + b.send("bbbb"); + const auto path_a = trim_crlf(http_body(a.receive())); + const auto path_b = trim_crlf(http_body(b.receive())); + CHECK(path_a != path_b); + CHECK(read_all(path_a) == "AAAAaaaa"); + CHECK(read_all(path_b) == "BBBBbbbb"); + std::filesystem::remove(path_a); + std::filesystem::remove(path_b); app.stop(); } From cffeec895867cbc0d1e82ae961754499aaee3e0e Mon Sep 17 00:00:00 2001 From: "Anton N. Petrov" Date: Wed, 2 Sep 2026 23:48:51 +0300 Subject: [PATCH 05/14] Rework the request body sink per review: shared handle, opt-in file header Addresses ssubbotin's review on PR #1235 (rework, keeping the seam): - request::body_sink is now a public shared_ptr instead of a parser-owned unique_ptr plus body_file_path/take_body_file(); every copy of the request shares the sink, so a handler can hand it off (e.g. to a deferred response) without the file being deleted early, and calling "keep" on a copy now actually reaches the underlying sink. - FileBodySink moves to a new opt-in header, crow/file_body_sink.h, never included by crow.h or crow/parser.h, so the core no longer needs RTTI (verified building a core-only program with -fno-rtti -DASIO_NO_TYPEID). .body_file()/app.body_file_directory() are gone; the file sink is now .body_sink(crow::FileBodySink::factory(dir)), with FileBodySink::from(), ->path() and ->keep() replacing has_body_file()/take_body_file(). - write()/finish() are now called inside catch(...): an uncaught exception used to unwind into the worker loop (or end a non-std::exception thread silently) instead of producing a 500. - A nullptr from a user factory now means "keep the body in req.body" instead of 500; FileBodySink's own open failure throws instead, so it still 500s. - remote_ip_address is set before the sink factory runs instead of after, so a factory can apply a per-peer policy. - The file is no longer unlinked after just the first chunk of a streamed response: do_write_sync() only clears the parser (and drops the connection's body_sink reference) on the last write of a response, not every chunk. - FileBodySink::factory() resolves its directory to an absolute path once and requires it to already exist (throws otherwise), instead of a silently-discarded create_directories() at builder time against whatever the cwd happens to be later; the resolved path is cached instead of calling temp_directory_path() per request. Uses mkostemp on POSIX and CreateFileW (path.wstring()) on Windows. - Deduplicated the three near-identical rule-lookup blocks in routing.h behind a single matched_rule() helper. - Generalized the "lingering close" fix from #1234 (drain instead of hard-closing on a rejection, to avoid RSTing a client still mid-upload) from the 413 path to the new sink-failure 500 path; confirmed the regression test hangs without it. Rewrote tests/body_file_tests.cpp for the new API and added the coverage ssubbotin flagged as missing: throwing write()/finish() (including a non-std::exception throw), the copy-trap regression, the early-cleanup timing regression, a chunked over-limit request on a sink route, the nullptr-factory fallback, and a per-request (post-setup) open failure. Updated docs/guides/body-file.md, routes.md, and the example to match. Full suite passes: 1133 assertions, 150 test cases. --- docs/guides/body-file.md | 101 ++++--- docs/guides/routes.md | 2 +- examples/example_body_file.cpp | 18 +- include/crow/app.h | 24 +- include/crow/body_sink.h | 177 +----------- include/crow/file_body_sink.h | 215 ++++++++++++++ include/crow/http_connection.h | 77 +++-- include/crow/http_request.h | 30 +- include/crow/parser.h | 54 ++-- include/crow/routing.h | 93 ++---- tests/body_file_tests.cpp | 504 ++++++++++++++++++++++++++------- 11 files changed, 840 insertions(+), 455 deletions(-) create mode 100644 include/crow/file_body_sink.h diff --git a/docs/guides/body-file.md b/docs/guides/body-file.md index c7275f1247..33c0c4282e 100644 --- a/docs/guides/body-file.md +++ b/docs/guides/body-file.md @@ -4,52 +4,72 @@ By default Crow stores a request body in `req.body` (`std::string`). That is the right default for JSON and form fields, but a large upload then occupies the same amount of RAM as the file. -A route can divert body bytes **while they arrive** to a sink. The built-in -sink is a unique file; you can also supply your own (flash, SD, a bounded -buffer) when there is no writable filesystem. +A route can divert body bytes **while they arrive** to a sink instead, via +`#!cpp .body_sink(factory)`. The built-in sink is a unique file +(`crow::FileBodySink`, an opt-in header — see below); you can also supply +your own (flash, SD, a bounded buffer) when there is no writable filesystem. The handler still runs only after the full body has been received. Writes run -synchronously on the connection's io thread. +synchronously on the connection's io thread, before the `Host` header check, +before middleware, and before `100 Continue`. A sink that throws, or returns +`false` from `write()`/`finish()`, ends the request as a 500 with the +connection closed; the route handler never runs. This is the request-side counterpart of returning a file with `response.set_static_file_info()`: the payload is never held as one string. Size limits are `app.max_body_size()` / `.max_body_size()` — see [Request body size](app.md#request-body-size). Over-limit requests are 413 -and the connection closes; no file is left behind. +and the connection closes; nothing the sink wrote is kept. ## File convenience ```cpp +#include "crow.h" +#include "crow/file_body_sink.h" // opt-in: not pulled in by crow.h + +crow::SimpleApp app; +app.max_body_size(64ull * 1024 * 1024); + CROW_ROUTE(app, "/upload") .methods(crow::HTTPMethod::Put, crow::HTTPMethod::Post) - .body_file()([](const crow::request& req) { - if (!req.has_body_file()) - return crow::response(500); - // req.body is empty; the bytes are in req.body_file_path - std::ifstream in(req.body_file_path, std::ios::binary); + .body_sink(crow::FileBodySink::factory("uploads")) // empty: system temp directory + ([](const crow::request& req) { + auto* file = crow::FileBodySink::from(req); + if (!file) + return crow::response(200); // e.g. an empty PUT: see "Empty bodies" below + // req.body is empty; the bytes are in file->path() + std::ifstream in(file->path(), std::ios::binary); // ... return crow::response(200); }); ``` -Pass a directory to `.body_file()` to override the app default for -that route. Crow always generates a unique file name, so concurrent requests -do not share a path. The descriptor is kept open until the body is complete; -the handler then reads the path. - -```cpp -app.body_file_directory("uploads"); // default: system temp directory -app.max_body_size(64ull * 1024 * 1024); -``` +`crow::FileBodySink::factory(directory)` resolves `directory` to an absolute +path once, at the point you call it, and throws +`std::filesystem::filesystem_error` if it does not already exist — Crow does +not create it for you, and does not fail silently. Crow always generates a +unique file name (POSIX: `mkostemp`; Windows: a retried `CREATE_NEW`), so +concurrent requests never share a path. The descriptor is kept open until the +body is complete; the handler then reads `file->path()`. -The file is deleted after the response is sent. Call `req.take_body_file()` -if the application will use the file after the handler returns (for example -after renaming it into place). +The file is deleted when the last `crow::request` copy that reached the +handler is destroyed — normally right after the response has been fully +sent. Call `file->keep()` if the application will use the file after that +(for example after renaming it into place); the destructor then only closes +the descriptor. A client that disconnects before the body is complete never reaches the -handler; the partial file is removed. Bodyless GET/HEAD (and `Content-Length: 0`) -do not create a file. +handler; the partial file is removed. Bodyless GET/HEAD (and +`Content-Length: 0`) do not create a file — `crow::FileBodySink::from(req)` +returns `nullptr` in that case, same as for any route where the body didn't +go to this sink. + +`crow::FileBodySink::from()` uses `dynamic_cast` and so needs RTTI; that is +why this header is opt-in and never pulled in by `crow.h` or +`crow/parser.h`. A build with `-fno-rtti` (or asio's `-DASIO_NO_TYPEID`) can +use `crow::BodySink` directly and simply not include +`crow/file_body_sink.h`. ## Custom sink @@ -61,7 +81,7 @@ struct FlashSink : crow::BodySink { CROW_ROUTE(app, "/upload") .methods(crow::HTTPMethod::Post) - .body_sink([](const crow::request& req) { + .body_sink([](const crow::request& req) -> std::unique_ptr { return std::make_unique(req); }) ([](const crow::request&) { @@ -69,18 +89,33 @@ CROW_ROUTE(app, "/upload") }); ``` -The factory runs after headers, before the body. Each request gets its own -sink. `req.body` stays empty. +The factory runs once per request, after headers and before the body; `req` +carries the method, url, headers and `remote_ip_address` (already set, so a +factory can apply a per-peer policy), but not `body` or `body_sink` yet. +Returning `nullptr` from the factory means "no sink for this request" — the +body goes to `req.body` as usual, it is not an error. Throwing from the +factory, or from `write()`/`finish()`, is a 500. + +Calling `.body_sink(...)` again on the same route replaces the factory +(last-call-wins). + +## Empty bodies -Do not set both `.body_file()` and `.body_sink()` on the same route; the last -call wins. +A `.body_sink(...)` route only opens a sink when a body is actually coming +(`Content-Length` > 0, or chunked framing); a bodyless request never calls +the factory, so `req.body_sink` (and `crow::FileBodySink::from(req)`) is +`nullptr`. Handle that case explicitly — see the example above. Note that an +empty **chunked** body (`0\r\n\r\n`) still counts as "a body is coming" and +does open the sink (into an empty file, for the built-in one), unlike +`Content-Length: 0`. ## What this is not -`.body_file()` / `.body_sink()` store the **raw** request body. -`multipart/form-data` is still parsed from `req.body` by -`crow::multipart::message`. Saving individual multipart parts as they arrive -is a separate feature. +`.body_sink(...)` stores the **raw** request body. `multipart/form-data` is +parsed from `req.body` by `crow::multipart::message`; a route with a sink +diverts every body regardless of `Content-Type`, so `req.body` is empty and +`crow::multipart::message` cannot be used on it. Saving individual multipart +parts as they arrive is a separate feature. The route handler is not invoked per chunk. Incremental processing belongs in `BodySink::write`. diff --git a/docs/guides/routes.md b/docs/guides/routes.md index 68a2dcb0af..ad9dc11a71 100644 --- a/docs/guides/routes.md +++ b/docs/guides/routes.md @@ -84,7 +84,7 @@ You can also access the URL parameters in the handler using `#!cpp req.url_param To cap the request body for one route, use `#!cpp .max_body_size(bytes)` (see [Request body size](app.md#request-body-size)). Without a per-route value, `#!cpp app.max_body_size()` applies, including on 404 and 405. -To write a large request body to a file (or a custom sink) while it is received instead of filling `req.body`, use `#!cpp .body_file()` or `#!cpp .body_sink(...)`. See [Request body files](body-file.md). +To write a large request body to a file (or a custom sink) while it is received instead of filling `req.body`, use `#!cpp .body_sink(...)`. See [Request body files](body-file.md). !!! note "Note      [:octicons-feed-tag-16: master](https://github.com/CrowCpp/Crow)" diff --git a/examples/example_body_file.cpp b/examples/example_body_file.cpp index 0a3d763935..9aeed82c2f 100644 --- a/examples/example_body_file.cpp +++ b/examples/example_body_file.cpp @@ -1,4 +1,5 @@ #include "crow.h" +#include "crow/file_body_sink.h" #include #include @@ -7,16 +8,23 @@ int main() { crow::SimpleApp app; - app.body_file_directory("uploads").max_body_size(64ull * 1024 * 1024); + app.max_body_size(64ull * 1024 * 1024); // PUT or POST the raw body; Crow writes it to disk as the bytes arrive. CROW_ROUTE(app, "/upload") .methods(crow::HTTPMethod::Put, crow::HTTPMethod::Post) - .body_file()([](const crow::request& req) { - if (!req.has_body_file()) - return crow::response(500); + .body_sink(crow::FileBodySink::factory("uploads"))([](const crow::request& req) { + auto* file = crow::FileBodySink::from(req); + if (!file) + { + // No incoming body (e.g. an empty PUT): the sink never opened. + crow::json::wvalue reply; + reply["bytes"] = 0; + reply["preview"] = ""; + return crow::response(reply); + } - std::ifstream in(req.body_file_path, std::ios::binary); + std::ifstream in(file->path(), std::ios::binary); const std::string contents((std::istreambuf_iterator(in)), std::istreambuf_iterator()); crow::json::wvalue reply; diff --git a/include/crow/app.h b/include/crow/app.h index 69eb48875d..8dec28545b 100644 --- a/include/crow/app.h +++ b/include/crow/app.h @@ -23,7 +23,6 @@ #include #include #include -#include #include #include #include @@ -546,30 +545,10 @@ namespace crow return res_stream_threshold_; } - /// \brief Directory used for request body files created by routes that call `body_file()`. - /// - /// Empty (the default) uses the system temporary directory. Unique names are generated per request. - /// The directory is created when this is set. - self_t& body_file_directory(std::string directory) - { - body_file_directory_ = std::move(directory); - if (!body_file_directory_.empty()) - { - std::error_code ec; - std::filesystem::create_directories(body_file_directory_, ec); - } - return *this; - } - - const std::string& body_file_directory() const - { - return body_file_directory_; - } - /// \brief Create a body sink for the matched route, or nullptr for in-memory `req.body`. std::unique_ptr make_body_sink(const routing_handle_result& found, const request& req) const { - return router_.make_body_sink(found, req, body_file_directory_); + return router_.make_body_sink(found, req); } bool uses_body_sink(const routing_handle_result& found) const @@ -977,7 +956,6 @@ namespace crow detail::socket::tcp_socket_options tcp_socket_options_{}; detail::socket::tcp_socket_options websocket_tcp_socket_options_{}; size_t res_stream_threshold_ = 1048576; - std::string body_file_directory_; Router router_; bool static_routes_added_{false}; diff --git a/include/crow/body_sink.h b/include/crow/body_sink.h index b3c2f14fd9..366de441ba 100644 --- a/include/crow/body_sink.h +++ b/include/crow/body_sink.h @@ -1,31 +1,20 @@ #pragma once -#include #include -#include -#include #include #include -#include -#include - -#ifndef _WIN32 -#include -#include -#include -#else -#ifndef WIN32_LEAN_AND_MEAN -#define WIN32_LEAN_AND_MEAN -#endif -#include -#endif - -#include "crow/http_request.h" -#include "crow/utility.h" namespace crow { + struct request; + /// Destination for request body bytes as they arrive from the parser. + /// + /// `write()`/`finish()` run on the connection's io thread, at headers-complete, + /// before the `Host` check, before middleware, and before `100 Continue`. A + /// thrown exception or a `false` return is a 500 with the connection closed; + /// the route handler never runs. If `finish()` was never called, the body did + /// not arrive in full (client disconnect, over-limit body, or a write failure). struct BodySink { virtual ~BodySink() = default; @@ -33,152 +22,8 @@ namespace crow virtual bool finish() = 0; }; + /// `req` carries method, url, headers and `remote_ip_address` (set before the + /// factory runs); `req.body` and `req.body_sink` are not yet populated. + /// Returning `nullptr` keeps the body in `req.body` instead of diverting it. using BodySinkFactory = std::function(const request&)>; - - /// File-backed sink used by `.body_file()`. Owns the descriptor until `finish()`. - class FileBodySink : public BodySink - { - public: - static std::unique_ptr create(const std::string& directory) - { - std::error_code ec; - std::filesystem::path dir = directory.empty() ? std::filesystem::temp_directory_path(ec) : - std::filesystem::path(directory); - if (ec) - return nullptr; - - for (int attempt = 0; attempt < 128; ++attempt) - { - std::string name; - try - { - name = "crow-body-" + utility::random_alphanum(16); - } - catch (...) - { - return nullptr; - } - const auto path = dir / name; -#ifndef _WIN32 - const int fd = ::open(path.string().c_str(), O_CREAT | O_EXCL | O_WRONLY | O_CLOEXEC, 0600); - if (fd >= 0) - return std::unique_ptr(new FileBodySink(path.string(), fd)); - if (errno != EEXIST) - return nullptr; -#else - SECURITY_ATTRIBUTES sa{}; - sa.nLength = sizeof(sa); - sa.bInheritHandle = FALSE; - HANDLE handle = CreateFileA(path.string().c_str(), GENERIC_WRITE, 0, &sa, CREATE_NEW, - FILE_ATTRIBUTE_NORMAL, nullptr); - if (handle != INVALID_HANDLE_VALUE) - return std::unique_ptr(new FileBodySink(path.string(), handle)); - if (GetLastError() != ERROR_FILE_EXISTS) - return nullptr; -#endif - } - return nullptr; - } - - FileBodySink(const FileBodySink&) = delete; - FileBodySink& operator=(const FileBodySink&) = delete; - - ~FileBodySink() override - { - close_handle(); - if (!persist_ && !path_.empty()) - { - std::error_code ec; - std::filesystem::remove(path_, ec); - } - } - - bool write(const char* data, std::size_t length) override - { -#ifndef _WIN32 - if (fd_ < 0) - return false; - std::size_t written = 0; - while (written < length) - { - const ssize_t n = ::write(fd_, data + written, length - written); - if (n < 0) - { - if (errno == EINTR) - continue; - return false; - } - if (n == 0) - return false; - written += static_cast(n); - } - return true; -#else - if (handle_ == INVALID_HANDLE_VALUE) - return false; - std::size_t written = 0; - while (written < length) - { - DWORD n = 0; - const DWORD chunk = static_cast( - std::min(length - written, static_cast(MAXDWORD))); - if (!WriteFile(handle_, data + written, chunk, &n, nullptr) || n == 0) - return false; - written += n; - } - return true; -#endif - } - - bool finish() override - { - return close_handle(); - } - - const std::string& path() const - { - return path_; - } - - void persist() - { - persist_ = true; - } - - private: -#ifndef _WIN32 - FileBodySink(std::string path, int fd): - path_(std::move(path)), fd_(fd) - {} -#else - FileBodySink(std::string path, HANDLE handle): - path_(std::move(path)), handle_(handle) - {} -#endif - - bool close_handle() - { -#ifndef _WIN32 - if (fd_ < 0) - return true; - const int fd = fd_; - fd_ = -1; - return ::close(fd) == 0; -#else - if (handle_ == INVALID_HANDLE_VALUE) - return true; - const HANDLE handle = handle_; - handle_ = INVALID_HANDLE_VALUE; - return CloseHandle(handle) != 0; -#endif - } - - std::string path_; - bool persist_{false}; -#ifndef _WIN32 - int fd_{-1}; -#else - HANDLE handle_{INVALID_HANDLE_VALUE}; -#endif - }; } // namespace crow diff --git a/include/crow/file_body_sink.h b/include/crow/file_body_sink.h new file mode 100644 index 0000000000..4f226a40a5 --- /dev/null +++ b/include/crow/file_body_sink.h @@ -0,0 +1,215 @@ +#pragma once + +// Opt-in: not included by "crow.h" or "crow/parser.h". A build that has no +// writable filesystem, or that disables RTTI (`-fno-rtti`, `-DASIO_NO_TYPEID`), +// can use `crow::BodySink` directly and never pull this header in. + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#ifndef _WIN32 +#include +#include +#include +#else +#ifndef WIN32_LEAN_AND_MEAN +#define WIN32_LEAN_AND_MEAN +#endif +#include +#include "crow/utility.h" +#endif + +#include "crow/body_sink.h" +#include "crow/http_request.h" + +namespace crow +{ + /// File-backed `BodySink`. Give it to `.body_sink(...)` via `factory()`; read + /// the result back from the handler with `from()`. + /// + /// The descriptor is created `O_CREAT|O_EXCL|O_WRONLY` (Windows: `CREATE_NEW`), + /// mode 0600, and kept open until `finish()`. The file is unlinked when the + /// sink is destroyed, unless `keep()` was called; since `request::body_sink` + /// is a `shared_ptr`, that is when the last copy of the request goes away. + class FileBodySink : public BodySink + { + public: + /// A `BodySinkFactory` that writes each request's body to a uniquely + /// named file under `directory`. `directory` is resolved to an absolute + /// path once, here, and must already exist; empty uses the system + /// temporary directory. Throws `std::filesystem::filesystem_error` if it + /// cannot be resolved — call this while setting up the route, not per + /// request, so a bad directory fails loudly at startup. + static BodySinkFactory factory(std::string directory = {}) + { + auto resolved = std::make_shared(resolve_directory(directory)); + return [resolved](const request&) -> std::unique_ptr { + return create(*resolved); + }; + } + + /// The `FileBodySink` behind `req.body_sink`, or `nullptr` if the route + /// used a different sink (or none). + static FileBodySink* from(const request& req) + { + return dynamic_cast(req.body_sink.get()); + } + + FileBodySink(const FileBodySink&) = delete; + FileBodySink& operator=(const FileBodySink&) = delete; + + ~FileBodySink() override + { + close_handle(); + if (!keep_ && !path_.empty()) + { + std::error_code ec; + std::filesystem::remove(path_, ec); + } + } + + bool write(const char* data, std::size_t length) override + { +#ifndef _WIN32 + if (fd_ < 0) + return false; + std::size_t written = 0; + while (written < length) + { + const ssize_t n = ::write(fd_, data + written, length - written); + if (n < 0) + { + if (errno == EINTR) + continue; + return false; + } + if (n == 0) + return false; + written += static_cast(n); + } + return true; +#else + if (handle_ == INVALID_HANDLE_VALUE) + return false; + std::size_t written = 0; + while (written < length) + { + DWORD n = 0; + const DWORD chunk = static_cast( + std::min(length - written, static_cast(MAXDWORD))); + if (!WriteFile(handle_, data + written, chunk, &n, nullptr) || n == 0) + return false; + written += n; + } + return true; +#endif + } + + bool finish() override + { + return close_handle(); + } + + /// The path of the file, valid once the factory has created it. + const std::string& path() const + { + return path_; + } + + /// Do not delete the file when this sink is destroyed (i.e. when the + /// last copy of the request that reached the handler goes away). + void keep() + { + keep_ = true; + } + + private: + static std::string resolve_directory(const std::string& directory) + { + std::error_code ec; + std::filesystem::path dir = directory.empty() ? + std::filesystem::temp_directory_path(ec) : + std::filesystem::absolute(directory, ec); + if (ec) + throw std::filesystem::filesystem_error("body_file directory unavailable", directory, ec); + if (!std::filesystem::is_directory(dir, ec) || ec) + throw std::filesystem::filesystem_error( + "body_file directory does not exist", dir, + std::make_error_code(std::errc::no_such_file_or_directory)); + return dir.string(); + } + + static std::unique_ptr create(const std::string& directory) + { +#ifndef _WIN32 + std::string tmpl = (std::filesystem::path(directory) / "crow-body-XXXXXX").string(); + std::vector buf(tmpl.begin(), tmpl.end()); + buf.push_back('\0'); + const int fd = ::mkostemp(buf.data(), O_CLOEXEC); + if (fd < 0) + throw std::system_error(errno, std::generic_category(), "crow::FileBodySink: mkostemp"); + return std::unique_ptr(new FileBodySink(std::string(buf.data()), fd)); +#else + const std::filesystem::path dir(directory); + for (int attempt = 0; attempt < 128; ++attempt) + { + const std::string name = "crow-body-" + utility::random_alphanum(16); + const auto path = dir / name; + SECURITY_ATTRIBUTES sa{}; + sa.nLength = sizeof(sa); + sa.bInheritHandle = FALSE; + HANDLE handle = CreateFileW(path.wstring().c_str(), GENERIC_WRITE, 0, &sa, CREATE_NEW, + FILE_ATTRIBUTE_NORMAL, nullptr); + if (handle != INVALID_HANDLE_VALUE) + return std::unique_ptr(new FileBodySink(path.string(), handle)); + if (GetLastError() != ERROR_FILE_EXISTS) + throw std::system_error(static_cast(GetLastError()), std::system_category(), + "crow::FileBodySink: CreateFileW"); + } + throw std::runtime_error("crow::FileBodySink: could not find a unique file name"); +#endif + } + +#ifndef _WIN32 + FileBodySink(std::string path, int fd): + path_(std::move(path)), fd_(fd) + {} +#else + FileBodySink(std::string path, HANDLE handle): + path_(std::move(path)), handle_(handle) + {} +#endif + + bool close_handle() + { +#ifndef _WIN32 + if (fd_ < 0) + return true; + const int fd = fd_; + fd_ = -1; + return ::close(fd) == 0; +#else + if (handle_ == INVALID_HANDLE_VALUE) + return true; + const HANDLE handle = handle_; + handle_ = INVALID_HANDLE_VALUE; + return CloseHandle(handle) != 0; +#endif + } + + std::string path_; + bool keep_{false}; +#ifndef _WIN32 + int fd_{-1}; +#else + HANDLE handle_{INVALID_HANDLE_VALUE}; +#endif + }; +} // namespace crow diff --git a/include/crow/http_connection.h b/include/crow/http_connection.h index 20ed2397c7..fbc153714e 100644 --- a/include/crow/http_connection.h +++ b/include/crow/http_connection.h @@ -125,6 +125,8 @@ namespace crow int handle_header() { body_error_status_ = 0; + // Set before a body_sink factory can run, so a sink can apply a per-peer policy. + req_.remote_ip_address = adaptor_.address(); const uint64_t limit = handler_->effective_max_body_size(*routing_handle_result_); parser_.set_max_body_size(limit); @@ -141,11 +143,7 @@ namespace crow { try { - if (!parser_.open_body_sink(handler_->make_body_sink(*routing_handle_result_, req_))) - { - body_error_status_ = status::INTERNAL_SERVER_ERROR; - return 1; - } + parser_.open_body_sink(handler_->make_body_sink(*routing_handle_result_, req_)); } catch (...) { @@ -153,12 +151,15 @@ namespace crow return 1; } } - else if (limit != UINT64_MAX && - parser_.content_length != CROW_ULLONG_MAX && - parser_.content_length <= req_.body.max_size()) + if (!req_.body_sink && + limit != UINT64_MAX && + parser_.content_length != CROW_ULLONG_MAX && + parser_.content_length <= req_.body.max_size()) { // Finite cap only: never allocate from an untrusted Content-Length - // when unlimited, and never on a sink route (req.body stays empty). + // when unlimited, and never when the body is going to a sink + // (req.body stays empty, whether the route always uses one or the + // factory declined this particular request). req_.body.reserve(static_cast(parser_.content_length)); } @@ -208,7 +209,6 @@ namespace crow req_.middleware_context = static_cast(&ctx_); req_.middleware_container = static_cast(middlewares_); req_.io_context = &adaptor_.get_io_context(); - req_.remote_ip_address = adaptor_.address(); add_keep_alive_ = req_.keep_alive; close_connection_ = req_.close_connection; @@ -372,7 +372,7 @@ namespace crow while (is.gcount() > 0) { buffers[0] = asio::buffer(buf, is.gcount()); - error_code ec = do_write_sync(buffers); + error_code ec = do_write_sync(buffers, /*clear_parser=*/false); if (ec) { CROW_LOG_ERROR << ec << " - buffer write error happened while sending content of file " << res.file_info.path << ". Writing stopped premature."; @@ -429,7 +429,7 @@ namespace crow { size_t to_transfer = CROW_MIN(16384UL, length - transferred); buffers[0] = asio::const_buffer(data + transferred, to_transfer); - ec = do_write_sync(buffers); + ec = do_write_sync(buffers, /*clear_parser=*/false); if (ec) { CROW_LOG_ERROR << ec << " - " << transferred << " - buffer write error happened while sending response. Writing stopped premature."; break; @@ -471,9 +471,21 @@ namespace crow { self->cancel_deadline_timer(); self->parser_.done(); - self->adaptor_.shutdown_read(); - self->adaptor_.close(); CROW_LOG_DEBUG << self << " from read(1) with description: \"" << http_errno_description(static_cast(self->parser_.http_errno)) << '\"'; + if (self->body_error_status_) + { + // The rejection response (413, or a sink write/finish + // failure's 500) is already written; shut down the write + // side and drain whatever the client still sends instead + // of closing on unread bytes, which can RST a peer that + // is still mid-upload before it reads the response. + self->linger_close(); + } + else + { + self->adaptor_.shutdown_read(); + self->adaptor_.close(); + } } else if (self->close_connection_) { @@ -494,6 +506,34 @@ namespace crow }); } + /// Shut down the write side (FIN, not RST) after a rejection response and + /// discard whatever the client still sends, instead of closing on unread + /// bytes. Bounded by the same deadline timer a slow client already gets. + void linger_close() + { + adaptor_.shutdown_write(); + start_deadline(); + do_linger_read(); + } + + void do_linger_read() + { + auto self = this->shared_from_this(); + adaptor_.socket().async_read_some( + asio::buffer(buffer_), + [self](const error_code& ec, std::size_t /*bytes_transferred*/) { + if (!ec && self->adaptor_.is_open()) + { + self->do_linger_read(); + } + else + { + self->cancel_deadline_timer(); + self->adaptor_.close(); + } + }); + } + void do_write() { auto self = this->shared_from_this(); @@ -527,7 +567,12 @@ namespace crow }); } - inline error_code do_write_sync(std::vector& buffers) + /// `clear_parser` must stay false for every call but the last one writing + /// a single response, static file, or streamed body: clearing resets + /// `req` (dropping the connection's `body_sink` reference, e.g. deleting + /// a `FileBodySink`'s file) and must not happen until that whole response + /// has been written, not after its first chunk. + inline error_code do_write_sync(std::vector& buffers, bool clear_parser = true) { error_code ec; asio::write(adaptor_.socket(), buffers, ec); @@ -543,7 +588,7 @@ namespace crow { this->continue_requested = false; } - else + else if (clear_parser) { this->parser_.clear(); } diff --git a/include/crow/http_request.h b/include/crow/http_request.h index 7c736ebc84..0874f680ae 100644 --- a/include/crow/http_request.h +++ b/include/crow/http_request.h @@ -10,6 +10,7 @@ #endif #include +#include #include "crow/common.h" #include "crow/ci_map.h" @@ -21,6 +22,8 @@ namespace crow // NOTE: Already documented in "crow/app.h" namespace asio = boost::asio; #endif + struct BodySink; + /// Remove CR (\r) and LF (\n) characters from a header name or value to prevent header injection. inline void sanitize_header_value(std::string& s) { @@ -51,7 +54,11 @@ namespace crow // NOTE: Already documented in "crow/app.h" query_string url_params; ///< The parameters associated with the request. (everything after the `?` in the URL) ci_map headers; std::string body; - std::string body_file_path; ///< Path of the file that received the body, if the route used `body_file()`. Empty when the body is in `body`. + /// The sink the route diverted the body to (`.body_sink(...)`), if any. Set + /// before the handler runs; every copy of the request shares it, so it (and + /// whatever it owns, e.g. a `FileBodySink`'s file) lives as long as the last + /// copy. `nullptr` when the body is in `body` instead. + std::shared_ptr body_sink; std::string remote_ip_address; ///< The IP address from which the request was sent. unsigned char http_ver_major, http_ver_minor; bool keep_alive, ///< Whether or not the server should send a `connection: Keep-Alive` header to the client. @@ -62,9 +69,6 @@ namespace crow // NOTE: Already documented in "crow/app.h" void* middleware_container{}; asio::io_context* io_context{}; - template - friend struct HTTPParser; - /// Construct an empty request. (sets the method to `GET`) request(): method(HTTPMethod::Get) @@ -96,25 +100,12 @@ namespace crow // NOTE: Already documented in "crow/app.h" /// /// This is meant to be used with requests of type "application/x-www-form-urlencoded". - /// It reads `body`; it does not open `body_file_path`. + /// It reads `body`; a route with `.body_sink(...)` leaves it empty. const query_string get_body_params() const { return query_string(body, false); } - /// True when the request body was written to `body_file_path` instead of `body`. - bool has_body_file() const - { - return !body_file_path.empty(); - } - - /// Keep the body file and return its path. The parser will not delete it after the response. - std::string take_body_file() const - { - persist_body_file_ = true; - return body_file_path; - } - /// Send data to whoever made this request with a completion handler and return immediately. template void post(CompletionHandler handler) @@ -128,8 +119,5 @@ namespace crow // NOTE: Already documented in "crow/app.h" { asio::dispatch(io_context, handler); } - - private: - mutable bool persist_body_file_{false}; }; } // namespace crow diff --git a/include/crow/parser.h b/include/crow/parser.h index c537a27e8f..6d40bb445b 100644 --- a/include/crow/parser.h +++ b/include/crow/parser.h @@ -98,9 +98,18 @@ namespace crow self->handler_->reject_body(status::PAYLOAD_TOO_LARGE); return 1; } - if (self->body_sink_) + if (self->req.body_sink) { - if (!self->body_sink_->write(at, length)) + bool ok = false; + try + { + ok = self->req.body_sink->write(at, length); + } + catch (...) + { + ok = false; + } + if (!ok) { self->handler_->reject_body(status::INTERNAL_SERVER_ERROR); return 1; @@ -118,11 +127,17 @@ namespace crow HTTPParser* self = static_cast(self_); self->message_complete = true; - if (self->body_sink_) + if (self->req.body_sink) { - const bool ok = self->body_sink_->finish(); - if (auto* file = dynamic_cast(self->body_sink_.get())) - self->req.body_file_path = file->path(); + bool ok = false; + try + { + ok = self->req.body_sink->finish(); + } + catch (...) + { + ok = false; + } if (!ok) { self->handler_->reject_body(status::INTERNAL_SERVER_ERROR); @@ -140,11 +155,6 @@ namespace crow http_parser_init(this); } - ~HTTPParser() - { - cleanup_body_sink(); - } - // return false on error /// Parse a buffer into the different sections of an HTTP request. bool feed(const char* buffer, int length) @@ -178,7 +188,6 @@ namespace crow void clear() { - cleanup_body_sink(); req = crow::request(); header_field.clear(); header_value.clear(); @@ -195,13 +204,11 @@ namespace crow max_body_size_ = bytes; } - bool open_body_sink(std::unique_ptr sink) + /// `sink` may be null: the factory declining means "keep the body in + /// `req.body`", not an error. A factory that wants a 500 should throw. + void open_body_sink(std::unique_ptr sink) { - cleanup_body_sink(); - if (!sink) - return false; - body_sink_ = std::move(sink); - return true; + req.body_sink = std::move(sink); } bool has_incoming_body() const @@ -251,21 +258,10 @@ namespace crow request req; private: - void cleanup_body_sink() - { - if (auto* file = dynamic_cast(body_sink_.get())) - { - if (req.persist_body_file_) - file->persist(); - } - body_sink_.reset(); - } - int header_building_state = 0; bool message_complete = false; uint64_t body_bytes_{0}; uint64_t max_body_size_{UINT64_MAX}; - std::unique_ptr body_sink_; std::string header_field; std::string header_value; diff --git a/include/crow/routing.h b/include/crow/routing.h index 6e69fd0d1c..47eac80980 100644 --- a/include/crow/routing.h +++ b/include/crow/routing.h @@ -8,8 +8,6 @@ #include #include #include -#include -#include #include #include @@ -165,8 +163,6 @@ namespace crow // NOTE: Already documented in "crow/app.h" bool added_{false}; uint64_t max_body_size_{UINT64_MAX}; bool max_body_size_override_{false}; - bool body_file_{false}; - std::string body_file_directory_; BodySinkFactory body_sink_factory_; std::unique_ptr rule_to_upgrade_; @@ -635,35 +631,17 @@ namespace crow // NOTE: Already documented in "crow/app.h" return static_cast(*this); } - /// Write the request body to a user-provided sink while it is received. + /// Write the request body to a sink while it is received, instead of + /// filling `req.body`. /// - /// The factory runs after headers, before the body. The route handler still - /// runs only after the full body has been received. `req.body` stays empty. - /// Replaces `.body_file()` if both are set on the same route. + /// The factory runs after headers, before the body; see `BodySinkFactory`. + /// The route handler still runs only after the full body has been + /// received. Calling this again (last-call-wins) replaces the factory. + /// `crow::FileBodySink::factory(directory)` (`crow/file_body_sink.h`, + /// not pulled in by this header) is the built-in file-backed sink. self_t& body_sink(BodySinkFactory factory) { static_cast(this)->body_sink_factory_ = std::move(factory); - static_cast(this)->body_file_ = false; - return static_cast(*this); - } - - /// Write the request body to a unique file while it is received. - /// - /// `req.body` stays empty. The handler reads `req.body_file_path`. - /// An empty `directory` uses `app.body_file_directory()`, or the system - /// temporary directory. The file is deleted after the response unless the - /// handler calls `req.take_body_file()`. - /// Replaces `.body_sink()` if both are set on the same route. - self_t& body_file(std::string directory = {}) - { - static_cast(this)->body_file_ = true; - static_cast(this)->body_sink_factory_ = {}; - static_cast(this)->body_file_directory_ = std::move(directory); - if (!static_cast(this)->body_file_directory_.empty()) - { - std::error_code ec; - std::filesystem::create_directories(static_cast(this)->body_file_directory_, ec); - } return static_cast(*this); } }; @@ -1890,20 +1868,27 @@ namespace crow // NOTE: Already documented in "crow/app.h" return blueprints_; } + /// The rule a `handle_initial()` result matched, or `nullptr` for 404, + /// 405, a slash-redirect, or an unmatched `OPTIONS`. + const BaseRule* matched_rule(const routing_handle_result& found) const + { + if (found.catch_all || found.rule_index <= RULE_SPECIAL_REDIRECT_SLASH) + return nullptr; + if (found.method >= HTTPMethod::InternalMethodCount) + return nullptr; + const auto& rules = per_methods_[static_cast(found.method)].rules; + if (found.rule_index >= rules.size()) + return nullptr; + return rules[found.rule_index]; + } + /// Effective request body limit for a `handle_initial()` result. /// /// 404, 405, slash-redirect, and unmatched OPTIONS use `app_default`. /// A matched rule uses its override when set, otherwise `app_default`. uint64_t effective_max_body_size(const routing_handle_result& found, uint64_t app_default) const { - if (found.catch_all || found.rule_index <= RULE_SPECIAL_REDIRECT_SLASH) - return app_default; - if (found.method >= HTTPMethod::InternalMethodCount) - return app_default; - const auto& rules = per_methods_[static_cast(found.method)].rules; - if (found.rule_index >= rules.size()) - return app_default; - const BaseRule* rule = rules[found.rule_index]; + const BaseRule* rule = matched_rule(found); if (!rule || !rule->max_body_size_override_) return app_default; return rule->max_body_size_; @@ -1911,38 +1896,16 @@ namespace crow // NOTE: Already documented in "crow/app.h" bool uses_body_sink(const routing_handle_result& found) const { - if (found.catch_all || found.rule_index <= RULE_SPECIAL_REDIRECT_SLASH) - return false; - if (found.method >= HTTPMethod::InternalMethodCount) - return false; - const auto& rules = per_methods_[static_cast(found.method)].rules; - if (found.rule_index >= rules.size()) - return false; - const BaseRule* rule = rules[found.rule_index]; - return rule && (rule->body_file_ || static_cast(rule->body_sink_factory_)); + const BaseRule* rule = matched_rule(found); + return rule && static_cast(rule->body_sink_factory_); } - std::unique_ptr make_body_sink(const routing_handle_result& found, const request& req, - const std::string& app_directory) const + std::unique_ptr make_body_sink(const routing_handle_result& found, const request& req) const { - if (found.catch_all || found.rule_index <= RULE_SPECIAL_REDIRECT_SLASH) - return nullptr; - if (found.method >= HTTPMethod::InternalMethodCount) + const BaseRule* rule = matched_rule(found); + if (!rule || !rule->body_sink_factory_) return nullptr; - const auto& rules = per_methods_[static_cast(found.method)].rules; - if (found.rule_index >= rules.size()) - return nullptr; - const BaseRule* rule = rules[found.rule_index]; - if (!rule) - return nullptr; - if (rule->body_sink_factory_) - return rule->body_sink_factory_(req); - if (rule->body_file_) - { - const std::string& dir = rule->body_file_directory_.empty() ? app_directory : rule->body_file_directory_; - return FileBodySink::create(dir); - } - return nullptr; + return rule->body_sink_factory_(req); } std::function& exception_handler() diff --git a/tests/body_file_tests.cpp b/tests/body_file_tests.cpp index db03c83103..25e02388ce 100644 --- a/tests/body_file_tests.cpp +++ b/tests/body_file_tests.cpp @@ -19,6 +19,7 @@ #include "catch2/catch_all.hpp" #include "crow.h" +#include "crow/file_body_sink.h" using namespace crow; @@ -127,6 +128,17 @@ namespace asio::write(socket_, asio::buffer(data, size)); } + // Reads exactly `size` more bytes into an internal buffer without + // waiting for the response to complete. Used to pause a client + // mid-response and observe server-side state while it is still + // streaming. + std::string read_some(std::size_t size) + { + std::string out(size, '\0'); + asio::read(socket_, asio::buffer(out)); + return out; + } + std::string receive() { std::string response; @@ -189,6 +201,26 @@ namespace bool finish() override { return false; } }; + struct ThrowingWriteSink : BodySink + { + bool write(const char*, std::size_t) override { throw std::runtime_error("write blew up"); } + bool finish() override { return true; } + }; + + struct ThrowingFinishSink : BodySink + { + bool write(const char*, std::size_t) override { return true; } + bool finish() override { throw std::runtime_error("finish blew up"); } + }; + + // Throws something that is not a std::exception, to exercise the sink's + // catch(...) boundary rather than a catch(std::exception&). + struct NonStdThrowingWriteSink : BodySink + { + bool write(const char*, std::size_t) override { throw 42; } + bool finish() override { return true; } + }; + std::string trim_crlf(std::string s) { while (!s.empty() && (s.back() == '\n' || s.back() == '\r')) @@ -201,36 +233,41 @@ TEST_CASE("request_body_file", "[http][body_file]") { TempDir dir; SimpleApp app; - app.body_file_directory(dir.path.string()).max_body_size(1024 * 1024); + app.max_body_size(1024 * 1024); CROW_ROUTE(app, "/memory") .methods("POST"_method)([](const request& req) { - return std::string(req.has_body_file() ? "file" : "memory") + ':' + req.body; + return std::string(FileBodySink::from(req) ? "file" : "memory") + ':' + req.body; }); CROW_ROUTE(app, "/upload") .methods("POST"_method) - .body_file()([](const request& req) { - if (!req.has_body_file()) + .body_sink(FileBodySink::factory(dir.path.string()))([](const request& req) { + auto* file = FileBodySink::from(req); + if (!file) return std::string("nobody:") + req.body; - return std::string("file:") + read_all(req.body_file_path); + return std::string("file:") + read_all(file->path()); }); CROW_ROUTE(app, "/keep") .methods("POST"_method) - .body_file()([](const request& req) { - return req.take_body_file(); + .body_sink(FileBodySink::factory(dir.path.string()))([](const request& req) { + auto* file = FileBodySink::from(req); + file->keep(); + return file->path(); }); + const auto custom_dir = dir.path / "route-dir"; + std::filesystem::create_directories(custom_dir); CROW_ROUTE(app, "/custom") .methods("POST"_method) - .body_file((dir.path / "route-dir").string())([](const request& req) { - return req.body_file_path; + .body_sink(FileBodySink::factory(custom_dir.string()))([](const request& req) { + return FileBodySink::from(req)->path(); }); CROW_ROUTE(app, "/getfile") - .body_file()([](const request& req) { - return req.has_body_file() ? "file" : "nobody"; + .body_sink(FileBodySink::factory(dir.path.string()))([](const request& req) { + return FileBodySink::from(req) ? "file" : "nobody"; }); auto server = app.bindaddr(LOCALHOST_ADDRESS).port(0).run_async(); @@ -292,6 +329,19 @@ TEST_CASE("request_body_file", "[http][body_file]") CHECK(no_crow_body_files(dir.path)); } + { + // An empty *chunked* body still counts as an incoming body and opens + // the sink, unlike Content-Length: 0. + TestClient client(port); + client.send( + "POST /upload HTTP/1.1\r\n" + "Host: localhost\r\n" + "Transfer-Encoding: chunked\r\n" + "\r\n" + "0\r\n\r\n"); + CHECK(http_body(client.receive()) == "file:"); + } + { TestClient client(port); client.send( @@ -328,10 +378,12 @@ TEST_CASE("request_body_file", "[http][body_file]") { const auto response = post("/custom", "Z"); const auto custom_path = trim_crlf(http_body(response)); - CHECK(custom_path.find((dir.path / "route-dir").string()) != std::string::npos); + CHECK(custom_path.find(custom_dir.string()) != std::string::npos); } { + // Keep-alive: two uploads reusing the same socket both work, and the + // first upload's file is not disturbed by the second request. TestClient client(port); client.send( "POST /upload HTTP/1.1\r\n" @@ -362,16 +414,110 @@ TEST_CASE("request_body_file", "[http][body_file]") app.stop(); } +TEST_CASE("request_body_file keep() survives a copied request", "[http][body_file]") +{ + // Regression test for the copy trap: the flag that used to live on + // `request` (persist_body_file_) was a plain bool copied by value, so + // calling the "keep" operation on a copy never reached the parser's own + // request and the file was deleted anyway. `body_sink` is a shared_ptr + // now, so every copy shares the same underlying FileBodySink. + TempDir dir; + SimpleApp app; + app.max_body_size(1024 * 1024); + + CROW_ROUTE(app, "/keep-via-copy") + .methods("POST"_method) + .body_sink(FileBodySink::factory(dir.path.string()))([](const request& req) { + crow::request copy = req; // a distinct object, sharing body_sink + FileBodySink::from(copy)->keep(); + return FileBodySink::from(req)->path(); + }); + + auto server = app.bindaddr(LOCALHOST_ADDRESS).port(0).run_async(); + app.wait_for_server_start(); + + TestClient client(app.port()); + client.send( + "POST /keep-via-copy HTTP/1.1\r\n" + "Host: localhost\r\n" + "Content-Length: 3\r\n" + "\r\n" + "abc"); + const auto path = trim_crlf(http_body(client.receive())); + REQUIRE(std::filesystem::exists(path)); + CHECK(read_all(path) == "abc"); + std::filesystem::remove(path); + + app.stop(); +} + +TEST_CASE("request_body_file deleted only after the full response is sent", "[http][body_file]") +{ + // Regression test: the file used to be unlinked as soon as the *first* + // chunk of a streamed response was written (parser_.clear() ran inside + // the per-chunk write helper), not after the whole response. Force the + // streaming path with a body well above the default 1MiB threshold, then + // pause the client mid-response (without finishing the read) and check + // the file is still on disk while more of it is still in flight. + TempDir dir; + SimpleApp app; + app.max_body_size(1024 * 1024); + app.stream_threshold(1024); + + const std::size_t response_body_size = 8 * 1024 * 1024; + CROW_ROUTE(app, "/upload") + .methods("POST"_method) + .body_sink(FileBodySink::factory(dir.path.string()))([response_body_size](const request&) { + return std::string(response_body_size, 'R'); + }); + + auto server = app.bindaddr(LOCALHOST_ADDRESS).port(0).run_async(); + app.wait_for_server_start(); + + TestClient client(app.port()); + client.send( + "POST /upload HTTP/1.1\r\n" + "Host: localhost\r\n" + "Content-Length: 3\r\n" + "\r\n" + "abc"); + + // Read the response headers one byte at a time so nothing beyond them is + // consumed, note the advertised Content-Length, then read a small prefix + // of the body and stop — leaving the server's write loop stalled with + // most of the body still unsent. + std::string headers; + while (headers.find("\r\n\r\n") == std::string::npos) + headers += client.read_some(1); + const auto length_pos = headers.find("Content-Length:"); + REQUIRE(length_pos != std::string::npos); + const auto content_length = static_cast(std::stoul(headers.substr(length_pos + 15))); + REQUIRE(content_length == response_body_size); + + const std::size_t prefix_size = 4096; + client.read_some(prefix_size); + std::this_thread::sleep_for(std::chrono::milliseconds(300)); + + CHECK_FALSE(directory_is_empty(dir.path)); + + // Drain exactly the rest of this one response (no more, so a bound read + // never blocks on the still-open keep-alive connection) so the server + // can finish and the test can clean up. + client.read_some(content_length - prefix_size); + + app.stop(); +} + TEST_CASE("request_body_file_too_large", "[http][body_file]") { TempDir dir; std::atomic handler_ran{false}; SimpleApp app; - app.body_file_directory(dir.path.string()).max_body_size(8); + app.max_body_size(8); CROW_ROUTE(app, "/upload") .methods("POST"_method) - .body_file()([&handler_ran](const request&) { + .body_sink(FileBodySink::factory(dir.path.string()))([&handler_ran](const request&) { handler_ran = true; return "ran"; }); @@ -395,16 +541,62 @@ TEST_CASE("request_body_file_too_large", "[http][body_file]") app.stop(); } +TEST_CASE("request_body_file_too_large chunked, over limit on a sink route", "[http][body_file]") +{ + // The Content-Length fast path at headers-complete can't catch a chunked + // body (the size isn't known yet), so this exercises the over-limit + // check inside on_body, after the sink has already been opened. + TempDir dir; + std::atomic handler_ran{false}; + SimpleApp app; + app.max_body_size(8); + + CROW_ROUTE(app, "/upload") + .methods("POST"_method) + .body_sink(FileBodySink::factory(dir.path.string()))([&handler_ran](const request&) { + handler_ran = true; + return "ran"; + }); + + auto server = app.bindaddr(LOCALHOST_ADDRESS).port(0).run_async(); + app.wait_for_server_start(); + + TestClient client(app.port()); + client.send( + "POST /upload HTTP/1.1\r\n" + "Host: localhost\r\n" + "Transfer-Encoding: chunked\r\n" + "\r\n" + "a\r\n0123456789\r\n" + "0\r\n\r\n"); + const auto response = client.receive(); + CHECK(response.find("HTTP/1.1 413") != std::string::npos); + CHECK(response.find("Connection: close") != std::string::npos); + CHECK_FALSE(handler_ran.load()); + + const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(2); + bool empty = false; + while (std::chrono::steady_clock::now() < deadline) + { + empty = directory_is_empty(dir.path); + if (empty) + break; + std::this_thread::sleep_for(std::chrono::milliseconds(20)); + } + CHECK(empty); + + app.stop(); +} + TEST_CASE("request_body_file_disconnect_cleans_up", "[http][body_file]") { TempDir dir; std::atomic handler_ran{false}; SimpleApp app; - app.body_file_directory(dir.path.string()); CROW_ROUTE(app, "/upload") .methods("POST"_method) - .body_file()([&handler_ran](const request&) { + .body_sink(FileBodySink::factory(dir.path.string()))([&handler_ran](const request&) { handler_ran = true; return "ran"; }); @@ -455,7 +647,7 @@ TEST_CASE("request_body_sink", "[http][body_file]") })([buf, &body_size, &body_capacity, &had_file](const request& req) { body_size = req.body.size(); body_capacity = req.body.capacity(); - had_file = req.has_body_file(); + had_file = static_cast(FileBodySink::from(req)); return req.body.empty() ? *buf : req.body; }); @@ -481,11 +673,11 @@ TEST_CASE("request_body_file does not reserve req.body", "[http][body_file]") std::atomic body_size{999}; std::atomic body_capacity{999}; SimpleApp app; - app.body_file_directory(dir.path.string()).max_body_size(1024 * 1024); + app.max_body_size(1024 * 1024); CROW_ROUTE(app, "/upload") .methods("POST"_method) - .body_file()([&](const request& req) { + .body_sink(FileBodySink::factory(dir.path.string()))([&](const request& req) { body_size = req.body.size(); body_capacity = req.body.capacity(); return "ok"; @@ -545,6 +737,52 @@ TEST_CASE("request_body_sink write failure is 500", "[http][body_file]") app.stop(); } +TEST_CASE("request_body_sink write failure lets a large in-flight upload finish writing before the 500 is read", "[http][body_file]") +{ + // Regression test: a client that writes its whole over-limit body before + // reading must not see its write fail (e.g. with a broken pipe). The + // server must linger and keep draining the socket instead of closing on + // unread bytes, which can RST a peer that is still mid-upload and lose + // the response it already sent. A small payload (4 bytes, as in the test + // above) cannot reproduce this: it needs to be big enough that the + // client's blocking send() has to wait on the server to keep reading, + // rather than completing into socket buffers before the server has even + // reacted. + std::atomic handler_ran{false}; + SimpleApp app; + app.max_body_size(64ull * 1024 * 1024); + + CROW_ROUTE(app, "/sink") + .methods("POST"_method) + .body_sink([](const request&) { + return std::make_unique(); + })([&handler_ran](const request&) { + handler_ran = true; + return "ran"; + }); + + auto server = app.bindaddr(LOCALHOST_ADDRESS).port(0).run_async(); + app.wait_for_server_start(); + + const std::string payload(5'000'000, 'x'); + TestClient client(app.port()); + const std::string http_request = + "POST /sink HTTP/1.1\r\n" + "Host: localhost\r\n" + "Content-Length: " + + std::to_string(payload.size()) + + "\r\n" + "\r\n" + + payload; + + REQUIRE_NOTHROW(client.send(http_request)); + const auto response = client.receive(); + CHECK(response.find("HTTP/1.1 500") != std::string::npos); + CHECK_FALSE(handler_ran.load()); + + app.stop(); +} + TEST_CASE("request_body_sink finish failure is 500", "[http][body_file]") { std::atomic handler_ran{false}; @@ -578,80 +816,157 @@ TEST_CASE("request_body_sink finish failure is 500", "[http][body_file]") app.stop(); } -TEST_CASE("request_body_sink last call wins over body_file", "[http][body_file]") +TEST_CASE("request_body_sink a throwing write() or finish() is 500", "[http][body_file]") { - TempDir dir; - auto buf = std::make_shared(); + // Regression test: write()/finish() used to be called bare; an uncaught + // exception unwound into the worker loop's catch(std::exception&), + // logged "Worker Crash" and left the client with no response at all + // (and a non-std::exception ended the worker thread silently). + std::atomic handler_ran{false}; SimpleApp app; - app.body_file_directory(dir.path.string()).max_body_size(1024 * 1024); + app.max_body_size(1024 * 1024); - CROW_ROUTE(app, "/sink-last") + CROW_ROUTE(app, "/throw-write") .methods("POST"_method) - .body_file() - .body_sink([buf](const request&) { - auto sink = std::make_unique(); - sink->buf = buf; - return sink; - })([buf](const request& req) { - return req.has_body_file() ? std::string("file") : *buf; + .body_sink([](const request&) { + return std::make_unique(); + })([&handler_ran](const request&) { + handler_ran = true; + return "ran"; }); - CROW_ROUTE(app, "/file-last") + CROW_ROUTE(app, "/throw-finish") .methods("POST"_method) - .body_sink([buf](const request&) { - auto sink = std::make_unique(); - sink->buf = buf; - return sink; - }) - .body_file()([](const request& req) { - return req.has_body_file() ? std::string("file:") + read_all(req.body_file_path) : std::string("sink"); + .body_sink([](const request&) { + return std::make_unique(); + })([&handler_ran](const request&) { + handler_ran = true; + return "ran"; + }); + + CROW_ROUTE(app, "/throw-write-nonstd") + .methods("POST"_method) + .body_sink([](const request&) { + return std::make_unique(); + })([&handler_ran](const request&) { + handler_ran = true; + return "ran"; }); auto server = app.bindaddr(LOCALHOST_ADDRESS).port(0).run_async(); app.wait_for_server_start(); const auto port = app.port(); - { + auto post_fail = [&](const std::string& target) { TestClient client(port); client.send( - "POST /sink-last HTTP/1.1\r\n" + "POST " + target + + " HTTP/1.1\r\n" "Host: localhost\r\n" - "Content-Length: 3\r\n" + "Content-Length: 4\r\n" "\r\n" - "abc"); - CHECK(http_body(client.receive()) == "abc"); - CHECK(directory_is_empty(dir.path)); - } + "fail"); + return client.receive(); + }; + for (const std::string target : {"/throw-write", "/throw-finish", "/throw-write-nonstd"}) { - TestClient client(port); - client.send( - "POST /file-last HTTP/1.1\r\n" - "Host: localhost\r\n" - "Content-Length: 3\r\n" - "\r\n" - "xyz"); - CHECK(http_body(client.receive()) == "file:xyz"); + handler_ran = false; + const auto response = post_fail(target); + CHECK(response.find("HTTP/1.1 500") != std::string::npos); + CHECK(response.find("Connection: close") != std::string::npos); + CHECK_FALSE(handler_ran.load()); } + // The server is still responsive: a throw doesn't take down the worker. + TestClient probe(port); + probe.send( + "POST /throw-write HTTP/1.1\r\n" + "Host: localhost\r\n" + "Content-Length: 1\r\n" + "\r\n" + "x"); + CHECK(probe.receive().find("HTTP/1.1 500") != std::string::npos); + app.stop(); } -TEST_CASE("request_body_sink open failure is 500", "[http][body_file]") +TEST_CASE("request_body_sink calling body_sink() again replaces the factory", "[http][body_file]") { + TempDir dir; + auto buf = std::make_shared(); + SimpleApp app; + app.max_body_size(1024 * 1024); + + CROW_ROUTE(app, "/last-wins") + .methods("POST"_method) + .body_sink(FileBodySink::factory(dir.path.string())) + .body_sink([buf](const request&) { + auto sink = std::make_unique(); + sink->buf = buf; + return sink; + })([buf](const request& req) { + return FileBodySink::from(req) ? std::string("file") : *buf; + }); + + auto server = app.bindaddr(LOCALHOST_ADDRESS).port(0).run_async(); + app.wait_for_server_start(); + + TestClient client(app.port()); + client.send( + "POST /last-wins HTTP/1.1\r\n" + "Host: localhost\r\n" + "Content-Length: 3\r\n" + "\r\n" + "abc"); + CHECK(http_body(client.receive()) == "abc"); + CHECK(directory_is_empty(dir.path)); + + app.stop(); +} + +TEST_CASE("request_body_sink a factory returning nullptr keeps the body in req.body", "[http][body_file]") +{ + // A user factory declining (nullptr) is not an error: unlike the file + // sink's own open failure, it means "use req.body as usual" for this + // request. std::atomic handler_ran{false}; SimpleApp app; app.max_body_size(1024 * 1024); - CROW_ROUTE(app, "/null") + CROW_ROUTE(app, "/maybe") .methods("POST"_method) .body_sink([](const request&) -> std::unique_ptr { return nullptr; - })([&handler_ran](const request&) { + })([&handler_ran](const request& req) { handler_ran = true; - return "ran"; + return std::string("memory:") + req.body; }); + auto server = app.bindaddr(LOCALHOST_ADDRESS).port(0).run_async(); + app.wait_for_server_start(); + + TestClient client(app.port()); + client.send( + "POST /maybe HTTP/1.1\r\n" + "Host: localhost\r\n" + "Content-Length: 5\r\n" + "\r\n" + "hello"); + const auto response = client.receive(); + CHECK(response.find("HTTP/1.1 200") != std::string::npos); + CHECK(http_body(response) == "memory:hello"); + CHECK(handler_ran.load()); + + app.stop(); +} + +TEST_CASE("request_body_sink a throwing factory is 500", "[http][body_file]") +{ + std::atomic handler_ran{false}; + SimpleApp app; + app.max_body_size(1024 * 1024); + CROW_ROUTE(app, "/throw") .methods("POST"_method) .body_sink([](const request&) -> std::unique_ptr { @@ -663,53 +978,53 @@ TEST_CASE("request_body_sink open failure is 500", "[http][body_file]") auto server = app.bindaddr(LOCALHOST_ADDRESS).port(0).run_async(); app.wait_for_server_start(); - const auto port = app.port(); - auto post_fail = [&](const std::string& target) { - TestClient client(port); - client.send( - "POST " + target + - " HTTP/1.1\r\n" - "Host: localhost\r\n" - "Content-Length: 4\r\n" - "\r\n" - "fail"); - return client.receive(); - }; - - { - const auto response = post_fail("/null"); - CHECK(response.find("HTTP/1.1 500") != std::string::npos); - CHECK(response.find("Connection: close") != std::string::npos); - CHECK_FALSE(handler_ran.load()); - } - - handler_ran = false; - { - const auto response = post_fail("/throw"); - CHECK(response.find("HTTP/1.1 500") != std::string::npos); - CHECK(response.find("Connection: close") != std::string::npos); - CHECK_FALSE(handler_ran.load()); - } + TestClient client(app.port()); + client.send( + "POST /throw HTTP/1.1\r\n" + "Host: localhost\r\n" + "Content-Length: 4\r\n" + "\r\n" + "fail"); + const auto response = client.receive(); + CHECK(response.find("HTTP/1.1 500") != std::string::npos); + CHECK(response.find("Connection: close") != std::string::npos); + CHECK_FALSE(handler_ran.load()); app.stop(); } -TEST_CASE("request_body_file open failure is 500 and leaves no file", "[http][body_file]") +TEST_CASE("FileBodySink::factory rejects a directory that does not exist", "[http][body_file]") { TempDir dir; + const auto missing = dir.path / "does-not-exist"; + CHECK_THROWS_AS(FileBodySink::factory(missing.string()), std::filesystem::filesystem_error); + const auto not_a_dir = dir.path / "not-a-dir"; { std::ofstream out(not_a_dir); out << "x"; } + CHECK_THROWS_AS(FileBodySink::factory(not_a_dir.string()), std::filesystem::filesystem_error); +} + +TEST_CASE("request_body_file per-request open failure is 500 and leaves no file", "[http][body_file]") +{ + // The directory exists (and is required to) when the route is set up; + // simulate it disappearing before a request actually opens a file. + TempDir dir; + const auto vanishing = dir.path / "vanishing"; + std::filesystem::create_directories(vanishing); + auto factory = FileBodySink::factory(vanishing.string()); + std::filesystem::remove(vanishing); + std::atomic handler_ran{false}; SimpleApp app; app.max_body_size(1024 * 1024); CROW_ROUTE(app, "/upload") .methods("POST"_method) - .body_file(not_a_dir.string())([&handler_ran](const request&) { + .body_sink(std::move(factory))([&handler_ran](const request&) { handler_ran = true; return "ran"; }); @@ -728,12 +1043,7 @@ TEST_CASE("request_body_file open failure is 500 and leaves no file", "[http][bo CHECK(response.find("HTTP/1.1 500") != std::string::npos); CHECK(response.find("Connection: close") != std::string::npos); CHECK_FALSE(handler_ran.load()); - - std::error_code ec; - for (const auto& entry : std::filesystem::directory_iterator(dir.path, ec)) - { - CHECK(entry.path().filename() == "not-a-dir"); - } + CHECK_FALSE(std::filesystem::exists(vanishing)); app.stop(); } @@ -742,12 +1052,14 @@ TEST_CASE("request_body_file concurrent uploads get distinct paths", "[http][bod { TempDir dir; SimpleApp app; - app.body_file_directory(dir.path.string()).max_body_size(1024 * 1024); + app.max_body_size(1024 * 1024); CROW_ROUTE(app, "/keep") .methods("POST"_method) - .body_file()([](const request& req) { - return req.take_body_file(); + .body_sink(FileBodySink::factory(dir.path.string()))([](const request& req) { + auto* file = FileBodySink::from(req); + file->keep(); + return file->path(); }); auto server = app.bindaddr(LOCALHOST_ADDRESS).port(0).run_async(); From 0b3a6dc0acc1506a95db4b480faa95bd56978607 Mon Sep 17 00:00:00 2001 From: "Anton N. Petrov" Date: Thu, 3 Sep 2026 23:19:45 +0300 Subject: [PATCH 06/14] Address code review: portable file sink, drain doc, comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace mkostemp (glibc-only) with a portable open(O_CREAT|O_EXCL) loop using the same random-name scheme as the Windows branch, so the opt-in file_body_sink.h header builds on macOS/BSD. Fix app.md's stale claim that an over-limit body "is not read or drained" — it is, via linger_close()/do_linger_read(). Add explanatory comments on the linger-read buffer reuse and the uploads/ directory requirement in the example. --- docs/guides/app.md | 2 +- examples/example_body_file.cpp | 3 ++- include/crow/file_body_sink.h | 22 +++++++++++++--------- include/crow/http_connection.h | 2 ++ 4 files changed, 18 insertions(+), 11 deletions(-) diff --git a/docs/guides/app.md b/docs/guides/app.md index 654137da42..b7d631b5bb 100644 --- a/docs/guides/app.md +++ b/docs/guides/app.md @@ -57,7 +57,7 @@ app.tcp_nodelay(true) By default Crow accepts a request body of any size. `#!cpp app.max_body_size(bytes)` sets an app-wide limit (`UINT64_MAX` is unlimited). A route can override it with `#!cpp .max_body_size(bytes)`. -The advertised `Content-Length` is checked when headers complete, before `100 Continue`. Chunked bodies are counted as they arrive. An over-limit request is answered `413 Payload Too Large` and the connection is closed; the body is not read or drained, and the route handler does not run. The same cap applies to 404, 405, and slash-redirects. +The advertised `Content-Length` is checked when headers complete, before `100 Continue`. Chunked bodies are counted as they arrive. An over-limit request is answered `413 Payload Too Large`; the route handler does not run. The remaining body is drained (discarded, not parsed) and the connection is then closed, bounded by the same deadline timer (`app.timeout()`, default 5s) a slow client already gets. The same cap applies to 404, 405, and slash-redirects. ```cpp crow::SimpleApp app; diff --git a/examples/example_body_file.cpp b/examples/example_body_file.cpp index 9aeed82c2f..59e31c7589 100644 --- a/examples/example_body_file.cpp +++ b/examples/example_body_file.cpp @@ -13,7 +13,8 @@ int main() // PUT or POST the raw body; Crow writes it to disk as the bytes arrive. CROW_ROUTE(app, "/upload") .methods(crow::HTTPMethod::Put, crow::HTTPMethod::Post) - .body_sink(crow::FileBodySink::factory("uploads"))([](const crow::request& req) { + .body_sink(crow::FileBodySink::factory("uploads")) // requires ./uploads to exist + ([](const crow::request& req) { auto* file = crow::FileBodySink::from(req); if (!file) { diff --git a/include/crow/file_body_sink.h b/include/crow/file_body_sink.h index 4f226a40a5..1702155fa4 100644 --- a/include/crow/file_body_sink.h +++ b/include/crow/file_body_sink.h @@ -12,7 +12,6 @@ #include #include #include -#include #ifndef _WIN32 #include @@ -23,11 +22,11 @@ #define WIN32_LEAN_AND_MEAN #endif #include -#include "crow/utility.h" #endif #include "crow/body_sink.h" #include "crow/http_request.h" +#include "crow/utility.h" namespace crow { @@ -149,13 +148,18 @@ namespace crow static std::unique_ptr create(const std::string& directory) { #ifndef _WIN32 - std::string tmpl = (std::filesystem::path(directory) / "crow-body-XXXXXX").string(); - std::vector buf(tmpl.begin(), tmpl.end()); - buf.push_back('\0'); - const int fd = ::mkostemp(buf.data(), O_CLOEXEC); - if (fd < 0) - throw std::system_error(errno, std::generic_category(), "crow::FileBodySink: mkostemp"); - return std::unique_ptr(new FileBodySink(std::string(buf.data()), fd)); + const std::filesystem::path dir(directory); + for (int attempt = 0; attempt < 128; ++attempt) + { + const std::string name = "crow-body-" + utility::random_alphanum(16); + const std::string path = (dir / name).string(); + const int fd = ::open(path.c_str(), O_CREAT | O_EXCL | O_WRONLY | O_CLOEXEC, 0600); + if (fd >= 0) + return std::unique_ptr(new FileBodySink(path, fd)); + if (errno != EEXIST) + throw std::system_error(errno, std::generic_category(), "crow::FileBodySink: open"); + } + throw std::runtime_error("crow::FileBodySink: could not find a unique file name"); #else const std::filesystem::path dir(directory); for (int attempt = 0; attempt < 128; ++attempt) diff --git a/include/crow/http_connection.h b/include/crow/http_connection.h index fbc153714e..8100ca4f29 100644 --- a/include/crow/http_connection.h +++ b/include/crow/http_connection.h @@ -516,6 +516,8 @@ namespace crow do_linger_read(); } + // Reuses the normal read path's buffer_; safe here because parsing has + // already aborted, so nothing else reads from or writes to it. void do_linger_read() { auto self = this->shared_from_this(); From a0e67330f91e4fac0c29f821123d5adb49301212 Mon Sep 17 00:00:00 2001 From: "Anton N. Petrov" Date: Fri, 4 Sep 2026 14:09:07 +0300 Subject: [PATCH 07/14] Drop the Content-Length reserve() unconditionally, clear req.body on reject The merge of feature/max-body-size into this branch (71c9f86) reintroduced a finite-cap-conditioned req_.body.reserve(content_length) that #1234 had already removed unconditionally on its own head (d24c987). A client can still send headers only, advertising a large Content-Length within a route's configured max_body_size, and make every such connection reserve that much memory before a single body byte arrives - concurrent connections can still exhaust memory this way, per ssubbotin's still-open review comment on include/crow/http_connection.h:163. Also port over the req_.body.clear() on the body_error_status_ reject path from the same upstream fix, so a chunked reject that buffered an accepted prefix and a Content-Length reject (always empty) leave after-handlers a consistent, empty request body. --- include/crow/http_connection.h | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/include/crow/http_connection.h b/include/crow/http_connection.h index 8100ca4f29..109f3c1890 100644 --- a/include/crow/http_connection.h +++ b/include/crow/http_connection.h @@ -151,18 +151,6 @@ namespace crow return 1; } } - if (!req_.body_sink && - limit != UINT64_MAX && - parser_.content_length != CROW_ULLONG_MAX && - parser_.content_length <= req_.body.max_size()) - { - // Finite cap only: never allocate from an untrusted Content-Length - // when unlimited, and never when the body is going to a sink - // (req.body stays empty, whether the route always uses one or the - // factory declined this particular request). - req_.body.reserve(static_cast(parser_.content_length)); - } - // HTTP 1.1 Expect: 100-continue if (req_.http_ver_major == 1 && req_.http_ver_minor == 1 && get_header_value(req_.headers, "expect") == "100-continue") { @@ -214,6 +202,7 @@ namespace crow if (body_error_status_) { + req_.body.clear(); res = response(body_error_status_); res.set_header("Connection", "close"); res.end(); From 7dca1ef85732d5a4a8ceb5e3bca5ef7df125c4e3 Mon Sep 17 00:00:00 2001 From: "Anton N. Petrov" Date: Fri, 4 Sep 2026 14:09:13 +0300 Subject: [PATCH 08/14] Keep file_body_sink.h out of the amalgamated crow_all.h scripts/merge_all.py globbed every include/crow/*.h* into the amalgamated header, so file_body_sink.h was pulled into crow_all.h even though it is documented as opt-in. Compiling that generated header with -fno-rtti -DASIO_NO_TYPEID (asio's supported RTTI-free configuration, per ssubbotin's review) then failed at the dynamic_cast in FileBodySink::from(). Exclude file_body_sink.h from the crow/*.h* glob via an opt_in_headers set, mirroring how middlewares are already assembled separately rather than glob-included unconditionally. Verified crow_all.h no longer references FileBodySink/dynamic_cast and compiles clean under -fno-rtti -DASIO_NO_TYPEID. --- scripts/merge_all.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/scripts/merge_all.py b/scripts/merge_all.py index 9fc7871c60..c5597ec31d 100755 --- a/scripts/merge_all.py +++ b/scripts/merge_all.py @@ -44,10 +44,16 @@ middlewares_actual = middlewares print("Middlewares: " + str(middlewares_actual)) +# Headers that are opt-in and must not be pulled into crow_all.h: they impose +# a requirement (e.g. RTTI) that the rest of the library doesn't need, so a +# build that can't meet it can still use the amalgamated header. +opt_in_headers = {'file_body_sink.h'} + re_depends = re.compile('^#include \"(.*)\"\n', re.MULTILINE) re_pragma = re.compile('^(.*)#pragma once(.*)\n', re.MULTILINE) headers = [x.rsplit(sep, 1)[-1] for x in glob(pt.join(header_path, '*.h*'))] -headers += ['crow'+sep + x.rsplit(sep, 1)[-1] for x in glob(pt.join(header_path, 'crow'+sep+'*.h*'))] +headers += ['crow'+sep + x.rsplit(sep, 1)[-1] for x in glob(pt.join(header_path, 'crow'+sep+'*.h*')) + if x.rsplit(sep, 1)[-1] not in opt_in_headers] headers += [('crow'+sep+'middlewares'+sep + x + '.h') for x in middlewares_actual] print(headers) edges = defaultdict(list) From ec10ad8a77af6637edd09cd3760c84809955fcdc Mon Sep 17 00:00:00 2001 From: "Anton N. Petrov" Date: Fri, 4 Sep 2026 14:09:18 +0300 Subject: [PATCH 09/14] Fix stale mkostemp reference in body-file guide The guide said Crow generates unique file names via POSIX mkostemp, per ssubbotin's review comment. The implementation was changed to a retried open(O_CREAT | O_EXCL) loop for portability (mkostemp is glibc-only, unavailable on macOS/BSD and older musl) but the doc wasn't updated to match. --- docs/guides/body-file.md | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/docs/guides/body-file.md b/docs/guides/body-file.md index 33c0c4282e..3f6de974f7 100644 --- a/docs/guides/body-file.md +++ b/docs/guides/body-file.md @@ -49,9 +49,10 @@ CROW_ROUTE(app, "/upload") path once, at the point you call it, and throws `std::filesystem::filesystem_error` if it does not already exist — Crow does not create it for you, and does not fail silently. Crow always generates a -unique file name (POSIX: `mkostemp`; Windows: a retried `CREATE_NEW`), so -concurrent requests never share a path. The descriptor is kept open until the -body is complete; the handler then reads `file->path()`. +unique file name (POSIX: a retried `open(O_CREAT | O_EXCL)`; Windows: a +retried `CREATE_NEW`), so concurrent requests never share a path. The +descriptor is kept open until the body is complete; the handler then reads +`file->path()`. The file is deleted when the last `crow::request` copy that reached the handler is destroyed — normally right after the response has been fully From 27799292184b65f93a62c5faaaae7d2e8d50615d Mon Sep 17 00:00:00 2001 From: "Anton N. Petrov" Date: Fri, 4 Sep 2026 14:09:23 +0300 Subject: [PATCH 10/14] Fix CIFuzz build: DummyHandler out of sync with HTTPParser handler interface handle_header() now returns int (0/1) instead of void, and the parser calls handler_->reject_body(status) and handler_->parser_should_abort() on the rejection/message-complete paths (the general form this branch's body-sink-failure handling generalized #1234's reject_payload_too_large() into). tests/fuzz/http_fuzzer.cpp's DummyHandler stand-in wasn't updated when the interface changed, so http_fuzzer.cpp failed to compile. --- tests/fuzz/http_fuzzer.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/fuzz/http_fuzzer.cpp b/tests/fuzz/http_fuzzer.cpp index c77956ca26..6690d6c86b 100644 --- a/tests/fuzz/http_fuzzer.cpp +++ b/tests/fuzz/http_fuzzer.cpp @@ -4,8 +4,10 @@ struct DummyHandler { void handle_url() {} - void handle_header() {} + int handle_header() { return 0; } void handle() {} + void reject_body(int) {} + bool parser_should_abort() const { return false; } size_t stream_threshold() { return 1024*1024; } }; From 300378a7d9622aa59f785ba5975bb52acaa10c0e Mon Sep 17 00:00:00 2001 From: "Anton N. Petrov" Date: Sun, 6 Sep 2026 00:26:01 +0300 Subject: [PATCH 11/14] Address PR review: scope drain wording, make send_file test cwd-independent - docs/guides/app.md: scope the "drained" wording to requests rejected while their body is still streaming in (chunked 413, sink 500). A request rejected on Content-Length alone has no body yet to drain; the connection is simply closed after the 413 write. - tests/unittest.cpp: the send_file test stat()'d and served "tests/img/..." relative to the process cwd, failing when unittest is run from anywhere but the build-dir root. CROW_STATIC_FILE's sanitizer rejects absolute Unix paths outright, so instead chdir into the CMake-provided repo root for the scope of the test (restored via RAII), keeping the paths relative as the API requires. --- docs/guides/app.md | 2 +- tests/CMakeLists.txt | 1 + tests/unittest.cpp | 12 ++++++++++++ 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/docs/guides/app.md b/docs/guides/app.md index b7d631b5bb..5dbcf7499f 100644 --- a/docs/guides/app.md +++ b/docs/guides/app.md @@ -57,7 +57,7 @@ app.tcp_nodelay(true) By default Crow accepts a request body of any size. `#!cpp app.max_body_size(bytes)` sets an app-wide limit (`UINT64_MAX` is unlimited). A route can override it with `#!cpp .max_body_size(bytes)`. -The advertised `Content-Length` is checked when headers complete, before `100 Continue`. Chunked bodies are counted as they arrive. An over-limit request is answered `413 Payload Too Large`; the route handler does not run. The remaining body is drained (discarded, not parsed) and the connection is then closed, bounded by the same deadline timer (`app.timeout()`, default 5s) a slow client already gets. The same cap applies to 404, 405, and slash-redirects. +The advertised `Content-Length` is checked when headers complete, before `100 Continue`. Chunked bodies are counted as they arrive. An over-limit request is answered `413 Payload Too Large`; the route handler does not run. If the request is rejected while its body is still streaming in (e.g. a chunked body that crosses the limit), the remaining bytes are drained (discarded, not parsed) before the connection closes, bounded by the same deadline timer (`app.timeout()`, default 5s) a slow client already gets. When the advertised `Content-Length` alone already exceeds the limit, there is no body yet to drain — the connection is simply closed after the 413 is written (this does not reset a peer that is still sending). The same cap applies to 404, 405, and slash-redirects. ```cpp crow::SimpleApp app; diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index e01edeff2c..6d8e237386 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -35,6 +35,7 @@ set(TEST_SRCS add_executable(unittest ${TEST_SRCS}) target_link_libraries(unittest Crow::Crow Catch2::Catch2WithMain) +target_compile_definitions(unittest PRIVATE CROW_TEST_REPO_DIR="${CMAKE_SOURCE_DIR}") add_warnings_optimizations(unittest) add_sanitizer_flags(unittest) diff --git a/tests/unittest.cpp b/tests/unittest.cpp index 1e60242bd2..8ecdf3f00d 100644 --- a/tests/unittest.cpp +++ b/tests/unittest.cpp @@ -3,6 +3,7 @@ #include #include +#include #include #include #include @@ -1892,6 +1893,17 @@ TEST_CASE("multipart_view") TEST_CASE("send_file") { + // CROW_STATIC_FILE's internal path is deliberately relative (its sanitizer + // strips a leading '/' to reject Unix absolute paths), so "tests/img/..." + // can't be replaced with an absolute path. Instead, chdir into the + // CMake-provided repo root for the scope of this test, so the relative + // paths below resolve regardless of where `unittest` is invoked from. + struct ScopedCwd + { + std::filesystem::path previous = std::filesystem::current_path(); + ScopedCwd(const std::filesystem::path& dir) { std::filesystem::current_path(dir); } + ~ScopedCwd() { std::filesystem::current_path(previous); } + } scoped_cwd(CROW_TEST_REPO_DIR); struct stat statbuf_cat; stat("tests/img/cat.jpg", &statbuf_cat); From e4e4a265adbc518994eef029a818ca3fefb1495d Mon Sep 17 00:00:00 2001 From: "Anton N. Petrov" Date: Sun, 6 Sep 2026 01:38:20 +0300 Subject: [PATCH 12/14] Explicit skip_body on body_error_status_ responses, matching max-body-size This branch forked from feature/max-body-size right after 225034f, before d24c98755 ("Address ssubbotin's PR review: linger on 413, drop reserve(), and cleanup") landed there, so it never picked up that commit's skip_body handling. Port the equivalent fix here: - handle(): explicitly set res.skip_body on the body_error_status_ branch (413 from an over-limit body, or 500 from a body_sink failure) instead of relying on response::operator=(&&) happening to omit the member. - write_header_into_buffer: skip the automatic status-text body when skip_body is set. Without this, setting skip_body explicitly surfaces a real bug: end() already writes "Content-Length: 0" while skip_body is true, but the default status text was still being force-appended to body afterward, so a HEAD 413/500 shipped a nonzero-length body under a Content-Length: 0 header. - Add a regression test covering a matched-route HEAD/OPTIONS request with an over-limit body (413, Connection: close, and for HEAD an empty body with Content-Length: 0), mirroring the one already added on feature/max-body-size. --- include/crow/http_connection.h | 4 +++ include/crow/http_response.h | 2 +- tests/max_body_size_tests.cpp | 48 ++++++++++++++++++++++++++++++++++ 3 files changed, 53 insertions(+), 1 deletion(-) diff --git a/include/crow/http_connection.h b/include/crow/http_connection.h index 109f3c1890..25b7a3978d 100644 --- a/include/crow/http_connection.h +++ b/include/crow/http_connection.h @@ -204,6 +204,10 @@ namespace crow { req_.body.clear(); res = response(body_error_status_); + // Explicit, rather than relying on response::operator=(&&) happening to + // omit skip_body: a HEAD request must never get a body, regardless of + // which error status (413, or 500 from a body_sink failure) sent it here. + res.skip_body = (req_.method == HTTPMethod::Head); res.set_header("Connection", "close"); res.end(); close_connection_ = true; diff --git a/include/crow/http_response.h b/include/crow/http_response.h index 71a9e0c7a9..61cd83625e 100644 --- a/include/crow/http_response.h +++ b/include/crow/http_response.h @@ -412,7 +412,7 @@ namespace crow auto& status = statusCodes.find(code)->second; buffers.emplace_back(status.data(), status.size()); - if (code >= 400 && body.empty()) + if (code >= 400 && body.empty() && !skip_body) body = statusCodes[code].substr(9); for (auto& kv : headers) diff --git a/tests/max_body_size_tests.cpp b/tests/max_body_size_tests.cpp index 31425da508..a6320dc3fd 100644 --- a/tests/max_body_size_tests.cpp +++ b/tests/max_body_size_tests.cpp @@ -441,3 +441,51 @@ TEST_CASE("max_body_size 413 skips before-handlers", "[http][max_body_size]") app.stop(); } + +TEST_CASE("max_body_size applies to a matched-route HEAD/OPTIONS request", "[http][max_body_size]") +{ + // A HEAD/OPTIONS request that matches a route reaches the max_body_size + // check in handle_header(). A HEAD 413 must still carry no body + // (skip_body set explicitly in the body_error_status_ branch, not left + // to accident), while OPTIONS gets a normal 413 body. + SimpleApp app; + app.max_body_size(8); + + CROW_ROUTE(app, "/upload") + .methods("GET"_method)([] { + return "ok"; + }); + + auto server = app.bindaddr(LOCALHOST_ADDRESS).port(0).run_async(); + app.wait_for_server_start(); + const auto port = app.port(); + + { + TestClient client(port); + client.send( + "HEAD /upload HTTP/1.1\r\n" + "Host: localhost\r\n" + "Content-Length: 100\r\n" + "\r\n"); + const auto resp = client.receive(); + CHECK(status_of(resp) == 413); + CHECK(resp.find("Connection: close") != std::string::npos); + CHECK(resp.find("Content-Length: 0") != std::string::npos); + const auto header_end = resp.find("\r\n\r\n"); + REQUIRE(header_end != std::string::npos); + CHECK(resp.size() == header_end + 4); // no body after the headers + } + { + TestClient client(port); + client.send( + "OPTIONS /upload HTTP/1.1\r\n" + "Host: localhost\r\n" + "Content-Length: 100\r\n" + "\r\n"); + const auto resp = client.receive(); + CHECK(status_of(resp) == 413); + CHECK(resp.find("Connection: close") != std::string::npos); + } + + app.stop(); +} From 16fa250c6652b69019a2c17d264b17ed87a70ccb Mon Sep 17 00:00:00 2001 From: "Anton N. Petrov" Date: Sun, 6 Sep 2026 13:23:27 +0300 Subject: [PATCH 13/14] Address PR review: fix inaccurate drain wording, init rule_index docs/guides/app.md claimed the connection is "simply closed" with nothing to drain when a request is rejected on the advertised Content-Length alone. That's inaccurate: http_connection.h's do_read error path calls linger_close() for every body_error_status_ case, so the server always drains and discards whatever the client still sends, regardless of which check triggered the 413. Reworded to match the verified behavior. Also default-initialize routing_handle_result::rule_index so a default-constructed instance never carries an indeterminate value. --- docs/guides/app.md | 2 +- include/crow/common.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/guides/app.md b/docs/guides/app.md index 5dbcf7499f..9225124093 100644 --- a/docs/guides/app.md +++ b/docs/guides/app.md @@ -57,7 +57,7 @@ app.tcp_nodelay(true) By default Crow accepts a request body of any size. `#!cpp app.max_body_size(bytes)` sets an app-wide limit (`UINT64_MAX` is unlimited). A route can override it with `#!cpp .max_body_size(bytes)`. -The advertised `Content-Length` is checked when headers complete, before `100 Continue`. Chunked bodies are counted as they arrive. An over-limit request is answered `413 Payload Too Large`; the route handler does not run. If the request is rejected while its body is still streaming in (e.g. a chunked body that crosses the limit), the remaining bytes are drained (discarded, not parsed) before the connection closes, bounded by the same deadline timer (`app.timeout()`, default 5s) a slow client already gets. When the advertised `Content-Length` alone already exceeds the limit, there is no body yet to drain — the connection is simply closed after the 413 is written (this does not reset a peer that is still sending). The same cap applies to 404, 405, and slash-redirects. +The advertised `Content-Length` is checked when headers complete, before `100 Continue`. Chunked bodies are counted as they arrive. An over-limit request is answered `413 Payload Too Large`; the route handler does not run. Whether the rejection happens on the advertised `Content-Length` alone or partway through a streaming (chunked) body, the connection drains and discards whatever the client still sends rather than closing on unread bytes, which could reset a peer that is still mid-upload; the drain is bounded by the same deadline timer (`app.timeout()`, default 5s) a slow client already gets. The same cap applies to 404, 405, and slash-redirects. ```cpp crow::SimpleApp app; diff --git a/include/crow/common.h b/include/crow/common.h index facd98dd12..aaf395fb27 100644 --- a/include/crow/common.h +++ b/include/crow/common.h @@ -288,7 +288,7 @@ namespace crow struct routing_handle_result { bool catch_all{false}; - size_t rule_index; + size_t rule_index{0}; std::vector blueprint_indices; routing_params r_params; HTTPMethod method{}; From 99d7368a961ed4c205ee858bbed539a97348b285 Mon Sep 17 00:00:00 2001 From: "Anton N. Petrov" Date: Sun, 6 Sep 2026 23:20:47 +0300 Subject: [PATCH 14/14] Address PR review: linger-drain on body errors, dedupe test helpers - do_write_static()/do_write_general() no longer shut the socket down themselves for a body-error (413/500) response; that's left to do_read()'s linger_close(), so a small stream_threshold() no longer races the drain and RSTs a peer still mid-upload. Default-threshold responses were never affected, which is why this was latent. - Add regression tests: the above with a 1-byte stream_threshold, and that a body_sink finish() failure sends exactly one response (traced through the parser, on_message_complete()'s existing early return already prevents the double-handle() the review described - this pins the current, correct behavior down). - Extract response_complete/status_of/http_body/TestClient, duplicated verbatim across max_body_size_tests.cpp and body_file_tests.cpp, into tests/http_test_utils.h. - Content-Length parsing in the test helpers now uses stoull instead of stoul. - Note the 32-bit/64-bit MAXDWORD cast is exact on both in FileBodySink's Windows WriteFile chunking loop. --- include/crow/file_body_sink.h | 3 + include/crow/http_connection.h | 9 +- tests/body_file_tests.cpp | 139 ++++++++------------------- tests/http_test_utils.h | 166 +++++++++++++++++++++++++++++++++ tests/max_body_size_tests.cpp | 127 +++++++++---------------- 5 files changed, 262 insertions(+), 182 deletions(-) create mode 100644 tests/http_test_utils.h diff --git a/include/crow/file_body_sink.h b/include/crow/file_body_sink.h index 1702155fa4..35f52a5cda 100644 --- a/include/crow/file_body_sink.h +++ b/include/crow/file_body_sink.h @@ -101,6 +101,9 @@ namespace crow while (written < length) { DWORD n = 0; + // static_cast(MAXDWORD) is still exact on 32-bit size_t + // (both are 32-bit there), so this chunking is correct on 32- + // and 64-bit Windows alike. const DWORD chunk = static_cast( std::min(length - written, static_cast(MAXDWORD))); if (!WriteFile(handle_, data + written, chunk, &n, nullptr) || n == 0) diff --git a/include/crow/http_connection.h b/include/crow/http_connection.h index 25b7a3978d..379281911e 100644 --- a/include/crow/http_connection.h +++ b/include/crow/http_connection.h @@ -374,7 +374,10 @@ namespace crow is.read(buf, sizeof(buf)); } } - if (close_connection_) + // A body-error response leaves the read side open for do_read()'s + // linger_close() to drain instead of closing here, so a peer still + // mid-upload isn't RST'd before it reads the response. + if (close_connection_ && !body_error_status_) { adaptor_.shutdown_readwrite(); adaptor_.close(); @@ -430,7 +433,9 @@ namespace crow transferred += to_transfer; } } - if (close_connection_) + // See the matching comment in do_write_static(): leave the + // close to do_read()'s linger_close() on a body error. + if (close_connection_ && !body_error_status_) { adaptor_.shutdown_readwrite(); adaptor_.close(); diff --git a/tests/body_file_tests.cpp b/tests/body_file_tests.cpp index 25e02388ce..d881193eff 100644 --- a/tests/body_file_tests.cpp +++ b/tests/body_file_tests.cpp @@ -32,6 +32,9 @@ using asio_error_code = asio::error_code; #define LOCALHOST_ADDRESS "127.0.0.1" +#include "http_test_utils.h" +using namespace crow_test_utils; + namespace { std::string read_all(const std::string& path) @@ -63,104 +66,6 @@ namespace return true; } - bool response_complete(const std::string& data) - { - auto search_from = std::size_t{0}; - while (true) - { - const auto status_line = data.find("HTTP/1.1 ", search_from); - if (status_line == std::string::npos) - return false; - const auto code = std::atoi(data.c_str() + status_line + 9); - const auto header_end = data.find("\r\n\r\n", status_line); - if (header_end == std::string::npos) - return false; - if (code >= 100 && code < 200) - { - search_from = header_end + 4; - continue; - } - const auto length_pos = data.find("Content-Length:", status_line); - if (length_pos == std::string::npos || length_pos > header_end) - return true; - const auto length = static_cast(std::stoul(data.substr(length_pos + 15))); - return data.size() >= header_end + 4 + length; - } - } - - std::string http_body(const std::string& response) - { - auto search_from = std::size_t{0}; - while (true) - { - const auto status_line = response.find("HTTP/1.1 ", search_from); - if (status_line == std::string::npos) - return {}; - const auto code = std::atoi(response.c_str() + status_line + 9); - const auto header_end = response.find("\r\n\r\n", status_line); - if (header_end == std::string::npos) - return {}; - if (code >= 100 && code < 200) - { - search_from = header_end + 4; - continue; - } - return response.substr(header_end + 4); - } - } - - class TestClient - { - public: - TestClient(uint16_t port): - socket_(io_context_) - { - socket_.connect(asio::ip::tcp::endpoint(asio::ip::make_address(LOCALHOST_ADDRESS), port)); - } - - void send(const std::string& data) - { - asio::write(socket_, asio::buffer(data)); - } - - void send(const char* data, std::size_t size) - { - asio::write(socket_, asio::buffer(data, size)); - } - - // Reads exactly `size` more bytes into an internal buffer without - // waiting for the response to complete. Used to pause a client - // mid-response and observe server-side state while it is still - // streaming. - std::string read_some(std::size_t size) - { - std::string out(size, '\0'); - asio::read(socket_, asio::buffer(out)); - return out; - } - - std::string receive() - { - std::string response; - std::array buffer{}; - const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(5); - while (!response_complete(response)) - { - REQUIRE(std::chrono::steady_clock::now() < deadline); - asio_error_code ec; - const auto n = socket_.read_some(asio::buffer(buffer), ec); - if (ec) - break; - response.append(buffer.data(), n); - } - return response; - } - - private: - asio::io_context io_context_{}; - asio::ip::tcp::socket socket_; - }; - struct TempDir { std::filesystem::path path; @@ -491,7 +396,7 @@ TEST_CASE("request_body_file deleted only after the full response is sent", "[ht headers += client.read_some(1); const auto length_pos = headers.find("Content-Length:"); REQUIRE(length_pos != std::string::npos); - const auto content_length = static_cast(std::stoul(headers.substr(length_pos + 15))); + const auto content_length = static_cast(std::stoull(headers.substr(length_pos + 15))); REQUIRE(content_length == response_body_size); const std::size_t prefix_size = 4096; @@ -816,6 +721,42 @@ TEST_CASE("request_body_sink finish failure is 500", "[http][body_file]") app.stop(); } +TEST_CASE("request_body_sink finish failure sends exactly one response", "[http][body_file]") +{ + // Regression test: on_message_complete() must not fall through to + // process_message() (a second handle() call) after reject_body() already + // wrote the 500 for a finish() failure - that would put two status lines + // on one connection. + SimpleApp app; + app.max_body_size(1024 * 1024); + + CROW_ROUTE(app, "/sink") + .methods("POST"_method) + .body_sink([](const request&) { + return std::make_unique(); + })([](const request&) { + return "ran"; + }); + + auto server = app.bindaddr(LOCALHOST_ADDRESS).port(0).run_async(); + app.wait_for_server_start(); + + TestClient client(app.port()); + client.send( + "POST /sink HTTP/1.1\r\n" + "Host: localhost\r\n" + "Content-Length: 4\r\n" + "\r\n" + "fail"); + const auto response = client.receive(); + CHECK(response.find("HTTP/1.1 500") != std::string::npos); + + const auto leftover = client.read_leftover(std::chrono::milliseconds(200)); + CHECK(leftover.empty()); + + app.stop(); +} + TEST_CASE("request_body_sink a throwing write() or finish() is 500", "[http][body_file]") { // Regression test: write()/finish() used to be called bare; an uncaught diff --git a/tests/http_test_utils.h b/tests/http_test_utils.h new file mode 100644 index 0000000000..5503b4e15b --- /dev/null +++ b/tests/http_test_utils.h @@ -0,0 +1,166 @@ +#pragma once + +// Shared helpers for tests that drive a running Crow server over a raw +// socket (max_body_size_tests.cpp, body_file_tests.cpp). Free functions are +// `inline` because this header is included by more than one translation +// unit linked into the same `unittest` binary. + +#include +#include +#include +#include +#include + +#include "catch2/catch_all.hpp" + +namespace crow_test_utils +{ + // True once `data` holds a full response (status line through body, per + // Content-Length; a response with none is complete at the header + // terminator). Skips past any 1xx interim responses (e.g. 100 Continue). + inline bool response_complete(const std::string& data) + { + auto search_from = std::size_t{0}; + while (true) + { + const auto status_line = data.find("HTTP/1.1 ", search_from); + if (status_line == std::string::npos) + return false; + const auto code = std::atoi(data.c_str() + status_line + 9); + const auto header_end = data.find("\r\n\r\n", status_line); + if (header_end == std::string::npos) + return false; + if (code >= 100 && code < 200) + { + search_from = header_end + 4; + continue; + } + const auto length_pos = data.find("Content-Length:", status_line); + if (length_pos == std::string::npos || length_pos > header_end) + return true; + const auto length = static_cast(std::stoull(data.substr(length_pos + 15))); + return data.size() >= header_end + 4 + length; + } + } + + // The status code of the last non-interim response in `response`. + inline int status_of(const std::string& response) + { + auto search_from = std::size_t{0}; + while (true) + { + const auto status_line = response.find("HTTP/1.1 ", search_from); + if (status_line == std::string::npos) + return 0; + const auto code = std::atoi(response.c_str() + status_line + 9); + if (code >= 100 && code < 200) + { + search_from = status_line + 9; + continue; + } + return code; + } + } + + // The body of the last non-interim response in `response`. + inline std::string http_body(const std::string& response) + { + auto search_from = std::size_t{0}; + while (true) + { + const auto status_line = response.find("HTTP/1.1 ", search_from); + if (status_line == std::string::npos) + return {}; + const auto code = std::atoi(response.c_str() + status_line + 9); + const auto header_end = response.find("\r\n\r\n", status_line); + if (header_end == std::string::npos) + return {}; + if (code >= 100 && code < 200) + { + search_from = header_end + 4; + continue; + } + return response.substr(header_end + 4); + } + } + + // A bare-socket HTTP client, for tests that need to control framing + // (partial sends, reading mid-response, checking what's left on the wire) + // more precisely than a real HTTP client would allow. + // + // Relies on `asio`/`asio_error_code` and `LOCALHOST_ADDRESS` already + // being visible at the point this header is included (each including + // .cpp sets those up first, matching the CROW_USE_BOOST/standalone asio + // switch already in effect for the rest of the file). + class TestClient + { + public: + TestClient(uint16_t port): + socket_(io_context_) + { + socket_.connect(asio::ip::tcp::endpoint(asio::ip::make_address(LOCALHOST_ADDRESS), port)); + } + + void send(const std::string& data) + { + asio::write(socket_, asio::buffer(data)); + } + + void send(const char* data, std::size_t size) + { + asio::write(socket_, asio::buffer(data, size)); + } + + // Reads exactly `size` more bytes into an internal buffer without + // waiting for the response to complete. Used to pause a client + // mid-response and observe server-side state while it is still + // streaming. + std::string read_some(std::size_t size) + { + std::string out(size, '\0'); + asio::read(socket_, asio::buffer(out)); + return out; + } + + // Collects whatever bytes show up within `budget`, to check for a + // second response wrongly sent after a first one is already complete. + std::string read_leftover(std::chrono::milliseconds budget) + { + std::string leftover; + std::array buffer{}; + socket_.non_blocking(true); + const auto deadline = std::chrono::steady_clock::now() + budget; + while (std::chrono::steady_clock::now() < deadline) + { + asio_error_code ec; + const auto n = socket_.read_some(asio::buffer(buffer), ec); + if (!ec && n > 0) + leftover.append(buffer.data(), n); + else + std::this_thread::sleep_for(std::chrono::milliseconds(5)); + } + return leftover; + } + + std::string receive() + { + std::string response; + std::array buffer{}; + const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(5); + while (!response_complete(response)) + { + REQUIRE(std::chrono::steady_clock::now() < deadline); + asio_error_code ec; + const auto n = socket_.read_some(asio::buffer(buffer), ec); + if (ec) + break; + response.append(buffer.data(), n); + } + return response; + } + + private: + asio::io_context io_context_{}; + asio::ip::tcp::socket socket_; + }; +} // namespace crow_test_utils diff --git a/tests/max_body_size_tests.cpp b/tests/max_body_size_tests.cpp index a6320dc3fd..4f217c7093 100644 --- a/tests/max_body_size_tests.cpp +++ b/tests/max_body_size_tests.cpp @@ -23,87 +23,8 @@ using asio_error_code = asio::error_code; #define LOCALHOST_ADDRESS "127.0.0.1" -namespace -{ - bool response_complete(const std::string& data) - { - auto search_from = std::size_t{0}; - while (true) - { - const auto status_line = data.find("HTTP/1.1 ", search_from); - if (status_line == std::string::npos) - return false; - const auto code = std::atoi(data.c_str() + status_line + 9); - const auto header_end = data.find("\r\n\r\n", status_line); - if (header_end == std::string::npos) - return false; - if (code >= 100 && code < 200) - { - search_from = header_end + 4; - continue; - } - const auto length_pos = data.find("Content-Length:", status_line); - if (length_pos == std::string::npos || length_pos > header_end) - return true; - const auto length = static_cast(std::stoul(data.substr(length_pos + 15))); - return data.size() >= header_end + 4 + length; - } - } - - class TestClient - { - public: - TestClient(uint16_t port): - socket_(io_context_) - { - socket_.connect(asio::ip::tcp::endpoint(asio::ip::make_address(LOCALHOST_ADDRESS), port)); - } - - void send(const std::string& data) - { - asio::write(socket_, asio::buffer(data)); - } - - std::string receive() - { - std::string response; - std::array buffer{}; - const auto deadline = std::chrono::steady_clock::now() + std::chrono::seconds(5); - while (!response_complete(response)) - { - REQUIRE(std::chrono::steady_clock::now() < deadline); - asio_error_code ec; - const auto n = socket_.read_some(asio::buffer(buffer), ec); - if (ec) - break; - response.append(buffer.data(), n); - } - return response; - } - - private: - asio::io_context io_context_{}; - asio::ip::tcp::socket socket_; - }; - - int status_of(const std::string& response) - { - auto search_from = std::size_t{0}; - while (true) - { - const auto status_line = response.find("HTTP/1.1 ", search_from); - if (status_line == std::string::npos) - return 0; - const auto code = std::atoi(response.c_str() + status_line + 9); - if (code >= 100 && code < 200) - { - search_from = status_line + 9; - continue; - } - return code; - } - } -} // namespace +#include "http_test_utils.h" +using namespace crow_test_utils; TEST_CASE("max_body_size advertised length", "[http][max_body_size]") { @@ -489,3 +410,47 @@ TEST_CASE("max_body_size applies to a matched-route HEAD/OPTIONS request", "[htt app.stop(); } + +TEST_CASE("max_body_size 413 lingers instead of closing even with a small stream_threshold", "[http][max_body_size]") +{ + // Regression test: complete_request() has two response-writing paths, + // chosen by stream_threshold(), and both used to shut the socket down + // outright when close_connection_ was set - including for a 413/500 + // body-error response, racing ahead of do_read()'s linger_close() drain. + // A default-sized threshold (1 MiB) never routes a small error body + // through the streamed path, so the race was latent unless an app set a + // small stream_threshold(); this pins it down explicitly. + std::atomic handler_ran{false}; + SimpleApp app; + app.max_body_size(8); + app.stream_threshold(1); // force even the tiny 413 body through the streamed path + + CROW_ROUTE(app, "/upload") + .methods("POST"_method)([&handler_ran](const request& req) { + handler_ran = true; + return req.body; + }); + + auto server = app.bindaddr(LOCALHOST_ADDRESS).port(0).run_async(); + app.wait_for_server_start(); + const auto port = app.port(); + + TestClient client(port); + // Headers only, declaring a body well over the cap that the client sends next. + client.send( + "POST /upload HTTP/1.1\r\n" + "Host: localhost\r\n" + "Content-Length: 5000000\r\n" + "\r\n"); + const auto resp = client.receive(); + CHECK(status_of(resp) == 413); + CHECK_FALSE(handler_ran); + + // The client can still write its whole declared body without the write + // failing (broken pipe / connection reset), because the server drains + // instead of closing on unread bytes. + const std::string payload(5'000'000, 'x'); + REQUIRE_NOTHROW(client.send(payload)); + + app.stop(); +}