-
Notifications
You must be signed in to change notification settings - Fork 3.8k
[webview_flutter_wkwebview] Fixes crash when sending undefined message via javascript channel #8776
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
Changes from 7 commits
6abb674
2de63f3
0905d5e
153a85c
1362056
5c7801b
83b1aed
feb84e2
77bec50
db6b43a
ef6ccae
4bde9bc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -443,6 +443,7 @@ class WebKitWebViewController extends PlatformWebViewController { | |
| webKitParams.name, | ||
| ), | ||
| ]); | ||
| await _injectPostMessageOverride(webKitParams.name); | ||
| } | ||
|
|
||
| @override | ||
|
|
@@ -768,6 +769,47 @@ window.addEventListener("error", function(e) { | |
| await controller.addUserScript(overrideScript); | ||
| } | ||
|
|
||
| Future<void> _injectPostMessageOverride(String channelName) async { | ||
| final WKUserScript overrideScript = | ||
| _webKitParams.webKitProxy.newWKUserScript( | ||
| source: ''' | ||
| var _flutter_webview_plugin_overrides = _flutter_webview_plugin_overrides || {}; | ||
|
|
||
| _flutter_webview_plugin_overrides.originalPostMessageFunctions = _flutter_webview_plugin_overrides.originalPostMessageFunctions || {}; | ||
|
|
||
| (function() { | ||
| if (window.webkit && window.webkit.messageHandlers) { | ||
| const handlerName = "$channelName"; | ||
| const handler = window.webkit.messageHandlers[handlerName]; | ||
|
|
||
| if (handler && | ||
| typeof handler.postMessage === 'function' && | ||
| !_flutter_webview_plugin_overrides.originalPostMessageFunctions[handlerName]) { | ||
|
|
||
| _flutter_webview_plugin_overrides.originalPostMessageFunctions[handlerName] = handler.postMessage; | ||
|
|
||
| handler.postMessage = function(data) { | ||
| if (data === undefined) { | ||
| return _flutter_webview_plugin_overrides.originalPostMessageFunctions[handlerName].call(this, "undefined"); | ||
| } | ||
| if (data === null) { | ||
| return _flutter_webview_plugin_overrides.originalPostMessageFunctions[handlerName].call(this, "null"); | ||
| } | ||
| return _flutter_webview_plugin_overrides.originalPostMessageFunctions[handlerName].call(this, data); | ||
| }; | ||
| } | ||
| } | ||
| })(); | ||
| ''', | ||
| injectionTime: UserScriptInjectionTime.atDocumentStart, | ||
| isForMainFrameOnly: false, | ||
| ); | ||
|
|
||
| final WKUserContentController controller = | ||
| await _webView.configuration.getUserContentController(); | ||
| await controller.addUserScript(overrideScript); | ||
| } | ||
|
|
||
| // WKWebView does not support removing a single user script, so all user | ||
| // scripts and all message handlers are removed instead. And the JavaScript | ||
| // channels that shouldn't be removed are re-registered. Note that this | ||
|
|
@@ -906,7 +948,7 @@ class WebKitJavaScriptChannelParams extends JavaScriptChannelParams { | |
| return (_, __, WKScriptMessage message) { | ||
| if (weakReference.target != null) { | ||
| weakReference.target!( | ||
| JavaScriptMessage(message: message.body!.toString()), | ||
| JavaScriptMessage(message: message.body?.toString() ?? ''), | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How was The linked issue notably says that it expects a different value (the string
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. From my testing, passing
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like Android returns the string
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since I can't get the
Since I am not familiar with Android, I did not handle the crash issue when passing
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Injecting JS into every single page is problematic; it can lead to a lot of unexpected and hard-to-debug edge cases. It should be a method of absolutely last resort only if there is no other way to accomplish necessary goals. Polyfilling slight differences between iOS and Android behaviors is not sufficiently critical to warrant such an approach.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it makes sense to return the string I think the main problem is that Your thoughts @stuartmorgan?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Yes, ideally we should have designed this to be more consistent with Given the constraints we have now, I think I would vote for
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay, now I've adjusted it to return |
||
| ); | ||
| } | ||
| }; | ||
|
|
||




There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is misleading example code as written, since
.toStringcan be called onnull; the?? ''part can never execute. You could just remove the!from the previous code.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done