Add request body file storage while receiving

[anton-n-petrov]

Addresses #1064.

Why this is needed

Crow currently appends every request body into req.body (std::string). That is the right default for JSON and form fields, but a GB-class upload then occupies the same amount of RAM as the file. There is no existing pull request that writes the request body to disk while it is received (searched open/closed PRs for body-to-file, streaming upload, and large request body). The closest issue is #1064.

Receiving a body already has a read loop; the only change is to divert each decoded body span to a file instead of string::insert. Writes are still synchronous and run on the connection executor, same as today's string append.

API

app.body_file_directory("uploads");          // default: system temp directory
app.max_body_file_size(64ull * 1024 * 1024); // default: 0 = unlimited

CROW_ROUTE(app, "/upload")
  .methods(crow::HTTPMethod::Put, crow::HTTPMethod::Post)
  .body_file()([](const crow::request& req) {
      // req.body is empty; bytes are in req.body_file_path
      std::ifstream in(req.body_file_path, std::ios::binary);
      ...
  });

.body_file(<directory>) overrides the app directory for that route. Crow always generates a unique name, so concurrent requests do not share a path.

req.keep_body_file() keeps the file after the response. Otherwise it is deleted once the connection is done with the request.

Behaviour

• The route is chosen after headers, so the decision to use a file is made before the body arrives.
• Chunked request bodies are decoded by the existing parser and written as they are delivered.
• Expect: 100-continue still works; the file is opened after the 100 response.
• When max_body_file_size would be exceeded, the partial file is discarded, remaining body bytes are consumed so keep-alive stays in sync, and the client gets 413 without the handler running.
• A client that disconnects mid-body never reaches the handler; the partial file is removed.
• Ordinary routes are unchanged: they still buffer in req.body.

This stores the raw body. multipart/form-data is still parsed from req.body. Saving individual multipart parts to disk as they arrive is a separate feature.

Coverage and documentation

Tests cover in-memory vs file routes, a 64 KiB payload received in multiple reads, binary data, empty bodies, chunked request bodies, 100-continue, keep_body_file(), a route-level directory, keep-alive reuse, 413, and disconnect cleanup.

The guide is docs/guides/body-file.md. examples/example_body_file.cpp shows a raw PUT/POST upload.

[ssubbotin]

Reviewed at 1a6465f5 against e91d0e75. I read the diff plus the surrounding request path in full, and I checked the load-bearing claims against a server built from this branch rather than only from the source. Items marked [measured] were reproduced against that running server.

First, the good news: the branch is healthy. It builds clean with -DCROW_BUILD_TESTS=ON, body_file_tests.cpp passes (4 cases, 56 assertions), and the full suite passes 128/128 (1025 assertions) from the repository root, so no regressions. Keep-alive after a 413 works exactly as the guide claims: a second request on the same socket answers 200. [measured] The size-cap arithmetic at parser.h:100-102 is correct: length > max is tested before max - length, so there is no wraparound, and a body of exactly max bytes is accepted. Routing before the body is correct too: handle_url() runs from on_url (http_connection.h:112), so the rule really is known before body bytes arrive, and Router::get_rule()'s index guards are right, since RULE_SPECIAL_REDIRECT_SLASH == 1 (routing.h:786) and PerMethod(): rules(2) (routing.h:1924-1925) reserve indices 0 and 1. Blueprint rules land in the same per_methods_[m].rules (routing.h:1387), so .body_file() works inside a Blueprint, and OPTIONS synthesis is excluded by the early return you added at http_connection.h:139-145.

My overall read is rework rather than merge, for two reasons: the patch does not close the issue it cites, and its failure policy makes the size limit cost more than having no limit at all. Details below, then a concrete proposal.

Blocking

1. The limit is enforced after the bytes, so 413 is an amplifier

handle_header() never looks at Content-Length, even though the vendored parser has already produced it: content_length is a member of http_parser (http_parser_merged.h:159), assigned before state = s_headers_done and before the on_headers_complete callback fires at :1623. Instead the cap is applied per chunk in on_body (parser.h:100-105), and an over-limit body is drained (parser.h:280 sets body_discard_, checked at :96), with the 413 going out only at on_message_complete.

With max_body_file_size(8) and a client sending Expect: 100-continue and Content-Length: 5000000, the server answered HTTP/1.1 100 Continue before a single body byte, accepted all 5,000,000 bytes, and only then sent 413. [measured] The Expect mechanism exists precisely so the server can refuse before the body. The connection deadline does not bound this either, because it is re-armed after every successful read (http_connection.h:439).

One detail that matters for the fix: content_length is decremented as identity-body bytes are consumed (http_parser_merged.h:1737), so the header callback is the only point where the advertised length is still intact.

2. The knob guards only the opted-in route, so #1064 stays open

max_body_file_size applies only where .body_file() was set. Every other path still runs req.body.insert(...) (parser.h:116) unbounded, and Crow has no request body limit anywhere else. A near miss of the route is enough: POST /probe/ against a rule registered as /probe answered 404 after buffering 200,000 bytes into req.body, with the cap not applied. [measured] The same holds for an unknown URL, a wrong method (405, routing.h:1748), and a slash redirect.

The spool also runs before any request validation. POST /probe HTTP/1.1 with no Host header and a 20 MB body wrote ~8.7 MB into the spool directory while the client was still sending, and answered 400 only after the whole body had landed on disk. [measured] The Host check lives in handle() (http_connection.h:180), which runs at on_message_complete.

Crow already ships the idiom worth copying here: app.websocket_max_payload() (app.h:340) with the per-rule override WebSocketRule::max_payload (routing.h:478), and websocket.h:515-521 closes on over-limit instead of draining.

3. The temporary file loses its exclusive handle

utility.h:833-836 creates the file with O_CREAT | O_EXCL | O_WRONLY, 0600, then closes the descriptor and returns a path, and parser.h:149 reopens that path with std::ofstream(..., trunc). Everything O_EXCL bought is discarded in that window. A process running as the server's own uid can unlink the name and drop a symlink in its place (the sticky bit on /tmp stops other uids, not the same uid), and the reopen has no O_NOFOLLOW. If the name is merely unlinked, the reopen creates a fresh file with 0666 & ~umask, so an upload meant to be 0600 becomes world-readable. That is CWE-367 plus CWE-59.

A second pathname race survives even after fixing the first: the parser closes the file at on_message_complete (parser.h:123) and the handler then reopens body_file_path by name.

The Windows branch (utility.h:840-847) never creates exclusively at all: exists() followed by a plain ofstream, so uniqueness is not atomic and the file inherits the directory ACL. The error_code declared at :821 and last written by create_directories at :825 is reused for that exists() call and never re-checked, so a failed query is indistinguishable from "the file is already there".

4. A failed final flush is reported as a successful upload

close_body_file_stream() (parser.h:257-264) calls flush() and close() and checks neither. on_message_complete then sets req.body_file_size = body_bytes_ and runs the handler (parser.h:123-127). A filesystem that fills while the last stream buffer drains hands the handler a truncated file that reports the full advertised byte count. This is the one pure data-integrity defect in the patch, and it falsifies the feature's core promise under exactly the condition the feature exists to survive. Related: std::filesystem::remove errors are swallowed at parser.h:271-273 and the path is cleared regardless, so a partial upload can be left behind while the request state says it was discarded.

5. Neither middleware nor after-handlers run for a rejected body

req.body_error_code sets is_invalid_request (http_connection.h:174-178); the before-handler pass sits inside if (!is_invalid_request) (:210) and need_to_call_after_handlers_ is left false (:209), so complete_request() skips the after-handlers. A CORS middleware never stamps the 413, so a browser surfaces an opaque network error instead of the status, and an access-log middleware never records it. The pre-existing missing-Host 400 has the same shape, but that path is rare, whereas this one is the feature's own expected outcome.

