[webview_flutter] Fix platform exception on iOS 14+ when evaluating any JavaScript that evaluates to null or undefined. #4328
Conversation
… evaluating any JavaScript evaluating to null or undefined.
github-actions
bot
added
p: webview_flutter
| }; | ||
| if (@available(iOS 14, *)) { | ||
| jsString = [NSString | ||
| stringWithFormat:@"return eval(\"%@\");", [self escapeStringForJavaScript:jsString]]; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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...
There was a problem hiding this comment.
(make
evaluateJavaScriptbehave likecallAsyncJavaScript), 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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
|
Superseded by #4401 |
Currently, a PlatformException is thrown whenever
evaluateJavascriptis run for any JavaScript that evaluates tonullorundefined.This is due to (I assume to be) a regression in webkit's evaluateJavascript function, which no longer supports
nullorundefinedfor the given JavaScript to evaluate to, as of iOS 14. (Still tested working on iOS 11.4 and 12.4)This PR attempts to fix this problem by instead evaluating JavaScript using callAsyncJavaScript on iOS 14+.
Relevant issue:
Pre-launch Checklist
dart format.)[shared_preferences]"null"for bothnullandundefinedto match the Android side's behavior, rather than"(null)"fornulland"<null>"forundefined. I am not entirely certain if this should be considered a breaking change.///).