Add chunked content provider for streaming responses by ssubbotin · Pull Request #1213 · CrowCpp/Crow

ssubbotin · 2026-08-05T18:42:45Z

Closes #16.

Today a response body has to exist in full before it can be sent: response::body is a
std::string and response::write() appends to it. The only path that actually writes
data out in pieces is do_write_static(), and it needs a regular file on disk, since
set_static_file_info() calls stat and checks S_ISREG. That leaves no way to answer
with a body that is generated on the fly, or one that is simply too large to hold in
memory.

This adds a chunk provider to response and a third write path in Connection next to
the existing static and general ones.

Usage

CROW_ROUTE(app, "/numbers")
([](const crow::request&, crow::response& res) {
    int remaining = 100;
    res.set_chunked_content_provider(
      [remaining](std::string& chunk) mutable -> bool {
          if (remaining == 0)
              return false;
          chunk = std::to_string(100 - remaining) + '\n';
          --remaining;
          return true;
      },
      "text/plain");
    res.end();
});

The provider fills the given string with the next piece of the body and returns true
while more data is coming, false on its last invocation. Leaving the string empty is
allowed and sends no chunk. set_chunked_content_provider() sets
Transfer-Encoding: chunked and suppresses Content-Length through the existing
manual_length_header flag.

Notes on the implementation

The connection deadline is cancelled before the write loop, the same way
do_write_general() does for large bodies. Without it a body that takes longer to
produce than the timeout gets cut short, which is the failure mode described in Streaming response #16.
do_write_sync() clears the response on every write, so the provider is moved out of
the response before the loop starts.
A kept-alive connection is put back into reading state after the last chunk, since the
deadline was cancelled during the transfer.
A HEAD request drops the provider in response::end(), so no body is produced.

Tests

Four cases added to tests/unittest.cpp: a normal multi-chunk response, a provider that
yields no data at all, a body larger than a single chunk, and a HEAD request.

Locally the full suite passes except send_file, which fails identically on an
unmodified master in my environment and is unrelated to this change.

anton-n-petrov

Blocking on the two header bugs; suggestions are optional.

