guzba / zippy

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

Add initial support for deflate64 zip files #84

Open iffy opened 1 month ago

iffy commented 1 month ago

This is a shot in the dark attempt at adding Deflate64 support for #11 . I've made this change by trial and error with the given sample Deflate64 zipfile. I haven't read any spec about Deflate64 and I don't know if the magic numbers I put in internal.nim are correct. But it unzips the file now.

If someone with more knowledge of how the zip file format works could chime in, that would be very welcome.

guzba commented 1 month ago

Thanks for the PR however I do have concerns.

This PR changes internal.nim values for maxDistanceCodes and a couple other important arrays which concerns me. These changes are invalid for Deflate which is by far the primary and most important use of Zippy. Deflate64 is hyper niche by comparison.

I am not going to merge something that makes Zippy violate the Deflate spec so we'll need to find a way to signal "this is Deflate vs this is Deflate64" and have separate values for that.

iffy commented 1 month ago

Absolutely understandable. If I get some more time on this, I'll try to work out a change that doesn't modify existing Deflate stuff.

guzba commented 1 month ago

One thought I had was adding dfDeflate64 as a new format parameter for compress/uncompress. I think (but will need to confirm) that a compressed blob out of Deflate is valid Deflate64 input so that'll be easy. The harder part will be doing Deflate64 uncompress correctly without it blowing up the complexity of an already very touchy code path.