[Content Extensions] resource-type and load-type should be independent.
authorachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 1 Jun 2015 22:40:47 +0000 (22:40 +0000)
committerachristensen@apple.com <achristensen@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 1 Jun 2015 22:40:47 +0000 (22:40 +0000)
https://bugs.webkit.org/show_bug.cgi?id=145528
rdar://problem/21190765

Reviewed by Benjamin Poulain.

Source/WebCore:

Covered by existing tests and a new API test.

Right now we use the same uint16_t to store all the load-type and resource-type flags,
then we just do a bitwise and to check both at the same time. This results in a trigger
with load-type and resource-type firing if either condition is met, not both conditions.
A trigger with both resource-type and load-type conditions should only fire if both conditions are met.

* contentextensions/DFABytecodeInterpreter.cpp:
(WebCore::ContentExtensions::DFABytecodeInterpreter::interpretTestFlagsAndAppendAction):
Check and correctly handle rules with both resource-type and load-type flags.
* loader/ResourceLoadInfo.h:
Add masks to separate flags from resource-type and load-type.

Tools:

* TestWebKitAPI/Tests/WebCore/ContentExtensions.cpp:
(TestWebKitAPI::TEST_F):

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

Source/WebCore/ChangeLog
Source/WebCore/contentextensions/DFABytecodeInterpreter.cpp
Source/WebCore/loader/ResourceLoadInfo.h
Tools/ChangeLog
Tools/TestWebKitAPI/Tests/WebCore/ContentExtensions.cpp

index a4752bc..defb9b7 100644 (file)
@@ -1,5 +1,26 @@
 2015-06-01  Alex Christensen  <achristensen@webkit.org>
 
+        [Content Extensions] resource-type and load-type should be independent.
+        https://bugs.webkit.org/show_bug.cgi?id=145528
+        rdar://problem/21190765
+
+        Reviewed by Benjamin Poulain.
+
+        Covered by existing tests and a new API test.
+        
+        Right now we use the same uint16_t to store all the load-type and resource-type flags,
+        then we just do a bitwise and to check both at the same time. This results in a trigger
+        with load-type and resource-type firing if either condition is met, not both conditions.
+        A trigger with both resource-type and load-type conditions should only fire if both conditions are met.
+
+        * contentextensions/DFABytecodeInterpreter.cpp:
+        (WebCore::ContentExtensions::DFABytecodeInterpreter::interpretTestFlagsAndAppendAction):
+        Check and correctly handle rules with both resource-type and load-type flags.
+        * loader/ResourceLoadInfo.h:
+        Add masks to separate flags from resource-type and load-type.
+
+2015-06-01  Alex Christensen  <achristensen@webkit.org>
+
         [Content Extensions] Reduce DFA memory usage.
         https://bugs.webkit.org/show_bug.cgi?id=145526
 
index 3adb30f..195b770 100644 (file)
@@ -65,7 +65,15 @@ void DFABytecodeInterpreter::interpretTestFlagsAndAppendAction(unsigned& program
 {
     ASSERT(getBits<DFABytecodeInstruction>(m_bytecode, m_bytecodeLength, programCounter, m_pagesUsed) == DFABytecodeInstruction::TestFlagsAndAppendAction
         || getBits<DFABytecodeInstruction>(m_bytecode, m_bytecodeLength, programCounter, m_pagesUsed) == DFABytecodeInstruction::TestFlagsAndAppendActionWithIfDomain);
-    if (flags & getBits<uint16_t>(m_bytecode, m_bytecodeLength, programCounter + sizeof(DFABytecode), m_pagesUsed))
+    uint16_t flagsToCheck = getBits<uint16_t>(m_bytecode, m_bytecodeLength, programCounter + sizeof(DFABytecode), m_pagesUsed);
+
+    uint16_t loadTypeFlags = flagsToCheck & LoadTypeMask;
+    uint16_t ressourceTypeFlags = flagsToCheck & ResourceTypeMask;
+    
+    bool loadTypeMatches = loadTypeFlags ? (loadTypeFlags & flags) : true;
+    bool ressourceTypeMatches = ressourceTypeFlags ? (ressourceTypeFlags & flags) : true;
+    
+    if (loadTypeMatches && ressourceTypeMatches)
         actions.add((ifDomain ? IfDomainFlag : 0) | static_cast<uint64_t>(getBits<unsigned>(m_bytecode, m_bytecodeLength, programCounter + sizeof(DFABytecode) + sizeof(uint16_t), m_pagesUsed)));
     programCounter += instructionSizeWithArguments(DFABytecodeInstruction::TestFlagsAndAppendAction);
     ASSERT(instructionSizeWithArguments(DFABytecodeInstruction::TestFlagsAndAppendAction) == instructionSizeWithArguments(DFABytecodeInstruction::TestFlagsAndAppendActionWithIfDomain));
index 3e48158..22465f1 100644 (file)
@@ -44,12 +44,14 @@ enum class ResourceType : uint16_t {
     PlugInStream = 0x0100,
     Popup = 0x0200,
 };
+const uint16_t ResourceTypeMask = 0x03FF;
 
 enum class LoadType : uint16_t {
     Invalid = 0x0000,
     FirstParty = 0x0400,
     ThirdParty = 0x0800,
 };
+const uint16_t LoadTypeMask = 0x0C00;
 
 typedef uint16_t ResourceFlags;
 
index 6ec15f5..fa065cf 100644 (file)
@@ -1,3 +1,14 @@
+2015-06-01  Alex Christensen  <achristensen@webkit.org>
+
+        [Content Extensions] resource-type and load-type should be independent.
+        https://bugs.webkit.org/show_bug.cgi?id=145528
+        rdar://problem/21190765
+
+        Reviewed by Benjamin Poulain.
+
+        * TestWebKitAPI/Tests/WebCore/ContentExtensions.cpp:
+        (TestWebKitAPI::TEST_F):
+
 2015-06-01  Daniel Bates  <dabates@apple.com>
 
         Add ATS keys to WebKitTestRunnerApp
index b2b819c..bac45d7 100644 (file)
@@ -654,6 +654,15 @@ TEST_F(ContentExtensionTest, ResourceType)
     testRequest(backend, mainDocumentRequest("http://block_only_images.org", ResourceType::Document), { });
 }
 
+TEST_F(ContentExtensionTest, ResourceAndLoadType)
+{
+    auto backend = makeBackend("[{\"action\":{\"type\":\"block\"},\"trigger\":{\"url-filter\":\"BlockOnlyIfThirdPartyAndScript\",\"resource-type\":[\"script\"],\"load-type\":[\"third-party\"]}}]");
+    
+    testRequest(backend, subResourceRequest("http://webkit.org/BlockOnlyIfThirdPartyAndScript.js", "http://webkit.org", ResourceType::Script), { });
+    testRequest(backend, subResourceRequest("http://webkit.org/BlockOnlyIfThirdPartyAndScript.png", "http://not_webkit.org", ResourceType::Image), { });
+    testRequest(backend, subResourceRequest("http://webkit.org/BlockOnlyIfThirdPartyAndScript.js", "http://not_webkit.org", ResourceType::Script), { ContentExtensions::ActionType::BlockLoad });
+}
+
 TEST_F(ContentExtensionTest, ResourceOrLoadTypeMatchingEverything)
 {
     auto backend = makeBackend("[{\"action\":{\"type\":\"block\"},\"trigger\":{\"url-filter\":\".*\",\"resource-type\":[\"image\"]}},"