Skip to content

fix: unzipping with backslashes in the name on UNIX#834

Merged
nmoinvaz merged 3 commits into
zlib-ng:developfrom
Coeur:coeur/backslash_unzip
Apr 14, 2025
Merged

fix: unzipping with backslashes in the name on UNIX#834
nmoinvaz merged 3 commits into
zlib-ng:developfrom
Coeur:coeur/backslash_unzip

Conversation

@Coeur
Copy link
Copy Markdown
Collaborator

@Coeur Coeur commented Nov 24, 2024

While #832 was for fixing zipping, this PR is for fixing unzipping.

The expectation is that archiving+unarchiving data on UNIX should preserve its structure, no matter if there are backslashes (or dots) in the name of files or folders.
So backslash should only be seen as a separator on Windows, never on UNIX filesystems.

This is the current behaviour with the default macOS /usr/bin/zip and /usr/bin/unzip as well as default behaviour in Finder when right-click -> compress, followed by double-click to uncompress.

@Coeur
Copy link
Copy Markdown
Collaborator Author

Coeur commented Nov 24, 2024

OK, I guess I need to look at the unit tests.

@nmoinvaz
Copy link
Copy Markdown
Member

nmoinvaz commented Nov 24, 2024

The thing I worry about is if the zip was improperly created in the Windows style of path slashes in the zip entry. It would unzip on Unix anymore.

@Coeur
Copy link
Copy Markdown
Collaborator Author

Coeur commented Nov 25, 2024 •
edited
Loading

Do we have Windows sample archives like that? As written in 4.4.17.1, it would not have been allowed in the first place.

OK, here is what I suggest: a compile option just like I did in #804 with MZ_TARGET_APPSTORE. Flipping such new compile option would allow for either behaviour.

@nmoinvaz nmoinvaz force-pushed the develop branch 2 times, most recently from f33bdd3 to 68dc2f3 Compare April 5, 2025 17:52
@Coeur Coeur force-pushed the coeur/backslash_unzip branch from 66a6413 to 05cb9d8 Compare April 12, 2025 14:40
@Coeur Coeur marked this pull request as draft April 12, 2025 14:41
@Coeur
Copy link
Copy Markdown
Collaborator Author

Coeur commented Apr 12, 2025 •
edited
Loading

I'll update TEST_P(path_resolve, os) this weekend.
[edit]
da7607a

@Coeur Coeur marked this pull request as ready for review April 13, 2025 14:07
@Coeur Coeur marked this pull request as draft April 13, 2025 14:09
@Coeur
Copy link
Copy Markdown
Collaborator Author

Coeur commented Apr 13, 2025

OK, next mission, a compile option...

@Coeur Coeur force-pushed the coeur/backslash_unzip branch from da7607a to 172eaa0 Compare April 13, 2025 14:12
@Coeur Coeur force-pushed the coeur/backslash_unzip branch from 172eaa0 to dfd0058 Compare April 13, 2025 21:35
@Coeur Coeur marked this pull request as ready for review April 13, 2025 22:00
@Coeur
Copy link
Copy Markdown
Collaborator Author

Coeur commented Apr 13, 2025 •
edited
Loading

I've added MZ_PRESERVE_NATIVE_STRUCTURE in 216e242

I've adopted the default behaviour which matches the other zipping/unzipping tools available on macOS and avoids data corruption.

@nmoinvaz ready for review.

@Coeur
Copy link
Copy Markdown
Collaborator Author

Coeur commented Apr 13, 2025 •
edited
Loading

The thing I worry about is if the zip was improperly created in the Windows style of path slashes in the zip entry. It would unzip on Unix anymore.

If the filepath was previously a/b\\c/d\\e, the Windows style or archiving would have made it a\\b\\c\\d\\e so that archive would have been already corrupted and is irrecoverable to its original structure, no matter which compilation flag you use. Hopefully, with #832 we shouldn't have corrupted archives anymore. And as previously mentioned, the regression was introduced in #644, so only a subset of minizip versions had the bug.

@nmoinvaz nmoinvaz merged commit f894149 into zlib-ng:develop Apr 14, 2025
29 checks passed
@Coeur Coeur deleted the coeur/backslash_unzip branch April 15, 2025 01:31
@Coeur Coeur mentioned this pull request Apr 29, 2025
Merged
@Coeur
Copy link
Copy Markdown
Collaborator Author

Coeur commented Apr 30, 2025 •
edited
Loading

For reference, this is the (expected) impact of this PR on default filesystems.

Without MZ_PRESERVE_NATIVE_STRUCTURE:

Input Archive format Unzip on Linux Unzip on Windows
Windows A\b A/b A/b A\b
Linux A/b A/b A/b A\b
Linux a\b a\b a/b [non-preserved] a\b [non-preserved]
Linux A/b\c A/b\c A/b/c [non-preserved] A\b\c [non-preserved]

With MZ_PRESERVE_NATIVE_STRUCTURE:

Input Archive format Unzip on Linux Unzip on Windows
Windows A\b A/b A/b A\b
Linux A/b A/b A/b A\b
Linux a\b a\b a\b a\b [non-preserved]
Linux A/b\c A/b\c A/b\c A\b\c [non-preserved]

@remia remia mentioned this pull request May 5, 2025
Closed
algitbot pushed a commit to alpinelinux/aports that referenced this pull request May 22, 2025
@ncopa
community/opencolorio: skip test that broke with recent minizip-ng
939f263 
minizip-ng 4.0.10 no longer consider backslash as a folder separator,
which broke a test in opencolorio. Disable this test.

ref: zlib-ng/minizip-ng#834
ref: AcademySoftwareFoundation/OpenColorIO#2157
sergio-nsk
sergio-nsk reviewed Aug 26, 2025
View reviewed changes
Comment thread mz_os.h
@Coeur Coeur mentioned this pull request Oct 5, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nmoinvaz nmoinvaz nmoinvaz approved these changes

+1 more reviewer

@sergio-nsk sergio-nsk sergio-nsk left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Coeur @nmoinvaz @sergio-nsk