Skip to content
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

Use ZipEntry.CompressionMethodForHeader when writing the header entries #440

Merged
merged 2 commits into from
Apr 12, 2020

Conversation

Numpsy
Copy link
Contributor

@Numpsy Numpsy commented Apr 10, 2020

When writing the local/central directory entries in ZipFile, use the CompressionMethodForHeader property of ZipEntry instead of the CompressionMethod property, in order to handle the case where the entry method is WinZipAes.

Add a couple of extra unit tests, which both try to verify using SharpZipLib locally (#317 aside) and with 7-Zip.

refs #439

I certify that I own, and have sufficient rights to contribute, all source code and related material intended to be compiled or integrated with the source code for the SharpZipLib open source product (the "Contribution"). My Contribution is licensed under the MIT License.

@Numpsy
Copy link
Contributor Author

Numpsy commented Apr 10, 2020

The CompressionMethodForHeader property shouid only differ from the CompressionMethod property for AES encrypted entries, and as you can't presently add those via ZipFile the change should only effect updates to existing files that contain AES encrypted entries.

I don't think there are any situations where this would break anything, given that the current code already breaks such entries when the file is updated?

@piksel
Copy link
Member

piksel commented Apr 11, 2020

No, it shouldn't afaik.

@piksel piksel added the ready PR ready for merging when appropriate label Apr 11, 2020
@piksel piksel merged commit 8f3fa5c into icsharpcode:master Apr 12, 2020
@Numpsy Numpsy deleted the rw/zipfile/439 branch April 12, 2020 13:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready PR ready for merging when appropriate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants