Skip to content

optimize: JSMethod #108

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.

Sign up for GitHub

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

Open
LazyImmortal wants to merge 4 commits into
base: main
Choose a base branch
from
Open

optimize: JSMethod #108

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
dd4eb6e
添加 混淆规则(确保JSMethod不被移除)
LazyImmortal Nov 25, 2025
a261eec
添加 JSMethod注解值(确保代码混淆后方法仍能被正确调用)
LazyImmortal Nov 25, 2025
a15f5c2
调整 消除警告(提高代码健壮性)
LazyImmortal Nov 25, 2025
90b8278
优化 Java类对象设置(确保类没有无参构造方法时仍能调用静态方法)
LazyImmortal Nov 25, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion wrapper-android/consumer-rules.pro
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
-keepclasseswithmembers class * {
native <methods>;
}
-keep class com.whl.quickjs.**{*;}
-keep class com.whl.quickjs.**{*;}
-keepclassmembers,allowobfuscation class * {
@com.whl.quickjs.wrapper.JSMethod <methods>;
Comment on lines +5 to +6
Copy link

@sourcery-ai sourcery-ai bot Nov 25, 2025

Choose a reason for hiding this comment

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

issue (bug_risk): 允许对带有 @JSmethod 的方法进行混淆，可能会破坏依赖默认方法名的 JS API。

当前绑定逻辑会使用注解中的 value，如果 value 为空则回退到 method.getName()。如果这些方法被混淆，那么任何依赖默认（空）注解值的 JS API 在发布构建中就会暴露为混淆后的方法名。仅当所有暴露给 JS 的方法都显式指定了 @JSmethod("...") 的值时，这才是安全的。如果无法保证这一点，建议对这些方法去掉 allowobfuscation，或者拆分规则，只对注解值为空的 @JSmethod 方法保持不混淆。

Original comment in English

issue (bug_risk): Allowing obfuscation of @JSmethod methods can break JS APIs that rely on default method names.

Because the binding uses the annotation value or falls back to method.getName() when the value is blank, obfuscating these methods means any JS API that relies on the default (blank) annotation will now be exposed under an obfuscated name in release builds. This is only safe if all JS-exposed methods use an explicit @JSmethod("...") value. If that isn’t guaranteed, consider removing allowobfuscation for these methods or splitting the rule so that only methods with blank annotation values remain unobfuscated.

}
20 changes: 10 additions & 10 deletions wrapper-android/src/androidTest/java/com/whl/quickjs/wrapper/QuickJSTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -491,33 +491,33 @@ public void testStackSizeWithLimited() {
}

