fix: Default "platform" field in zip files should be set to the local platform, rather than always "Unix" (#470) by roynor · Pull Request #471 · zip-rs/zip2

roynor · 2025-11-05T08:13:16Z

Fixed the issue where the compression platform was set to Unix in the Windows environment. Now the platform can be automatically set to Dos or Unix based on the specific operating system.

fix #470

gemini-code-assist · 2025-11-05T08:13:26Z

Summary of Changes

Hello @roynor, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request resolves a bug where the platform field in generated zip files was incorrectly set to "Unix" even when created on a Windows operating system. The change introduces automatic detection of the operating system during zip file creation, ensuring that the system field is accurately set to "Dos" for Windows environments and "Unix" for Linux environments, improving compatibility and correctness of zip archives.

Highlights

Platform Auto-detection: The ZipFileData structure now automatically detects the operating system (Windows, Linux) to set the appropriate compression platform (Dos, Unix) for zip files.
Bug Fix: Addresses an issue where zip files created on Windows incorrectly reported their platform as Unix, ensuring correct platform identification.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request correctly addresses the issue of the zip platform being hardcoded to Unix by introducing OS detection. The platform is now correctly set to DOS on Windows. However, the check for Unix-like systems is too specific, using cfg!(linux), which will incorrectly classify other common platforms like macOS as System::Unknown. I've provided a suggestion to use the more general cfg!(unix) to properly support all Unix-like systems.

src/types.rs

pzhlkj6612 · 2025-11-28T17:17:42Z

failures:
    read::test::test_cannot_symlink_outside_destination
    write::test::write_mimetype_zip
    write::test::write_non_utf8
    write::test::write_symlink_simple
    write::test::write_symlink_wonky_paths
    write::test::write_zip_dir

test result: FAILED. 82 passed; 6 failed

Well, I have no idea why all test suites in the "tests" directory was not be counted without specifying --lib ¹. For now, I encountered failed tests in the test suite "end_to_end" on Windows and I think I know the reason.

In initialize_local_block(), we have no parameter to designate if the current ZipFileData is related to a directory or a file, So the external_attributes field is always permissions << 16 ².

Because of the incorrect external_attributes, the System::Dos branch in unix_mode() always produces ffi::S_IFREG | 0o0664 == 0o0100664 ³. This incorrect Unix mode has at least two negative effects on our ZIP file:

It may cause an incorrect version_made_by field by making the check mode & S_IFDIR == S_IFDIR in version_needed() always fail ⁴.
It does cover the real Unix mode of every item by a constant 0o664 ⁵ when someone calls the unix_mode() function.

The 2nd point is the reason why the "end_to_end" suite failed.

roynor · 2025-12-01T08:25:49Z

Sorry,I have limited knowledge of the zip format, so fixing these issues is a bit challenging for me.

failures:
    read::test::test_cannot_symlink_outside_destination
    write::test::write_mimetype_zip
    write::test::write_non_utf8
    write::test::write_symlink_simple
    write::test::write_symlink_wonky_paths
    write::test::write_zip_dir

test result: FAILED. 82 passed; 6 failed

Well, I have no idea why all test suites in the "tests" directory was not be counted without specifying --lib 1. For now, I encountered failed tests in the test suite "end_to_end" on Windows and I think I know the reason.

In initialize_local_block(), we have no parameter to designate if the current ZipFileData is related to a directory or a file, So the external_attributes field is always permissions << 16 2.

Because of the incorrect external_attributes, the System::Dos branch in unix_mode() always produces ffi::S_IFREG | 0o0664 == 0o0100664 3. This incorrect Unix mode has at least two negative effects on our ZIP file:

* It may cause an incorrect `version_made_by` field by making the check `mode & S_IFDIR == S_IFDIR` in `version_needed()` always fail [4](#user-content-fn-version_needed-8c3be6dcdb5b1bc6ee133d5759ee1fd4).

* It does cover the real Unix mode of every item by a constant `0o664` [5](#user-content-fn-end_to_end-8c3be6dcdb5b1bc6ee133d5759ee1fd4) when someone calls the `unix_mode()` function.

The 2nd point is the reason why the "end_to_end" suite failed.

Footnotes

1. https://github.com/zip-rs/zip2/actions/runs/19553408314/job/55990236771?pr=471 [↩](#user-content-fnref-ci-8c3be6dcdb5b1bc6ee133d5759ee1fd4)

2. [/src/types.rs#L750](https://github.com/zip-rs/zip2/blob/a157695153d9dc61b6c757ba26d15aae71096e14/src/types.rs#L750) [↩](#user-content-fnref-external_attributes-8c3be6dcdb5b1bc6ee133d5759ee1fd4)

3. [/src/types.rs#L626-L638](https://github.com/zip-rs/zip2/blob/a157695153d9dc61b6c757ba26d15aae71096e14/src/types.rs#L626-L638) [↩](#user-content-fnref-unix_mode-8c3be6dcdb5b1bc6ee133d5759ee1fd4)

4. [/src/types.rs#L669-L677](https://github.com/zip-rs/zip2/blob/a157695153d9dc61b6c757ba26d15aae71096e14/src/types.rs#L669-L677) [↩](#user-content-fnref-version_needed-8c3be6dcdb5b1bc6ee133d5759ee1fd4)

5. [/tests/end_to_end.rs#L252](https://github.com/zip-rs/zip2/blob/a157695153d9dc61b6c757ba26d15aae71096e14/tests/end_to_end.rs#L252) [↩](#user-content-fnref-end_to_end-8c3be6dcdb5b1bc6ee133d5759ee1fd4)

pzhlkj6612 · 2025-12-01T09:39:39Z

Sorry,I have limited knowledge of the zip format, so fixing these issues is a bit challenging for me.

Don't worry.

We need to change internal things. For now, I think adding a field which could be called as dos_attributes in the FileOptions structure looks reasonable.

zip2/src/write.rs

Lines 269 to 282 in a157695

    
           pub struct FileOptions<'k, T: FileOptionExtension> { 
        
               pub(crate) compression_method: CompressionMethod, 
        
               pub(crate) compression_level: Option<i64>, 
        
               pub(crate) last_modified_time: DateTime, 
        
               pub(crate) permissions: Option<u32>, 
        
               pub(crate) large_file: bool, 
        
               pub(crate) encrypt_with: Option<EncryptWith<'k>>, 
        
               pub(crate) extended_options: T, 
        
               pub(crate) alignment: u16, 
        
               #[cfg(feature = "deflate-zopfli")] 
        
               pub(super) zopfli_buffer_size: Option<usize>, 
        
               #[cfg(feature = "aes-crypto")] 
        
               pub(crate) aes_mode: Option<(AesMode, AesVendorVersion, CompressionMethod)>, 
        
           }

Speaking of other errors. The test data was made on platforms other than Windows, so it should fail. We need to add new ones and surround related statements in unit tests by cfg!(windows) or other similar things. Negative testing is also required, btw.

Pr0methean · 2026-01-17T01:47:06Z

Speaking of other errors. The test data was made on platforms other than Windows, so it should fail.

ZIP files are supposed to transfer between platforms, so instead of restricting tests to only Windows, we should adapt them to whichever platform they're running on.

Windows; need auto-detection or manual configuration

refactor: simplify setup system of platform Co-authored-by: Mozi <29089388+pzhlkj6612@users.noreply.github.com> Signed-off-by: ztzuo <zztwangyi@163.com>

pzhlkj6612 · 2026-01-18T04:33:29Z

@Pr0methean:

Thanks for your work! I'll test v7.2.0 soon.

ZIP files are supposed to transfer between platforms, so instead of restricting tests to only Windows, we should adapt them to whichever platform they're running on.

I thought that we need cross-testing. For example, on Windows, read ZIP files made on Linux.

Pr0methean · 2026-01-18T05:44:16Z

@Pr0methean:

Thanks for your work! I'll test v7.2.0 soon.

ZIP files are supposed to transfer between platforms, so instead of restricting tests to only Windows, we should adapt them to whichever platform they're running on.

I thought that we need cross-testing. For example, on Windows, read ZIP files made on Linux.

We have that; the tests that were previously failing were ones that didn't actually open the zip file but compared it byte-for-byte with a known good one.

gemini-code-assist bot reviewed Nov 5, 2025

View reviewed changes

src/types.rs Outdated Show resolved Hide resolved

Pr0methean enabled auto-merge November 19, 2025 22:44

Pr0methean changed the title ~~Fix #470:Default "platform" field in zip files is set to "Unix" on Windows; need auto-detection or manual configuration~~ fix: Default "platform" field in zip files should be set to the local platform, rather than always "Unix" (#470) Nov 19, 2025

pzhlkj6612 reviewed Nov 28, 2025

View reviewed changes

src/types.rs Outdated Show resolved Hide resolved

auto-merge was automatically disabled December 1, 2025 08:22
Head branch was pushed to by a user without write access

Pr0methean added the Please fix failing tests Tests are failing with this change; please fix them. label Jan 16, 2026

zuozongtao and others added 6 commits January 16, 2026 18:22

Fix zip-rs#470:Default "platform" field in zip files is set to "Unix" on

7d3fc5f

Windows; need auto-detection or manual configuration

fix: platform set to Dos on windows and set to Unix on others.

179d63e

Update src/types.rs

cfcdb2b

refactor: simplify setup system of platform Co-authored-by: Mozi <29089388+pzhlkj6612@users.noreply.github.com> Signed-off-by: ztzuo <zztwangyi@163.com>

Fixes for platform byte and DOS directory byte

714c290

Fix: write symlinks with Unix mode on all platforms

072abec

Set rustfmt::skip on byte arrays, and cargo fmt --all

5290fbe

Pr0methean force-pushed the fix-issue-470 branch from 8f86545 to 5290fbe Compare January 17, 2026 02:58

Pr0methean enabled auto-merge January 17, 2026 02:58

Pr0methean removed the Please fix failing tests Tests are failing with this change; please fix them. label Jan 17, 2026

Pr0methean added this to the 7.2.0 milestone Jan 17, 2026

Pr0methean disabled auto-merge January 17, 2026 17:30

Pr0methean merged commit 9e9badd into zip-rs:master Jan 17, 2026
205 of 206 checks passed

Pr0methean mentioned this pull request Jan 17, 2026

chore: release v7.2.0 #526

Merged

konstin mentioned this pull request Feb 13, 2026

feat: Provide option to override platform byte #656

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Default "platform" field in zip files should be set to the local platform, rather than always "Unix" (#470)#471

fix: Default "platform" field in zip files should be set to the local platform, rather than always "Unix" (#470)#471
Pr0methean merged 6 commits intozip-rs:masterfrom
roynor:fix-issue-470

roynor commented Nov 5, 2025 •

edited by Its-Just-Nans

Loading

Uh oh!

gemini-code-assist bot commented Nov 5, 2025

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

Uh oh!

Uh oh!

pzhlkj6612 commented Nov 28, 2025

Uh oh!

roynor commented Dec 1, 2025

Footnotes

Uh oh!

pzhlkj6612 commented Dec 1, 2025

Uh oh!

Pr0methean commented Jan 17, 2026

Uh oh!

Uh oh!

pzhlkj6612 commented Jan 18, 2026

Uh oh!

Pr0methean commented Jan 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

roynor commented Nov 5, 2025 • edited by Its-Just-Nans Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gemini-code-assist bot commented Nov 5, 2025

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

pzhlkj6612 commented Nov 28, 2025

Footnotes

Uh oh!

roynor commented Dec 1, 2025

Footnotes

Uh oh!

pzhlkj6612 commented Dec 1, 2025

Uh oh!

Pr0methean commented Jan 17, 2026

Uh oh!

Uh oh!

pzhlkj6612 commented Jan 18, 2026

Uh oh!

Pr0methean commented Jan 18, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

roynor commented Nov 5, 2025 •

edited by Its-Just-Nans

Loading