ReVanced / revanced-manager

💊 Application to use ReVanced on Android
https://revanced.app
GNU General Public License v3.0
17.33k stars 725 forks source link

bug: ApkFormatException: Malformed ZIP entry: assets/fonts/minecraft.ttf #2083

Closed SSUPII closed 1 month ago

SSUPII commented 1 month ago

Bug description

Trying to apply "Change package name" patch only. Objective is to be able to keep both this older release and the latest updated release from Play Store installed on the same device.

How to reproduce: 1) Installed my own backup of the app to be patched (via ADB, installed while passing --bypass-low-target-sdk-block) 2) Select the app in Revanced Manager, leave the selected patches as the default "Change package name" only. 3) Attempt to patch

Device running Revanced Manager: Samsung Galaxy Tab S9+, Android 14 (OneUI 6.1)

Version of ReVanced Manager and version & name of app you are patching

Manager: 1.20.1 App failing to be patched: Minecraft Pocket Edition 0.8.0

Installation method

None

ReVanced Manager logs

https://pastebin.com/GP18qM2e

Patch logs

- Device Info
ReVanced Manager: 1.20.1
Model: SM-X810
Android version: 14
Supported architectures: arm64-v8a, armeabi-v7a, armeabi
Root permissions: No

- Patch Info
App: com.mojang.minecraftpe v0.8.0 (Suggested: Any)
Patches version: v4.11.0
Patches added: Change package name
Patches removed: None
Default patch options changed: None

- Settings
Allow changing patch selection: true
Version compatibility check: true
Show universal patches: true
Patches source: revanced/revanced-patches
Integration source: revanced/revanced-integrations

- Logs
Reading APK
Decoding app manifest
Loading patches
Deleting existing temporary files directory
Decoding resources
Executing patches
Applied 1 patches
Compiling patched dex files
Compiled 1 dex files
Compiling modified resources
Aligning APK
Signing APK
An error occurred:
com.android.apksig.apk.ApkFormatException: Malformed ZIP entry: assets/fonts/minecraft.ttf
    at com.android.apksig.ApkSigner.sign(SourceFile:0)
    at com.android.apksig.ApkSigner.sign(SourceFile:0)
    at app.revanced.library.ApkSigner$Signer.signApk(SourceFile:0)
    at app.revanced.library.ApkUtils.sign(SourceFile:0)
    at app.revanced.manager.flutter.MainActivity.runPatcher$lambda$34(Unknown Source:553)
    at app.revanced.manager.flutter.MainActivity.$r8$lambda$hTJfH-rv2j-9Y91G5H_L5jFDN6U(SourceFile:0)
    at app.revanced.manager.flutter.MainActivity$$ExternalSyntheticLambda5.run(SourceFile:0)
    at java.lang.Thread.run(Thread.java:1012)
Caused by: com.android.apksig.zip.ZipFormatException: Data Descriptor presence mismatch between Local File Header and Central Directory for entry assets/fonts/minecraft.ttf. LFH: true, CD: false
    at com.android.apksig.internal.zip.LocalFileRecord.getRecord(SourceFile:0)
    at com.android.apksig.internal.zip.LocalFileRecord.getRecord(SourceFile:0)
    ... 8 more

Acknowledgements

oSumAtrIX commented 1 month ago

As the error says, the APK is malformed because it has a malformed ZipEntry. Usually fixable by removing such a broken file or letting the entry be fixed by some ZIP program

SSUPII commented 1 month ago

As the error says, the APK is malformed because it has a malformed ZipEntry. Usually fixable by removing such a broken file or letting the entry be fixed by some ZIP program

Thing is, this file is installed on the system and is a valid APK file despite Manager claiming it is malformed. It is weird that it is a valid APK file but not a valid ZIP file.

As I remember a thing or two about modding this older Minecraft PE releases I can try rebuilding a fresh APK with no modifications.

SSUPII commented 1 month ago

I can instead confirm rebuilding a fresh APK allows Manager to patch it. Manager cannot install it due to Android's package installer refusing to install apps from deprecated SDKs, but the resulting apk file can be saved and eventually installed manually via adb.

It installed and worked.

oSumAtrIX commented 1 month ago

ReVanced Manager isn't the one claiming it's broken. It's an underlying official Android library (apkzlib) which evidently fails to read that fail. The reason your APK installs is because for installation reading all the entries in an APK is not required. When you run the app and Android tried to read the entry, it'll crash or gracefully exit

AbdurazaaqMohammed commented 1 month ago

ReVanced Manager isn't the one claiming it's broken. It's an underlying official Android library (apkzlib) which evidently fails to read that fail. The reason your APK installs is because for installation reading all the entries in an APK is not required. When you run the app and Android tried to read the entry, it'll crash or gracefully exit

