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

Fix the elusive invalid zip archive issue that has been a problem for ages! #142

Merged
merged 2 commits into from
Jun 3, 2024

Conversation

dellis1972
Copy link
Contributor

@dellis1972 dellis1972 commented May 29, 2024

Fixes dotnet/android#8988

We had this odd corrupt zip file issue which kept cropping up on our Azure Pipelines builds.
We had no idea what caused it until now. Some of the data for the local headers of an item (not the central directory) would be written incorrectly. This would result in a zip which may or may not be extractable, it would depend on how resilient the software extracting the data would be.

So, what was happening here was that (sometimes) libzip would start writing some data (most likely the local file header) using our stream source callback, and it would seek a few bytes into the data and then tried to seek back to the beginning. The latter seek was done by giving the seek operation of the callback an offset of 0 which, unfortunately, was also used by the code as a guard as to whether or not to even perform the seek operation. The effect was that we ignored the seek to 0 and the stream remained at whatever the previous seek location was requested, thus corrupting data. It happened only on the very first entry, since that was the only one which would have position 0 within its range.

We discovered that just enabling the strict consistency checks would uncover the issue, so that has been enabled in
a number of unit tests. Once we did that it turns out we were writting the corrupt data ALL the TIME!.
Fixing up the seeking code to take into account that we might want to see to 0 fixed the issue.

@dellis1972 dellis1972 changed the title Testzip Fix the Illusive invalid zip archive issue that has been a problem for ages! May 30, 2024
@dellis1972 dellis1972 marked this pull request as ready for review May 31, 2024 09:32
@dellis1972
Copy link
Contributor Author

@grendello when you get a mo can you check this over and check the commit message to see if its accurate please?

@dellis1972 dellis1972 requested a review from grendello June 3, 2024 07:39
…r ages!

Fixes dotnet/android#8988

We had this odd corrupt zip file issue which kept cropping up on our
Azure Pipelines builds. We had no idea what caused it until now.
It turns out some of the logic we were using to control the stream
position was not working correctly. It stopped us from navigating to a
0 offset. As a result some of the data for the local headers of an item
(not the central directory) would be written incorrectly.
This would result in a zip which may or may not be extractable, it would
depend on how resilient the software extracting the data would be.

We discovered that just enabling the strict consistency checks would
uncover the issue, so that has been enabled in a number of unit tests.
Once we did that it turns out we were writting the corrupt data
ALL the TIME!. Fixing up the seeking code to take into account that we
might want to see to 0 fixed the issue.
@grendello
Copy link
Contributor

So, what was happening here was that (sometimes) libzip would start writing some data (most likely the local file header) using our stream source callback, and it would seek a few bytes into the data and then tried to seek back to the beginning. The latter seek was done by giving the seek operation of the callback an offset of 0 which, unfortunately, was also used by the code as a guard as to whether or not to even perform the seek operation. The effect was that we ignored the seek to 0 and the stream remained at whatever the previous seek location was requested, thus corrupting data. It happened only on the very first entry, since that was the only one which would have position 0 within its range.

@grendello grendello changed the title Fix the Illusive invalid zip archive issue that has been a problem for ages! Fix the elusive invalid zip archive issue that has been a problem for ages! Jun 3, 2024
@dellis1972 dellis1972 merged commit b541b87 into main Jun 3, 2024
9 checks passed
@dellis1972 dellis1972 deleted the testzip branch June 3, 2024 09:04
jonpryor pushed a commit to dotnet/android that referenced this pull request Jun 26, 2024
Context: #8988

Changes: dotnet/android-libzipsharp@3.1.1...3.3.0

  * dotnet/android-libzipsharp@de57dcc: Add xml comments. Centralize the dotnet target framework (dotnet/android-libzipsharp#143)
  * dotnet/android-libzipsharp@b541b87: Fix the elusive invalid zip archive issue that has been a problem for ages! (dotnet/android-libzipsharp#142)
  * dotnet/android-libzipsharp@c2ae332: Update OneLocBuildToken (dotnet/android-libzipsharp#141)
  * dotnet/android-libzipsharp@4fef46a: Bump library versions for the latest upstream releases (dotnet/android-libzipsharp#140)
  * dotnet/android-libzipsharp@14f591c: Remove LZMA (XZ) support (dotnet/android-libzipsharp#139)
  * dotnet/android-libzipsharp@336a86f: [ci] Use managed identity for API Scan (dotnet/android-libzipsharp#138)
  * dotnet/android-libzipsharp@8bc799c: [ci] Add API Scan job (dotnet/android-libzipsharp#132)
  * dotnet/android-libzipsharp@afef4b2: [ci] Improve binskim scan performance (dotnet/android-libzipsharp#137)
  * dotnet/android-libzipsharp@577147e: [ci] Migrate to the 1ES template (dotnet/android-libzipsharp#135)

Changes: xamarin/monodroid@c6aae9e...e11d9a5

  * xamarin/monodroid@e11d9a5af: Bump to LibZipSharp 3.3.0 (xamarin/monodroid#1493)
  * xamarin/monodroid@c9e71ebe5: Bump to xamarin/xamarin-android/main@eb7fdf7 (xamarin/monodroid#1495)
  * xamarin/monodroid@5c344d7c2: Bump to xamarin/android-sdk-installer@cc43d20d (xamarin/monodroid#1498)
  * xamarin/monodroid@004875391: Bump to xamarin/androidtools@0884384b (xamarin/monodroid#1496)

dotnet/android-libzipsharp@b541b87 fixes an odd corrupt zip file
issue which kept cropping up on our Azure Pipelines builds.
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.

Making nuget package with aar file always has first resource file corrupted
2 participants