Repository navigation
Conversation
cli: ignore trailing data after the last frame, and report corrupted archives
|
Wasn't that good enough? |
Author
|
I thought I had to make additional changes. I'll make a new PR right now |
Author
|
Uploaded:) |
Well, yes. I meant that in case of actually corrupted archives must return en error, an non-zero value on exit, and that archive cannot be deleted, as it was not successfully decompressed. It also gives the user chance to have a look and possibly recover that data, which will be impossible if archive is deleted. Other than that, I have no objections. |
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.
What and why
Both parts follow the discussion on the issue. They are separate commits, so they can be taken
independently.
Trailing data after the last frame (first commit). When valid frames were followed by data that
is not a frame, zstd stopped with "unsupported format" (4 bytes or more) or "unknown header"
(1 to 3 bytes) and exit status 1, although every frame had already been decoded. It now
decompresses all frames, ignores the trailing data and warns:
zstd: WARNING: <file>: trailing data after the last frame is ignored. The exit status is 0.gzip and bzip2 also tolerate trailing data with a warning (gzip exits with 2). Input that never
contained a frame keeps its errors, and
--pass-throughstill copies the trailing data.Damaged archives (second commit). To tell that case apart from a genuinely damaged archive, a
zstd frame that is cut short or damaged now ends with
zstd: <file>: corrupted or incomplete archive; decoded <N> bytes, after the existing error.N counts the bytes decoded and written before the failure, across all frames of the file. It is
printed for a frame that is cut short and for the errors that mean damaged data (corruption
detected, checksum mismatch, wrong literals header, wrong source size). Other decoding errors,
such as dictionary mismatch or a window size over the memory limit, are unchanged.
Notes for reviewers
cat file | zstd -dc(the case in the issue) must still show it.-qqsilences it.zstd -d --rmremoves the source file in that case (as gzip does), and
zstd -tpasses.archive now decompresses only the frames before it, with the warning as the only signal. Before,
it failed.
change. The second commit does not depend on it.
an exact decoded size.
Tests
tests/cli-tests/decompression/trailing-garbage.sh and corrupted-archive.sh. Both fail without the
change.
dictionaries/dictionary-mismatch.sh shows that unrelated decoding errors are unchanged. The
CLI test suite and tests/playTests.sh pass;
Checklist
A gain in one configuration is not a general gain: see the Performance section of CONTRIBUTING.md.
Tools used
Claude Code (Claude Sonnet 5.5) was used to find the code path, write the documentation, and the
tests. I ran the tests locally