@oSumAtrIX This is incorrect. This issue is likely being caused by merging a split APK to single APK using REAndroid APKEditor or AntiSplit M which is based on it or Apktool M. Merging split APKs is now practically necessary since it seems the Play Store is no longer supplying non split APKs which people used to download from sites like APKMirror.

This is not an actual problem; the APKs merged in this way install and work perfectly fine and have been doing so for a long time. If you simply remove the exception from the smali of ReVanced Manager, you can patch and install merged APKs perfectly. Considering that only one other person has ever created an issue for this error without specifying if the APK was merged, ReVanced should use a custom implementation of apksig/apkzlib with this particular exception having better validation or simply removed. I would do this myself and make a pull request but I don't have a computer right now.

oSumAtrIX commented 1 month ago

The APK is corrupted as mentioned earlier:

Caused by: com.android.apksig.zip.ZipFormatException: Data Descriptor presence mismatch between Local File Header and Central Directory for entry assets/fonts/minecraft.ttf. LFH: true, CD: false.

I don't know what tools you are using but they are not gonna be taken in account. ReVanced Manager expects a working and not a broken APK. How and where you get one is your concern.

AbdurazaaqMohammed commented 1 month ago

The APK is corrupted as mentioned earlier:

Caused by: com.android.apksig.zip.ZipFormatException: Data Descriptor presence mismatch between Local File Header and Central Directory for entry assets/fonts/minecraft.ttf. LFH: true, CD: false.

I don't know what tools you are using but they are not gonna be taken in account. ReVanced Manager expects a working and not a broken APK. How and where you get one is your concern.

Did you read anything I said? The APK is not corrupted, apksig is performing validation that it should not. There are no more non split APKs being released by Google Play for some apps, it will become necessary to use these tools.

oSumAtrIX commented 1 month ago

If apkzlib performs validation "that it should not", feel free to report that issue to apkzlib and claim to them that the APK isn't broken when the exception is clear proof for it.

SSUPII commented 1 month ago

@AbdurazaaqMohammed Split APKs weren't a thing in 2013, year of release of this Minecraft version. I don't even think API 9 (Android 2.3) can even understand split APKs.

All I needed to do was uncompress and recompress as zip the apk file, then sign it with a debugging key. It was just a malformed file table in my original archive.

AbdurazaaqMohammed commented 1 month ago

If apkzlib performs validation "that it should not", feel free to report that issue to apkzlib and claim to them that the APK isn't broken when the exception is clear proof for it.

Why are you blindly trusting that the exception is correct? You know that typical requirements for a ZIP to be valid do not apply to APKs. You can test it yourself with a merged APK and removing the exception from smali, or I can record the video and show you.

SSUPII commented 1 month ago

If you have an issue with patching merged APKs you need to make a new issue. This one is for an ancient game backup I made back in the day that happened to not be properly constructed.

And as already said, rebuilding the archive structure fixed the issue.

AbdurazaaqMohammed commented 1 month ago

Ok, pretty big coincidence that the first actually valid report of this error happens right as it starts occurring for a different reason.

AbdurazaaqMohammed commented 1 month ago

If you have an issue with patching merged APKs you need to make a new issue. This one is for an ancient game backup I made back in the day that happened to not be properly constructed.

And as already said, rebuilding the archive structure fixed the issue.

Actually if the app still worked when you first installed the unpatched version, this still isn't a valid exception

oSumAtrIX commented 1 month ago

that typical requirements for a ZIP to be valid do not apply to APK

Yes they do. An APK is a ZIP file and inherits all it's properties. The APK needs to be fully read and fully written during a patching process. A broken APK prevents that.

Actually if the app still worked when you first installed the unpatched version, this still isn't a valid exception

Yes it is. The APK may install but that file can't be read that was broken during the runtime

AbdurazaaqMohammed commented 1 month ago

that typical requirements for a ZIP to be valid do not apply to APK

Yes they do. An APK is a ZIP file and inherits all it's properties. The APK needs to be fully read and fully written during a patching process. A broken APK prevents that.

A ZIP with entries with invalid CRC are invalid but an APK with them will install and work perfectly fine. Spoofing CRCs is one way pairip was bypassed. Again, if you remove this exception, it will patch, install and work perfectly.

Actually if the app still worked when you first installed the unpatched version, this still isn't a valid exception

Yes it is. The APK may install but that file can't be read that was broken during the runtime

I mean if the app was opened and working.

oSumAtrIX commented 1 month ago

you remove this exception, it will patch, install and work perfectly.

Refer to

The APK may install but that file can't be read that was broken during the runtime

and

The APK needs to be fully read and fully written during a patching process. A broken APK prevents that

We won't assume erroneous ZIP files. Supply a working APK file or complain to apkzlib.

SSUPII commented 1 month ago

