gildas-lormeau / zip.js

JavaScript library to zip and unzip files supporting multi-core compression, compression streams, zip64, split files and encryption.
https://gildas-lormeau.github.io/zip.js
BSD 3-Clause "New" or "Revised" License
3.38k stars 510 forks source link

Possible issue writing Zip64 format #386

Closed AshleyScirra closed 1 year ago

AshleyScirra commented 1 year ago

Apologies for the slightly vague report, but this is a tricky one to make a minimal repro for.

Recently we updated our software to produce files in Zip64 format using the zip.js library. We discovered that our ASP.net website can't read these zips with the built-in zip support, returning the error:

Split or spanned archives are not supported.

We tried another library, but it also fails with the error

Spanned archives with more than 65534 segments are not supported at this time.

Here is a sample file written in Zip64 format with zip.js: broken-sample.zip

If I open this file in WinRAR, add a dummy file, and then try uploading again, it succeeds (with both the previously problematic ASP.net zip libraries). According to WinRAR the file is still in Zip64 format after doing this. Here is the fixed version of the file as written by WinRAR: fixed-sample.zip

Perhaps the ASP.net zip libraries have a bug, but I suspect that zip.js is in fact writing something incorrectly in the Zip64 format which some but not all tools are tolerant to; when WinRAR updates the zip it rewrites the headers with correct data that all tools can read. This StackOverflow answer appears to include some of the details about the zip format and links to the code in other libraries that reads the file. It sounds like zip.js may be writing 0xFFFF in a field that indicates "spanning file with more than 65534 segments", hence the above error messages. Could you review this information and check if zip.js's Zip64 code is correct?

Thanks for your great work on zip.js!

gildas-lormeau commented 1 year ago

From my point of view, this is a bug in the ASP.net library. It looks like it does not fully support 4.4.19 and 4.4.20 in the spec here https://pkware.cachefly.net/webdocs/APPNOTE/APPNOTE-6.3.9.TXT. 0xFFFF is a special value used to indicate that the value is in fact stored in a zip64 data field. I'll try to improve the implementation to avoid this issue though. Out of curiosity, do you set the option zip64 explicitly to true in your code?

gildas-lormeau commented 1 year ago

Please let me know if the last version fixes the issue. If you set the zip64 option to true explicitly then you should also set supportZip64SplitFile to false. Note that you shouldn't need to set zip64 to true because zip.js can detect when to use zip64 automatically.

gildas-lormeau commented 1 year ago

More specifically, this piece of code shows that the ASP.net library does not fully support zip64.

https://github.com/DinoChiesa/DotNetZip/blob/7ac10dc7793cbf21212848e9814334ee946f26d3/Zip/ZipFile.Read.cs#L614-L616

AshleyScirra commented 1 year ago

Out of curiosity, do you set the option zip64 explicitly to true in your code?

Yes, we do actually. I want to make sure everything we need to interoperate with supports zip64 (otherwise things will seem to work until zips get large enough to become zip64 then suddenly stop working).

Please let me know if the last version fixes the issue.

I updated to v2.6.61 and also set supportZip64SplitFile to false. It seems to work now, thanks! We'll need to do some more testing to confirm, but it looks good so far.

gildas-lormeau commented 1 year ago

Thank you for the feedback.

Yes, we do actually. I want to make sure everything we need to interoperate with supports zip64 (otherwise things will seem to work until zips get large enough to become zip64 then suddenly stop working).

FYI, you shouldn't need to do so. zip.js should be able to add zip64 data automatically if necessary even if zip64 is not explicitly set to true. When it's set to true, all the zip64 data is always added even if it's not necessary. That's why you're facing this issue. The ASP.net library does not support the last disk number when it's written as zip64 data even if you're not using split zip files.