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

chore(core): handle RES_TABLE_TYPE_OVERLAY #1804

Merged
merged 1 commit into from
Mar 18, 2023

Conversation

jpstotz
Copy link
Collaborator

@jpstotz jpstotz commented Mar 18, 2023

The implementation to parse OVERLAY chunks in resource table.

Fixes #1748

@skylot skylot merged commit 6ba0e1d into skylot:master Mar 18, 2023
@skylot
Copy link
Owner

skylot commented Mar 18, 2023

@jpstotz thanks!
BTW, looks like we can make chunk parsing more general by moving calculating chunk size and skipping to the end in outer loop. This will allow fixing similar issues for other types and to skip not yet supported chunks without errors 🙂

@jpstotz
Copy link
Collaborator Author

jpstotz commented Mar 18, 2023

@skylot Yes that would be possible but I intentionally implemented it this way to make sure people create an issue and provide sample files. Otherwise I don't see a chance in finding matching APK files for evolving Jadx resource parsing.

It is possible to learn a lot by looking at the apktool source but often the apktool source code also doesn't work so it is IMHO better to raise an error, get the sample file and thus be able to implement the necessary code base don a real-world example.

@jpstotz jpstotz deleted the parseOverlayTypeChunk branch March 18, 2023 14:38
@skylot
Copy link
Owner

skylot commented Mar 18, 2023

to make sure people create an issue and provide sample files

@jpstotz agree, I like this strategy 👍🤣

@jpstotz
Copy link
Collaborator Author

jpstotz commented Mar 20, 2023

@skylot may be we can make it a little bit more user friendly: parse unknown resource table chunks, show an error dialog but continue loading the other chunks.

I implemented this approach as a test:
jpstotz@6aa46ad

The shown error dialog contains the complete chunk data base64 encoded. For common chunks that are smaller than 4KB this should be possible. Also this simplifies getting sample data in case the APK can not be shared.

@skylot
Copy link
Owner

skylot commented Mar 20, 2023

@jpstotz well, UncaughtExceptionHandler usage look very hacky and I think it can kill jadx-cli, also it can be unexpected for users of jadx as a library. In jadx-next branch I started a gui api for plugins and jadx script (JadxGuiContext) I think it is better to add a method there to show such errors, code will be like this:

JadxGuiContext guiContext = root.getDecompiler().getPluginsContext().getGuiContext();
if (guiContext != null) {
	// running in jadx-gui, show error dialog
	guiContext.showExceptionDialog(...)
}

@jpstotz
Copy link
Collaborator Author

jpstotz commented Mar 21, 2023

@skylot Yes that is a bit hacky, but I have tested that it does not cause problems for jadx-cli.

Of course the way JadxGuiContext is designed makes it much cleaner. I will take a look onto jadx-next...

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.

[core]APP parsing error, and the overlay cannot parse
2 participants