Add streaming gzip and deflate response decoding - #954
Conversation
codeon:
version: 1
authority: FWN-372
description: |-
Share gzip and zlib Content-Encoding stack selection between buffered RawResponse parsing and streaming body readers. Wrap sync and async ResponseBodyReaders with incremental reverse-order flate2 decoding while preserving empty-body skip, trailer visibility, CE/CL stripping only on success, raw headers for unsupported stacks, and decoded-size bounds on read_to_response.
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d942ccf4e2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| self.layers[index] | ||
| .write_all(¤t) | ||
| .map_err(error::decode)?; | ||
| current = self.layers[index].take_output().map_err(error::decode)?; | ||
| self.record_layer_bytes(index, current.len())?; |
There was a problem hiding this comment.
For bounded read_to_response calls, each Vec-backed decoder expands the entire input before record_layer_bytes checks the configured limit. A highly compressible response can therefore allocate far beyond max_buffered_response_body_bytes before returning BodyTooLarge; stacked encodings amplify this because one layer's multi-megabyte output is passed wholesale into the next layer, potentially causing an OOM despite the configured bound. Use a size-limited output sink or otherwise stop decompression as soon as the layer crosses the limit.
Useful? React with 👍 / 👎.
| let mut binary = if self.body.content_decode_succeeded() { | ||
| strip_content_encoding_and_length_from_head(&self.head)? | ||
| } else { | ||
| self.head.clone() | ||
| }; |
There was a problem hiding this comment.
When a compressed streaming response is converted with read_to_response, this branch replaces the wire headers and later appends the decoded body, so Response::binary() returns a synthesized decoded message rather than the original capture. This regresses the documented Response::binary() contract in response/mod.rs and differs from the buffered path, which decodes the exposed body while retaining the received bytes; consumers using the binary response for diagnostics, persistence, or signatures will receive altered data.
Useful? React with 👍 / 👎.
| if self.body.content_decode_succeeded() { | ||
| strip_content_encoding_and_length(&mut headers); |
There was a problem hiding this comment.
For the normal pattern of inspecting headers before consuming the body, content_decode_succeeded() is still false because it only becomes true after compressed EOF, yet body_mut().read() already returns decoded bytes. Consequently headers() reports Content-Encoding and the compressed Content-Length for a decoded stream—and its result changes after EOF—so a consumer honoring those headers can double-decode the body or use an incorrect length. The async implementation has the same condition.
Useful? React with 👍 / 👎.
1 participant
Summary
Test plan