libxposed / api

Apache License 2.0
127 stars 47 forks source link

Allow anonymous XposedInterface.Hooker implementation and provide a way to skip hooked methods #10

Open anonymix007 opened 1 year ago

anonymix007 commented 1 year ago

Sometimes it's needed to hook lots of different methods (about 50 in my case), so only one XposedInterface.Hooker class isn't enough, because dispatching between hooked methods will be ugly. Having anonymous implementation of XposedInterface.Hooker is more convenient, but it's now impossible:

hook(method, new @XposedHooker Hooker() {
    @BeforeInvocation
    public static @Nullable Hooker before(@NonNull Member member, @Nullable Object thisObject, @NonNull Object[] args) {
        // Do something
        return null;
    }
}.getClass());

will not compile because of '@XposedHooker' not applicable to type use It's also a bit problematic to create return value of before method as anonymous classes don't have constructors.

Another problem is that some methods need to be skipped (because of their side effects), and now it's also impossible. So maybe before and after methods should include new parameter, which will contain functions returnAndSkip, setThrowable and others?

Dr-TSNG commented 1 year ago

The new API is a more low-level design, and we will provide a pre-designed one in the helper to provide a hook interface similar to the old API (that is, providing a callback object, which can allow something like kotlin dsl).

Regarding the performance issue of dispatch, it is difficult for us to make a trade-off between convenience and functionality. For some reasons, the hooker must be a class rather than an object. You can use a hashmap to distinguish different methods, but we will also adopt the suggestion of removing the @XposedHook annotation to allow anonymous classes.

Skip is indeed something that was not considered before, we will add an interface.

yujincheng08 commented 1 year ago

74534aa0c1511f963dda33ff839504440096822e

anonymix007 commented 1 year ago

Do before and after methods have to be static? Consider the following example:

public void onPackageLoaded(@NonNull PackageLoadedParam param) {
    //...
    Method hookMe = someClass.getDeclaredMethod("methodName", int.class);
    Method invokeMe = someClass.getDeclaredMethod("anotherMethodName", int.class);
    Field f = someClass.getDeclaredField("fieldName"); // Assume integer
    hook(getCurrentCodecConfig, new @XposedHooker Hooker() {
        @BeforeInvocation
        public static Hooker before(BeforeHookCallback callback) {
            int val = (int) callback.getArgs()[0];
            if (val == 1) {
                int newF = (int) invokeMe.invoke(callback.getThisObject(), 2);
                f.set(newF);
                callback.returnAndSkip(null);
            }
            return null;
        }
        @AfterInvocation
        public static void after(AfterHookCallback callback, Object thisObject) {
        }
    }.getClass());
    //...
}

It will not compile: Non-static variable 'f' cannot be referenced from a static context (and the same for invokeMe)

This hacky solution will compile:

// The other code is the same
var h = new @XposedHooker Hooker() {
    static Field f;
    static Method invokeMe;
    //...
};
h.f = f;
h.invokeMe = invokeMe;
hook(hookMe, h.getClass());

But it's really annoying to do, especially if there are lots of such methods and fields (which is the case for my module). They could be also made global (like module in example. But if there are different methods with the same name in different classes, these variables will have to be called differently. It will definitely lead to many logical errors, so even hacky solution with static fields in Hooker instance is better. Before it was handled by Java automatically (with variable scopes).

Dr-TSNG commented 1 year ago

A better solution to make things easy is to declare a global hooker with a ConcurrentHashMap<Member, MyCallback> (which will be provided in the unfinished helper module), and then you can write something like the old API. The reason why before and after should be static is that non-static callback object (framework level) can be easily detected and cleared by target app, so we could only do this.

anonymix007 commented 1 year ago

Now that makes sense, thanks for explanation. So you're suggesting something like this, right?

interface BeforeCallback {
     Hooker before(BeforeHookCallback callback);
}

interface AfterCallback {
     void after(AfterHookCallback callback, Object thisObject);
}
@XposedHooker
class CustomHooker implements Hooker {
    static ConcurrentHashMap<Member, BeforeCallback> beforeCallbacks;
    static ConcurrentHashMap<Member, AfterCallback> afterCallbacks;
    @BeforeInvocation
    public static Hooker before(BeforeHookCallback callback) {
        BeforeCallback bc =  beforeCallbacks.get(callback.getMember());
        return bc == null ? null : bc.before(callback);
    }
    @AfterInvocation
    public static void after(AfterHookCallback callback, Object thisObject) {
        AfterCallback ac = afterCallbacks.get(callback.getMember());
        if (ac != null) ac.after(callback, state);
    }
}
void hookBefore(Method method, BeforeCallback callback) {
    //assert !CustomHooker.beforeCallbacks.containsKey(method);
    CustomHooker.beforeCallbacks.put(method, callback);
    hook(method, CustomHooker.class);
}

void hookAfter(Method method, AfterCallback callback) {
    //assert !CustomHooker.afterCallbacks.containsKey(method);
    CustomHooker.afterCallbacks.put(method, callback);
    hook(method, CustomHooker.class);
}

Is there any reason why Object getArg(int) was removed in favor of Object[] getArgs()? Maybe keep both? Method BeforeHookCallback.invokeOrigin() was useful as well. XposedInterfaceWrapper.invokeOrigin() seems to provide similar functionality, but it's less convenient to use in some cases as one will have to pass original parameters manually one by one. Maybe a version which takes parameters as an array of objects, so that it could be invoked like this: invokeOrigin(callback.getOrigin(), callback.getArgs())?

Also getResources() and getBaseContext() were removed. Why and what is the proposed way to get resources and inflater? I've used getResources().getStringArray(...) and getBaseContext().getSystemService(Context.LAYOUT_INFLATER_SERVICE) before their removal.

Will callback.getArgs()[0]=1003 do the same what callback.setArg(0, 1003) did previously?