public static class TestJava {
@JSMethod
public boolean test1(String message) {
@JSMethod("test1")
public boolean test(String message) {
return TextUtils.equals("arg1", message);
}

@JSMethod
public boolean test2(String message, int age) {
@JSMethod("test2")
public boolean test(String message, int age) {
return TextUtils.equals("arg1", message) && 18 == age;
}

@JSMethod
public boolean testArray(JSArray array) {
@JSMethod("testArray")
public boolean test(JSArray array) {
String arg1 = (String) array.get(0);
int age = (int) array.get(1);
array.release();
return TextUtils.equals("arg1", arg1) && 18 == age;
}

@JSMethod
public boolean testObject(JSObject jsObj) {
@JSMethod("testObject")
public boolean test(JSObject jsObj) {
String str = jsObj.stringify();
jsObj.release();
return TextUtils.equals("{\"arg1\":\"a\",\"arg2\":18}", str);
}

@JSMethod
public boolean testAll(JSObject object, JSArray message, int age, String name) {
@JSMethod("testAll")
public boolean test(JSObject object, JSArray message, int age, String name) {
String oStr = object.stringify();
String mStr = message.stringify();
object.release();
Expand Down
1 change: 1 addition & 0 deletions wrapper-java/src/main/java/com/whl/quickjs/wrapper/JSMethod.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,5 @@
@Retention(value = RetentionPolicy.RUNTIME)
@Target(value = {ElementType.METHOD})
public @interface JSMethod {
String value() default "";
}
55 changes: 30 additions & 25 deletions wrapper-java/src/main/java/com/whl/quickjs/wrapper/QuickJSObject.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@

import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.lang.reflect.Modifier;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
import java.util.HashSet;
import java.util.Map;

/**
Expand Down Expand Up @@ -104,33 +104,37 @@ private void setPropertyObject(String name, Object o) {
public void setProperty(String name, Class<?> clazz) {
Object javaObj = null;
try {
javaObj = clazz.newInstance();
} catch (IllegalAccessException e) {
javaObj = clazz.getConstructor().newInstance();
} catch (IllegalAccessException
| InstantiationException
| InvocationTargetException
| NoSuchMethodException e) {
e.printStackTrace();
} catch (InstantiationException e) {
e.printStackTrace();
}

if (javaObj == null) {
throw new NullPointerException("The JavaObj cannot be null. An error occurred in newInstance!");
}

JSObject jsObj = context.createNewJSObject();
Comment on lines 106 to 115
Copy link

@sourcery-ai sourcery-ai bot Nov 25, 2025

Choose a reason for hiding this comment

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

issue (bug_risk): 构造函数失败时，只打印堆栈而没有进一步处理，会导致实例方法被静默忽略。

在改为使用 clazz.getConstructor().newInstance() 之后，如果没有无参构造函数、无参构造不可访问，或者构造函数本身抛异常，javaObj 就会保持为 null。后续循环只会暴露静态方法，这个失败则被隐藏在 e.printStackTrace() 中。这样会让配置错误（例如只添加了带必填参数的构造函数）很难被发现，并且会在不明显的情况下破坏 JS 绑定。请更明确地暴露这类失败：例如抛出清晰的运行时异常、通过你们的标准日志组件记录并尽早失败，或者在无法直接实例化时支持调用方提供工厂方法。

Original comment in English

issue (bug_risk): Constructor failure leads to silent dropping of instance methods with only a stack trace as signal.

With the new use of clazz.getConstructor().newInstance(), a missing/inaccessible no-arg constructor (or a constructor that throws) leaves javaObj as null. The loop then only exposes static methods, and the failure is effectively hidden behind e.printStackTrace(). This makes misconfigurations (e.g., adding only a required-args constructor) hard to detect and can silently break JS bindings. Please surface this failure more explicitly: e.g., throw a clear runtime exception, log via your standard logger and fail fast, or support a caller-provided factory when direct instantiation isn’t possible.

Method[] methods = clazz.getMethods();
Object finalJavaObj = javaObj;
for (Method method : methods) {
if (method.isAnnotationPresent(JSMethod.class)) {
Object finalJavaObj = javaObj;
jsObj.setProperty(method.getName(), args -> {
try {
return method.invoke(finalJavaObj, args);
} catch (IllegalAccessException e) {
e.printStackTrace();
} catch (InvocationTargetException e) {
e.printStackTrace();
}
return null;
});
if (finalJavaObj == null && !Modifier.isStatic(method.getModifiers())) {
continue;
}
JSMethod jsMethod = method.getAnnotation(JSMethod.class);
if (jsMethod == null) {
continue;
}
String jsMethodName = jsMethod.value();
if (jsMethodName.isBlank()) {
jsMethodName = method.getName();
}
jsObj.setProperty(jsMethodName, args -> {
try {
return method.invoke(finalJavaObj, args);
} catch (IllegalAccessException | InvocationTargetException e) {
e.printStackTrace();
}
return null;
});
}

setProperty(name, jsObj);
Expand Down Expand Up @@ -288,6 +292,7 @@ public ArrayList<Object> toArray(MapFilter filter) {
return toArray(filter, null, null);
}

@SuppressWarnings("unchecked")
protected void convertToMap(Object target, Object map, HashMap<Long, Object> circulars, MapFilter filter, Object extra, MapCreator mapCreator) {
circulars.put(((JSObject) target).getPointer(), map);

Expand All @@ -314,7 +319,7 @@ protected void convertToMap(Object target, Object map, HashMap<Long, Object> cir
Object refValue = circulars.get(pointer);
if (map instanceof Map) {
((Map<String, Object>) map).put(key, refValue);
} else if (map instanceof ArrayList){
} else if (map instanceof ArrayList) {
((ArrayList<Object>) map).add(refValue);
}
continue;
Expand All @@ -332,7 +337,7 @@ protected void convertToMap(Object target, Object map, HashMap<Long, Object> cir
convertToMap(value, list, circulars, filter, extra, mapCreator);
if (map instanceof Map) {
((Map<String, Object>) map).put(key, list);
} else if (map instanceof ArrayList){
} else if (map instanceof ArrayList) {
((ArrayList<Object>) map).add(list);
}
((JSArray) value).release();
Expand All @@ -344,7 +349,7 @@ protected void convertToMap(Object target, Object map, HashMap<Long, Object> cir
convertToMap(value, valueMap, circulars, filter, extra, mapCreator);
if (map instanceof Map) {
((Map<String, Object>) map).put(key, valueMap);
} else if (map instanceof ArrayList){
} else if (map instanceof ArrayList) {
((ArrayList<Object>) map).add(valueMap);
}
((JSObject) value).release();
Expand All @@ -354,7 +359,7 @@ protected void convertToMap(Object target, Object map, HashMap<Long, Object> cir
// Primitive types.
if (map instanceof Map) {
((Map<String, Object>) map).put(key, value);
} else if (map instanceof ArrayList){
} else if (map instanceof ArrayList) {
((ArrayList<Object>) map).add(value);
}
}
Expand Down