anton-n-petrov · 2026-08-06T12:22:59Z

                if (skip_body)
                {
+                    chunk_provider_ = nullptr;
                    set_header("Content-Length", std::to_string(body.size()));


[bug] For HEAD, end() nulls the chunk provider (good) but then always sets Content-Length to body.size() (usually 0) while leaving Transfer-Encoding: chunked in place. That violates RFC 7230 §3.3.2/§3.3.3 (sender must not send both) and is wrong for HEAD semantics: the length should match what GET would produce, and for streaming length is unknown so TE: chunked without a body and without Content-Length is the right shape. The HEAD test only asserts the literal "body" is absent, so this slips through.

Suggestion: When skip_body and a chunk provider is present, drop the provider, keep Transfer-Encoding: chunked and manual_length_header = true, clear body, and do not set Content-Length. Extend chunked_response_head_request to assert Transfer-Encoding: chunked is present, Content-Length is absent, and the body is empty.

Fixed in 0981e28: when skip_body finds a chunk provider, the providers are dropped, the body is cleared, manual_length_header is set and Content-Length is not written, so a HEAD response keeps Transfer-Encoding: chunked alone. chunked_response_head_request now asserts TE present, Content-Length absent, empty body.

anton-n-petrov · 2026-08-06T12:22:59Z

+        void set_chunked_content_provider(chunk_provider_t provider, std::string content_type = "")
+        {
+            chunk_provider_ = std::move(provider);
+            manual_length_header = true;


[bug] set_chunked_content_provider sets manual_length_header = true and Transfer-Encoding: chunked, but never removes an existing Content-Length header. Any prior set_header("Content-Length", ...) (or other path that added it) is still emitted from write_header_into_buffer, producing both headers on the wire for a chunked response.

Suggestion: Erase Content-Length when enabling the chunked provider (e.g. headers.erase("Content-Length") via the ci_map), and/or document that callers must not set Content-Length with this API. Prefer the erase so the API is hard to misuse.

Fixed in ec62cee: enabling the chunked provider now erases any previously set Content-Length (both overloads go through this path), and the doc comment states it.

anton-n-petrov · 2026-08-06T12:22:59Z

+            {
+                chunk.clear();
+                more = provider(chunk);
+                if (chunk.empty())


[suggestion] Empty chunks are skipped with continue while more remains true. A provider that repeatedly returns true with an empty string (the docs encourage empty chunks when "no bytes yet") busy-loops forever on the connection thread, never yielding, never hitting the deadline (already cancelled), and never completing the response.

Suggestion: Document that empty+true must not spin (caller should block/sleep until data is ready, or return false). Optionally guard with a max consecutive empty iterations, or treat N empty returns as an error and abort the write with a log. At minimum, strengthen the streaming guide so "not ready yet" is not read as a poll-in-a-tight-loop API.

Addressed in 2026027 by documenting the contract (both provider doc comments and the streaming guide): an empty chunk is fine as an occasional occurrence, but a provider with no data at hand should block until data is available or finish the transfer. I deliberately did not add a max-consecutive-empty guard: any threshold would be arbitrary and could break legitimate providers with rare pauses, while the contract makes the intent explicit.

anton-n-petrov · 2026-08-06T12:22:59Z

            headers = std::move(r.headers);
            completed_ = r.completed_;
            file_info = std::move(r.file_info);
+            chunk_provider_ = std::move(r.chunk_provider_);


[suggestion] Move-assignment now moves chunk_provider_ and headers (including Transfer-Encoding: chunked) but still does not move manual_length_header. A moved chunked response can therefore keep TE: chunked while manual_length_header stays false on the target, causing write_header_into_buffer to also inject Content-Length: 0.

Suggestion: Move (or copy) manual_length_header (and ideally skip_body / compressed) in operator=, matching the other response state that affects wire format.

Fixed in f5ab73f: move assignment now carries manual_length_header, skip_body and (under CROW_ENABLE_COMPRESSION) compressed.

anton-n-petrov · 2026-08-06T12:22:59Z

+            while (more && !ec)
+            {
+                chunk.clear();
+                more = provider(chunk);


[suggestion] provider(chunk) is invoked outside any try/catch. Route-level exception handlers only wrap handler execution, not the later write path. A throwing provider aborts mid-stream (headers may already be sent, no final chunk), can escape into the Asio completion stack, and leaves the connection unclean.

Suggestion: Catch exceptions around the provider call, log, stop the chunk loop, close the connection, and avoid restarting keep-alive reads. Document that providers should not throw.

Fixed in aefa8f4: the provider call is wrapped in try/catch; an exception is logged and treated as an abort (no terminating frame, forced close, completion handler gets clean == false, no keep-alive restart). Covered by the new chunked_response_throwing_provider test, and the doc comment now says providers should not throw.

anton-n-petrov · 2026-08-06T12:22:59Z

+    app.wait_for_server_start();
+
+    HttpClient client(LOCALHOST_ADDRESS, 45451);
+    client.send("GET /chunks HTTP/1.0\r\n\r\n");


[suggestion] All new chunked tests speak HTTP/1.0. Chunked transfer coding is an HTTP/1.1 feature; real 1.0 clients are not required to understand it. The tests pass only because the harness is a raw TCP client looking for the 0\r\n\r\n trailer.

Suggestion: Use HTTP/1.1 with a Host header in these tests (and/or auto-upgrade the response version when chunked is used). Optionally assert behavior for keep-alive + a second request on the same connection after a chunked response.

Fixed in 8bf9162: all chunked tests now speak HTTP/1.1 with a Host header, and chunked_response additionally issues a second request on the same kept-alive connection to verify the connection is returned to reading after a chunked transfer.

anton-n-petrov · 2026-08-06T12:22:59Z

+    for (std::string::size_type pos = response.find("400\r\n"); pos != std::string::npos;
+         pos = response.find("400\r\n", pos + 1))
+        ++seen;
+    CHECK(seen >= chunk_count);


[nit] CHECK(seen >= chunk_count) is a weak lower bound. A regression that duplicated chunk headers would still pass. Body integrity is never checked for the large case.

Suggestion: Assert seen == chunk_count and/or verify decoded payload length chunk_count * chunk_size after a minimal chunk decode.

Fixed in 485e609: the test now decodes the chunked body frame by frame and asserts seen == chunk_count plus the total decoded length of chunk_count * chunk_size.

A chunk provider can now return chunk_result (more/done/abort) instead of bool: abort closes the connection without the terminating frame, so the client sees a truncated body. An optional completion handler reports whether the body was written cleanly. The bool provider overload keeps its exact behaviour by wrapping into the new one.

A HEAD response to a chunked route used to get Content-Length: 0 while Transfer-Encoding: chunked was still set, sending both headers at once (forbidden by RFC 7230) and misrepresenting what a GET would return. Now the provider is dropped, the body stays empty, Transfer-Encoding: chunked is kept and Content-Length is not set.

A handler that set Content-Length before calling set_chunked_content_provider() would send both Content-Length and Transfer-Encoding: chunked, which RFC 7230 forbids. The header is now erased when the provider is installed; the bool overload delegates to the chunk_result one, so both are covered.

skip_body, manual_length_header and (under CROW_ENABLE_COMPRESSION) compressed were left at their defaults when a response was move-assigned. A moved chunked response would then have manual_length_header == false and write_header_into_buffer() would append Content-Length: 0 next to Transfer-Encoding: chunked.

The provider used to be called outside any try/catch, so an exception would propagate into the Asio write path. It is now caught in do_write_chunked(), logged, and handled exactly like chunk_result::abort: no terminating frame, forced close, completion handler called with clean == false, no return to keep-alive reading.

An empty chunk is allowed as an occasional occurrence; a provider that has no data yet should block until data is available or finish the transfer, since returning empty chunks in a tight loop spins the connection thread needlessly. Stated in the doxygen of both provider types and in the streaming guide.

Chunked transfer encoding belongs to HTTP/1.1, so the tests now send HTTP/1.1 requests with a Host header, matching the other 1.1 tests. The basic test also sends a second request on the same connection to verify that a kept-alive connection goes back to reading state after the chunked transfer.

Counting occurrences of the hex size line only proved a lower bound. The test now walks the chunked body frame by frame and checks the exact frame count and the exact decoded body length.

anton-n-petrov

Requesting changes for incomplete framing on write failure in do_write_chunked: unlike chunk_result::abort, a mid-transfer ec can leave keep-alive and restart do_read() without a terminating chunk. Please force-close (and skip the keep-alive restart) whenever the chunked body did not finish cleanly.

Other notes (completion-handler exceptions, HEAD not calling the completion handler, static vs chunked mutual exclusion) are non-blocking suggestions. Prior review items look fixed.

anton-n-petrov · 2026-08-06T15:26:29Z

+
+            // The deadline was cancelled for the duration of the transfer, so a kept-alive
+            // connection has to be put back into reading state explicitly.
+            if (!aborted && !close_connection_ && need_to_start_read_after_complete_)


[bug] On a mid-transfer write failure (ec set after a chunk or after headers), the code does not force-close the socket the way chunk_result::abort does. The terminating frame is correctly omitted (result == done && !ec), but if the connection is keep-alive (!close_connection_ and need_to_start_read_after_complete_), do_read() is restarted on a connection whose HTTP message framing is incomplete. Abort was carefully designed so clients see a truncated body; write errors produce the same incomplete framing without the same connection lifecycle. (Socket write errors often imply a dead peer, but when the socket remains open this desynchronizes keep-alive.)

Suggestion: Treat write failures like abort for connection policy: if ec after any incomplete chunked body (no successful terminating frame), shut down/close the adaptor and do not restart do_read(). e.g. const bool force_close = aborted || static_cast<bool>(ec); and gate the keep-alive restart on !force_close.

Fixed in 2fb5f6d: a write failure now follows the same connection policy as an explicit abort (force_close = aborted || ec) - the socket is shut down and keep-alive reads are not restarted on a connection with incomplete framing.

anton-n-petrov · 2026-08-06T15:26:29Z

+
+            if (completion_handler)
+            {
+                completion_handler(result == response::chunk_result::done && !ec);


[suggestion] Provider exceptions are caught and converted to abort, but completion_handler(...) is invoked without a try/catch. If the handler throws, control never reaches the subsequent close_connection_ shutdown, res.clear(), or keep-alive do_read() restart, and the exception can escape into the Asio callback stack—the same class of problem previously fixed for providers.

Suggestion: Wrap the completion-handler call in try/catch (log and continue cleanup), matching the provider path. Document that the handler should not throw.

Fixed in 2fb5f6d: the completion handler is now invoked under try/catch (logged and swallowed), so an exception cannot skip the cleanup or escape into the Asio callback stack. Documented that the handler should not throw.

anton-n-petrov · 2026-08-06T15:26:29Z

+                        // "Transfer-Encoding: chunked" is kept and "Content-Length" is not
+                        // set (RFC 7230 forbids sending both at once). The body itself is
+                        // skipped, so the provider is dropped without being called.
+                        chunk_provider_ = nullptr;


[suggestion] On HEAD (skip_body), providers are nulled without being called (good, and covered by tests/docs), but chunk_complete_ is left set and is never invoked: write goes through do_write_general because is_chunked_type() is already false, and res.clear() later discards the handler. The docs present the completion handler as the place “to release the source of the data,” so HEAD silently skips that cleanup path. RAII via provider captures still works when the std::function is destroyed; relying only on the completion handler does not.

Suggestion: Either invoke the completion handler for HEAD (e.g. clean=true if headers were sent, or document a dedicated meaning), or explicitly document that HEAD never runs the completion handler and that resource lifetime must be tied to the provider (or other RAII), not only to set_chunked_completion_handler.

Fixed in 8c6cf80: the completion handler now runs for HEAD as well (with clean == true) at the point the providers are dropped, so it stays the single release point for the data source. Covered by an extension of chunked_response_head_request and documented in the guide.

anton-n-petrov · 2026-08-06T15:26:29Z

+        /// and "Content-Length" must not be sent together.
+        void set_chunked_content_provider(chunk_provider_ex_t provider, std::string content_type = "")
+        {
+            chunk_provider_ex_ = std::move(provider);


[suggestion] set_chunked_content_provider does not clear file_info, and set_static_file_info_unsafe does not clear chunk providers. complete_request prefers is_static_type() over is_chunked_type(). If both are set, the static path sends the raw file body while headers may still include Transfer-Encoding: chunked (and possibly a Content-Length from the file after a reverse call order)—illegal/conflicting framing relative to RFC 7230.

Suggestion: Make body sources mutually exclusive: in set_chunked_content_provider, clear file_info (and ideally body); in set_static_file_info_unsafe, null chunk providers/completion and erase Transfer-Encoding when installing Content-Length. Optionally assert/log if both were set.

Fixed in 408cadf: the setters now discard each other's body source. set_chunked_content_provider clears file_info and the string body; set_static_file_info_unsafe drops the providers, the completion handler and the Transfer-Encoding header before installing Content-Length. Whichever source is configured last wins; covered by chunked_provider_excludes_other_body_sources.

The router marks a HEAD request by setting skip_body on the connection's response before the handler runs. Copying the flag from the source response in operator= let a handler that assigns a freshly built response reset it, so HEAD responses to plain routes carried the GET body again (caught by the http_method test).

…andler A write failure after the headers or a chunk leaves the message framing incomplete: the terminating frame is never sent. Restarting keep-alive reads on such a connection desynchronizes it, so write errors now follow the same connection policy as an explicit abort: force the close and do not reuse the socket. The completion handler is also invoked under try/catch now, matching the provider: an exception escaping it skipped the connection cleanup and could propagate into the Asio callback stack.

The docs present the completion handler as the place to release the source of the data, but a HEAD response dropped the provider and never invoked the handler: the write went through the general path and res.clear() discarded it silently. The handler now runs (with clean == true) at the point the providers are dropped, so it stays the single release point regardless of the request method.

complete_request prefers the static path over the chunked one, so a response carrying both sent the raw file bytes while the headers still announced "Transfer-Encoding: chunked" next to the file's "Content-Length" - conflicting framing either way the calls were ordered. Each setter now discards the other body source together with its framing header: the source configured last wins.

ssubbotin · 2026-08-06T17:51:25Z

All four comments addressed (2fb5f6d, 8c6cf80, 408cadf). The red CI had two causes: most jobs died on a GitHub infra hiccup ("Service Unavailable" while resolving actions), and the one real failure (http_method on macOS) was a regression from the previous round - moving skip_body in the response move-assignment let a handler-built response overwrite the flag the router sets for HEAD. Fixed in 44a443e by keeping skip_body out of the move. Full suite passes locally (133 cases / 1144 assertions).

anton-n-petrov

LGTM — re-reviewed after the follow-up commits (HEAD headers, Content-Length erase, abort/write-error handling, completion on HEAD, static vs chunked exclusivity, tests).

Prior review items are addressed. Residual note only: empty chunk + more can still spin the connection thread; that's documented as contract, fine for merge.

ssubbotin added 4 commits August 5, 2026 20:33

Add chunked content provider to response

5835259

Write chunked responses in the connection

5d6c835

Add tests for chunked responses

5a52b50

Document chunked responses

a96ac1f

anton-n-petrov suggested changes Aug 6, 2026

View reviewed changes

ssubbotin added 8 commits August 6, 2026 14:35

Strengthen the large body test with real chunked decoding

485e609

Counting occurrences of the hex size line only proved a lower bound. The test now walks the chunked body frame by frame and checks the exact frame count and the exact decoded body length.

anton-n-petrov suggested changes Aug 6, 2026

View reviewed changes

ssubbotin added 4 commits August 6, 2026 19:47

anton-n-petrov approved these changes Aug 6, 2026

View reviewed changes

Uh oh!

Uh oh!

Conversation

ssubbotin commented Aug 5, 2026

Usage

Notes on the implementation

Tests

Uh oh!

anton-n-petrov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

anton-n-petrov left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ssubbotin commented Aug 6, 2026

Uh oh!

anton-n-petrov left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants