Skip to content

Conversation

@ducaale
Copy link
Contributor

@ducaale ducaale commented Nov 26, 2025

Motivation

When decompressing a chunked response with extra bytes like in https://github.com/seanmonstar/reqwest/blob/a2aa5a34e48724be0c1089b0f5afe49b82ece30e/tests/gzip.rs#L306, the client hangs and doesn't return error until the server drops connection.

I think this happens because those extra bytes are treated as the start of another gzip frame.

Solution

We can disable multiple_members and assert that any frames after decompressing body are trailers.

@ducaale ducaale force-pushed the decompression-extra-bytes branch from 9e78561 to 5fb15ed Compare November 26, 2025 21:37
@ducaale ducaale force-pushed the decompression-extra-bytes branch from 5fb15ed to ada3d5d Compare November 26, 2025 21:59
frame.map_data(|mut data| data.copy_to_bytes(data.remaining()))
))),
Some(Ok(frame)) => {
if let Ok(bytes) = frame.into_data() {
Copy link
Contributor Author

@ducaale ducaale Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to check if frame is not empty before erroring. See seanmonstar/reqwest#2507 (comment)

@seanmonstar
Copy link
Collaborator

@0x676e67 I noticed you filed #548, and while I approved that, I no longer remember why. Seems like we're undoing that here, and perhaps never should have enabled it? I see it is enabled in reqwest for zstd, but all that I remember from gzip, that perhaps is wrong there too?

@0x676e67
Copy link
Contributor

@0x676e67 I noticed you filed #548, and while I approved that, I no longer remember why. Seems like we're undoing that here, and perhaps never should have enabled it? I see it is enabled in reqwest for zstd, but all that I remember from gzip, that perhaps is wrong there too?

@seanmonstar Yes, I remember the context now — it started from these two issues/PRs:
seanmonstar/reqwest#2574
seanmonstar/reqwest#2583

@ducaale
Copy link
Contributor Author

ducaale commented Nov 27, 2025

In that case, I will re-enable multiple_members for zstd. We only need to disable it for gzip to get reqwest tests passing

@ducaale ducaale changed the title Disable multiple_members option for decoders Disable multiple_members option for gzip decoder Nov 27, 2025
@ducaale ducaale force-pushed the decompression-extra-bytes branch from c53bb2d to 397dcce Compare November 27, 2025 22:20
@ducaale ducaale force-pushed the decompression-extra-bytes branch from 397dcce to 86c5b4d Compare November 27, 2025 22:20
@seanmonstar seanmonstar merged commit 1fe4c09 into tower-rs:main Nov 28, 2025
11 checks passed
@ducaale ducaale deleted the decompression-extra-bytes branch November 28, 2025 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants