Skip to content

GitLab

T
tangled
Commit 48ff1397 authored by Shimi Zhang's avatar Shimi Zhang Committed by Commit Bot
Browse files
Options

Revert "[JJI] Add validation for jsObjectName" 

This reverts commit a72a5f6b.

Reason for revert: After a discussion with Torne and Changwan, we feel it is less meaningful to put a restriction there. Hence the revert.

Original change's description:
> [JJI] Add validation for jsObjectName
> 
> Since we are not using jsObjectName until the next navigation, it will
> only until then we could tell if the parameter is valid or not, if it
> is not valid, the injection will fail sliently. We want to provide a
> early exception for developer to know if the object name is valid or
> not.
> 
> Since it is hard for us to valid the full set of the allowed JavaScript
> variable name, we limited the scope to a manageable subset of it.
> 
> Bug: 918065
> Change-Id: I74909768fc9695e6d33302d880eb211a4b250c62
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1894181
> Commit-Queue: Shimi Zhang <ctzsm@chromium.org>
> Reviewed-by: Changwan Ryu <changwan@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#711795}

TBR=torne@chromium.org,changwan@chromium.org,ctzsm@chromium.org

Change-Id: Ic86d9b1681f7c806e085679b75cb713cc3b6a5c6
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 918065
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1896048Reviewed-by: default avatarShimi Zhang <ctzsm@chromium.org>
Commit-Queue: Shimi Zhang <ctzsm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#711875}
parent c61f25a1
Hide whitespace changes
Inline Side-by-side
Showing with 7 additions and 39 deletions
android_webview/java/src/org/chromium/android_webview/AwContents.java
View file @ 48ff1397
...@@ -170,9 +170,6 @@ public class AwContents implements SmartClipProvider { ...@@ -170,9 +170,6 @@ public class AwContents implements SmartClipProvider {
private static final Pattern sDataURLWithSelectorPattern = private static final Pattern sDataURLWithSelectorPattern =
Pattern.compile("^[^#]*(#[A-Za-z][A-Za-z0-9\\-_:.]*)$"); Pattern.compile("^[^#]*(#[A-Za-z][A-Za-z0-9\\-_:.]*)$");
// Subset of valid JavaScript variable names.
private static final Pattern sJsObjectNamePattern = Pattern.compile("^[\\$_][\\$A-Za-z0-9_]*$");
private static class ForceAuxiliaryBitmapRendering { private static class ForceAuxiliaryBitmapRendering {
private static final boolean sResult = lazyCheck(); private static final boolean sResult = lazyCheck();
private static boolean lazyCheck() { private static boolean lazyCheck() {
...@@ -2493,11 +2490,6 @@ public class AwContents implements SmartClipProvider { ...@@ -2493,11 +2490,6 @@ public class AwContents implements SmartClipProvider {
throw new IllegalArgumentException("jsObjectName shouldn't be null or empty string"); throw new IllegalArgumentException("jsObjectName shouldn't be null or empty string");
} }
if (!validateJsObjectName(jsObjectName)) {
throw new IllegalArgumentException("Invalid jsObjectName: " + jsObjectName
+ ", see our doc of addWebMessageListener for the format of the parameter.");
}
for (int i = 0; i < allowedOriginRules.length; ++i) { for (int i = 0; i < allowedOriginRules.length; ++i) {
if (TextUtils.isEmpty(allowedOriginRules[i])) { if (TextUtils.isEmpty(allowedOriginRules[i])) {
throw new IllegalArgumentException( throw new IllegalArgumentException(
...@@ -2527,14 +2519,6 @@ public class AwContents implements SmartClipProvider { ...@@ -2527,14 +2519,6 @@ public class AwContents implements SmartClipProvider {
mNativeAwContents, AwContents.this, jsObjectName); mNativeAwContents, AwContents.this, jsObjectName);
} }
// To validate the full set of JavaScript variable names requires a lot of maintenance effort,
// so we only allow a subset of it. The variable name starts with '$' or '_' with unlimited
// number of '$' or word characters follow.
@VisibleForTesting
public static boolean validateJsObjectName(String jsObjectName) {
return sJsObjectNamePattern.matcher(jsObjectName).matches();
}
/** /**
* @see android.webkit.WebView#getScale() * @see android.webkit.WebView#getScale()
* *
......
android_webview/javatests/src/org/chromium/android_webview/test/JsJavaInteractionTest.java
View file @ 48ff1397
...@@ -48,8 +48,8 @@ public class JsJavaInteractionTest { ...@@ -48,8 +48,8 @@ public class JsJavaInteractionTest {
private static final String HELLO = "Hello"; private static final String HELLO = "Hello";
private static final String NEW_TITLE = "new_title"; private static final String NEW_TITLE = "new_title";
private static final String JS_OBJECT_NAME = "_myObject"; private static final String JS_OBJECT_NAME = "myObject";
private static final String JS_OBJECT_NAME_2 = "$myObject2"; private static final String JS_OBJECT_NAME_2 = "myObject2";
private static final String DATA_HTML = "<html><body>data</body></html>"; private static final String DATA_HTML = "<html><body>data</body></html>";
private static final int MESSAGE_COUNT = 10000; private static final int MESSAGE_COUNT = 10000;
...@@ -523,22 +523,6 @@ public class JsJavaInteractionTest { ...@@ -523,22 +523,6 @@ public class JsJavaInteractionTest {
} }
} }
@Test
@SmallTest
@Feature({"AndroidWebView", "JsJavaInterfaction"})
public void testValidateJsObjectName() {
Assert.assertFalse(AwContents.validateJsObjectName("myObject"));
Assert.assertFalse(AwContents.validateJsObjectName("."));
Assert.assertFalse(AwContents.validateJsObjectName("1"));
Assert.assertFalse(AwContents.validateJsObjectName("$_ಠ_ಠ"));
Assert.assertFalse(AwContents.validateJsObjectName("$你好"));
Assert.assertTrue(AwContents.validateJsObjectName("_myObject123_"));
Assert.assertTrue(AwContents.validateJsObjectName("$myObject"));
Assert.assertTrue(AwContents.validateJsObjectName("_"));
Assert.assertTrue(AwContents.validateJsObjectName("$"));
}
@Test @Test
@MediumTest @MediumTest
@Feature({"AndroidWebView", "JsJavaInterfaction"}) @Feature({"AndroidWebView", "JsJavaInterfaction"})
......
android_webview/test/data/post_message_receives_reply.html
View file @ 48ff1397
...@@ -2,10 +2,10 @@ ...@@ -2,10 +2,10 @@
<head> <head>
<title>reply</title> <title>reply</title>
<script> <script>
_myObject.onmessage = function (event) { myObject.onmessage = function (event) {
document.title = event.data; document.title = event.data;
}; };
_myObject.postMessage("Hello"); myObject.postMessage("Hello");
</script> </script>
</head> </head>
<body> <body>
......
android_webview/test/data/post_message_repeat.html
View file @ 48ff1397
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
<head> <head>
<script> <script>
for (let i = 0; i < 10000; ++i) for (let i = 0; i < 10000; ++i)
_myObject.postMessage("Hello:" + i); myObject.postMessage("Hello:" + i);
</script> </script>
</head> </head>
<body> <body>
......
android_webview/test/data/post_message_simple.html
View file @ 48ff1397
<html> <html>
<head> <head>
<script> <script>
_myObject.postMessage("Hello"); myObject.postMessage("Hello");
</script> </script>
</head> </head>
<body> <body>
......
android_webview/test/data/post_message_with_ports.html
View file @ 48ff1397
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
<head> <head>
<script> <script>
let channel = new MessageChannel(); let channel = new MessageChannel();
_myObject.postMessage("Hello", [channel.port1]); myObject.postMessage("Hello", [channel.port1]);
channel.port2.onmessage = function(e) { channel.port2.onmessage = function(e) {
document.title = e.data; document.title = e.data;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment