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

Set FileOffset field in central directory header to special value when including FileOffset in ZIP64 extra fields #171

Merged
merged 1 commit into from
Feb 27, 2024

Conversation

DonBrinn
Copy link
Contributor

@DonBrinn DonBrinn commented Dec 13, 2023

This fixes the root cause of issue 371 in node-archiver. This should directly fix this issue when this PR is merged here, when node-zip-stream is updated to use this new version of node-compress-commons, and then when node-archiver is updated to use the new version of node-zip-stream.

Reason for the defect:

The order of the fields in the ZIP64 extended information record is fixed, but the fields will only appear if the corresponding Local or Central directory record field is set to 0xFFFF or 0xFFFFFFFF.

Source: https://libzip.org/specifications/extrafld.txt

In the ZipArchiveOutputStream._writeCentralFileHeader method here, if the File Offset was smaller than the Zip64 limit (0xFFFFFFFF) but the Zip64 fields were needed for another large value (example: if the uncompressed file size is larger than 0xFFFFFFFF) then the File Offset was included in the Zip64 extra field but the normal 4-byte File Offset field in the central directory header, rather than the required value of 0xFFFFFFFF.

Validation:

  • I reproduced the problem by using the archiver v6.0.1 module in NodeJs to stream about 5 GB of data to a single file within a zip archive with options { zlib: { level: 6 }, forceZip64: true }. Using 7Zip, I can unzip the file successfully but it gives the warning message "Warnings: Headers Error". Opening the zip file in 7Zip and viewing the properties of the file within, it shows "Characteristics: Extra_ERROR Zip64_ERROR : Descriptor".
  • I validated the fix by using a locally-patched version of the archiver v6.0.1 module with the same changes as in this PR, to stream the same ~5 GB data as above to a single file within a zip archive with options { zlib: { level: 6 }, forceZip64: true }. Using 7Zip, I can unzip the file successfully with no warning or error messages. Opening the zip file in 7Zip and viewing the properties of the file within, it shows "Characteristics: Zip64 : Descriptor".

I have not added nor updated any unit tests, as I didn't find any tests that cover the modified areas of code or provide a framework to test these specific changes.

…uding file offset in central diretory extra fields
@DonBrinn DonBrinn changed the title Set FileOffset field in central directory heder to special value when including FileOffset in ZIP64 extra fields Set FileOffset field in central directory header to special value when including FileOffset in ZIP64 extra fields Jan 24, 2024
Copy link
Member

@ctalkington ctalkington left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the research here

@ctalkington ctalkington merged commit da44c38 into archiverjs:master Feb 27, 2024
@ctalkington ctalkington mentioned this pull request Feb 27, 2024
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.

3 participants