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

[release/10.0-preview2] Revert "version needed to extract" fix #112923

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Feb 25, 2025

Backport of #112922 to release/10.0-preview2

/cc @carlossanlop

The dotnet/android team uses a tool called aapt2 to create zip files. The entries added by this tool set a “version needed to extract” metadata field with a value of 0.0. But sometimes this tool uses deflate compression, which requires a version of 2.0, which they don’t set correctly (full discussion in #112017).

When these zip files are then opened using our ZipArchive in Update mode and new entries are added to the archive, the central directory is correctly updated with the correct version, but the local file header for existing (unmodified) entries isn’t rewritten by us (because we didn’t modify them), leading now to a mismatch.

We tried to fix this by forcing the local file header to be rewritten when the “version needed to extract” changes, ensuring consistency between the local file header and the central directory header. Unfortunately, this attempt to increase correctness made matters worse, as tools like 7zip now report that the zip files are corrupt.

So this PR reverts that fix to avoid breaking other tools when they open zip files with this issue that are then modified by our ZipArchive.

The dotnet/android team has a workaround: instead of using aapt2 they can use a tool called android-libzipsharp, which has the drawback of being less performant, but it works fine.

@edwardneal @jonathanpeppers @ericstj @ViktorHofer @artl93

Customer Impact

  • Customer reported
  • Found internally

Regression

  • Yes
  • No

Regression introduced with #112032 which is what we're reverting here.

Testing

Revert to previous behavior which was working fine with other tools like 7zip.

Risk

Low. We know the previous behavior worked as expected and the dotnet/android team has a workaround.

…ntral and local directories when updating entries.

This reverts commit 68a88d1.
Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-system-io-compression
See info in area-owners.md if you want to be subscribed.

@carlossanlop carlossanlop self-assigned this Feb 25, 2025
@carlossanlop carlossanlop added the Servicing-consider Issue for next servicing release review label Feb 25, 2025
@carlossanlop carlossanlop added Servicing-approved Approved for servicing release and removed Servicing-consider Issue for next servicing release review labels Feb 25, 2025
@carlossanlop
Copy link
Member

Approved by Tactics via email.

@carlossanlop
Copy link
Member

The CI didn't trigger. I'll close and reopen the PR to see if that triggers it.

@carlossanlop
Copy link
Member

/ba-g The nativeaot leg is stuck

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants