[BUG] ZipDataSource mempty compresses to invalid zip files; add test#15
Open
ulidtko wants to merge 1 commit intodylex:masterfrom
Open
[BUG] ZipDataSource mempty compresses to invalid zip files; add test#15ulidtko wants to merge 1 commit intodylex:masterfrom
ZipDataSource mempty compresses to invalid zip files; add test#15ulidtko wants to merge 1 commit intodylex:masterfrom
Conversation
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.
Hi again @dylex!
When using the compression part of the library in production, we stumbled over a corner-case. Turns out, if archive member data is provided through a conduit (via
ZipDataSource), and that conduit never yields — the output zip archive is... I don't want to say "corrupted" (ubuntu'sunzipstill partially extracts it — but not all decompressors do, including the one in this library) — let's say, definitely "not entirely valid".Please try the added test. It fails unexpectedly, and you can check the produced output. Infozip's
unzipsaysinvalid compressed data to inflateon it:I looked at it, and attempted a fix — but it didn't help, I don't quite understand why.
(I also don't understand why
empty_OK_2.txthas compressed length 2, and not 0).In practice, this wasn't a major issue for us — as there's an easy workaround, of wrapping all
ZipDataSourcearguments in a little(yield "" >>)in user-code relative tozip-stream. But I just thought, as a FOSS courtesy, you would better be made aware. Perhaps you can think of a fix for this that will improve the library.By all means, feel free to discard this PR, or merge after some reworks — it's really only a bug report, with a patch attached to demonstrate the issue that's quite specific.