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.33k stars 506 forks source link

Corrupt zip file created using BlobReader on File object #508

Closed tmccoid-tech closed 2 months ago

tmccoid-tech commented 2 months ago

Using the example provided at https://github.com/gildas-lormeau/zip.js/blob/gh-pages/demos/demo-create-file.js, I implemented a basic zip routine for use in an extension/add-on for the Mozilla Thunderbird email client.

The zip file is being generated without apparent error, and opening the archive in Windows File Explorer (or WinRAR, for that matter) displays the expected files. However, these files are un-extractable, with WinRAR indicating a checksum error (file is corrupt) on any included file.

Here is an abstraction of the code I am using:

async function add(fileName, fileData, creationDate) { const zipWriter = new zip.ZipWriter( new zip.BlobWriter("application/zip"), { bufferedWrite: true, useCompressionStream: false } ); await zipWriter.add(fileName, new zip.BlobReader(fileData), { creationDate: creationDate, useWebWorkers: false } ); const result = await zipWriter.close(); return result; }

The fileData parameter is a JavaScript File object.

I feel as if I must be missing something basic here. Any guidance would be appreciated.

Screenshot 2024-04-22 140444

tmccoid-tech commented 2 months ago

One misconception I had is that compression would be off (level 0) by default; however, the default setting is 5. I have discovered that setting it to 0 rectifies the problem, but am still uncertain as to why having the default (and perhaps any non-zero) compression level would result in archive corruption. Is there a correlated option for either the BlobWriter or the add method of the ZipWriter object that needs to be set? Thanks.

gildas-lormeau commented 2 months ago

Thank you for the bug report. I think I was able to reproduce the issue. It looks like it happens only when importing zip.js from the dist folder, and setting useCompressionStream and useWebWorkers to false.

The tests below show the issue. The only difference is how zip.js is included (cf. lines 10-11).

gildas-lormeau commented 2 months ago

I just realized my mistake, I didn't include the good script. I have to include <script src="https://unpkg.com/@zip.js/zip.js/dist/zip-full.js"></script> when not using web workers. By any chance, did you make the same mistake?

See https://plnkr.co/edit/p9K9N5c04YMlDBvS

gildas-lormeau commented 2 months ago

It looks like you made the same mistake, see https://github.com/tmccoid-tech/extract-em/blob/main/module/zip.js. You should use https://github.com/gildas-lormeau/zip.js/blob/master/dist/zip-full.js instead (or https://github.com/gildas-lormeau/zip.js/blob/master/dist/zip-no-worker-deflate.min.js) when setting useWebWorkers to false.

I think it's still a bug in zip.js though. It should never create invalid zip files. Or it should at least warn the user in that case. I'll look into how to avoid this.

tmccoid-tech commented 2 months ago

Thanks for looking into this and providing a solution so quickly.

I wonder if you might want to update the demo I referenced in the first comment to use zip-full.js rather than zip.js, and maybe set the compression level explicitly and provide a comment to the effect of "0 = no compression, 5 = default, 9 = max" or the like.

gildas-lormeau commented 2 months ago

The issue related to zip-full.js/zip.js is fixed in the version 2.7.42. Now, it will work for people who copy and paste from the demo code. I've also updated the demos to use zip-full.min.js. For the record, the value from 1 to 9 of the level option is effective only when useCompressionStream is set to false. However, most of the time, the gain is negligible.