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

用一个更严格的正则表达式来把 .method( 替换为 ._c("method")( #139

Merged
merged 1 commit into from
Nov 12, 2015

Conversation

Arthraim
Copy link
Contributor

我改了什么?

我把 \.\s*(\w+)\s*\( 换成了 (?<!\\)\.\s*(\w+)\s*\(,意思是在匹配 .method( 的同时,不匹配 \.method(

原来有什么问题?

我需要用 JSPatch 来在 webview 中 evaluate 一段 js 代码,但是这段代码符合替换的条件,所以也被替换了,造成了 JSPatch 的脚本本身有语法错误。

(lldb) po _regex.pattern
\.\s*(\w+)\s*\(

(lldb) po script
self.webView().evaluateJavaScript_completionHandler("window.alert('hello')", null)

(lldb) po formatedScript
self.__c("webView")().__c("evaluateJavaScript_completionHandler")("window.__c("alert")('hello')", null)

改动后,1、要求 webview 需要执行的 js 脚本里的 .method 都写成 \.method。2、正则表达式也如这个 PR 这样改过。于是达到了效果:

(lldb) po _regex.pattern
(?<!\\)\.\s*(\w+)\s*\(

(lldb) po script
self.webView().evaluateJavaScript_completionHandler("window\.alert('hello')", null)

(lldb) po formatedScript
self.__c("webView")().__c("evaluateJavaScript_completionHandler")("window\.alert('hello')", null)

为什么这么改?

思考后,我发现要改正这个 bug,有三种办法。

  1. 找到一个正确的正则表达式,只匹配字符串以外的 .method(
    于是学艺不精,没有找到 >.< 情况实在太复杂了,还是希望有人能研究出来。

  2. 在 webview 要执行的 js 代码里做 hack,用别的字符代替 .,运行时再替换回来。
    比如:

    self.webView().evaluateJavaScript_completionHandler("window[¬_¬]alert('hello')".replace("[¬_¬]","."), null)
    

    显然,太 hacky 了,对写 patch 的同学来说真是……

  3. 两边都改。javascript string 里用 \. 代替 . 本身就合法。相对应的,替换的正则表达式的改动也很小,情况变得很简单,也不影响旧版本。新的正则表达式只要确认原来匹配的方法前,没有 \ 出现。

所以希望……

  1. 我的 PR 没有别的 bug。
  2. 有人找到更好的办法 1
  3. merge, merge, merge -___,-

bang590 added a commit that referenced this pull request Nov 12, 2015
用一个更严格的正则表达式来把 `.method(` 替换为 `._c("method")(`
@bang590 bang590 merged commit 9d4aebe into bang590:master Nov 12, 2015
@bang590
Copy link
Owner

bang590 commented Nov 12, 2015

赞~

@Arthraim
Copy link
Contributor Author

@bang590 👍 👍

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.

2 participants