guzba / zippy

Pure Nim implementation of deflate, zlib, gzip and zip.
MIT License
246 stars 29 forks source link

Incorrect file permissions when using extractAll #43

Closed huantianad closed 2 years ago

huantianad commented 2 years ago

When using zippy to extract zip files, I've noticed it mess up file permissions on Linux. Not sure what's causing it, but here's an example of it happening.

example.zip

import zippy/ziparchives

extractAll("example.zip", "example")

results in

.-----x---    overflow1.png ... overflow10.png
.rw-r--r--    overflowneg1.png ... overflowneg10.png

instead of

.rw-r--r--    overflow1.png ... overflow10.png
.rw-r--r--    overflowneg1.png ... overflowneg10.png
guzba commented 2 years ago

This is happening because your example.zip does not store Linux file permissions in it (or does not store them correctly). Currently, in the external file attribute area for permissions, your example.zip has decimal0 for all the g files. It has decimal 8 for the other files. In this case, 0 falls back to the default permissions (rw-r--r--). In the case of 8, Zippy interprets that as a specific Linux permission, which happens to be Group Exec.

Zip is a very very bad way to manage file permissions, especially across operating systems, and even more so if you do not control both the creation of and extracting of the zip.

In this case, you'll need to find a way to produce a .zip that encodes the permissions you want correctly (or just puts 0), or override the file permissions to what you want them to be after extracting.

huantianad commented 2 years ago

Makes sense, I don't expect to get file permissions out of the zip files I'm handling so I'll just override all the file permissions after extractions. Thanks for the info!

guzba commented 2 years ago

I would love to just remove the Zip file permission handling (since it is very strange) but then others would open issues to add support again lol.

guzba commented 2 years ago

Actually, I should totally add a ignorePermissions optional bool to extractAll for .zip files... Will consider this.

huantianad commented 2 years ago

Yeah that seems quite sensible, ig it would have to be false for backwards compat :stuck_out_tongue:

This issue only affects Unix-like systems right?