Non-blocking, but worth fixing

• A body file is created for requests with no body. A bodyless GET on a .body_file() route reached the handler with has_file=1 size=0 path=.../crow-body-DNQIKCbUZqXHrjHh, and every HEAD that falls back to the GET rule does the same. [measured] Each one costs a random_device open, an O_EXCL create, an ofstream open, a close and an unlink. The parser already knows there is no body.
• create_body_file_path() can throw out of a C callback. random_alphanum constructs a fresh std::random_device per call (utility.h:799), which can throw std::system_error on fd exhaustion or in a container without /dev. handle_header() has no try/catch, so the throw unwinds through http_parser_execute up to io_context::run(), where http_server.h:197-208 logs "Worker Crash" and the connection is dropped with no response. Worth noting that High CPU load when no FD available #1228 just landed fd-exhaustion handling for the acceptor, so that state is live.
• The 128-attempt retry loop is errno-blind (utility.h:829-849). It retries on any ::open failure, not only EEXIST. On EACCES, ENOSPC or EMFILE it burns 128 random_device constructions and 128 open() calls on the io thread before returning "" and a 500.
• create_directories runs on every request (utility.h:825): one extra stat/mkdir pair per request, silently creating directories with 0777 & ~umask.
• routing_handle_result leaves method uninitialised in its three-argument constructor (common.h:298-301), which is the one both Trie::find returns use (routing.h:947, :1074). get_rule() reads found.method at routing.h:1878 and is correct only because the || at :1876 short-circuits first. Now that get_rule() is public, that is one refactor away from UB. Default member initialisers on the struct would settle it.
• Deletion is tied to parser_.clear(), which fires at surprising times. For a response above res_stream_threshold_ (1 MiB), do_write_general calls do_write_sync per 16 KiB chunk (http_connection.h:385), and each call runs parser_.clear() (:501), so the upload is unlinked after the first chunk rather than "after the response is sent" (docs/guides/body-file.md:45). Incidentally that same call empties res.body while the loop keeps streaming from a pointer captured at :379-380; that is pre-existing and works only because std::string::clear() does not deallocate, but it is not a code path I would want to hang filesystem side effects on.
• Blocking writes on the io thread at 4 KiB granularity. buffer_ is 4096 bytes (http_connection.h:533), so each socket read produces one ofstream::write on the io_context thread that also owns the deadline timer (http_server.h:189-191). The static file response has the same property, so this is a precedent rather than a new class of bug, but it is the operational limit of a feature sold for GB uploads and the guide does not mention it.
• A crash leaks the partial file. SIGKILL, the OOM killer or abort() all skip ~HTTPParser. This follows from handing the handler a path: the file cannot be unlinked at creation time, which is the POSIX idiom that makes the leak impossible.
• keep_body_file() on a copy of request is a silent no-op, because the parser only ever consults its own req.persist_body_file (parser.h:204).
• The example undoes the feature: examples/example_body_file.cpp:19-20 reads the whole spooled file back into a std::string, which is the allocation the feature exists to avoid.

API surface

Load-bearing, and worth keeping: .body_file() on the route builder (chaining like .methods() is the right Crow shape), app.body_file_directory(), req.body_file_path, req.has_body_file().

The rest reads as internals in public headers:

• app.should_save_body_to_file() and app.create_body_file_path() (app.h:553, :560): two public Crow<> methods whose own comments say "used by the connection", each repeating the same get_rule lookup. One private helper covers both.
• Router::get_rule() (routing.h:1874): hands a raw BaseRule* out of the router's private index, and exports the rule-index conventions along with it.
• HTTPParser::open_body_file() (parser.h:143) and HTTPParser::discard_remaining_body() (parser.h:166): HTTPParser is a struct whose private: does not start until :256, so both are public API on a type users instantiate.
• req.body_error_code (http_request.h:56): a parser-to-connection channel parked on every request. A handler can never observe it, since a non-zero value is exactly the condition under which the handler is not invoked.
• req.body_file_size (http_request.h:55): a write counter that is 0 on the failure path and diverges from reality when the final flush fails (issue 4 above).
• req.persist_body_file as a public mutable bool flipped by a const method (http_request.h:66, :116): nothing else in request is mutable, and there is no way to undo it. std::string take_body_file() expresses the same ownership transfer without the trick, and removes the copy-of-request trap.
• utility::create_temporary_file() (utility.h:819): a security-sensitive primitive published under a generic name, returning a path rather than a handle, with a prefix parameter that can contain path separators or an absolute path, in which case path::operator/ discards dir entirely.
• The per-route directory override looks speculative to me. An app-global directory plus a rename() in the handler covers the cases I can name.
• BaseRule::stores_body_in_file() / body_file_directory() (routing.h:158, :164) add bool + std::string to every rule deriving from BaseRule, including WebSocketRule and the static-file rules, which cannot use them.

For comparison, the response side solves the same problem and keeps it private: static_file_info file_info lives in response's private section behind friend class crow::Connection (http_response.h:462), exposing only set_static_file_info() and is_static_type(). The request side here exposes four raw public members plus six methods across four headers. Mirroring the response side would be more consistent.

Two smaller idiom points: max_payload_{UINT64_MAX} (app.h:961) and max_body_file_size_{0} (app.h:969) use opposite sentinels for "unlimited" eight lines apart, and 0 is a legitimate policy ("reject all bodies") that the current choice makes inexpressible. And body_file() / stores_body_in_file() / body_file_directory() are three stems for one bit.

What I would merge instead

Step 1, its own PR: a general request body limit

This is the actual fix for #1064, and it is small.

// app.h, next to websocket_max_payload (app.h:340)
self_t&  max_body_size(uint64_t bytes);   // UINT64_MAX = unlimited, matching max_payload_
uint64_t max_body_size() const;

// routing.h, on RuleParameterTraits, mirroring WebSocketRule::max_payload (routing.h:478)
self_t& max_body_size(uint64_t bytes);    // per-route override

Mechanism, in handle_header(), before the Expect: 100-continue write at http_connection.h:131:

1. Resolve the effective limit (route override, else app value) through a private router helper, and apply it on the catch-all, 404, 405 and slash-redirect paths too.
2. If parser_.content_length != CROW_ULLONG_MAX && content_length > limit: answer 413 with Connection: close, do not send 100 Continue, close. Read no body at all.
3. Chunked bodies have no advertised length, so count in on_body and close on exceed, matching websocket.h:515-521. Do not drain.
4. While there, req.body.reserve(content_length) when it is known and under the limit, which also removes the std::string reallocation doubling.

One plumbing note: on_headers_complete currently always returns 0 (parser.h:91) and process_header() / handle_header() are void (parser.h:222-225), so returning 1 to set F_SKIPBODY (http_parser_merged.h:1633-1634) needs that return value threaded through, and skipping the body has to be paired with closing the connection, otherwise the socket desyncs.

Step 2: the spool, on an internal sink seam

Keep .body_file() as the public API. Make the internal mechanism a body destination (memory / file / discard) instead of a hard-coded ofstream branch, so the file case owns a descriptor.

// routing.h, on RuleParameterTraits
self_t& body_file();                       // app directory
self_t& body_file(std::string directory);  // optional, if a case for it turns up

// http_request.h
std::string body_file_path;                // empty => the body is in `body`
bool        has_body_file() const;
std::string take_body_file();              // transfers ownership; the parser will not delete it

Required properties, each answering one of the issues above:

