Skip to content

Skip Zip64 extra fields when strict input doesn't require it.#127

Merged
mrkkrp merged 3 commits intomrkkrp:masterfrom
quytelda:issue126
May 25, 2025
Merged

Skip Zip64 extra fields when strict input doesn't require it.#127
mrkkrp merged 3 commits intomrkkrp:masterfrom
quytelda:issue126

Conversation

@quytelda
Copy link
Copy Markdown
Contributor

@quytelda quytelda commented May 23, 2025

Close #126.

When entry data is provided as a strict ByteString, we don't need to stream any data to determine its uncompressed size. Thus, we can rule out the need for Zip64 extra fields early when writing the local header. This applies mainly to addEntry, which takes its input as a strict ByteString.

In summary, I added a new PendingAction called StrictEntry and a new EntryOrigin (StrictOrigin Int) which tracks the size of strict data for use when building the header. I decided to pass that to putHeader as part of the LocalHeader HeaderType because the information is only really relevant when writing the local header (nothing is different for the CD headers).

I also made some additional minor code simplifications that don't change any behavior. Some Maps were being iterated over many times because of a combination of forM_ and (!) from Data.Map.Strict. My change reduces those to a single pass where possible while avoiding (!), which is no longer recommended.

All the tests are passing for me.

@mrkkrp
Copy link
Copy Markdown
Owner

mrkkrp commented May 24, 2025

Great stuff! Thanks for simplifying the code and fixing my silly typos :) Could you please do the following:

  • Run Ormolu on your changes (ideally amend each commit).
  • Make sure your commit messages do not end with a full stop.
  • Add a changelog entry in CHANGELOG.md (add a new section named "Unpublished").

quytelda added 2 commits May 24, 2025 20:32
Use foldlWithKey, foldMapWithKey, and traverseWithKey to process Maps
in a single pass and avoid using (!), which is a partial function.
The case statement in question evaluates to an identical expression in
all cases, so we can elide it.
@quytelda quytelda force-pushed the issue126 branch 2 times, most recently from 3a3fda9 to 2480edf Compare May 25, 2025 03:42
@quytelda
Copy link
Copy Markdown
Contributor Author

Okay, all three points should be addressed now :)

I also adjusted the checkEntry test as suggested. Does there still need to be a new test added?

When entry data is provided as a strict ByteString, we don't need to
stream the data to determine its uncompressed size. Thus, we can rule
out the need for Zip64 extra fields early.

This addresses issue mrkkrp#126.
@mrkkrp mrkkrp merged commit 7496676 into mrkkrp:master May 25, 2025
4 checks passed
@mrkkrp
Copy link
Copy Markdown
Owner

mrkkrp commented May 25, 2025

Thanks! I will release a new version (2.2.0) on Hackage later today.

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.

Zip64 extra field breaks MIME type detection for zip-based formats

2 participants