Skip to content

[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.

Sign up for GitHub

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

Merged
auto-submit merged 12 commits into from
Mar 31, 2025
Merged

[webview_flutter_wkwebview] Fixes crash when sending undefined message via javascript channel #8776

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
6abb674
Fixes crash when sending undefined message via javascript channel
LinXunFeng Mar 4, 2025
2de63f3
Merge branch 'main' into fix_crash_when_send_undefined
LinXunFeng Mar 4, 2025
0905d5e
Merge branch 'main' of github.com:flutter/packages into fix_crash_whe…
bparrishMines Mar 21, 2025
153a85c
javascript spelling
bparrishMines Mar 21, 2025
1362056
Preserve undefined and null output
LinXunFeng Mar 24, 2025
5c7801b
Code formatting
LinXunFeng Mar 24, 2025
83b1aed
update
LinXunFeng Mar 24, 2025
feb84e2
Revert "update"
LinXunFeng Mar 24, 2025
77bec50
Revert "Code formatting"
LinXunFeng Mar 24, 2025
db6b43a
Revert "Preserve undefined and null output"
LinXunFeng Mar 24, 2025
ef6ccae
Return '(null)' when message.body is null
LinXunFeng Mar 25, 2025
4bde9bc
Remove unnecessary symbols
LinXunFeng Mar 26, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/webview_flutter/webview_flutter_wkwebview/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 3.18.5

* Fixes crash when sending undefined message via JavaScript channel.

## 3.18.4

* Fixes crash when native `WKFrameInfo.request` is nil.
Expand Down
41 changes: 41 additions & 0 deletions ...view_flutter/webview_flutter_wkwebview/example/integration_test/webview_flutter_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,47 @@ Future<void> main() async {
await expectLater(channelCompleter.future, completion('hello'));
});

testWidgets('JavaScriptChannel can receive undefined',
(WidgetTester tester) async {
final Completer<void> pageFinished = Completer<void>();
final PlatformWebViewController controller = PlatformWebViewController(
const PlatformWebViewControllerCreationParams(),
);
unawaited(controller.setJavaScriptMode(JavaScriptMode.unrestricted));
final PlatformNavigationDelegate delegate = PlatformNavigationDelegate(
const PlatformNavigationDelegateCreationParams(),
);
unawaited(delegate.setOnPageFinished((_) => pageFinished.complete()));
unawaited(controller.setPlatformNavigationDelegate(delegate));

final Completer<String> channelCompleter = Completer<String>();
await controller.addJavaScriptChannel(
JavaScriptChannelParams(
name: 'Channel',
onMessageReceived: (JavaScriptMessage message) {
channelCompleter.complete(message.message);
},
),
);

await controller.loadHtmlString(
'data:text/html;charset=utf-8;base64,PCFET0NUWVBFIGh0bWw+',
);

await tester.pumpWidget(Builder(
builder: (BuildContext context) {
return PlatformWebViewWidget(
PlatformWebViewWidgetCreationParams(controller: controller),
).build(context);
},
));

await pageFinished.future;

await controller.runJavaScript('Channel.postMessage(undefined);');
await expectLater(channelCompleter.future, completion(''));
});

testWidgets('resize webview', (WidgetTester tester) async {
final Completer<void> buttonTapResizeCompleter = Completer<void>();
final Completer<void> onPageFinished = Completer<void>();
Expand Down
2 changes: 1 addition & 1 deletion packages/webview_flutter/webview_flutter_wkwebview/lib/src/common/weak_reference_utils.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
/// ) {
/// weakReference.target?.onJavascriptChannelMessage(
/// message.name,
/// message.body!.toString(),
/// message.body?.toString() ?? '',
Copy link
Collaborator

@stuartmorgan-g stuartmorgan-g Mar 25, 2025

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 .toString can be called on null; the ?? '' part can never execute. You could just remove the ! from the previous code.

Copy link
Member Author

@LinXunFeng LinXunFeng Mar 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

/// );
/// };
/// },
Expand Down
2 changes: 1 addition & 1 deletion packages/webview_flutter/webview_flutter_wkwebview/lib/src/webkit_webview_controller.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -906,7 +906,7 @@ class WebKitJavaScriptChannelParams extends JavaScriptChannelParams {
return (_, __, WKScriptMessage message) {
if (weakReference.target != null) {
weakReference.target!(
JavaScriptMessage(message: message.body!.toString()),
JavaScriptMessage(message: message.body?.toString() ?? ''),
Copy link
Collaborator

@stuartmorgan-g stuartmorgan-g Mar 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How was '' chosen as the value here? Is this what the iOS implementation returned prior to the proxy API migration? Is it what webview_android returns? Other?

The linked issue notably says that it expects a different value (the string undefined), so I'd like to make sure we're picking '' for a clear reason.

Copy link
Contributor

@bparrishMines bparrishMines Mar 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From my testing, passing undefined to the JavaScriptChannel leads to a null value in this callback. So in practice, undefined seems indistinguishable from a null value. However, I could see a rational for passing the string 'null' instead. I will check what happens on Android.

Copy link
Contributor

@bparrishMines bparrishMines Mar 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like Android returns the string 'undefined' and crashes when passing null.

Copy link
Member Author

@LinXunFeng LinXunFeng Mar 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since I can't get the undefined passed through postMessage in the userContentController method, I've now overridden the postMessage method to preserve the undefined and null output.

image

Since I am not familiar with Android, I did not handle the crash issue when passing null. This may require another PR to handle.

Copy link
Collaborator

@stuartmorgan-g stuartmorgan-g Mar 24, 2025

Choose a reason for hiding this comment

The 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.

Copy link
Member Author

@LinXunFeng LinXunFeng Mar 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used webview_flutter: 2.8.0 and the value of message.message is "(null)", as shown in the screenshot of the code below.

image image

Copy link
Member Author

@LinXunFeng LinXunFeng Mar 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The message.body was handled on the OC side in the older version of the iOS plugin, But now the it is passed directly to the Dart side, maybe we can use "${message.body}" to handle it like OC, what do you all think?

image

Copy link
Contributor

@bparrishMines bparrishMines Mar 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it makes sense to return the string null when WKScriptMessage.body == null. I think this is better because a user could still differentiate between receiving a null or an empty string the majority of the time.

I think the main problem is that JavaScriptMessage.message is non-null in the platform interface, but changing this is a breaking change.

Your thoughts @stuartmorgan?

Copy link
Collaborator

@stuartmorgan-g stuartmorgan-g Mar 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the main problem is that JavaScriptMessage.message is non-null in the platform interface, but changing this is a breaking change.

Yes, ideally we should have designed this to be more consistent with runJavaScript, where we distinguish unsupported values (in that case with an error, but we'd need something else here).

Given the constraints we have now, I think I would vote for '(null)' for consistency with previous implementations on iOS.

Copy link
Member Author

@LinXunFeng LinXunFeng Mar 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, now I've adjusted it to return '(null)' when WKScriptMessage.body == null.

);
}
};
Expand Down
2 changes: 1 addition & 1 deletion packages/webview_flutter/webview_flutter_wkwebview/pubspec.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: webview_flutter_wkwebview
description: A Flutter plugin that provides a WebView widget based on Apple's WKWebView control.
repository: https://github.com/flutter/packages/tree/main/packages/webview_flutter/webview_flutter_wkwebview
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+webview%22
version: 3.18.4
version: 3.18.5

environment:
sdk: ^3.5.0
Expand Down