Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

[webview_flutter] Fix platform exception on iOS 14+ when evaluating any JavaScript that evaluates to null or undefined. #4328

Closed
BeMacized wants to merge 5 commits into from
Closed

[webview_flutter] Fix platform exception on iOS 14+ when evaluating any JavaScript that evaluates to null or undefined. #4328

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
42bf7f0
[webview_flutter] Fix platform exception being thrown on iOS 14+ when…
BeMacized Sep 9, 2021
22428f1
Merge branch 'master' into issue/66318
BeMacized Sep 9, 2021
da7d890
Merge remote-tracking branch 'origin/master' into issue/66318
BeMacized Sep 10, 2021
2219761
Bring up to date with master 1/2
BeMacized Sep 27, 2021
85400e0
Bring up to date with master 2/2
BeMacized Sep 27, 2021
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/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 2.0.14

* Fixed platform exception being thrown on iOS 14+ when evaluating any JavaScript evaluating to null or undefined.

## 2.0.13

* Send URL of File to download to the NavigationDelegate on Android just like it is already done on iOS.
Expand Down
125 changes: 125 additions & 0 deletions packages/webview_flutter/webview_flutter/example/ios/RunnerTests/FLTWebViewTests.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,4 +88,129 @@ - (void)testContentInsetsSumAlwaysZeroAfterSetFrame {
}
}

- (void)testEvaluateJavaScriptShouldCallCallAsyncJavaScriptIOS14AndAbove {
if (@available(iOS 14, *)) {
// Setup
FLTWebViewController *controller =
[[FLTWebViewController alloc] initWithFrame:CGRectMake(0, 0, 300, 400)
viewIdentifier:1
arguments:nil
binaryMessenger:self.mockBinaryMessenger];
FLTWKWebView *mockView = OCMClassMock(FLTWKWebView.class);
[OCMStub([mockView callAsyncJavaScript:[OCMArg any]
arguments:[OCMArg any]
inFrame:[OCMArg any]
inContentWorld:[OCMArg any]
completionHandler:[OCMArg any]]) andDo:^(NSInvocation *invocation) {
// __unsafe_unretained: https://github.com/erikdoe/ocmock/issues/384#issuecomment-589376668
__unsafe_unretained void (^evalResultHandler)(id, NSError *);
[invocation getArgument:&evalResultHandler atIndex:6];
evalResultHandler(@"RESULT", nil);
}];
controller.webView = mockView;
XCTestExpectation *resultExpectation = [self
expectationWithDescription:@"Should return successful result over the method channel."];

// Run
[controller onMethodCall:[FlutterMethodCall methodCallWithMethodName:@"evaluateJavascript"
arguments:@"console.log('test')"]
result:^(id _Nullable result) {
XCTAssertTrue([@"RESULT" isEqualToString:result]);
[resultExpectation fulfill];
}];

// Verify
OCMVerify([mockView
callAsyncJavaScript:[OCMArg isEqual:@"return eval(\"console.log('test')\");"]
arguments:[OCMArg any]
inFrame:[OCMArg any]
inContentWorld:[OCMArg any]
completionHandler:[OCMArg any]]);
OCMReject([mockView evaluateJavaScript:[OCMArg any] completionHandler:[OCMArg any]]);
[self waitForExpectationsWithTimeout:30.0 handler:nil];
}
}

- (void)testEvaluateJavaScriptShouldCallEvaluateJavaScriptBelowIOS14 {
if (@available(iOS 14, *)) {
} else {
// Setup
FLTWebViewController *controller =
[[FLTWebViewController alloc] initWithFrame:CGRectMake(0, 0, 300, 400)
viewIdentifier:1
arguments:nil
binaryMessenger:self.mockBinaryMessenger];
FLTWKWebView *mockView = OCMClassMock(FLTWKWebView.class);
[OCMStub([mockView evaluateJavaScript:[OCMArg any]
completionHandler:[OCMArg any]]) andDo:^(NSInvocation *invocation) {
// __unsafe_unretained: https://github.com/erikdoe/ocmock/issues/384#issuecomment-589376668
__unsafe_unretained void (^evalResultHandler)(id, NSError *);
[invocation getArgument:&evalResultHandler atIndex:3];
evalResultHandler(@"RESULT", nil);
}];
controller.webView = mockView;
XCTestExpectation *resultExpectation = [self
expectationWithDescription:@"Should return successful result over the method channel."];

// Run
[controller onMethodCall:[FlutterMethodCall methodCallWithMethodName:@"evaluateJavascript"
arguments:@"console.log('test')"]
result:^(id _Nullable result) {
XCTAssertTrue([@"RESULT" isEqualToString:result]);
[resultExpectation fulfill];
}];

// Verify
OCMVerify([mockView evaluateJavaScript:@"console.log('test')" completionHandler:[OCMArg any]]);
[self waitForExpectationsWithTimeout:30.0 handler:nil];
}
}

