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

Maybe fix the unable to open file as zip archive error #7759

Merged
merged 8 commits into from
Feb 28, 2023

Conversation

grendello
Copy link
Contributor

@grendello grendello commented Feb 1, 2023

Context: https://gist.github.com/grendello/55b9a74c80c611dd48b726ee6f16d3c9
Context: dotnet/android-libzipsharp#125
Context: dotnet/android-tools#204
Context: https://github.com/xamarin/androidtools/pull/377
Context: https://github.com/xamarin/monodroid/pull/1287

Sometimes in our tests we see an error from LibZipSharp which claims an file
cannot be opened as a valid ZIP archive. The reason for this is ZIP format
corruption happening sometimes in the BuildApk task.

The way the process works is that we first run aapt2 to produce an APK archive
named packaged_resources, which we then copy to the destination APK (using
standard File.Copy) and open it to append our content. The first item we
always append is the classes.dex file which is also the only corrupted e
ntry in the broken APK files.

ZIP files contain two records describing each entry in the archive:

  1. in the Central Directory (which is located at the end of the file)
  2. local header in the ZIP data stream, offset of whose is contained
    in the above Central Directory record.

After close examination, it appears that the broken APK files contain 1 above
for classes.dex but not 2. Instead of 2 we see a copy of packaged_resource
package's Central Directory record.

The corruption may be caused by invalid stream position after we open the
packaged_resources copy and append classes.dex entry to it.

Calling Flush () after adding classex.dex may fix the problem.

If it doesn't, we should consider copying data from packaged_resources to the
final APK entry by entry by decompressing them from the original archive and
adding them to the destination one.

Also, bump LibZipSharp to a version which aims to address the same problem but
from a different angle. Changes to LibZipSharp involve improvements and possible
bug fixes around the code which handles the native stream operations requested by
the native libzip library.

Context: https://gist.github.com/grendello/55b9a74c80c611dd48b726ee6f16d3c9

Sometimes in our tests we see an error from LibZipSharp which claims
an file cannot be opened as a valid ZIP archive.  The reason for this
is ZIP format corruption happening sometimes in the `BuildApk` task.

The way the process works is that we first run `aapt2` to produce
an APK archive named `packaged_resources`, which we then copy to the
destination APK (using standard `File.Copy`) and open it to append
our content.  The first item we always append is the `classes.dex`
file which is also the **only** corrupted entry in the broken APK
files.

ZIP files contain two records describing each entry in the archive:

  1. in the Central Directory (which is located at the end of the
     file)
  2. local header in the ZIP data stream, offset of whose is contained
     in the above Central Directory record.

After close examination, it appears that the broken APK files contain
`1` above for `classes.dex` but not `2`.  Instead of `2` we see a copy
of `packaged_resource` package's Central Directory record.

The corruption may be caused by invalid stream position after we open
the `packaged_resources` copy and append `classes.dex` entry to it.

Calling `Flush ()` after adding `classex.dex` **may** fix the problem.

If it doesn't, we should consider copying data from `packaged_resources`
to the final APK entry by entry by decompressing them from the original
archive and adding them to the destination one.
* main:
  [ci] Remove some Classic XA test stages/jobs. (dotnet#7770)
  [ci] Remove classic Mono.Android-Tests runs (dotnet#7778)
  $(AndroidPackVersionSuffix)=preview.2; net8 is 34.0.0-preview.2 (dotnet#7761)
  Localized file check-in by OneLocBuild Task (dotnet#7758)
* main:
  Add Unit Test for testOnly apps (dotnet#7637)
  [build] Only build the latest API level (dotnet#7786)
  [Xamarin.Android.Build.Tasks] Improve aapt2+file not found handling (dotnet#7644)
  [MSBuildDeviceIntegration] Fix duplicated test parameter (dotnet#7809)
  [tests] Bump NUnit versions to latest (dotnet#7802)
  [Microsoft.Android.Sdk.ILLink] target `net7.0` temporarily (dotnet#7803)
  [tests] `InstallAndroidDependenciesTest` can use `platform-tools` 34.0.0 (dotnet#7800)
  Bump to xamarin/Java.Interop/main@9e0a469 (dotnet#7797)
  [Xamarin.Android.Build.Tasks] FileWrites&libraryprojectimports.cache (dotnet#7780)
  Bump to dotnet/installer@d25a3bb 8.0.100-preview.2.23105.6 (dotnet#7769)
  [lgtm] Fix LGTM-reported issues (dotnet#1074)
  [ci] Report issues in the API docs build log (dotnet#7784)
* main:
  LEGO: Merge pull request 7825
  Bump to xamarin/Java.Interop/main@bbaeda6f (dotnet#7799)
  Bump NDK to r25c (dotnet#7808)
  [Xamarin.Android.Build.Tests] Improve logcat logging in failed tests (dotnet#7816)
  [Mono.Android] Update api-compat reference file for current API-33 (dotnet#7822)
  Localized file check-in by OneLocBuild Task (dotnet#7820)
* main:
  Revert "Bump to dotnet/installer@d25a3bb 8.0.100-preview.2.23105.6 (dotnet#7769)"
  LEGO: Merge pull request 7828
* main:
  Localized file check-in by OneLocBuild (dotnet#7827)
@grendello grendello added the do-not-merge PR should not be merged. label Feb 27, 2023
@grendello grendello removed the do-not-merge PR should not be merged. label Feb 27, 2023
@jonpryor
Copy link
Member

jonpryor commented Feb 27, 2023

Unified commit message:

Fixes? https://github.com/xamarin/xamarin-android/issues/6067

Context: https://gist.github.com/grendello/55b9a74c80c611dd48b726ee6f16d3c9
Context: https://github.com/xamarin/LibZipSharp/pull/125

Context: https://github.com/xamarin/xamarin-android/issues/7622

Changes: http://github.com/xamarin/monodroid/compare/50faac94c6a0c27864564829ac83f3988c82f8ef...602aca98245744882a129206b79b5a5e093dae44

  * xamarin/monodroid@602aca982: Bump androidtools for new LibZipSharp (xamarin/monodroid#1287)
  * xamarin/monodroid@1f52d5873: [tools/msbuild] Deploy *resources.dll from PackageReference (xamarin/monodroid#1276)

Changes: https://github.com/xamarin/androidtools/compare/f11d16350b088742f1293a58a3078e99f7deca09...c0bcb66c0beadcb3d1488349ce0fc906f2f3d8e4

  * xamarin/androidtools@c0bcb66: Bump xamarin-android-tools for new LibZipSharp (xamarin/androidtools#377)

Changes: https://github.com/xamarin/xamarin-android-tools/compare/099fd95f5459d9df78af0ad28f46e3016dd7ca1d...dbe42bf1688e79e1ca474dff4a1f8060158cafd9

  * xamarin/xamarin-android-tools@dbe42bf: Bump LibZipSharp version (xamarin/xamarin-android-tools#204)

Sometimes in our tests we see an error from LibZipSharp which claims
a file cannot be opened as a valid ZIP archive:

	error ANDZA0000: Unable to open '…/example.apk' as zip archive

The reason for this is ZIP format corruption happening sometimes in
the `<BuildApk/>` task.

The way the process works is that we first run `aapt2` to produce
an APK archive named `packaged_resources`, which we then copy to the
destination APK (using standard `File.Copy()`) and open it to append
our content.  The first item we always append is the `classes.dex`
file which is also the **only** corrupted entry in the broken APK
files.

ZIP files contain two records describing each entry in the archive:

 1. in the Central Directory (located at the end of the file)

 2. local header in the ZIP data stream, offset of which is contained
    in the above Central Directory record.

After close examination, it appears that the broken APK files contain
(1) for `classes.dex` but not (2).  Instead of (2) we see a *copy*
of `packaged_resource` package's Central Directory record:

	% zipinfo -vv example.apk
	
	Central directory entry #9:
	---------------------------

	  There are an extra 2 bytes preceding this file.

	  classes.dex

	  offset of local header from start of archive:   21680
	

In particular, "There are an extra 2 bytes preceding this file" is a
sign that something "isn't right".  If we use `hexdump` to look at
the 128 bytes starting at offset 21680:

	% hexdump -C -s 21680 -n 128 example.apk 
	000054b0  50 4b 01 02 00 00 00 00  00 00 08 00 00 00 21 00  |PK............!.|
	000054c0  5e b2 00 5e cc 03 00 00  5c 0b 00 00 13 00 00 00  |^..^....\.......|
	000054d0  00 00 00 00 00 00 00 00  00 00 00 00 00 00 41 6e  |..............An|
	000054e0  64 72 6f 69 64 4d 61 6e  69 66 65 73 74 2e 78 6d  |droidManifest.xm|
	000054f0  6c 50 4b 01 02 00 00 00  00 00 00 00 00 00 00 21  |lPK............!|
	00005500  00 5c 71 d9 26 d2 05 00  00 d2 05 00 00 1d 00 00  |.\q.&...........|
	00005510  00 00 00 00 00 00 00 00  00 00 00 fd 03 00 00 72  |...............r|
	00005520  65 73 2f 64 72 61 77 61  62 6c 65 2d 6d 64 70 69  |es/drawable-mdpi|

we see that it starts with "PK", the common magic signature for zip
archives, and the following bytes { 0x1 0x2 } indicate that this is a
*central directory* entry, while it *should* be { 0x3, 0x4 } for a
*local header* entry.

The corruption may be caused by invalid stream position after we open
the `packaged_resources` copy and append `classes.dex` entry to it;
see xamarin/LibZipSharp#125 for details.

We believe that the changes to `ZipArchive.stream_callback()` in
xamarin/LibZipSharp#125 alongside additional calls to `Flush()` after
adding `classex.dex` **may** fix the problem.

If it doesn't, we should consider copying data from
`packaged_resources` to the final APK entry by entry by decompressing
them from the original archive and adding them to the destination one.

@jonpryor jonpryor merged commit 0b2a982 into dotnet:main Feb 28, 2023
@grendello grendello deleted the maybe-fix-zip branch February 28, 2023 17:30
grendello added a commit to grendello/xamarin-android that referenced this pull request Feb 28, 2023
* main:
  [Xamarin.Android.Build.Tasks] `unable to open file as zip archive`? (dotnet#7759)
grendello added a commit to grendello/xamarin-android that referenced this pull request Feb 28, 2023
* main:
  [Xamarin.Android.Build.Tasks] Remove support for mkbundle (dotnet#7772)
  [Xamarin.Android.Build.Tasks] `unable to open file as zip archive`? (dotnet#7759)
  [monodroid] Properly process satellite assemblies (dotnet#7823)
  Bump to xamarin/java.interop/main@77800dda (dotnet#7824)
grendello added a commit to grendello/xamarin-android that referenced this pull request Mar 1, 2023
* main:
  Replace K4os.Hash.xxHash with System.IO.Hashing (dotnet#7831)
  $(AndroidPackVersionSuffix)=preview.3; net8 is 34.0.0-preview.3 (dotnet#7839)
  [Xamarin.Android.Build.Tasks] Remove support for mkbundle (dotnet#7772)
  [Xamarin.Android.Build.Tasks] `unable to open file as zip archive`? (dotnet#7759)
grendello added a commit to grendello/xamarin-android that referenced this pull request Mar 6, 2023
* main: (22 commits)
  Bump to dotnet/installer@632ddca 8.0.100-preview.3.23128.1 (dotnet#7836)
  LEGO: Merge pull request 7852
  [ci] Reduce overhead for MSBuildIntegration unit test jobs. (dotnet#7832)
  [ci] Allow dynamic`$(NuGetArtifactName)` values (dotnet#7848)
  [Xamarin.Android.Build.Tasks] guard `AutoImport.props` against empty values (dotnet#7837)
  [Mono.Android] Print type & member remapping info (dotnet#7844)
  [Mono.Android] Tweak AndroidMessageHandler behavior for WCF support (dotnet#7785)
  LEGO: Merge pull request 7845
  Localized file check-in by OneLocBuild Task (dotnet#7842)
  [ci] Use compliance stage template (dotnet#7818)
  [build] pass `--skip-sign-check` to `dotnet workload` (dotnet#7840)
  Replace K4os.Hash.xxHash with System.IO.Hashing (dotnet#7831)
  $(AndroidPackVersionSuffix)=preview.3; net8 is 34.0.0-preview.3 (dotnet#7839)
  [Xamarin.Android.Build.Tasks] Remove support for mkbundle (dotnet#7772)
  [Xamarin.Android.Build.Tasks] `unable to open file as zip archive`? (dotnet#7759)
  [monodroid] Properly process satellite assemblies (dotnet#7823)
  Bump to xamarin/java.interop/main@77800dda (dotnet#7824)
  [ci] Use AZDO built-in parallelization strategy. (dotnet#7804)
  Bump to dotnet/installer@e3ab0b5 8.0.100-preview.2.23123.10 (dotnet#7813)
  [ci] Run nunit tests with stable .NET version (dotnet#7826)
  ...
@github-actions github-actions bot locked and limited conversation to collaborators Jan 23, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants