fix: avoid stack overflow when decoding many DEFLATE blocks (#88) - #89
Merged
Conversation
`Read for Decoder` is defined with self-recursive calls in tail position. Rust does not guarantee tail-call elimination, so a DEFLATE stream that chains many empty stored blocks makes the recursion deep enough to overflow the thread stack. The new test builds such a stream (250_000 empty non-final stored blocks followed by a small final block) and decodes it via the gzip decoder; on master the process aborts with `has overflowed its stack`.
Both `deflate::Decoder::read` and `gzip::Decoder::read` were written as self-recursive tail calls that advance the stream one DEFLATE block (or one gzip member) per invocation. Rust does not guarantee tail-call elimination, so a stream carrying enough consecutive empty stored blocks — or a multi-member gzip file — can exhaust the thread stack and abort the process. Rewrite both methods as `loop`s that reuse the same stack frame, matching the pattern in the rest of the crate. Also gate the `make_large_deflate_stream` test helper on the `std` feature so `cargo test --no-default-features` still compiles.
- Rename `decode_large_deflate_stream` to `test_issue_88` to match the repo's naming convention for regression tests (`test_issue_64`, `test_issues_3`). - Drop the redundant `pub` on the test-only helper. - Hoist the WASM payload constant to module scope so the test and the helper share a single definition instead of two identical copies. - Refresh the helper's docstring so it no longer implies the block decoder is still recursive. - Guard `blocks - 1` with a `debug_assert!` so misuse (`blocks == 0`) fails loudly instead of wrap-then-OOM.
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.
Fixes #88.
impl Read for Decoderin bothdeflateandgzip::MultiDecoderwaswritten with self-recursive tail calls that advance one DEFLATE block
(or gzip member) per invocation. Rust does not guarantee tail-call
elimination, so a stream carrying many consecutive empty stored blocks
— or a gzip file with many members — can exhaust the thread stack and
abort the process. Both are rewritten as
loops that reuse one stackframe; every branch preserves the original semantics.
The other
Readimpls in the crate (zlib::Decoder,gzip::Decoder,non_blocking::{deflate,gzip,zlib}::Decoder) do not contain the samepattern, so no further changes are needed.
The reproduction payload and the loop-based fix were provided by
@michael-weigelt in the issue attachments; landed here (with minor
polish) after independent review.