Issue 6797 - Require active domains in snippet filters

test/snippets.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-present eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  *
  * You should have received a copy of the GNU General Public License
  * along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
  */
 
 /* eslint no-new-func: "off" */
 
 "use strict";
 
 const {createSandbox} = require("./_common");
 
 let Snippets = null;
 let parseScript = null;
 let compileScript = null;
 let Filter = null;
+let SnippetFilter = null;
 
 exports.setUp = function(callback)
 {
   let sandboxedRequire = createSandbox();
   (
-    {Filter} = sandboxedRequire("../lib/filterClasses"),
+    {Filter, SnippetFilter} = sandboxedRequire("../lib/filterClasses"),
     {Snippets, parseScript, compileScript} = sandboxedRequire("../lib/snippets")
   );
 
   callback();
 };
 
 exports.testDomainRestrictions = function(test)
 {
   function testScriptMatches(description, filters, domain, expectedMatches)
   {
-    for (let filter of filters)
-      Snippets.add(Filter.fromText(filter));
+    for (let filter of filters.map(Filter.fromText))
+    {
+      if (filter instanceof SnippetFilter)
+        Snippets.add(filter);
+    }
 
     let matches = Snippets.getScriptsForDomain(domain);
     test.deepEqual(matches.sort(), expectedMatches.sort(), description);
 
     Snippets.clear();
   }
 
   testScriptMatches(
+    "Ignore generic filters",
+    [
+      "#$#foo", "example.com#$#foo",

[kzar, 2018/08/08 11:31:50]

Shouldn't we use different snippet names for these filters? Then a match for the
"foo" snippet would mean more. At the moment, it seems to me like we could
accidentally match on one of the other filters here too without realising it.

[Manish Jethani, 2018/08/08 13:56:42]

Thanks, this makes sense.

Done.

+      "~example.com#$#foo"
+    ],
+    "example.com",
+    ["foo"]
+  );
+  testScriptMatches(
     "Ignore filters that include parent domain but exclude subdomain",
     [
       "~www.example.com,example.com#$#foo"
     ],
     "www.example.com",
     []
   );
   testScriptMatches(
     "Ignore filters for other subdomain",
     [
@@ skipping 170 matching lines @@
   // between executions. In the example below, assertFoo does not find 456 but
   // it doesn't find 123 either. It's the initial value 0.
   new Function(
     compileScript("setFoo 456; assertFoo 0", [
       ...libraries, "let foo = 1; exports.setFoo = value => { foo = value; };"
     ])
   )();
 
   test.done();
 };