Skip to content

fix(gradle): add apache http client support for api level 28+#1927

Merged
skylot merged 4 commits intoskylot:masterfrom
nitram84:optional_libraries
Jun 28, 2023
Merged

fix(gradle): add apache http client support for api level 28+#1927
skylot merged 4 commits intoskylot:masterfrom
nitram84:optional_libraries

Conversation

@nitram84
Copy link
Copy Markdown
Contributor

@nitram84 nitram84 commented Jun 22, 2023

I would like to propose some patches to fix legacy apache http client support in gradle files. To detect legacy apache http client usage the best way in my opinion is to use classpath information from jcst files. This has the advantage that all warnings like "Can't compare types, unknown hierarchy" should be gone for apks. My first idea was generating jcst files for each optional library. But this is a bad idea due to index based referencing of the jcst file format. To store the origin of classes I had to extend the jcst file format.

The first commit repairs the broken jcst generator.

In the second commit I extended the jcst file format, updated core to api level 34 and added all optional libraries (android.car and org.apache.http.legacy). With this PR the order of arguments matters when updating core.jcst. I updated core.jcst with this arguments:

<jadx_root>/jadx-core/src/main/resources/clst/core.jcst <sdk_root>/platforms/android-<api level>/android.jar <sdk_root>/platforms/android-<api level>/optional/android.car.jar <sdk_root>/platforms/android-<api level>/optional/org.apache.http.legacy.jar

For already decompiled apps it is important to clear jadx caches to rebuild usage info.

What is missing:

@skylot skylot merged commit 4467f91 into skylot:master Jun 28, 2023
@skylot
Copy link
Copy Markdown
Owner

skylot commented Jun 28, 2023

@nitram84 great work, thanks!
I made some fixes to disable loading of existing core.jcst file (it can fail if format changes) and also improve performance by disabling not needed prepare passes.

Also, I notice some possible issues with libraries detection:

  • now usage info can be loaded from cache file, so your checks will not execute.
    It can be a case if usage info cache set to disk and user reload project before export.
  • library will not be detected if all usage only in 'instructions'.
    Your checks not triggered on library method usage, only for library classes used as field types or methods args/return/exception types.

To fix these issues, I think, is it better to make a separate pass (with full instructions scan) and run it before export. But this fix looks complex and should be addressed in another issue/PR.

@nitram84
Copy link
Copy Markdown
Contributor Author

nitram84 commented Jul 1, 2023

@skylot Thank you for your feedback. At first I thought of a separate pass for all gradle relevant tasks, where classes have to be processed. But then, just for one simple check, I wasn't sure anymore. You are right, I should fix this, but it will take some time to test everything. Can you give me a good example for a full instructions scan as a reference? Thank you!

@nitram84 nitram84 deleted the optional_libraries branch July 1, 2023 10:58
@skylot
Copy link
Copy Markdown
Owner

skylot commented Jul 1, 2023

Can you give me a good example for a full instructions scan as a reference?

jadx/jadx-core/src/main/java/jadx/core/dex/visitors/usage/UsageInfoVisitor.java

Lines 112 to 118 in a992c93

codeReader.visitInstructions(insnData -> {
try {
processInsn(root, mth, insnData, usageInfo);
} catch (Exception e) {
mth.addError("Dependency scan failed at insn: " + insnData, e);
}
});

Same as in UsageInfoVisitor. This instruction visitor allows to quickly traverse all insns and decode only required.
But unlike UsageInfoVisitor you need to check all methods references (IMethodRef) from invoke insns, not only 'resolved' (i.e. from current app).

At first I thought of a separate pass for all gradle relevant tasks, where classes have to be processed. But then, just for one simple check, I wasn't sure anymore

Yeah, adjusting UsageInfoVisitor looks like the simplest way, but it requires too many changes in lots of places, so in result it is easier to just copy code to new visitor and change it to your needs.

Also, I want to mention, that now there is no way to nicely execute pass only for export. So I suggest adding new jadx pass type with name like JadxBeforeExportPass similar to existing JadxAfterLoadPass and next add it here, get all passes like this and run them before export.

@nitram84
Copy link
Copy Markdown
Contributor Author

nitram84 commented Jul 5, 2023

Thank you! This will help me a lot!

@nitram84 nitram84 mentioned this pull request Dec 10, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@nitram84 @skylot