• Create with O_CREAT | O_EXCL | O_WRONLY, 0600 (Windows: CREATE_NEW with a restrictive ACL) and keep the descriptor. Never reopen by path. (Issue 3.)
• Create the file only when the parser says a body is coming. (Bodyless GET/HEAD.)
• Check the final flush() / close() and turn failure into the 500 path. (Issue 4.)
• Route the 413/500 through the normal completion path so global middleware and after-handlers see it. (Issue 5.)
• Retry only on EEXIST, and return an error otherwise. Resolve the directory once at configuration time instead of calling create_directories per request.
• Say in the guide that writes are synchronous on the io thread.

That lets should_save_body_to_file, create_body_file_path, Router::get_rule, req.body_error_code, req.body_file_size, the public persist_body_file, utility::create_temporary_file and the public HTTPParser methods all disappear: roughly half the added API, same feature.

Step 3, separate proposal: a public streaming sink

A user callable invoked from on_body inside http_parser_execute, with no middleware context and its own answers for pipelining, exceptions and shutdown, changes Crow's "the handler runs once the request exists" contract. I think it is the right eventual primitive and that it subsumes the file case, but it should not be a precondition for closing #1064.

Tests

The four cases pass and cover real ground: memory vs file, 64 KiB across multiple reads, binary, empty, chunked, 100-continue, keep_body_file(), a route-level directory, keep-alive reuse, 413, disconnect cleanup, unique names. What is missing is the production contract:

• Content-Length above the cap at header time, and Expect: 100-continue with an over-limit body (issue 1);
• a large body to an unmatched URL, to a 405, to a slash redirect, and to a request with no Host (issue 2);
• the accumulate-across-chunks branch of the cap: the only over-limit test sends 9 bytes against a limit of 8, which a single on_body satisfies through length > max alone;
• the exact boundary body_bytes_ + length == max;
• open_body_file failure to 500, and a write or flush failure (issue 4);
• keep-alive survival after a 413, which the guide advertises and the current 413 test does not exercise, since it closes the socket immediately after reading the response;
• middleware behaviour on the 413 path (issue 5);
• GET and HEAD on a .body_file() route;
• concurrent uploads, pipelined bytes arriving in one feed(), and a deferred (req.post()) response;
• the 0600 mode of the created file, which is the property create_temporary_file's test should be asserting.

Three smaller test-side notes: directory_is_empty (tests/body_file_tests.cpp:49-53) swallows its error_code, so a failed directory_iterator compares equal to end() and the two assertions that depend on it (:332, :369) pass vacuously; the fixed ports 45580-45582 will flake in CI and prevent parallel runs, where Catch2 SECTIONs inside one TEST_CASE would not; and the :-delimited response parsing at :224-228 breaks on a Windows drive letter.

Thanks for the patch, and for the care that clearly went into the routing and lifetime plumbing. The core of it (route opts in, the parser diverts the body, the file is cleaned up on every termination path) is sound. My argument is only that the size limit belongs on every request rather than on this one route, that the limit has to be decided at headers-complete and close the connection instead of draining, and that the spool should own its descriptor.

[gittiver]

wouldn't it be better to use a stream interface instead of a file (there is not always a file system available especially on embedded systems or otherwise limited systems)?

[anton-n-petrov]

Superseded by two PRs, per the reviews below.

• Add a request body size limit that closes over-limit connections #1234 — the general request body size limit (max_body_size), which is the actual fix for Support for Streaming Large File Uploads (GB-level) in Crow 1.2.0 (C++14 Environment) #1064 and doesn't depend on any file/sink feature.
• Add a request body sink, with FileBodySink as a file-backed wrapper #1235 — a .body_sink() primitive with .body_file() as a file-backed wrapper, built on top of Add a request body size limit that closes over-limit connections #1234.

This follows @ssubbotin's review here (#1233 (comment)), which asked for the limit and the spool to be split into their own PRs, and @gittiver's comment here (#1233 (review)), which asked for a stream interface instead of a file-only approach — .body_sink() in #1235 is the direct answer to that.

Closing this one in favor of #1234 and #1235.