Define http::body::size() to frame responses with content_length. - #860
Closed
echennells wants to merge 1 commit into
Closed
Define http::body::size() to frame responses with content_length.#860echennells wants to merge 1 commit into
echennells wants to merge 1 commit into
Conversation
beast frames a response with a content_length only where the body type defines size(), otherwise it frames from the version alone, chunking a 1.1 response and leaving a 1.0 response unframed. http::body defined no size(), so no response carried a content_length, and a 1.0 response to a keep-alive request carried no delimiter at all. json-rpc clients that implement only content_length framing (such as those built on the rust jsonrpc crate) fail the response before any method is dispatched. http::body::size() reports the byte count that http::body::writer emits for the assigned inner body. Fixed bodies report their length directly. json and json-rpc are measured by json::body::length(), which serializes the model with the same serializer that the writer streams it with, and discards the serialization as it measures it, so the response text is never materialized. The json-rpc model is derived from its message once and retained, so the measure and the write serialize one model rather than converting the message twice. Serializing it twice, once to measure and once to write, is the cost of the content_length. beast detects a sized body as a compile time trait of the body type, so this cannot be applied to the json-rpc inner type alone; the narrowest available scope is the multiplexing body, leaving json::body itself unsized. rpc::body<>::size() is defined with the writer that it measures. The emitted part is the batch framing prefix, the serialized message (absent on a batch close part), the batch close character, and the terminator where the transport wires termination. An http singleton is therefore exactly the serialized message, with no terminator. beast derives framing from the header fields alone and never reconciles the declared length against the bytes that the body writer emits, so a declared length that the writer does not honor desynchronizes the connection. A buffer body streams an indeterminate number of buffers while more is set, and so has no length to declare. body::streaming() reports this, and proxy::write refuses a response carrying such a body unless the caller has set chunked encoding, which clears the length. A model that cannot be measured is measured as zero, and the writer that emits it performs the same conversion and serialization, so it fails on the same model. A peer body is selected only by preselection and is written by proxy::write(frame&&), so it is never framed by beast and this measure is never obtained. A batch response is framed by the caller from its first part, measured before the part is stamped with batch state and before the remaining parts are produced. proxy::write replaces that body and sets chunked on the header it writes, which erases the content_length, so the batch is framed by the transfer-encoding alone. The framing characters and the bytes they occupy are declared once, so the framing that get() emits is the framing that size() measures. proxy::write refuses a modeled body measured as zero. No serializable model produces zero bytes, as a null model produces four, so a zero measure is a measure that failed, and writing the header would frame an empty success for a body that does not follow it.
Member
The implementation is not obliged to declare a (full) length, and this would force the interface to stream all results into arbitrarily-size fixed server-side buffers before copying it out in chunks over TCP - a terrible design, and not a compatibility requirement. bitcoind's json-rpc interface is http1.1, which requires that callers support chunked encoding. It does not matter that bitcoind doesn't send it (and it does accept it), RFC9112 is explicit: §7.1 (Chunked Transfer Coding) and §6.1 (Transfer-Encoding).
The bugs are in any client that doesn't support http1.1. I recommend filing this issue with them. |
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
2 participants
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.
bs frames http responses with
Transfer-Encoding: chunked. bitcoind never does: its writer takes the complete body and declaresContent-Lengthon 1.1 always (src/httpserver.cpp, master). Clients of that interface reject the alternative. Against stock master,bitcoincore-rpc0.19 failsgetblockcountbefore dispatch: "transport error: The server replied with a chunked response which is not supported" (rust-jsonrpc,src/http/simple_http.rs:225-229). That is its default transport, and the path ord, electrs, bdk and payjoin take to a node.This reverses the choice recorded at
http_body.hpp:100and onjson_value: nosize(), so responses chunk and the serialize stays streaming. That rationale holds, and costs more than a second pass. A length cannot be produced without serializing the whole model, so declaring one forfeits the laziness the streaming writer exists for. An interface obliged to declare a length is better served serializing once and keeping the result, which is a server change that follows this one, covering the rest route; json-rpc responses continue to measure.http::body::size()reports the bytes the writer emits: a length for fixed bodies, and for json a measure driving the writer's own serializer, discarded as it goes, so the text is never materialized. The json-rpc model is derived once for both the measure and the write, and the framing characters and their byte counts are declared once, so whatget()emits is whatsize()measures.proxy::writerefuses what it cannot frame: a buffer body withmoreset that the caller has not chunked, and a zero measure of a body that cannot legitimately be empty.beast detects a sized body as a compile time trait of the body type, so this changes the framing default for every interface served over http, not the bitcoind route alone. A call site that wants chunked frames itself, as the batch path does. Two consequences are accepted: btcd shares the bitcoind sender and so declares a length its clients do not require, its own server declaring neither framing; and the websocket path prepares a payload whose header that transport discards.
Only a body still holding a model measures: json, and the json-rpc request and response. The six fixed alternatives report a length they hold, and a peer body is not framed here. Against chunked, which measures nothing, the measure costs what the write costs, being the same serialization: 0.27ms against 0.28ms at 500 KB, 4.2 against 4.4 at 5 MB, 33.8 against 33.8 at 29 MB (Ryzen 9 3900X, g++ 12.2 -O3, boost 1.86, system and network built at -O3 from master).
Tests assert
size()against the bytes emitted, for the singleton and for batch open, continue and close, and cover both refusals.