Skip to content

[release/8.0-staging] Set version in ZIP local header to ZIP64 when file offset is >4GB #103006

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

Conversation

carlossanlop
Copy link
Contributor

@carlossanlop carlossanlop commented Jun 3, 2024

Backport of #102053 to release/8.0-staging.
Fixes #94899

Customer Impact

Affecting two partner areas separately.

dotnet/Open-XML-SDK

Initially reported here: dotnet/wpf#2676

An external customer that develops PowerPoint Add-Ins uses DocumentFormat.OpenXML (a package owned by dotnet/Open-XML-SDK) to add custom xml parts to the PowerPoint presentation.

When they attempt to reopen the modified PowerPoint file either using Open XML SDK 2.5 Productivity Tool for Microsoft Office (an asset owned and released also by dotnet/Open-XML-SDK) or once again the DocumentFormat.OpenXML APIs, they fail stating that "The file contains corrupted data".

But they have no problem opening the file with PowerPoint, or with other zip tools like WinRAR, 7-Zip.

microsoft/DacFx

This is also affecting our partners at microsoft/DacFx: microsoft/DacFx#363

Azure DevOps uses SqlPackage to export databases into bacpac files and then import them into Azure Sql Server instances.

When they import the bacpac using either Local Sql Server or Azure Sql Server, they have no problems. But when they import it using their Import Database UI or az sql db import, there's a failure stating the file contains corrupted data.

All four cases use System.IO.Packaging to generate the bacpac file, but only the failing two cases use System.IO.Packaging again to attempt to import the file.

Root cause

Both issues have a common root cause: #94899

ZipArchive behaves differently in .NET and .NET Framework when creating or reading files larger than 4GB where the algorithm switches to using ZIP64 (described in this comment). The spec was not clear about that as described in this comment, but other tools do the right thing and can handle the files without issues, so we should match that behavior and ensure there's no discrepancy between .NET and .NET Framework.

The fix consists in making sure that both the central directory header and the local file header store the size offset and when the file is larger than 4GB, set the ZIP64 flag on both, and make sure .NET validates this just as .NET Framework used to do.

Testing

New unit tests added:

  • In System.IO.Compression, a test that confirms that ZipArchive can open a zip file larger than 4GB that was created with .NET Framework.
  • In System.IO.Compression, a test that uses reflection to confirm the correct ZIP64 value is used when creating archives larger than 4GB containing more than one file.
  • In System.IO.Packaging, a test similar to the previous one, but using the APIs in this namespace, which wrap System.IO.Compression internally.

Risk

Low, as the behavior was wrong in .NET when compared to .NET Framework or other ZIP tools.

Thanks to @karakasa for contributing with the original fix.

cc @pwoosam @SeenaAugusty @llali

…tnet#102053)

* ZipArchiveEntry didn't set ZIP64 in local headers for small files if their offset are > 4GB.
* Added System.IO.Compression and System.IO.Packaging tests.

---------

Co-authored-by: Gan Keyu <gankeyu@hotmail.com>
@carlossanlop carlossanlop force-pushed the backport-zip64offset-to-8 branch from e46b05a to c8e3f75 Compare June 5, 2024 01:06
@carlossanlop carlossanlop added the Servicing-approved Approved for servicing release label Jun 10, 2024
@carlossanlop
Copy link
Contributor Author

CI failure in osx is an infra issue unrelated to this PR:

/Users/runner/work/1/s/artifacts/toolset/restore.proj : error : Unable to load the service index for source https://pkgs.dev.azure.com/dnceng/public/_packaging/dotnet-libraries-transport/nuget/v3/index.json.

@carlossanlop carlossanlop merged commit 106b02f into dotnet:release/8.0-staging Jun 10, 2024
102 of 106 checks passed
@carlossanlop carlossanlop deleted the backport-zip64offset-to-8 branch June 10, 2024 22:14
@github-actions github-actions bot locked and limited conversation to collaborators Jul 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants