-
Notifications
You must be signed in to change notification settings - Fork 80
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix reading zip files containing zero-byte encrypted files #36
Conversation
It fails because compressionMethod is 0 because the file is empty, but compressedSize is non-zero because the file is encrypted
…ying to read zip containing an encrypted zero-byte file
Travis failing on the PR because of timestamp of a file in the zip, it should pass in about 20 minutes as then it's no longer created in the future. |
yauzl has a current design limitation that it does not handle encrypted files at all, as in, an encrypted file is an error condition. I don't want to speak for @thejoshwolfe but I am guessing that if he decides to support encryption at all, he will choose between a complete solution or status quo, unless there's a reason to special case zero byte encrypted files. Is there one? |
I'm not sure I understand your description of the situation, but I think I understand your code. I believe it disables a sanity check when uncompressed files are encrypted. And then, if I understand your test correctly, you're expecting that yauzl will extract an encrypted file by providing the encrypted bytes of the file. I'm not sure I like either one of these ideas. As @andrewrk predicted, I would like a complete solution to password-protected zip files. This feature has been on my TODO list for a long time. See #11. Although I'm rejecting this pull request, do know that you've given me motivation to look into encryption support. So you have contributed to the project indirectly :) |
I was aware of the design limitation, but the description was maybe a bit incomplete. What I intended with it, is to handle non-compressed (e.g. empty) encrypted files the same as yauzl currently handles compressed encrypted files, which is to at least spew out 'entry' events for them and allow the implementer to choose to extract (parts of) the zip. In my implementation of yauzl I'm intending to skip all the encrypted entries, so that I won't actually extract them - as that's indeed not behaving in a useful way currently - but I still am able to report to the application user which files were skipped. So this PR was not about trying to do anything smart about encrypted files themselves, just to support extracting other files which happen to be in a zip that also contains non-compressed encrypted files. |
This seems like a good usecase to support. I've looked into how "traditional encryption" works in zipfiles, which is probably what we're talking about here. It doesn't lend itself well to pure-JavaScript handling, since it involves doing per-byte processing where each byte requires several mathematical operations. That's the kind of thing that's going to be unbearably slow in a high-level language like JavaScript, and should really be written in a C library that the JavaScript code has bindings to. I don't want to add a natively compiled C component or dependency to yauzl, which means I either provide an unbearably inefficient solution, or don't support it at all. I think you can guess by my tone of writing which one I'd rather do. But I do have a plan that should be compatible with yauzl's goal of being pure javascript, and being low-level enough that abstractions can be built on top of it without any overhead. See #11. The proposal would include your original goal of getting the encrypted bytes of an empty, non-compressed, encrypted entry, but with my proposal, the client needs to know what they're getting themselves into in order for it to work, i.e. they need to pass |
The library can read out the central directory fine for zip files containing encrypted files.
However if a zip file contains a zero-byte file that happens to be encrypted, it fails, because yauzl sees a size mismatch between compressedSize and uncompressedSize, and zero-byte files aren't compressed.
This PR introduces a test for a zip with a zero-byte encrypted file, and changes the compressedSize === uncompressedSize check so it only applies if the file is neither compressed nor encrypted.