- (void)testEvaluateJavaScriptShouldSendErrorResultOnError {
// Setup
FLTWebViewController *controller =
[[FLTWebViewController alloc] initWithFrame:CGRectMake(0, 0, 300, 400)
viewIdentifier:1
arguments:nil
binaryMessenger:self.mockBinaryMessenger];
NSError *testError = [NSError errorWithDomain:@""
code:1
userInfo:@{NSLocalizedDescriptionKey : @"Test Error"}];
FLTWKWebView *mockView = OCMClassMock(FLTWKWebView.class);
XCTestExpectation *resultExpectation =
[self expectationWithDescription:@"Should return error result over the method channel."];
if (@available(iOS 14, *)) {
[OCMStub([mockView callAsyncJavaScript:[OCMArg any]
arguments:[OCMArg any]
inFrame:[OCMArg any]
inContentWorld:[OCMArg any]
completionHandler:[OCMArg any]]) andDo:^(NSInvocation *invocation) {
// __unsafe_unretained: https://github.com/erikdoe/ocmock/issues/384#issuecomment-589376668
__unsafe_unretained void (^evalResultHandler)(id, NSError *);
[invocation getArgument:&evalResultHandler atIndex:6];
evalResultHandler(nil, testError);
}];
} else {
[OCMStub([mockView evaluateJavaScript:[OCMArg any]
completionHandler:[OCMArg any]]) andDo:^(NSInvocation *invocation) {
// __unsafe_unretained: https://github.com/erikdoe/ocmock/issues/384#issuecomment-589376668
__unsafe_unretained void (^evalResultHandler)(id, NSError *);
[invocation getArgument:&evalResultHandler atIndex:3];
evalResultHandler(nil, testError);
}];
}
controller.webView = mockView;

// Run
[controller onMethodCall:[FlutterMethodCall methodCallWithMethodName:@"evaluateJavascript"
arguments:@"console.log('test')"]
result:^(id _Nullable result) {
XCTAssertTrue([result class] == [FlutterError class]);
[resultExpectation fulfill];
}];

// Verify
[self waitForExpectationsWithTimeout:30.0 handler:nil];
}

@end
20 changes: 12 additions & 8 deletions packages/webview_flutter/webview_flutter/ios/Classes/FlutterWebView.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,26 +7,30 @@

NS_ASSUME_NONNULL_BEGIN

/**
* The WkWebView used for the plugin.
*
* This class overrides some methods in `WKWebView` to serve the needs for the plugin.
*/
@interface FLTWKWebView : WKWebView
@end

@interface FLTWebViewController : NSObject <FlutterPlatformView, WKUIDelegate>

@property(nonatomic) FLTWKWebView* webView;

- (instancetype)initWithFrame:(CGRect)frame
viewIdentifier:(int64_t)viewId
arguments:(id _Nullable)args
binaryMessenger:(NSObject<FlutterBinaryMessenger>*)messenger;

- (UIView*)view;

- (void)onMethodCall:(FlutterMethodCall*)call result:(FlutterResult)result;
@end

@interface FLTWebViewFactory : NSObject <FlutterPlatformViewFactory>
- (instancetype)initWithMessenger:(NSObject<FlutterBinaryMessenger>*)messenger;
@end

/**
* The WkWebView used for the plugin.
*
* This class overrides some methods in `WKWebView` to serve the needs for the plugin.
*/
@interface FLTWKWebView : WKWebView
@end

NS_ASSUME_NONNULL_END
45 changes: 33 additions & 12 deletions packages/webview_flutter/webview_flutter/ios/Classes/FlutterWebView.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,14 @@ - (void)onCurrentUrl:(FlutterMethodCall*)call result:(FlutterResult)result {
result(_currentUrl);
}

- (NSString*)escapeStringForJavaScript:(NSString*)string {
NSArray* arrayForEncoding = @[ string ];
NSString* jsonString = [[NSString alloc]
initWithData:[NSJSONSerialization dataWithJSONObject:arrayForEncoding options:0 error:nil]
encoding:NSUTF8StringEncoding];
return [jsonString substringWithRange:NSMakeRange(2, jsonString.length - 4)];
}

