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

[BUG] HookParam call timing problem #47

Closed
cesaryuan opened this issue Sep 13, 2023 · 20 comments
Closed

[BUG] HookParam call timing problem #47

cesaryuan opened this issue Sep 13, 2023 · 20 comments
Labels
bug Something isn't working fixed This bug was fixed

Comments

@cesaryuan
Copy link

cesaryuan commented Sep 13, 2023

复现步骤:

  1. 创建模块,代码如下,启用模块
loadApp(name = "com.wibo.bigbang.ocr") {
  findClass(name = "okhttp3.Request\$Builder").hook {
      injectMember {
          method {
              name = "addHeader"
              param(StringClass,StringClass)
              returnType = "okhttp3.Request\$Builder"
          }
          beforeHook {
              val a1 = args().first().string();
              loggerD("checkEqual", "", LoggerType.LOGD)
              val a2 = args().first().string();
              loggerD(
                  tag = "checkEqual",
                  msg = "${a1 == a2} $a1 $a2"
              )
          }
      }
  }
}
  1. 安装宿主app:布丁扫描,启动宿主APP

  2. 观察Logcat,发现有好几次输出了false,按理来说应该始终是true才对
    image

  3. 把a1后面的loggerD("checkEqual", "", LoggerType.LOGD)删掉后,程序运行正常(始终输出true
    image

@fankes
Copy link
Collaborator

fankes commented Sep 13, 2023

试试 XposedBridge.log 以及 Log.d

@cesaryuan
Copy link
Author

试试 XposedBridge.log 以及 Log.d

谢谢大佬的回复,我试了下换成 Log.d 之后也还是一样的问题,但是出现 false 的几率变小了很多,然后我进一步测试,发现把 loggerD 换成 Thread.sleep(500) 后有一样的效果,出现 false 的几率特别大;但是换成 Thread.sleep(1) 之后就小了很多(还是会有 false)

@cesaryuan
Copy link
Author

补充一下,发现一个临时的解决方案,在最前面加上 val args = args

@fankes
Copy link
Collaborator

fankes commented Sep 13, 2023

因为 Log 会导致延迟,这个时候宿主的代码和你执行的时间不是同步的,执行的时间就会比你靠前,出现这种情况大概是正常的,这不是 API 的问题

因为 loggerD 会同时调用两个 Logd 耗时会更长

@fankes
Copy link
Collaborator

fankes commented Sep 13, 2023

这大概不是 API 的管辖范围内,所以关闭了,不能认为是 bug

@fankes fankes closed this as not planned Won't fix, can't repro, duplicate, stale Sep 13, 2023
@fankes fankes added the wontfix This will not be worked on label Sep 13, 2023
@cesaryuan
Copy link
Author

cesaryuan commented Sep 13, 2023

感觉有点反直觉诶,为什么在同一次hook中args会发生变化呢,那这样的话感觉是一个很严重的问题呀,理论上每一次hook执行不应该是相互隔离的嘛。

执行的时间就会比你靠前

这里没太懂,beforehook不是总在originmethod之前运行,并且beforehook运行完后origin method才会执行嘛

beforehook(param)
orginMethod(param.args)
afterhook(param)

我以为的大概是这个流程,那按理说每一次运行到beforehook时,param.args就应该不会发生改变了才对。现在的情况看起来是不同的method调用之间的param.args之间会互相混在一起?

所以其实对于每一次method调用,共享的都是一个param?所以如果method调用的比较快就会导致后一次调用的param把前一次的覆盖掉?

@fankes
Copy link
Collaborator

fankes commented Sep 13, 2023

你用原生 Xposed API 试一下,如果有这个问题你再告诉我

@cesaryuan
Copy link
Author

你用原生 Xposed API 试一下,如果有这个问题你再告诉我

大佬我试了下原生Xposed,不会出现这个问题,代码如下

package com.example.test_yuki_bug;

import android.util.Log;

import de.robv.android.xposed.IXposedHookLoadPackage;
import de.robv.android.xposed.XC_MethodHook;
import de.robv.android.xposed.XposedHelpers;
import de.robv.android.xposed.callbacks.XC_LoadPackage;

public class MainHook implements IXposedHookLoadPackage {
    @Override
    public void handleLoadPackage(XC_LoadPackage.LoadPackageParam lpparam) throws Throwable {
        if (!lpparam.packageName.equals("com.wibo.bigbang.ocr")) return;
        hook(lpparam);
    }

    private void hook(XC_LoadPackage.LoadPackageParam lpparam) {
        XposedHelpers.findAndHookMethod("okhttp3.Request$Builder", 
                lpparam.classLoader, "addHeader", String.class, String.class, new XC_MethodHook() {
                    @Override
                    protected void beforeHookedMethod(MethodHookParam param) throws InterruptedException {
                        String a1 = (String) param.args[0];
                        Thread.sleep(500);
                        String a2 = (String) param.args[0];
                        Log.d("checkEqual", a1.equals(a2) + " a1: " + a1 + " a2: " + a2);
                    } 
                });
    }
}

image

@fankes
Copy link
Collaborator

fankes commented Sep 14, 2023

那你把复现 apk 和 demo 都发上来,我有空试试

@fankes fankes reopened this Sep 14, 2023
@fankes
Copy link
Collaborator

fankes commented Sep 14, 2023

在注解声明 isUsingResourcesHook = false 再试试,无论如何先试试这个

如果依然不行,参考这个问题 #23,你可以切到这个版本试试可不可以修复,分别测试 1.1.4 和 1.1.5 版本

告诉我每个测试准确的结果。

@cesaryuan
Copy link
Author

分别测试 1.1.4 和 1.1.5 版本

image
在这里修改就可以嘛

@cesaryuan
Copy link
Author

cesaryuan commented Sep 14, 2023

在注解声明 isUsingResourcesHook = false 再试试

已测试,问题存在,且问题症状无明显区别(false出现几率相同)

分别测试 1.1.4 和 1.1.5 版本

已测试,都存在问题,且问题症状无明显区别(false出现几率相同)

你用原生 Xposed API 试一下

已测试,不存在问题,始终为 true

模块demo (Yuki):yuki_bug_test.zip
模块demo (原Xposed):同上(放在一个单独的Module里了)
宿主APK:(安装后需点击同意用户协议后运行到登录界面,但无需登录即可复现bug。如果出现闪退,禁用模块后运行一次宿主APP然后再启用模块即可)

@fankes
Copy link
Collaborator

fankes commented Sep 14, 2023

这次我也无法得知具体是什么原因了,看起来代理会破坏其原始结果,那么就先使用变量把它存起来吧,你大概也不会有短时间内重复调用它的硬性需求。

除了 args,你测试一下其它值会不会改变,比如 result hasThrowable member 等这些 param 里的对象,可以点进去看一下。

另外 args 调用方法由 args().first().string()

换成 args[0] 试试

@fankes
Copy link
Collaborator

fankes commented Sep 14, 2023

API 仅代理了 before 事件,replace 事件是 API 自己对 before 的一个封装,会设置 result = 目标替换值,只有这个事件的行为不同

↑ 不知道是否和这个有关

@fankes
Copy link
Collaborator

fankes commented Sep 14, 2023

我突然想到,现在只有一个控制变量不同了

你在外面套上这个 withProcess(mainProcessName) {
}

@cesaryuan
Copy link
Author

cesaryuan commented Sep 14, 2023

大佬我发现bug在哪了,beforeHookParam 的定义位置好像放错了,导致每次 hookcallback 调用共享了同一个 beforeHookParam 实例。

当被hook的method调用的比较慢时,放在外面没有问题,但是如果被hook的method调用的比较快,就会出现后一次调用的 YukiHookCallback.Param 覆盖掉 beforeHookParam.params 属性的情况。所以问题 #23 也可能是这个原因导致的

因此,把 beforeHookParam 放在 YukiMemberHook.beforeHookedMember 里面就没有问题了

image

@fankes
Copy link
Collaborator

fankes commented Sep 14, 2023

我差点忘了,这个类一直有历史遗留问题,这个位置是故意放外面的,当时这么改的原因就是为了确定唯一的 HookParam 对象,为了使用 extraData,但是没想到遇到了你这个问题。

还有就是因为 HookParam 重复创建会导致 hook ClassLoader 卡死 #14,所以后面就改成这样了,没事,这些东西大部分是我在 coding 很烂的时候写的,新的 API 将会重写这个类 #33,后面先会发小版本 1.2.0 修复一下这个错误和其它问题。

@fankes fankes changed the title loggerD导致args前后两次获取到的值不一样 [BUG] HookParam call timing problem Sep 14, 2023
@fankes fankes added enhancement New feature or request and removed wontfix This will not be worked on labels Sep 14, 2023
@fankes
Copy link
Collaborator

fankes commented Oct 1, 2023

很奇怪,我什么都没改的情况下,用你给的 demo 在我这复现不了问题

image

@cesaryuan
Copy link
Author

cesaryuan commented Oct 1, 2023

很奇怪,我什么都没改的情况下,用你给的 demo 在我这复现不了问题

image

大佬宿主APP运行到哪个页面了。

如何不用这个宿主,也可以自己写个宿主APP开多线程并发调用被hook函数

@fankes
Copy link
Collaborator

fankes commented Oct 7, 2023

1.2.0 版本已发布,该问题已被修复。

@fankes fankes closed this as completed Oct 7, 2023
@fankes fankes added bug Something isn't working fixed This bug was fixed and removed enhancement New feature or request labels Oct 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working fixed This bug was fixed
Projects
None yet
Development

No branches or pull requests

2 participants