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

throw 'unexpected EOF' #18

Closed
lutzroeder opened this issue Dec 21, 2020 · 10 comments
Closed

throw 'unexpected EOF' #18

lutzroeder opened this issue Dec 21, 2020 · 10 comments

Comments

@lutzroeder
Copy link

lutzroeder commented Dec 21, 2020

Repro steps

  1. Download and unzip fflate#18.zip
  2. Download and copy test-0001.zip to same folder
  3. npm install
  4. node index.js

Output

~/: node index.js

~/node_modules/fflate/lib/index.js:272
                throw 'unexpected EOF';
@101arrowz
Copy link
Owner

$ unzip test-0001.zip 
Archive:  test-0001.zip
  End-of-central-directory signature not found.  Either this file is not
  a zipfile, or it constitutes one disk of a multi-part archive.  In the
  latter case the central directory and zipfile comment will be found on
  the last disk(s) of this archive.
unzip:  cannot find zipfile directory in one of test-0001.zip or
        test-0001.zip.zip, and cannot find test-0001.zip.ZIP, period.

Even the CLI fails to unzip it. However, you are correct that Windows can unzip this natively, and so can 7zip, so I'll look into the issue and see what the problem is.

@lutzroeder
Copy link
Author

lutzroeder commented Dec 21, 2020

Can you double check the total file size of test-0001.zip is 655042854 bytes?

~/: unzip test-0001.zip 
Archive:  test-0001.zip
  inflating: test-0001        

The actual error is bit positions getting larger than 32 bit so JavaScript >>> won't work.

@101arrowz
Copy link
Owner

101arrowz commented Dec 21, 2020

That's true, I should mention the limit of 512 MB on synchronous unzip. However, streaming inflate does work on larger files, so I'll see if that works for this.

Yes, you were right, that was a PEBKAC, this ZIP is valid. I'll fix this by adding support for streaming when files become too large.

@lutzroeder
Copy link
Author

lutzroeder commented Dec 21, 2020

It shouldn't fail with a cryptic error for synchronous. Why not fix it to work via (pos / 8) >> 0?

@101arrowz
Copy link
Owner

I decided against that because I was worried that the division operator followed by a bit shift would be much more expensive than a single bit shift, but as it turns out what you suggested isn't any slower. I was thinking of (and just now finished) making the synchronous operation default to synchronous streaming when files were too large, but I suppose that's not as performant as your suggestion.

@101arrowz
Copy link
Owner

@lutzroeder I've finished making changes to support the full 4GB maximum supported by Uint8Array itself. Thanks for the suggestions, help, and debuggable examples ❤️ I'll be pushing an update shortly.

@101arrowz
Copy link
Owner

Check v0.4.4 on the NPM registry. Thanks for the bug report!

@101arrowz
Copy link
Owner

Hey @lutzroeder, I noticed you added a subsection of fflate into netron. I understand that depending upon the NPM package would be difficult, since netron's build process doesn't allow for tree shaking, but I would appreciate if you could still attribute the project in the source code or README. Thanks for your interest!

@lutzroeder
Copy link
Author

Reverted the change. The main fork is a rewrite from scratch using the reverse mapping algorithm from UZIP.js and has a UZIP.js reference added to the source.

@101arrowz
Copy link
Owner

The algorithm is actually from SO, you may want to cite that instead. Nothing special about the change except shift down a bit, since they're all 15 bit integers I managed to find a way to optimize further. Either way, hope your rewrite works well.

101arrowz added a commit that referenced this issue Jan 14, 2021
101arrowz added a commit that referenced this issue Jan 14, 2021
101arrowz added a commit that referenced this issue Jan 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants