[webkit-changes] [186965] trunk

Fri, 17 Jul 2015 14:47:56 -0700

Title: [186965] trunk
Revision
186965
Author
[email protected]
Date
2015-07-17 14:47:05 -0700 (Fri, 17 Jul 2015)

Log Message

[Content Extensions] Term::isUniversalTransition() incorrectly expects the end-of-line assertion in character sets
https://bugs.webkit.org/show_bug.cgi?id=147032

Patch by Benjamin Poulain <[email protected]> on 2015-07-17
Reviewed by Alex Christensen.

Source/WebCore:

* contentextensions/Term.h:
(WebCore::ContentExtensions::Term::isUniversalTransition):
The universal transition is not supposed to account for the end-of-line assertion,
it should be a transition matching any character.

Here, we were counting 128 transitions, the 127 characters plus the
transition on zero we are using for EOL.

The end result is Term::isUniversalTransition() was completely useless.
The only code using it is the pattern simplificaton phase. That part
was not working correclty and was allowing useless ".*" in the patterns.

Tools:

* TestWebKitAPI/Tests/WebCore/ContentExtensions.cpp:
Test that the useless terms are eliminated by counting
the number of NFA nodes generated.

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (186964 => 186965)


--- trunk/Source/WebCore/ChangeLog	2015-07-17 21:16:09 UTC (rev 186964)
+++ trunk/Source/WebCore/ChangeLog	2015-07-17 21:47:05 UTC (rev 186965)
@@ -1,3 +1,22 @@
+2015-07-17  Benjamin Poulain  <[email protected]>
+
+        [Content Extensions] Term::isUniversalTransition() incorrectly expects the end-of-line assertion in character sets
+        https://bugs.webkit.org/show_bug.cgi?id=147032
+
+        Reviewed by Alex Christensen.
+
+        * contentextensions/Term.h:
+        (WebCore::ContentExtensions::Term::isUniversalTransition):
+        The universal transition is not supposed to account for the end-of-line assertion,
+        it should be a transition matching any character.
+
+        Here, we were counting 128 transitions, the 127 characters plus the
+        transition on zero we are using for EOL.
+
+        The end result is Term::isUniversalTransition() was completely useless.
+        The only code using it is the pattern simplificaton phase. That part
+        was not working correclty and was allowing useless ".*" in the patterns.
+
 2015-07-17  Dan Bernstein  <[email protected]>
 
         WebCore part of <rdar://problem/21803781> The external URL policy is not reported correctly in navigation actions that create new windows

Modified: trunk/Source/WebCore/contentextensions/Term.h (186964 => 186965)


--- trunk/Source/WebCore/contentextensions/Term.h	2015-07-17 21:16:09 UTC (rev 186964)
+++ trunk/Source/WebCore/contentextensions/Term.h	2015-07-17 21:47:05 UTC (rev 186965)
@@ -579,7 +579,7 @@
         break;
     case TermType::CharacterSet:
         return (m_atomData.characterSet.inverted() && !m_atomData.characterSet.bitCount())
-            || (!m_atomData.characterSet.inverted() && m_atomData.characterSet.bitCount() == 128);
+            || (!m_atomData.characterSet.inverted() && m_atomData.characterSet.bitCount() == 127 && !m_atomData.characterSet.get(0));
     case TermType::Group:
         return m_atomData.group.terms.size() == 1 && m_atomData.group.terms.first().isUniversalTransition();
     }

Modified: trunk/Tools/ChangeLog (186964 => 186965)


--- trunk/Tools/ChangeLog	2015-07-17 21:16:09 UTC (rev 186964)
+++ trunk/Tools/ChangeLog	2015-07-17 21:47:05 UTC (rev 186965)
@@ -1,3 +1,14 @@
+2015-07-17  Benjamin Poulain  <[email protected]>
+
+        [Content Extensions] Term::isUniversalTransition() incorrectly expects the end-of-line assertion in character sets
+        https://bugs.webkit.org/show_bug.cgi?id=147032
+
+        Reviewed by Alex Christensen.
+
+        * TestWebKitAPI/Tests/WebCore/ContentExtensions.cpp:
+        Test that the useless terms are eliminated by counting
+        the number of NFA nodes generated.
+
 2015-07-17  Dan Bernstein  <[email protected]>
 
         Tests for <rdar://problem/21803781> The external URL policy is not reported correctly in navigation actions that create new windows

Modified: trunk/Tools/TestWebKitAPI/Tests/WebCore/ContentExtensions.cpp (186964 => 186965)


--- trunk/Tools/TestWebKitAPI/Tests/WebCore/ContentExtensions.cpp	2015-07-17 21:16:09 UTC (rev 186964)
+++ trunk/Tools/TestWebKitAPI/Tests/WebCore/ContentExtensions.cpp	2015-07-17 21:47:05 UTC (rev 186965)
@@ -922,6 +922,36 @@
     testRequest(backend, mainDocumentRequest("https://webkit.org/catso"), { });
 }
 
+TEST_F(ContentExtensionTest, UselessTermsMatchingEverythingAreEliminated)
+{
+    ContentExtensions::CombinedURLFilters combinedURLFilters;
+    ContentExtensions::URLFilterParser parser(combinedURLFilters);
+    EXPECT_EQ(ContentExtensions::URLFilterParser::ParseStatus::Ok, parser.addPattern(".*web", false, 0));
+    EXPECT_EQ(ContentExtensions::URLFilterParser::ParseStatus::Ok, parser.addPattern("(.*)web", false, 0));
+    EXPECT_EQ(ContentExtensions::URLFilterParser::ParseStatus::Ok, parser.addPattern("(.)*web", false, 0));
+    EXPECT_EQ(ContentExtensions::URLFilterParser::ParseStatus::Ok, parser.addPattern("(.+)*web", false, 0));
+    EXPECT_EQ(ContentExtensions::URLFilterParser::ParseStatus::Ok, parser.addPattern("(.?)*web", false, 0));
+    EXPECT_EQ(ContentExtensions::URLFilterParser::ParseStatus::Ok, parser.addPattern("(.+)?web", false, 0));
+    EXPECT_EQ(ContentExtensions::URLFilterParser::ParseStatus::Ok, parser.addPattern("(.?)+web", false, 0));
+
+    Vector<ContentExtensions::NFA> nfas = createNFAs(combinedURLFilters);
+    EXPECT_EQ(1ul, nfas.size());
+    EXPECT_EQ(7ul, nfas.first().nodes.size());
+
+    ContentExtensions::DFA dfa = ContentExtensions::NFAToDFA::convert(nfas.first());
+    Vector<ContentExtensions::DFABytecode> bytecode;
+    ContentExtensions::DFABytecodeCompiler compiler(dfa, bytecode);
+    compiler.compile();
+    ContentExtensions::DFABytecodeInterpreter interpreter(bytecode.data(), bytecode.size());
+    compareContents(interpreter.interpret("eb", 0), { });
+    compareContents(interpreter.interpret("we", 0), { });
+    compareContents(interpreter.interpret("weeb", 0), { });
+    compareContents(interpreter.interpret("web", 0), { 0 });
+    compareContents(interpreter.interpret("wweb", 0), { 0 });
+    compareContents(interpreter.interpret("wwebb", 0), { 0 });
+    compareContents(interpreter.interpret("http://theweb.com/", 0), { 0 });
+}
+
 TEST_F(ContentExtensionTest, LoadType)
 {
     auto backend = makeBackend("[{\"action\":{\"type\":\"block\"},\"trigger\":{\"url-filter\":\"webkit.org\",\"load-type\":[\"third-party\"]}},"

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to