Skip to content
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

Treat result of binding function as JSON, not JS #1002

Merged
merged 7 commits into from
Sep 16, 2023

Conversation

SteffenL
Copy link
Collaborator

@SteffenL SteffenL commented Aug 23, 2023

While the documentation of webview_return() states that the result must be valid JSON, it doesn't validate or parse the result as JSON, instead treats it as JavaScript code and evaluates it.

Not only can it be confusing to someone who expects the function to treat the result as JSON, it's also a potential security risk because it can allow loading of arbitrary JS code.

This work implements JSON string escaping and calls JSON.parse() on the JS side without evaluating the result as JS code.

Tests broke because of binding functions returning JS code that were expected to be evaluated, and has been adjusted to call webview::eval() instead.

If the string returned from a binding function is empty then the result on the JS side will be the primitive value undefined to retain the existing behavior.

If the string returned from a binding function can't be parsed by JSON.parse() then the promise on the JS side is rejected.

In any other case the promise on the JS side resolves with the parsed JSON value if the status value is 0; otherwise the promise rejects.

While the documentation of webview_return() states that the result must
be valid JSON, it doesn't validate or parse the result as JSON, instead
treats it as JavaScript code and evaluates it.

Not only can it be confusing to someone who expects the function to
treat the result as JSON, it's also a potential security risk because
it can allow loading of arbitrary JS code.

This work implements JSON string escaping and calls JSON.parse() on
the JS side without evaluating the result as JS code.

Tests broke because of binding functions returning JS code that were
expected to be evaluated, and has been adjusted to call webview::eval()
instead.

If the string returned from a binding function is empty then the
result on the JS side will be the primitive value "undefined" to
retain the existing behavior.

If the string returned from a binding function can't be parsed by
JSON.parse() then the promise on the JS side is rejected.

In any other case the promise on the JS side resolves with the parsed
JSON value if the status value is 0; otherwise the promise rejects.
@SteffenL SteffenL added the bug Something isn't working label Aug 23, 2023
@SteffenL SteffenL linked an issue Aug 23, 2023 that may be closed by this pull request
@SteffenL SteffenL force-pushed the binding-must-return-json-not-js branch from a954ba4 to 49b1b52 Compare August 23, 2023 16:13
@SteffenL SteffenL added this to the v0.11.0 milestone Sep 6, 2023
@SteffenL SteffenL marked this pull request as ready for review September 16, 2023 14:00
@SteffenL SteffenL merged commit b19ee7c into webview:master Sep 16, 2023
27 checks passed
@SteffenL SteffenL deleted the binding-must-return-json-not-js branch September 16, 2023 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging this pull request may close these issues.

Unexpected behavior for return/resolve—evaluates JavaScript
1 participant