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.

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

Jump to bottom

Properly escape JavaScript code on Android #20366

Closed
wants to merge 1 commit into from
Closed

Properly escape JavaScript code on Android #20366

wants to merge 1 commit into from

Conversation

LinusU
Copy link
Contributor

@LinusU LinusU commented Jul 24, 2018 •
edited
Loading

These changes will fix executing javascript with any special characters, by making use of the evaluateJavascript function on Android 4.4+, and by properly escaping the URI on Android <4.4.

Fixes #19611 • Fixes #20365 • Fixes #9749 • Closes #19655 • Closes #12321

This PR supersedes #19655 by patching the same problem in all the places, and fixing it for Android <4.4 as well.

Test Plan:

The snack from #20365 should now work on Android as it does on iOS. That is, it should show "12" three times.

https://snack.expo.io/rkSxWhNVm

It can also be tested by instantiating a WebView with injectedJavaScript set to document.write('1')\ndocument.write('2') and observe that "12" is written in the WebView.

Release Notes:

[ANDROID] [BUGFIX] [WebView] - Properly escape JavaScript code

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 24, 2018
@LinusU LinusU mentioned this pull request Jul 24, 2018
Closed
@jordoh jordoh mentioned this pull request Jul 24, 2018
Closed
@LinusU LinusU mentioned this pull request Jul 24, 2018
Closed
@LinusU LinusU force-pushed the escape-javascript branch from 9c06232 to 77a229a Compare July 31, 2018 08:59
@LinusU
Copy link
Contributor Author

LinusU commented Jul 31, 2018

Rebased on master to hopefully fix CI failure ☺️

@LinusU LinusU force-pushed the escape-javascript branch from 77a229a to feeab42 Compare August 1, 2018 09:56
@LinusU
Copy link
Contributor Author

LinusU commented Aug 1, 2018

Too long with no output (exceeded 10m0s)

rebasing again to restart the test...

@LinusU LinusU force-pushed the escape-javascript branch from feeab42 to 231be62 Compare August 1, 2018 10:19
LinusU referenced this pull request Aug 1, 2018
@facebook-github-bot
- Invalid use of destructuring
9d5bd50 
Summary: Pull Request resolved: #20178

Differential Revision: D8860733

Pulled By: TheSavior

fbshipit-source-id: ec4aa3050755652106dec9ea245394314c862e97
@LinusU LinusU force-pushed the escape-javascript branch from 231be62 to 826afaa Compare August 1, 2018 10:32
@LinusU
Copy link
Contributor Author

LinusU commented Aug 1, 2018 •
edited
Loading

Seems like master fails CI for code style...

@LinusU LinusU force-pushed the escape-javascript branch from 826afaa to c00bc80 Compare August 1, 2018 10:35
@LinusU
Copy link
Contributor Author

LinusU commented Aug 1, 2018

ping @mdvacca, seems like you have been touching this part of the code before, any chance of getting this merged? ☺️

@borisyankov borisyankov mentioned this pull request Aug 2, 2018
Closed
@LinusU LinusU force-pushed the escape-javascript branch from c00bc80 to 9c95db2 Compare August 6, 2018 11:47
@LinusU
Copy link
Contributor Author

LinusU commented Aug 6, 2018 •
edited
Loading

Rebased on master again but now the test is failing on something that seems related to iOS, and this PR only touches Android 🤔

@facebook-github-bot facebook-github-bot added the Import Started This pull request has been imported. This does not imply the PR has been approved. label Aug 9, 2018
@hramos
Copy link
Contributor

hramos commented Aug 9, 2018

Thanks for the PR!

facebook-github-bot
facebook-github-bot approved these changes Aug 9, 2018
View reviewed changes
Copy link
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

hramos is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@react-native-bot
Copy link
Collaborator

This pull request was closed by @LinusU in e6b305b.

Once this commit is added to a release, you will see the corresponding version tag below the description at e6b305b. If the commit has a single master tag, it is not yet part of a release.

@facebook facebook locked as resolved and limited conversation to collaborators Aug 9, 2018
@react-native-bot react-native-bot added the Merged This PR has been merged. label Aug 9, 2018
kelset pushed a commit that referenced this pull request Aug 13, 2018
@LinusU @kelset
Properly escape JavaScript code on Android (#20366)
e4f6694 
Summary:
These changes will fix executing javascript with any special characters, by making use of the `evaluateJavascript` function on Android 4.4+, and by properly escaping the URI on Android <4.4.

Fixes #19611 • Fixes #20365 • Fixes #9749 • Closes #19655 • Closes #12321

This PR supersedes #19655 by patching the same problem in all the places, and fixing it for Android <4.4 as well.
Pull Request resolved: #20366

Differential Revision: D9242968

Pulled By: hramos

fbshipit-source-id: f2e1abc786ba333dbd8aaa8922e716fd99ec26e0
@hramos hramos removed Import Started This pull request has been imported. This does not imply the PR has been approved. labels Feb 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@facebook-github-bot facebook-github-bot facebook-github-bot approved these changes

Assignees
No one assigned
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Platform: Android Android applications.
Projects
None yet
Milestone
No milestone
4 participants
@LinusU @hramos @react-native-bot @facebook-github-bot