search

google / archive-patcher

Automatically exported from code.google.com/p/archive-patcher
Apache License 2.0
528 stars 73 forks source link

Fix Zlib bug in android 12 and above #193

Open hamid97m opened 1 year ago

hamid97m commented 1 year ago

In Android 12 and above, the archive-patcher applier doesn't work because, in Android 12, the Zlib is updated. In this pull request, I just added a customDeflater class instead Deflater and load the suitable Zlib library version.

google-cla[bot] commented 1 year ago

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

berlix commented 1 year ago

Thanks a lot for this. At my organisation, we are currently dealing with the same issue, so your solution is really helpful to us.

I have two questions about your solution:

  1. What did you build libdifftools.so from (what library, what version)? You seem to have made some customisations, since the library contains JNI symbols specific to archive-patcher. It would be great to be able to see the sources, since otherwise we (and likely many others) wouldn't be able to adopt your solution for security reasons.
  2. You state that your solution fixes the issue on Android 12 and up. We have verified however that the issue was introduced in Android 11, not Android 12. Could you please clarify whether there is any reason to believe your solution won't work on Android 11?

Note that this repo appears to have been abandoned six years ago, so I wouldn't expect this PR to ever get merged.

hamid97m commented 1 year ago

Thanks a lot for this. At my organisation, we are currently dealing with the same issue, so your solution is really helpful to us.

I have two questions about your solution:

  1. What did you build libdifftools.so from (what library, what version)? You seem to have made some customisations, since the library contains JNI symbols specific to archive-patcher. It would be great to be able to see the sources, since otherwise we (and likely many others) wouldn't be able to adopt your solution for security reasons.
  2. You state that your solution fixes the issue on Android 12 and up. We have verified however that the issue was introduced in Android 11, not Android 12. Could you please clarify whether there is any reason to believe your solution won't work on Android 11?

Note that this repo appears to have been abandoned six years ago, so I wouldn't expect this PR to ever get merged.

Your welcome, Yes of course, the libdifftools.so is nothing but Zlib version 1.2.11; I made a repository here; you can also use your correct version of Zlib (Zlib is open source as you know). About your second issue, honestly, I didn't personally check this issue, and our QA team reported this issue; maybe they were wrong, and the problem is about Android 11, or maybe your problem isn't relative to Zlib; this possibility is not very likely because after my change we release a new version of our application and 45 million users have no problem yet.

berlix commented 1 year ago

Thanks, much appreciated! In case we make any improvements on top of your code, we'll be happy to contribute them back to your repos.

Regarding the affected Android versions, DefaultDeflateCompatibilityWindow().isCompatible() returns false from Android 11 onwards, so most likely that was when the zlib implementation changed.

So from your answer I gather there's no reason to believe that the fix will create problems on any Android version. We'll test it anyway on various versions from 5 through 13. Will report back here in case we encounter any issues.

ityancs commented 1 year ago

Can you share more information about the cause of the problem？I test it is work right on android 11。I guess maybe google use Zopfli replace zlib impl in android 12。

hamid97m commented 1 year ago

Can you share more information about the cause of the problem？I test it is work right on android 11。I guess maybe google use Zopfli replace zlib impl in android 12。

Zlib version has been updated in Android 12

ityancs commented 1 year ago

Can you share more information about the cause of the problem？I test it is work right on android 11。I guess maybe google use Zopfli replace zlib impl in android 12。

Zlib version has been updated in Android 12

I found standalone zlib version patch work right, but the native memory is not free right, do you have the same problem？

berlix commented 1 year ago

Thanks again for your solution. Inspired by it, we created our own, which bundles zlib 1.2.13 binaries (the currently latest version) and creating a version of Deflater that uses those. Note that this zlib version is indeed compatible, as indicated by the instrumented tests - no need to downgrade to any older version.

Sources: https://github.com/EIDU/archive-patcher-android

The library is on Maven Central. See the repo's README.md for usage instructions.

rakeshyguttal commented 3 months ago

Thanks again for your solution. Inspired by it, we created our own, which bundles zlib 1.2.13 binaries (the currently latest version) and creating a version of Deflater that uses those. Note that this zlib version is indeed compatible, as indicated by the instrumented tests - no need to downgrade to any older version.

Sources: https://github.com/EIDU/archive-patcher-android

The library is on Maven Central. See the repo's README.md for usage instructions.

We are currently dealing with the same issue. I understood that this change creates custom deflater which uses the bundled zlib. But trying to understand the issue with google archive patcher whether the issue with deflater or zlib. Because in the beginning of the conversation, @hamid97m mentioned issue due to Zlib version upgrade but you mentioned no need to downgrade to any older version. I have below queries related to this change. Please help me answer these.

  1. Can you please share more details on what is the exact issue and root cause ? what is the actual issue with java.util.zip.deflater or zlib in new android version ?
  2. What is the actual fix done in deflator or zlib to fix this issue ?
  3. why create custom deflater and bundle specific version of zlib binaries instead of fixing the issue with deflater or zlib ?
berlix commented 3 months ago

Excellent questions!

Can you please share more details on what is the exact issue and root cause ? what is the actual issue with java.util.zip.deflater or zlib in new android version ?

I don't actually know why zlib behaves differently on those newer Android versions, only that it does.

What is the actual fix done in deflator or zlib to fix this issue ?

No fix was necessary. Deflater and zlib are unmodified in our solution, except for the JNI code required to make the custom Deflater use the bundled zlib binaries.

why create custom deflater and bundle specific version of zlib binaries instead of fixing the issue with deflater or zlib ?

It might not actually be considered an "issue", because the output produced by the Android-bundled zlib isn't wrong as far as I'm aware, it's just different (perhaps it's even better in some way, who knows). But even if the behaviour was going to be reverted in Android's zlib, that would not fix our issue for devices that are already out there in the wild and will not receive an Android update anytime soon or ever.

sisong commented 3 months ago

@rakeshyguttal

The archive-patcher can decompressed APK & completely restore the APK file when patch, Can do this relies heavily on the compressed encoding of zlib library: because the encoding generated by different(or optimized) zlib versions is slightly different, the APK file will be corrupted.
Therefore, it is better to use the exact same version of the zlib library when diff & patch, rather than relying on the runtime system environment.