REGRESSION: Content Blocker: Blocking "a[href*=randomString]" doesn't work
authorachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 24 Mar 2017 18:17:48 +0000 (18:17 +0000)
committerachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 24 Mar 2017 18:17:48 +0000 (18:17 +0000)
https://bugs.webkit.org/show_bug.cgi?id=169167

Reviewed by Simon Fraser.

Source/WebCore:

When testing content extensions, we have always called an API function that internally
has called AtomicString::init somewhere before we start compiling the content extension.
On iOS, though, we call [_WKUserContentExtensionStore compileContentExtensionForIdentifier:...]
without having already called anything that calls AtomicString::init.  The new CSS parser is now
failing to parse some selectors because CSSSelectorParser::defaultNamespace is returning starAtom,
which is a null atomic string before AtomicString::init is called.

Covered by a new API test.

* contentextensions/ContentExtensionParser.cpp:
(WebCore::ContentExtensions::isValidCSSSelector):
(WebCore::ContentExtensions::loadAction):
(WebCore::ContentExtensions::isValidSelector): Deleted.
* contentextensions/ContentExtensionParser.h:
Call AtomicString::init before checking if a css selector is valid.

Tools:

* TestWebKitAPI/Tests/WebCore/ContentExtensions.cpp:
(TestWebKitAPI::TEST_F):
Test an example of a selector that was incorrectly determined to be invalid.

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

Source/WebCore/ChangeLog
Source/WebCore/contentextensions/ContentExtensionParser.cpp
Source/WebCore/contentextensions/ContentExtensionParser.h
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebCore/ContentExtensions.cpp

index 6cee3e4..c32d06c 100644 (file)
@@ -1,3 +1,26 @@
+2017-03-24  Alex Christensen  <achristensen@webkit.org>
+
+        REGRESSION: Content Blocker: Blocking "a[href*=randomString]" doesn't work
+        https://bugs.webkit.org/show_bug.cgi?id=169167
+
+        Reviewed by Simon Fraser.
+
+        When testing content extensions, we have always called an API function that internally
+        has called AtomicString::init somewhere before we start compiling the content extension.
+        On iOS, though, we call [_WKUserContentExtensionStore compileContentExtensionForIdentifier:...]
+        without having already called anything that calls AtomicString::init.  The new CSS parser is now
+        failing to parse some selectors because CSSSelectorParser::defaultNamespace is returning starAtom,
+        which is a null atomic string before AtomicString::init is called.
+
+        Covered by a new API test.
+
+        * contentextensions/ContentExtensionParser.cpp:
+        (WebCore::ContentExtensions::isValidCSSSelector):
+        (WebCore::ContentExtensions::loadAction):
+        (WebCore::ContentExtensions::isValidSelector): Deleted.
+        * contentextensions/ContentExtensionParser.h:
+        Call AtomicString::init before checking if a css selector is valid.
+
 2017-03-24  Youenn Fablet  <youenn@apple.com>
 
         Add libwebrtc backend support for RTCRtpSender::replaceTrack
index c0b0b21..1f78236 100644 (file)
@@ -231,8 +231,9 @@ static Expected<Trigger, std::error_code> loadTrigger(ExecState& exec, const JSO
     return WTFMove(trigger);
 }
 
-static bool isValidSelector(const String& selector)
+bool isValidCSSSelector(const String& selector)
 {
+    AtomicString::init();
     CSSParserContext context(HTMLQuirksMode);
     CSSParser parser(context);
     CSSSelectorList selectorList;
@@ -267,7 +268,7 @@ static Expected<std::optional<Action>, std::error_code> loadAction(ExecState& ex
             return makeUnexpected(ContentExtensionError::JSONInvalidCSSDisplayNoneActionType);
 
         String selectorString = asString(selector)->value(&exec);
-        if (!isValidSelector(selectorString)) {
+        if (!isValidCSSSelector(selectorString)) {
             // Skip rules with invalid selectors to be backwards-compatible.
             return {std::nullopt};
         }
index 35232e8..dc3809d 100644 (file)
@@ -39,6 +39,7 @@ namespace ContentExtensions {
 class ContentExtensionRule;
 
 Expected<Vector<ContentExtensionRule>, std::error_code> parseRuleList(String&&);
+WEBCORE_EXPORT bool isValidCSSSelector(const String&);
 
 } // namespace ContentExtensions
 } // namespace WebCore
index 215ce8d..b69dea9 100644 (file)
@@ -1,3 +1,14 @@
+2017-03-24  Alex Christensen  <achristensen@webkit.org>
+
+        REGRESSION: Content Blocker: Blocking "a[href*=randomString]" doesn't work
+        https://bugs.webkit.org/show_bug.cgi?id=169167
+
+        Reviewed by Simon Fraser.
+
+        * TestWebKitAPI/Tests/WebCore/ContentExtensions.cpp:
+        (TestWebKitAPI::TEST_F):
+        Test an example of a selector that was incorrectly determined to be invalid.
+
 2017-03-24  Jonathan Bedard  <jbedard@apple.com>
 
         Increase timeout for booting simulators.
index 507e82e..2db7731 100644 (file)
@@ -30,6 +30,7 @@
 #include <WebCore/CombinedURLFilters.h>
 #include <WebCore/ContentExtensionCompiler.h>
 #include <WebCore/ContentExtensionError.h>
+#include <WebCore/ContentExtensionParser.h>
 #include <WebCore/ContentExtensionsBackend.h>
 #include <WebCore/DFA.h>
 #include <WebCore/DFABytecodeCompiler.h>
@@ -2870,4 +2871,9 @@ TEST_F(ContentExtensionTest, CombinedQuantifiedOneOrMoreRangesCase11And13InGroup
     testRequest(searchBackend, mainDocumentRequest("zzz://www.ddjjyyy.xxx/"), { ContentExtensions::ActionType::CSSDisplayNoneSelector });
 }
 
+TEST_F(ContentExtensionTest, ValidSelector)
+{
+    EXPECT_TRUE(WebCore::ContentExtensions::isValidCSSSelector("a[href*=hsv]"));
+}
+
 } // namespace TestWebKitAPI