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

Patch APKTool to allow repeated entry offsets to appear #1683

Merged
merged 1 commit into from Dec 9, 2017

Conversation

Projects
None yet
2 participants
@Bricnic

Bricnic commented Dec 8, 2017

Hello! I'm sending this pull request on behalf of Facebook. We've been using a slightly modified APKTool internally- it would be great to get the changes incorporated back into this main open source repository.

There are two main changes:

  • Added a flag for only extracting and decoding AndroidManifest.xml (without unpacking resources or sources). We have a use case where all we need are this decoded manifest and the .yml file- this makes the execution significantly faster.
  • Added support for duplicate / repeated offsets (for resource entries). There are some built in assumptions in the ARSCDecoder- it assumes that offsets will always be in incrementing order, and that there will be no duplicates. These happen to be true for APK's that are built using standard android tools (aapt, aapt2), but they are not runtime requirements. We've performed some optimizations that result in collapsing certain entries into copies of other entries, which means duplicate offsets will appear (saving APK size).

Example beta build (version 153) of Facebook for Android which requires the 'repeated offsets' fix: https://www.dropbox.com/s/1n3dx7xhelxtik9/fb4a.153-beta.apk?dl=0

Attempting to decode this APK without the fix results in this error:

java -jar ./brut.apktool/apktool-cli/build/libs/apktool-cli-all.jar d -f -o /example_out_dir /path/to/fb4a.153-beta.apk

I: Using Apktool 2.3.1-88eed2-SNAPSHOT on fb4a.153-beta.apk
I: Loading resource table...
Exception in thread "main" brut.androlib.AndrolibException: Could not decode arsc file
	at brut.androlib.res.decoder.ARSCDecoder.decode(ARSCDecoder.java:52)
	at brut.androlib.res.AndrolibResources.getResPackagesFromApk(AndrolibResources.java:599)
	at brut.androlib.res.AndrolibResources.loadMainPkg(AndrolibResources.java:73)
	at brut.androlib.res.AndrolibResources.getResTable(AndrolibResources.java:65)
	at brut.androlib.Androlib.getResTable(Androlib.java:68)
	at brut.androlib.ApkDecoder.setTargetSdkVersion(ApkDecoder.java:228)
	at brut.androlib.ApkDecoder.decode(ApkDecoder.java:118)
	at brut.apktool.Main.cmdDecode(Main.java:163)
	at brut.apktool.Main.main(Main.java:72)
Caused by: java.io.IOException: Expected: 0x00000008, got: 0x0000000a
	at brut.util.ExtDataInput.skipCheckShort(ExtDataInput.java:56)
	at brut.androlib.res.decoder.ARSCDecoder.readValue(ARSCDecoder.java:320)
	at brut.androlib.res.decoder.ARSCDecoder.readEntry(ARSCDecoder.java:252)
	at brut.androlib.res.decoder.ARSCDecoder.readTableType(ARSCDecoder.java:231)
	at brut.androlib.res.decoder.ARSCDecoder.readTableTypeSpec(ARSCDecoder.java:156)
	at brut.androlib.res.decoder.ARSCDecoder.readTablePackage(ARSCDecoder.java:118)
	at brut.androlib.res.decoder.ARSCDecoder.readTableHeader(ARSCDecoder.java:80)
	at brut.androlib.res.decoder.ARSCDecoder.decode(ARSCDecoder.java:47)

The program executes correctly with the fix applied.
Thanks for your consideration!

@iBotPeaches

This comment has been minimized.

Show comment
Hide comment
@iBotPeaches

iBotPeaches Dec 8, 2017

Owner

Hi there. Thanks for sending things back upstream!

About 17 hours ago, we actually just merged a change into master for decoding just the manifest, if required. This can be seen here: 5734c68

Documentation hasn't been updated yet, but the idea is --force-manifest --no-res paired together will skip resource decoding (The real slowdown) but still decode the Manifest which should resolve your first issue. I see the goal for your use case was just the manifest & meta yaml file. I'll do some tests to check if the newly added feature is capable of that use-case so we don't have to balance two very similar feature flags.

Otherwise looking through the code, I see plenty of good fixes all around, so thanks again for sending them upstream. I'll pull it down and get this merged after I resolve the duplicate parameters for decoding just manifest.

Owner

iBotPeaches commented Dec 8, 2017

Hi there. Thanks for sending things back upstream!

About 17 hours ago, we actually just merged a change into master for decoding just the manifest, if required. This can be seen here: 5734c68

Documentation hasn't been updated yet, but the idea is --force-manifest --no-res paired together will skip resource decoding (The real slowdown) but still decode the Manifest which should resolve your first issue. I see the goal for your use case was just the manifest & meta yaml file. I'll do some tests to check if the newly added feature is capable of that use-case so we don't have to balance two very similar feature flags.

Otherwise looking through the code, I see plenty of good fixes all around, so thanks again for sending them upstream. I'll pull it down and get this merged after I resolve the duplicate parameters for decoding just manifest.

@Bricnic

This comment has been minimized.

Show comment
Hide comment
@Bricnic

Bricnic Dec 8, 2017

Good point, it does look like we can use that new addition instead. When combined with '--no-src' it's almost as fast as --manifest-only. Time comparisons:

--manifest-only : 7.8 seconds
--force-manifest --no-res --no-src : 8.1 seconds
--force-manifest --no-res : 1 min 3 seconds

All three options produce the files we need (AndroidManifest and the meta yaml file). It's no big deal that additional files are produced that we don't need, and options 1 and 2 are roughly as fast as each other, so I'm fine with the suggestion of getting rid of the new parameter to keep things simpler.

Thanks for the speedy response!

Bricnic commented Dec 8, 2017

Good point, it does look like we can use that new addition instead. When combined with '--no-src' it's almost as fast as --manifest-only. Time comparisons:

--manifest-only : 7.8 seconds
--force-manifest --no-res --no-src : 8.1 seconds
--force-manifest --no-res : 1 min 3 seconds

All three options produce the files we need (AndroidManifest and the meta yaml file). It's no big deal that additional files are produced that we don't need, and options 1 and 2 are roughly as fast as each other, so I'm fine with the suggestion of getting rid of the new parameter to keep things simpler.

Thanks for the speedy response!

@iBotPeaches iBotPeaches merged commit 88eed24 into iBotPeaches:master Dec 9, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@iBotPeaches

This comment has been minimized.

Show comment
Hide comment
@iBotPeaches

iBotPeaches Dec 9, 2017

Owner

Thanks again! Merged w/ removing duplicate parameter & slight code styling.

Owner

iBotPeaches commented Dec 9, 2017

Thanks again! Merged w/ removing duplicate parameter & slight code styling.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment