EOF semantics -- don't use empty read as EOF marker, but asio::error::eof to better conform to ASIO - #10
Merged
Conversation
Reading the end of a body now completes with asio::error::eof and zero bytes -- and keeps doing so for every read after it, even once the underlying stream is gone -- instead of the old zero-sized read. A body cut short still reports http::error::partial_message, so the two cases stay distinguishable. A zero-length read is not a read and completes immediately with success, wherever the body stands. Writing gains an explicit async_write_eof([buffer]) on client::Request and server::Response; impl::Writer::async_write() takes the eof flag alongside the buffer. The last bytes of a body and the flag that ends it travel in the same protocol element -- one DATA frame with END_STREAM, one QUIC STREAM frame with FIN, one final serializer pass -- so ending a body that has a tail of data left costs no second, empty write and no extra round trip. An empty async_write() is now a plain no-op that leaves the body open. All three backends answer the same write-entry ladder, which also survives reader/writer detach (the adapters latch an executor copy and how the body stood): an empty non-EOF write succeeds always; after the end, data through either entry point completes with errc::broken_pipe while a bare re-end is idempotent; only then do stream-level failures get their say. The immediate-completion idiom the ladder relies on is hoisted into common.hpp as complete_immediately(). Ending a body is split into intent and delivery everywhere, so a cancelled async_write_eof() stays re-issuable instead of silently succeeding with the terminator never sent: h1 latches eof_submitted only on write success, h2 keeps eof_requested (accepted) apart from eof_submitted (handed to nghttp2) and re-arms a still-owed FIN, and h3 tracks fin_offered and rolls the intent back when the FIN never reached nghttp3. On the h3 side, cancelling a *data-carrying* EOF write now cleans up exactly like a cancelled data write (ZeroCopy resets the stream, Staged retires the chunk) -- only a bare FIN may stay pending with a detached handler; keeping a data write active after its handler returned the buffer to the caller was a use-after-free. fail() releases a handler-less active write so a re-issued end cannot be adopted onto a stream nghttp3 will never poll again. server::Response initiations are bound to the response's executor, which is what lets tokens that need one -- cancel_after's timer -- be applied to these operations directly. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Bodies are ended with async_write_eof() -- with the last buffer attached where one is at hand: dump() and h2spec() send payload and end together, and serve_file() hands the whole mmap()ed file plus the end of the message to the transport in a single call, an empty file included. Read loops go through the new drain() helper (or its as_tuple shape), the one canonical read-to-end loop: read until asio::error::eof, let anything else surface as an exception. eat_request(), count(), try_receive() and client_main reuse it instead of keeping hand-rolled copies; client_main's private read_response() duplicate is gone in favor of anyhttp::read_response(). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Existing tests move off the empty-write-means-EOF convention. New coverage for the contract itself, across all three protocols: - WHEN_body_ends_THEN_read_reports_eof: eof with zero bytes, repeated for reads past the end -- surviving stream teardown in between -- and a zero-length read reporting nothing. - WHEN_empty_buffer_is_written_THEN_body_stays_open: an empty async_write() no longer ends anything. - WHEN_written_after_eof_THEN_reports_broken_pipe: bare re-end and empty write stay free after the end; data through either entry point is broken_pipe. - WHEN_server_cancels_write_eof_THEN_client_sees_truncated_body: cancelling a data-carrying async_write_eof() must stop referencing the caller's buffer (fails with a heap-use-after-free under ASAN without the h3 cancellation fix). - WHEN_client_cancels_write_eof_THEN_can_still_end: the FIN of a cancelled EOF write stays owed, and a re-issued async_write_eof() ends the (now shorter) body for real. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Looking up a query parameter and converting it meant fetching the params view, finding the key, and wrapping lexical_cast in a try/catch at every call site. get_param_as<T>() does all of that and returns an optional, using try_lexical_convert() so no call site has to catch anything. The "delay" handling in the test server is the first user. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
lexical_cast happily converts "-1" into an unsigned type, wrapping it around to SIZE_MAX. A handler sizing an allocation or a response body from such a parameter is then handed a value it will never survive. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The generate handler did the params()/from_chars()/range-check dance by hand. get_param_as<size_t>() covers all of it, including rejecting trailing junk, overflow and negative values, so the behaviour is unchanged -- only the "generate:" prefix on the warning is gone, the helper names the parameter instead. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
url() returns boost::url_view by value, and params() only references that view. Binding the result of url().params() directly left every later use of the parameter list -- the find(), the iterator, the value -- reading a destroyed stack temporary. ASAN reports this as a stack-use-after-scope inside boost::urls::detail::query_ref::nparam(). It also caused the garbage "length" value that made ExternalCustom.netcat_crazy_chunked attempt a 0x7ffff71a8790-byte allocation under TSAN. The default build passed either way, so the bug was only visible under a sanitizer. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Boost.ProgramOptions' bool_switch rejects a repeated option, so declare "verbose" as a zero-token, composing option and count its occurrences in the parsed command line: one "-v" selects debug, two or more select trace. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Replaces the "N * 1024" and "N * 1024 * 1024" spellings with the _k/_m literals from literals.hpp, which h3_session.cpp already uses. Plain 1024 stays as it is -- "1_k" reads worse than the number itself.
Both outcomes are worth seeing in a trace: how much of the body arrived before EOF, and which error cut it short before drain() throws.
send(request, size_t) generated a body of that many bytes, which reads like the send() range overload right next to it -- it is now generate(). read_response() does not hand back the response, it counts the body, so it is count_response(); count() was a thin wrapper that only logged what drain() now logs itself, so it is gone and its callers use drain(). Also assert the response is empty in IgnoreRequest, which had discarded the count it collected.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The nv arrays are small and short-lived, so keep them on the stack and reserve up front instead of letting a std::vector heap-allocate on every request and response. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Every request now starts with "METHOD URL" and every response with
"STATUS REASON", followed by the header block, on both sides of the
connection:
* HTTP/1.1 client logged neither its request nor its response headers.
* The HTTP/1.1 server logged the raw target; log the URL it derived
from it (scheme and authority included), which means moving the dump
behind the parsing.
* HTTP/2 could not label incoming headers as they arrived, because the
method and the status are only known once the frame is complete.
Collect them on the stream (only while debug logging is on, like the
HTTP/3 side does) and dump them after the request or status line.
* HTTP/3 knows both early, so it only needed the two missing lines on
the client side.
The markers this replaces (on_begin_header_callback:, on_request:,
on_response:, response headers: status=...) said nothing the request or
status line does not, and the HTTP/2 client dumped user-supplied headers
a second time at info level.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
read() and try_receive() ended their loops in two steps, returning from the loop and then reporting what happened afterwards -- try_receive() even reported it one level up, so only one of its two overloads logged at all. Report EOF and errors right where they are seen, and read with a 16k buffer instead of 1k while we are here. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… and drain functions
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.