Invalid CSS Selector for Content Blockers invalidates others
authorachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 6 Oct 2015 02:47:38 +0000 (02:47 +0000)
committerachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 6 Oct 2015 02:47:38 +0000 (02:47 +0000)
https://bugs.webkit.org/show_bug.cgi?id=148446
rdar://problem/22918235

Reviewed by Benjamin Poulain.

Source/WebCore:

Test: http/tests/contentextensions/invalid-selector.html

* contentextensions/ContentExtensionParser.cpp:
(WebCore::ContentExtensions::loadTrigger):
(WebCore::ContentExtensions::isValidSelector):
(WebCore::ContentExtensions::loadAction):
(WebCore::ContentExtensions::loadRule):
Add a check to see if a selector is valid before adding it.

LayoutTests:

* http/tests/contentextensions/invalid-selector-expected.txt: Added.
* http/tests/contentextensions/invalid-selector.html: Added.
* http/tests/contentextensions/invalid-selector.html.json: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@190602 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/http/tests/contentextensions/invalid-selector-expected.html [new file with mode: 0644]
LayoutTests/http/tests/contentextensions/invalid-selector.html [new file with mode: 0644]
LayoutTests/http/tests/contentextensions/invalid-selector.html.json [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/contentextensions/ContentExtensionParser.cpp

index 02cc9a2c4641b2447fbb4e29746b744e6ce86be3..18b6b508d95434013016b0b94d27cb6cf15f7e66 100644 (file)
@@ -1,3 +1,15 @@
+2015-10-05  Alex Christensen  <achristensen@webkit.org>
+
+        Invalid CSS Selector for Content Blockers invalidates others
+        https://bugs.webkit.org/show_bug.cgi?id=148446
+        rdar://problem/22918235
+
+        Reviewed by Benjamin Poulain.
+
+        * http/tests/contentextensions/invalid-selector-expected.txt: Added.
+        * http/tests/contentextensions/invalid-selector.html: Added.
+        * http/tests/contentextensions/invalid-selector.html.json: Added.
+
 2015-10-05  Jiewen Tan  <jiewen_tan@apple.com>
 
         CSSGradientValue should check whether gradientLength is zero or not.
diff --git a/LayoutTests/http/tests/contentextensions/invalid-selector-expected.html b/LayoutTests/http/tests/contentextensions/invalid-selector-expected.html
new file mode 100644 (file)
index 0000000..b7ef3d5
--- /dev/null
@@ -0,0 +1 @@
+This text should be visible because the class is an invalid selector.
diff --git a/LayoutTests/http/tests/contentextensions/invalid-selector.html b/LayoutTests/http/tests/contentextensions/invalid-selector.html
new file mode 100644 (file)
index 0000000..bc1a308
--- /dev/null
@@ -0,0 +1,5 @@
+<body>
+<p class="valid-selector">This text should not be visible.</p>
+<p class="non-universal-valid-selector">This text should not be visible.</p>
+<p class="body{">This text should be visible because the class is an invalid selector.</p>
+</body>
diff --git a/LayoutTests/http/tests/contentextensions/invalid-selector.html.json b/LayoutTests/http/tests/contentextensions/invalid-selector.html.json
new file mode 100644 (file)
index 0000000..9169c1c
--- /dev/null
@@ -0,0 +1,41 @@
+[{
+  "action" : {
+    "type" : "css-display-none",
+    "selector" : "body{"
+  },
+  "trigger" : {
+    "url-filter" : ".*"
+  }
+},{
+  "action" : {
+    "type" : "css-display-none",
+    "selector" : ".valid-selector"
+  },
+  "trigger" : {
+    "url-filter" : ".*"
+  }
+},{
+  "action" : {
+    "type" : "css-display-none",
+    "selector" : ".body{"
+  },
+  "trigger" : {
+    "url-filter" : ".*"
+  }
+},{
+  "action":{
+        "type":"css-display-none",
+        "selector":"body{background-image: url(http://127.0.0.1:8000/resources/square100.png)}"
+  },
+  "trigger":{
+        "url-filter":"html"
+  }
+},{
+  "action" : {
+    "type" : "css-display-none",
+    "selector" : ".non-universal-valid-selector"
+  },
+  "trigger" : {
+    "url-filter" : "invalid-selector\\.html"
+  }
+}]
index bee1e084daacd78731fef28d682f255ee65d01e7..a1e6e4ae7f42eb79654a160a17a3a63da2837b7e 100644 (file)
@@ -1,3 +1,20 @@
+2015-10-05  Alex Christensen  <achristensen@webkit.org>
+
+        Invalid CSS Selector for Content Blockers invalidates others
+        https://bugs.webkit.org/show_bug.cgi?id=148446
+        rdar://problem/22918235
+
+        Reviewed by Benjamin Poulain.
+
+        Test: http/tests/contentextensions/invalid-selector.html
+
+        * contentextensions/ContentExtensionParser.cpp:
+        (WebCore::ContentExtensions::loadTrigger):
+        (WebCore::ContentExtensions::isValidSelector):
+        (WebCore::ContentExtensions::loadAction):
+        (WebCore::ContentExtensions::loadRule):
+        Add a check to see if a selector is valid before adding it.
+
 2015-10-05  Jiewen Tan  <jiewen_tan@apple.com>
 
         CSSGradientValue should check whether gradientLength is zero or not.
index 5063bfbc06c4bd6c600198654c93012ab3b24540..63d4ee5e71be122340ca65c6692f40a0bbb7a47f 100644 (file)
@@ -28,6 +28,9 @@
 
 #if ENABLE(CONTENT_EXTENSIONS)
 
+#include "CSSParser.h"
+#include "CSSParserMode.h"
+#include "CSSSelectorList.h"
 #include "ContentExtensionError.h"
 #include "ContentExtensionRule.h"
 #include "ContentExtensionsBackend.h"
@@ -173,8 +176,18 @@ static std::error_code loadTrigger(ExecState& exec, const JSObject& ruleObject,
     return { };
 }
 
-static std::error_code loadAction(ExecState& exec, const JSObject& ruleObject, Action& action)
+static bool isValidSelector(const String& selector)
 {
+    CSSParserContext context(CSSQuirksMode);
+    CSSParser parser(context);
+    CSSSelectorList selectorList;
+    parser.parseSelector(selector, selectorList);
+    return selectorList.isValid();
+}
+
+static std::error_code loadAction(ExecState& exec, const JSObject& ruleObject, Action& action, bool& validSelector)
+{
+    validSelector = true;
     const JSValue actionObject = ruleObject.get(&exec, Identifier::fromString(&exec, "action"));
     if (!actionObject || exec.hadException() || !actionObject.isObject())
         return ContentExtensionError::JSONInvalidAction;
@@ -196,7 +209,13 @@ static std::error_code loadAction(ExecState& exec, const JSObject& ruleObject, A
         if (!selector || exec.hadException() || !selector.isString())
             return ContentExtensionError::JSONInvalidCSSDisplayNoneActionType;
 
-        action = Action(ActionType::CSSDisplayNoneSelector, selector.toWTFString(&exec));
+        String s = selector.toWTFString(&exec);
+        if (!isValidSelector(s)) {
+            // Skip rules with invalid selectors to be backwards-compatible.
+            validSelector = false;
+            return { };
+        }
+        action = Action(ActionType::CSSDisplayNoneSelector, s);
     } else
         return ContentExtensionError::JSONInvalidActionType;
 
@@ -211,11 +230,13 @@ static std::error_code loadRule(ExecState& exec, const JSObject& ruleObject, Vec
         return triggerError;
 
     Action action;
-    auto actionError = loadAction(exec, ruleObject, action);
+    bool validSelector;
+    auto actionError = loadAction(exec, ruleObject, action, validSelector);
     if (actionError)
         return actionError;
 
-    ruleList.append(ContentExtensionRule(trigger, action));
+    if (validSelector)
+        ruleList.append(ContentExtensionRule(trigger, action));
     return { };
 }