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

Use param names in annotations #1256

Closed
wants to merge 1 commit into from
Closed

Conversation

DanielFi
Copy link
Contributor

@DanielFi DanielFi commented Oct 1, 2021

Use the values in MethodParameters annotations as parameter names when available.

@@ -23,9 +22,7 @@ public static AnnotationsAttr pack(List<IAnnotation> annotationList) {
}
Map<String, IAnnotation> annMap = new HashMap<>(annotationList.size());
for (IAnnotation ann : annotationList) {
if (ann.getVisibility() != AnnotationVisibility.SYSTEM) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this change break many tests :(

private List<String> extractParamNamesFromAnnotation() {
AnnotationsAttr annotationList = mth.get(JadxAttrType.ANNOTATION_LIST);
if (annotationList != null) {
JadxAnnotation annotation = (JadxAnnotation) annotationList.get("Ldalvik/annotation/MethodParameters;");
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As soon as jadx now trying to decompile java bytecode and not only dex, dalvik specific annotations should be parsed in jadx-dex-input plugin and data class for common representation of such attribute should be placed in jadx-plugins-api.

@skylot
Copy link
Owner

skylot commented Oct 1, 2021

@DanielFi thank you for your work, but there are some issues which stops me from merging your PR.
Also, I don't implement parsing of these attributes because I never saw them used in dex or java bytecode, so if you have any sample with these attributes please share it. With sample, I can help you to fix your PR :)

@skylot
Copy link
Owner

skylot commented Oct 17, 2021

So, no reply :(

@skylot skylot closed this Oct 17, 2021
@nitram84
Copy link
Contributor

nitram84 commented Oct 21, 2021

Hi @skylot, sorry for commenting a closed PR. I recently found a real example that uses the MethodParameters-annotation: https://apkpure.com/de/js-run-javascript-editor-and-runner/com.mia.jsrun e.g. com.mia.jsrun.MainActivity.findTokenEnd()
Any versions since V1.1.0.24 are using this annotations. BUT: You can see this annotation only in smali view (jadx does not generate code for this annotation/annotation is ignored). This small app is not obfuscated, but the "real" parameter names are still helpful. In my opinion some things needs to be checked: According to the docs (https://source.android.com/devices/tech/dalvik/dex-format#dalvik-annotation-method-parameters) I would ignore this annotation for Android versions before 7.1 (may be used to fool jadx with wrong names) and all parameter names for Android 7.1+ must be validated. There is actually a behavior change issue for illegal parameter names (no runtime exception thrown in decompiled code) because jadx ignores this annotation.
You may want to reopen this PR.

@skylot
Copy link
Owner

skylot commented Oct 21, 2021

@nitram84 thank you for a nice sample! Indeed, names from that annotation can be very useful!
Unfortunately, this PR is nice POC, but required a lot of changes to be merged.
I will open a new issue for this case, to implement it later.

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.

None yet

3 participants