- (void)onEvaluateJavaScript:(FlutterMethodCall*)call result:(FlutterResult)result {
NSString* jsString = [call arguments];
if (!jsString) {
Expand All @@ -230,18 +238,31 @@ - (void)onEvaluateJavaScript:(FlutterMethodCall*)call result:(FlutterResult)resu
details:nil]);
return;
}
[_webView evaluateJavaScript:jsString
completionHandler:^(_Nullable id evaluateResult, NSError* _Nullable error) {
if (error) {
result([FlutterError
errorWithCode:@"evaluateJavaScript_failed"
message:@"Failed evaluating JavaScript"
details:[NSString stringWithFormat:@"JavaScript string was: '%@'\n%@",
jsString, error]]);
} else {
result([NSString stringWithFormat:@"%@", evaluateResult]);
}
}];
void (^evalResultHandler)(id, NSError*) = ^(_Nullable id evaluateResult,
NSError* _Nullable error) {
if (error) {
result([FlutterError
errorWithCode:@"evaluateJavaScript_failed"
message:@"Failed evaluating JavaScript"
details:[NSString
stringWithFormat:@"JavaScript string was: '%@'\n%@", jsString, error]]);
} else if (evaluateResult == /*null*/ [NSNull null] || evaluateResult == /*undefined*/ nil) {
result(@"null");
} else {
result([NSString stringWithFormat:@"%@", evaluateResult]);
}
};
if (@available(iOS 14, *)) {
jsString = [NSString
stringWithFormat:@"return eval(\"%@\");", [self escapeStringForJavaScript:jsString]];
Copy link
Copy Markdown
Contributor

@stuartmorgan-g stuartmorgan-g Sep 16, 2021

Choose a reason for hiding this comment

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

What's the reason for wrapping this in an eval? That seems like something that could cause side-effects. E.g., I believe CSP could completely break this.

Copy link
Copy Markdown
Contributor Author

@BeMacized BeMacized Sep 27, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

The reason for this is that unlike evaluateJavascript, callAsyncJavaScript executes the given string as the body of an async function, rather than directly evaluating it. As such, it will only return any value that is explicitly returned, not the result of the string being evaluated. I use eval here to make sure the behavior is identical to what evaluateJavascript does on iOS 13 and below and avoid a breaking change.

Personally I have not been able to find any edge cases or side effects, but I've also not tried anything to do with CSP. If you have any examples of this maybe we can find a way around it?

The only other way I can think of to avoid these side effects, assuming they're a thing, is to fix this the other way around:(make evaluateJavaScript behave like callAsyncJavaScript), forcing everyone to explicitly return a value in their JS strings, and consider it a breaking change.

Please let me know what you think. In the mean time I'll update this PR to move the fix to the separated iOS implementation.

Copy link
Copy Markdown
Contributor

@stuartmorgan-g stuartmorgan-g Sep 27, 2021

Choose a reason for hiding this comment

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

I've also not tried anything to do with CSP. If you have any examples of this

I don't have significant experience with CSP, but my expectation is that if you tried to run this on a site with CSP enabled and without unsafe-eval set, it would completely fail.

maybe we can find a way around it?

That would be worrying, since it would mean you we bypassing the browser's enforcement of the site's security policy...

Copy link
Copy Markdown
Contributor

@stuartmorgan-g stuartmorgan-g Sep 27, 2021

Choose a reason for hiding this comment

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

(make evaluateJavaScript behave like callAsyncJavaScript), forcing everyone to explicitly return a value in their JS strings, and consider it a breaking change.

How would that be better than the current behavior?

Copy link
Copy Markdown
Contributor Author

@BeMacized BeMacized Sep 27, 2021

Choose a reason for hiding this comment

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

I don't have significant experience with CSP, but my expectation is that if you tried to run this on a site with CSP enabled and without unsafe-eval set, it would completely fail.

Alright, I'll see if I can give this a test tomorrow.

How would that be better than the current behavior?

Assuming with "current behavior" you mean this branch rather than master: It would no longer require eval to be used, at the expense of introducing a breaking change.

Copy link
Copy Markdown
Contributor

@stuartmorgan-g stuartmorgan-g Sep 27, 2021

Choose a reason for hiding this comment

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

I mean master; isn't the issue you are trying to fix that a void return causes an error? Can't people already avoid that error right now by explicitly returning a value?

Copy link
Copy Markdown
Contributor Author

@BeMacized BeMacized Sep 27, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

On master the platform exception can indeed be avoided by ensuring the given string evaluates to a non-null/non-undefined value.

E.g:
Snackbar.postMessage("...") Throws on iOS 14+.
Snackbar.postMessage("...");1 Does not.

[_webView callAsyncJavaScript:jsString
arguments:nil
inFrame:nil
inContentWorld:WKContentWorld.pageWorld
completionHandler:evalResultHandler];
} else {
[_webView evaluateJavaScript:jsString completionHandler:evalResultHandler];
}
}

- (void)onAddJavaScriptChannels:(FlutterMethodCall*)call result:(FlutterResult)result {
Expand Down
3 changes: 0 additions & 3 deletions packages/webview_flutter/webview_flutter/lib/webview_flutter.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -762,9 +762,6 @@ class WebViewController {
return Future<String>.error(FlutterError(
'JavaScript mode must be enabled/unrestricted when calling evaluateJavascript.'));
}
// TODO(amirh): remove this on when the invokeMethod update makes it to stable Flutter.
// https://github.com/flutter/flutter/issues/26431
// ignore: strong_mode_implicit_dynamic_method
return _webViewPlatformController.evaluateJavascript(javascriptString);
}

Expand Down