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

Fix zip64 extra field #327

Merged
merged 3 commits into from
Jul 14, 2024
Merged

Conversation

Cruaier
Copy link

@Cruaier Cruaier commented Jul 4, 2024

Fixes problems with parsing ZIP64 extra fields that contain different content based on the ZIP64 central directory.

@ZJONSSON
Copy link
Owner

ZJONSSON commented Jul 5, 2024

Excellent, thank you @Cruaie. Can we create a unit test without including a massive zip file in the repo? Do you have a link to a file this would work on? Can we make a small zip file with the same central directory (no content) to ensure the directory is correctly parsed?

@Cruaier
Copy link
Author

Cruaier commented Jul 5, 2024

Can we create a unit test without including a massive zip file in the repo?

I see that you can easily create small zip files forcing ZIP64 (zip -fz test.zip test.txt) but that only gives the basic structure and has the uncompressed size in the extra field. I haven't found a way to make other fields appear (except files bigger than 5GB).

I'll have a look if I find a free available example on the web that showcases the offset length in the extra field.

@Cruaier
Copy link
Author

Cruaier commented Jul 5, 2024

Here an example file: https://testfile.org/files-5GB-zip
Output of zipdetails:

1291AC99F CENTRAL HEADER #31    02014B50
1291AC9A3 Created Zip Spec      1F '3.1'
1291AC9A4 Created OS            00 'MS-DOS'
1291AC9A5 Extract Zip Spec      14 '2.0'
1291AC9A6 Extract OS            00 'MS-DOS'
1291AC9A7 General Purpose Flag  0000
          [Bits 1-2]            0 'Normal Compression'
1291AC9A9 Compression Method    0008 'Deflated'
1291AC9AB Last Mod Time         56BE7DB3 'Tue May 30 15:45:38 2023'
1291AC9AF CRC                   42E3F3A3
1291AC9B3 Compressed Length     0B81CB9B
1291AC9B7 Uncompressed Length   0BBF2DE2
1291AC9BB Filename Length       0011
1291AC9BD Extra Length          0030
1291AC9BF Comment Length        0000
1291AC9C1 Disk Start            0000
1291AC9C3 Int File Attributes   0000
          [Bit 0]               0 'Binary Data'
1291AC9C5 Ext File Attributes   00000020
          [Bit 5]               Archive
1291AC9C9 Local Header Offset   FFFFFFFF
1291AC9CD Filename              'The Hive Mind.mp4'
1291AC9DE Extra ID #0001        0001 'ZIP64'
1291AC9E0   Length              0008
1291AC9E2   Offset to Local Dir 00000001079BAACD
1291AC9EA Extra ID #0002        000A 'NTFS FileTimes'
1291AC9EC   Length              0020
1291AC9EE   Reserved            00000000
1291AC9F2   Tag1                0001
1291AC9F4   Size1               0018
1291AC9F6   Mtime               01D992DFAEA545D0 'Tue May 30 17:15:38
                                2023 956948800ns'
1291AC9FE   Ctime               01D992DFAEA545D0 'Tue May 30 17:15:38
                                2023 956948800ns'
1291ACA06   Atime               01D992DF7CF8658F 'Tue May 30 17:14:15
                                2023 615630300ns'

However I am not sure if there is a feasible way to create a unit test for it. If you have an idea, let me know - I am happy to assist.

@ZJONSSON
Copy link
Owner

ZJONSSON commented Jul 5, 2024

Awesome @Cruaier , I think the best way to do that is to create a dummy file that just has the central directory copied over (binary) but modify the end of central directory to point to position 0 for offsetToStartOfCentralDirectory. Unzipping any entries won't work since the entries do not exist, but at least we can verify that the central directory will be accurately parsed.

@ZJONSSON
Copy link
Owner

ZJONSSON commented Jul 6, 2024

@Cruaier, I tried running your branch against the https://testfile.org/files-5GB-zip file, but the central directory was not parsed correctly. I will look into this, but I wanted to check if you had better luck.

@Cruaier
Copy link
Author

Cruaier commented Jul 8, 2024

Awesome @Cruaier , I think the best way to do that is to create a dummy file that just has the central directory copied over (binary) but modify the end of central directory to point to position 0 for offsetToStartOfCentralDirectory. Unzipping any entries won't work since the entries do not exist, but at least we can verify that the central directory will be accurately parsed.

@ZJONSSON I create 2 new test cases with other possible combinations of ZIP64 extra fields (using the existing zip64.zip as baseline). Please have a look if that's sufficient for you.

@Cruaier, I tried running your branch against the https://testfile.org/files-5GB-zip file, but the central directory was not parsed correctly. I will look into this, but I wanted to check if you had better luck.

It works fine from my side. My code for testing is:

const unzip = require('./unzip');

unzip.Open.file('Colttaine.zip')
  .then(zip => {
    zip.extract({path: './tmp'}).then(() => {
      console.log('done');
    });
  });

Looking at the existing test cases, I see it won't work with the parse method as the local file header doesn't save the offset length.

@Cruaier
Copy link
Author

Cruaier commented Jul 8, 2024

Additionally this PR also solves: #209

@ZJONSSON
Copy link
Owner

Thanks @Cruaier awesome work

@ZJONSSON ZJONSSON merged commit 1238dfc into ZJONSSON:master Jul 14, 2024
9 checks passed
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

Successfully merging this pull request may close these issues.

2 participants