ryanmcgrath / cacao

Rust bindings for AppKit (macOS) and UIKit (iOS/tvOS). Experimental, but working!
MIT License
1.8k stars 65 forks source link

Segmentation fault (EXC_BAD_ACCESS) on webview when receiving messages from the webpage. #12

Closed rrebelo62 closed 3 years ago

rrebelo62 commented 3 years ago

Problem:

When a message is sent from the web page to the WKWebView it generates a segmentation fault.

Versions:

Steps to reproduce it:

On the webview_custom_message.rs example do this:

  1. Insert this after line 71, on AppWindow:new() webview_config.add_handler("notify"); This will add an handler for javascript messages.

  2. Change the html code on line 37 into this <body onload="window.webkit.messageHandlers.notify.postMessage('Page loaded');">

This will send a message from the webpage to the handler abore. And will also generate an error.

I created a simple gist containing both the modified example and a Cargo.toml file to build it.

This error only hapens if the message is sent from the javascript page only. There will be no issue if the message is posted from the rust code, like this: webview_config.add_user_script("window.webkit.messageHandlers.notify.postMessage(\"loaded\");", InjectAt::End, true);

On my debuging I found that the problem is generated on the exit of the on_message() function on file webview/class.rs. More specifically, if this line 52 is commented out the problem won't happen: line 52: // let body = NSString::from_retained(msg_send![script_message, body]); line 53: delegate.on_message(name.to_str(), "dummy body value" ); It is during the execution of objc_autoreleasePoolPop->objc_release for the body variable that the problem happens.

ryanmcgrath commented 3 years ago

Probably just need to swap that call to be retained then is all. Give it a shot and feel free to PR it? Good catch!

On Sat, Jul 3, 2021 at 19:17, Rui A. Rebelo @.***> wrote:

Problem:

When a message is sent from the web page to the WKWebView it generates a segmentation fault.

Versions:

  • cacao 0.2.0, feature "Webview"
  • rust 1.51.0
  • OS-X Catalina on a Mac Mini

Steps to reproduce it:

On the webview_custom_message.rs example do this:

-

Insert this after line 71, on AppWindow:new() webview_config.add_handler("notify"); This will add an handler for javascript messages.

-

Change the html code on line 37 into this webview.zip webview.zip webview.zip This will send a message from the webpage to the handler abore. And will also generate an error.

I created a simple gist containing both the modified example and a Cargo.toml file to build it.

This error only hapens if the message is sent from the javascript page only. There will be no issue if the message is posted from the rust code, like this: webview_config.add_user_script("window.webkit.messageHandlers.notify.postMessage("loaded");", InjectAt::End, true);

On my debuging I found that the problem is generated on the exit of the on_message() function on file webview/class.rs. More specifically, if this line 52 is commented out the problem won't happen: line 52: // let body = NSString::from_retained(msg_send![script_message, body]); line 53: delegate.on_message(name.to_str(), "dummy body value" ); It is during the execution of objc_autoreleasePoolPop->objc_release for the body variable that the problem happens.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or unsubscribe.

rrebelo62 commented 3 years ago

Awesome! It seems to be working.

Will do a couple more tests and PR it, if everything goes OK.

Many thanks!

rrebelo62 commented 3 years ago

Yeah, the solution is working nice and doesn't interfere with injected javascript.

I guess I now need writing rights to push my commit for the PR, right?

Also, I understand I should commit it to a branch (e.g. "bug_fixes", "patch",...) never to trunk directly.

ryanmcgrath commented 3 years ago

Nope, just fork the repo to your account, branch and commit, then you should see a pull request option in the browser. Appreciate it!

On Sat, Jul 3, 2021 at 20:38, Rui A. Rebelo @.***> wrote:

Yeah, the solution is working nice and doesn't interfere with injected javascript.

I guess I now need writing rights to push my commit for the PR, right?

Also, I understand I should commit it to a branch (e.g. "bug_fixes", "patch",...) never to trunk directly.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.