wire-api: strip framing headers in generic streaming response renderers - #5389
Draft
jschaul wants to merge 1 commit into
Draft
wire-api: strip framing headers in generic streaming response renderers#5389jschaul wants to merge 1 commit into
jschaul wants to merge 1 commit into
Conversation
Both generic streaming renderers re-frame the body via Warp (chunked): * MultiVerb's `IsWaiBody (SourceIO ByteString)` instance * `LowLevelStream`'s servant `HasServer` route Each spliced the response's own headers into the `Wai.responseStream` verbatim. If a `Content-Length` (or `Transfer-Encoding`) rode along, Warp would honour it and serve the streamed body under that declared length instead of chunking; any mismatch between the declared length and the bytes actually streamed then desynchronises the caller's keep-alive HTTP/1.1 connection (the client reads past the response boundary into the next response). This is the same defect fixed in `Federator.Response.streamingResponseToWai`. No endpoint currently attaches such a header to a streaming response (the federator's inward path already strips to `Content-Type` only, and cargohold's `RespondStreaming` carries no length), so this is defense-in-depth: strip `Content-Length`/`Transfer-Encoding` in both renderers via a `stripFramingHeaders` helper so Warp always frames exactly what is streamed. Adds `Test.Wire.API.Routes.Streaming` covering both renderers (verified red without the strip, green with it). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
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.
A follow-up to #5381 in other places streaming is done.
== WARNING: AI written. Needs a bit more human review first, keeping this in draft for later... ===
Both generic streaming renderers re-frame the body via Warp (chunked):
IsWaiBody (SourceIO ByteString)instanceLowLevelStream's servantHasServerrouteEach spliced the response's own headers into the
Wai.responseStreamverbatim. If a
Content-Length(orTransfer-Encoding) rode along, Warpwould honour it and serve the streamed body under that declared length
instead of chunking; any mismatch between the declared length and the bytes
actually streamed then desynchronises the caller's keep-alive HTTP/1.1
connection (the client reads past the response boundary into the next
response). This is the same defect fixed in
Federator.Response.streamingResponseToWai.No endpoint currently attaches such a header to a streaming response (the
federator's inward path already strips to
Content-Typeonly, and cargohold'sRespondStreamingcarries no length), so this is defense-in-depth: stripContent-Length/Transfer-Encodingin both renderers via astripFramingHeadershelper so Warp always frames exactly what is streamed.
Adds
Test.Wire.API.Routes.Streamingcovering both renderers (verifiedred without the strip, green with it).
Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com
Checklist
changelog.d