Skip to content

Conversation

@baszalmstra
Copy link
Contributor

Fixes #411

This adds a test that verifies if receiving truncated compressed data yields an UnexpectedEof error.

While adding the test, I found three decoders that fail this test. I fixed all of them.

Added a generic truncated stream test to the test_cases! macro that
automatically tests all decoders (bzip2, gzip, deflate, zlib, xz, lzma,
lz4, zstd, brotli) for proper handling of incomplete streams.

The test compresses data, truncates it, then attempts decompression.
Decoders should return UnexpectedEof errors for truncated streams
instead of silently accepting incomplete data.

This test currently fails for bzip2, lz4, and zstd decoders, which
will be fixed in subsequent commits.
@baszalmstra baszalmstra force-pushed the claude/implement-issue-411-011CUpzAf5yUB1s58RpG9Fzp branch from 4bfae42 to 6c387f9 Compare November 5, 2025 17:13
Copy link
Collaborator

@NobodyXu NobodyXu left a comment

Choose a reason for hiding this comment

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

Thank you!

Just some feedback with the flush impl:

Fixes Nullus157#411

The async BzDecoder was silently accepting truncated bzip2 streams,
returning Ok(0) instead of raising an error. This contrasts with the
synchronous bzip2::read::BzDecoder which properly returns an
UnexpectedEof error.

Added state tracking to BzDecoder:
- Added stream_ended field to track if Status::StreamEnd was received
- Modified decode() to set stream_ended = true on Status::StreamEnd
- Updated finish() to check stream_ended and return UnexpectedEof if false

This ensures applications cannot accidentally accept corrupted or
incomplete compressed data as valid, matching the behavior of the
synchronous decoder.

The generic truncated test now passes for bzip2.
The LZ4 decoder was silently accepting truncated streams by not
validating stream completion in finish().

This issue was discovered by the generic truncated stream test.

Added state tracking to Lz4Decoder:
- Added stream_ended field to track if remaining == 0 was seen
- Modified decode() to set stream_ended = true when stream completes
- Updated finish() to check stream_ended and return UnexpectedEof if false

This matches the behavior of other decoders (bzip2, gzip, etc.) and
ensures applications cannot accidentally accept corrupted or incomplete
LZ4 data as valid.

The generic truncated test now passes for LZ4.
@baszalmstra baszalmstra force-pushed the claude/implement-issue-411-011CUpzAf5yUB1s58RpG9Fzp branch from 6c387f9 to cada816 Compare November 5, 2025 20:28
The Zstd decoder was silently accepting truncated streams by not
validating stream completion in finish().

This issue was discovered by the generic truncated stream test.

Added state tracking to ZstdDecoder:
- Added stream_ended field to track if remaining == 0 was seen
- Modified decode() to set stream_ended = true when stream completes
- Updated finish() to check stream_ended and return UnexpectedEof if false
- Updated all constructors to initialize stream_ended = false

This matches the behavior of other decoders (bzip2, gzip, lz4, etc.) and
ensures applications cannot accidentally accept corrupted or incomplete
zstd data as valid.

The generic truncated test now passes for Zstd.
@baszalmstra baszalmstra force-pushed the claude/implement-issue-411-011CUpzAf5yUB1s58RpG9Fzp branch from cada816 to 64b9392 Compare November 5, 2025 20:48
Copy link
Collaborator

@NobodyXu NobodyXu left a comment

Choose a reason for hiding this comment

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

Thank you!

@NobodyXu NobodyXu enabled auto-merge November 5, 2025 20:49
@NobodyXu
Copy link
Collaborator

NobodyXu commented Nov 5, 2025

I will try to cut a release after merging

@NobodyXu NobodyXu added this pull request to the merge queue Nov 5, 2025
Merged via the queue into Nullus157:main with commit e6b3819 Nov 5, 2025
20 checks passed
@github-actions github-actions bot mentioned this pull request Nov 5, 2025
@codecov
Copy link

codecov bot commented Nov 5, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 0.00%. Comparing base (83a06fe) to head (64b9392).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@     Coverage Diff     @@
##   main   #412   +/-   ##
===========================
===========================

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

BzDecoder does seem to propagate UnexpectedEof error from truncated streams

3 participants