The fct that the unpatched game worked doesn't mean the apk wasn't broken. Knowing the releases of Minecraft around that time there is good chance it had a programed workaround/fallback to system font in case minecraft.ttf was broken/missing as it is only every used for multiplayer chat. (Not that I can test as online functionalities for this version are long discontinued)

AbdurazaaqMohammed commented 1 month ago

you remove this exception, it will patch, install and work perfectly.

Refer to

The APK may install but that file can't be read that was broken during the runtime

and

The APK needs to be fully read and fully written during a patching process. A broken APK prevents that

We won't assume erroneous ZIP files. Supply a working APK file or complain to apkzlib.

apksig and apkzlib are by Google, I'm sure they will not be too eager to fix an issue for modifying APKs. On the other hand ReVanced Manager and Patcher are for modifying APKs. They should take these situations into account accordingly.

Since OP could not confirm if their APK was really corrupt or not, there are zero confirmed instances of this exception being thrown for an actually invalid APK (I could not find any unrelated to ReVanced). The merged APKs function perfectly; ZipInputStream, ZipOutputStream, ZipFile, ZipEntry itself, getAssets all do not nor does anything else on Android have any problem with these supposedly corrupt entries or APKs. In my original comment I suggested improving the validation of the conditions for throwing this exception. Either apksig is detecting a mismatch between LFH and CD incorrectly or that mismatch in fact does not matter and no exception should be thrown.

oSumAtrIX commented 1 month ago

apksig and apkzlib are by Google, I'm sure they will not be too eager to fix an issue for modifying APKs.

In your opinion, this is not an issue with ReVanced at all. You claim it is an issue of apkzlib having validations it should not have because "for APK files the exception thrown in apkzlib is unnecessary".

They should take these situations into account accordingly.

No. ReVanced relies on apkzlib to read and write the APK files. Any exception thrown in apkzlib is to be discussed with the maintainers of apkzlib and if apkzlib deems the APK file as corrupted because APK files are ZIP files and inherit its properties, you are supposed to complain to them handing APK files incorrectly (according to you).

In my original comment I suggested improving the validation of the conditions for throwing this exception.

Once again, the exception is thrown by apkzlib and you are welcome to raise an issue to them. If you can provide an APK file that is a valid ZIP and yet apkzlib throws a ZIP error, we can raise the issue on our behalf to apkzlib.

AbdurazaaqMohammed commented 1 month ago

This exception is from apksig, ReVanced Manager already successfully patched the APK.

Here are two solutions to fix this stupid problem

  1. Use something other than apksig to sign the APK file, here are two options

https://github.com/Aefyr/PseudoApkSigner

https://github.com/fornwall/apksigner

  1. If APK failed to sign give user the option to save the file to internal storage to try to sign it themselves

Extra suggestion: if user is rooted inform them about Core Patch and give them the option to install unsigned APKs

oSumAtrIX commented 1 month ago

This exception is from apksig

And it is rightfully.

ReVanced Manager already successfully patched the APK

Because it only read the APK file partially. In this case only entries which were not corrupted.

Here are two solutions to fix this stupid problem

I have already told you the course of action from here on. Supply a working APK file or file an issue to apkzlib if you think they made a mistake.

Extra suggestion: if user is rooted inform them about Core Patch and give them the option to install unsigned APKs

Rooted users can mount APK files. Signing is not necessary.

AbdurazaaqMohammed commented 1 month ago

This exception is from apksig

And it is rightfully.

No, it is not, we already established this.

ReVanced Manager already successfully patched the APK

Because it only read the APK file partially. In this case only entries which were not corrupted.

Again, the entries are not corrupted. No program except apksig reports it. The APK will install and run perfectly.

Here are two solutions to fix this stupid problem

I have already told you the course of action from here on. Supply a working APK file or file an issue to apkzlib if you think they made a mistake.

I have already explained why this and other exceptions should be removed by ReVanced for the purpose of modifying APKs.

Extra suggestion: if user is rooted inform them about Core Patch and give them the option to install unsigned APKs

Rooted users can mount APK files. Signing is not necessary.

Where is the option to disable signing?

oSumAtrIX commented 1 month ago

No, it is not, we already established this.

I have already explained multiple times that the ZIP file is corrupted. The exception is thrown rightfully.

Again, the entries are not corrupted.

Caused by: com.android.apksig.zip.ZipFormatException: Data Descriptor presence mismatch between Local File Header and Central Directory for entry assets/fonts/minecraft.ttf. LFH: true, CD: false.

No program except apksig reports it.

This is incorrect. Any zip validator will raise the same message.

The APK will install and run perfectly.

I have already said this multiple times now: The APK may install but that file can't be read that was broken during the runtime

should be removed by ReVanced

apkzlib throws the exception, we wont remove anything.

Where is the option to disable signing?

I said signing is not necessary.

I'll lock this since you are repeating the same messages despite having them answered already. If you have other issues with ReVanced Manager, create a new issue.