Issue 431/432 - Remove special handling for the $sitekey option

lib/filterClasses.js

 /*
  * This file is part of Adblock Plus <http://adblockplus.org/>,
  * Copyright (C) 2006-2014 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
@@ skipping 197 matching lines @@
 
   /**
    * See Filter.serialize()
    */
   serialize: function(buffer) {}
 };
 
 /**
  * Abstract base class for filters that can get hits
  * @param {String} text see Filter()
- * @param {String} domains  (optional) Domains that the filter is restricted to separated by domainSeparator e.g. "foo.com|bar.com|~baz.com"
+ * @param {String} [domains] Domains that the filter is restricted to separated by domainSeparator e.g. "foo.com|bar.com|~baz.com"
  * @constructor
  * @augments Filter
  */
 function ActiveFilter(text, domains)
 {
   Filter.call(this, text);
 
   this.domainSource = domains;
 }
 exports.ActiveFilter = ActiveFilter;
@@ skipping 145 matching lines @@
       }
 
       this.domainSource = null;
     }
 
     Object.defineProperty(this, "domains", {value: domains, enumerable: true});
     return this.domains;
   },
 
   /**
+   * Array containing public keys of websites that this filter should apply to
+   * @type Array of String
+   */
+  sitekeys: null,
+
+  /**
    * Checks whether this filter is active on a domain.
+   * @param {String} docDomain domain name of the document that loads the URL
+   * @param {String} [sitekey] public key provided by the document
+   * @return {Boolean} true in case of the filter being active
    */
-  isActiveOnDomain: function(/**String*/ docDomain) /**Boolean*/
+  isActiveOnDomain: function(docDomain, sitekey)
   {
+    if (this.sitekeys && (!sitekey || this.sitekeys.indexOf(sitekey.toUpperCase()) < 0))

[Wladimir Palant, 2014/08/29 20:13:59]

Ok, we have an issue here. This code works in the first approximation, but
sitekeys are actually case-sensitive. There is a very slight chance that two
different sitekeys will be identical when converted to upper-case.

Given that this issue was present in the original code as well, and that fixing
it is non-trivial (we would need to change the way filter options are parsed) -
could you just add a comment noting this problem?

[Thomas Greiner, 2014/09/01 09:51:46]

Done.

+      return false;
+
     // If no domains are set the rule matches everywhere
     if (!this.domains)
       return true;
 
     // If the document has no host name, match only if the filter isn't restricted to specific domains
     if (!docDomain)
       return this.domains[""];
 
     if (this.ignoreTrailingDot)
       docDomain = docDomain.replace(/\.+$/, "");
@@ skipping 46 matching lines @@
       if (this._lastHit)
         buffer.push("lastHit=" + this._lastHit);
     }
   }
 };
 
 /**
  * Abstract base class for RegExp-based filters
  * @param {String} text see Filter()
  * @param {String} regexpSource filter part that the regular expression should be build from
- * @param {Number} contentType  (optional) Content types the filter applies to, combination of values from RegExpFilter.typeMap
- * @param {Boolean} matchCase   (optional) Defines whether the filter should distinguish between lower and upper case letters
- * @param {String} domains      (optional) Domains that the filter is restricted to, e.g. "foo.com|bar.com|~baz.com"
- * @param {Boolean} thirdParty  (optional) Defines whether the filter should apply to third-party or first-party content only
+ * @param {Number} [contentType] Content types the filter applies to, combination of values from RegExpFilter.typeMap
+ * @param {Boolean} [matchCase] Defines whether the filter should distinguish between lower and upper case letters
+ * @param {String} [domains] Domains that the filter is restricted to, e.g. "foo.com|bar.com|~baz.com"
+ * @param {Boolean} [thirdParty] Defines whether the filter should apply to third-party or first-party content only
+ * @param {String} [sitekeys] Public keys of websites that this filter should apply to
  * @constructor
  * @augments ActiveFilter
  */
-function RegExpFilter(text, regexpSource, contentType, matchCase, domains, thirdParty)
+function RegExpFilter(text, regexpSource, contentType, matchCase, domains, thirdParty, sitekeys)
 {
-  ActiveFilter.call(this, text, domains);
+  ActiveFilter.call(this, text, domains, sitekeys);
 
   if (contentType != null)
     this.contentType = contentType;
   if (matchCase)
     this.matchCase = matchCase;
   if (thirdParty != null)
     this.thirdParty = thirdParty;
+  if (sitekeys != null)
+    this.sitekeySource = sitekeys;
 
   if (regexpSource.length >= 2 && regexpSource[0] == "/" && regexpSource[regexpSource.length - 1] == "/")
   {
     // The filter is a regular expression - convert it immediately to catch syntax errors
     let regexp = new RegExp(regexpSource.substr(1, regexpSource.length - 2), this.matchCase ? "" : "i");
     Object.defineProperty(this, "regexp", {value: regexp});
   }
   else
   {
     // No need to convert this filter to regular expression yet, do it on demand
@@ skipping 61 matching lines @@
    * @type Boolean
    */
   matchCase: false,
   /**
    * Defines whether the filter should apply to third-party or first-party content only. Can be null (apply to all content).
    * @type Boolean
    */
   thirdParty: null,
 
   /**
+   * String that the sitekey property should be generated from
+   * @type String
+   */
+  sitekeySource: null,
+
+  /**
+   * Array containing public keys of websites that this filter should apply to
+   * @type Array of String
+   */
+  get sitekeys()
+  {
+    let sitekeys = null;
+
+    if (this.sitekeySource)
+    {
+      sitekeys = this.sitekeySource.split("|");
+      this.sitekeySource = null;
+    }
+
+    Object.defineProperty(this, "sitekeys", {value: sitekeys, enumerable: true});
+    return this.sitekeys;
+  },
+
+  /**
    * Tests whether the URL matches this filter
    * @param {String} location URL to be tested
    * @param {String} contentType content type identifier of the URL
    * @param {String} docDomain domain name of the document that loads the URL
    * @param {Boolean} thirdParty should be true if the URL is a third-party request
+   * @param {String} sitekey public key provided by the document
    * @return {Boolean} true in case of a match
    */
-  matches: function(location, contentType, docDomain, thirdParty)
+  matches: function(location, contentType, docDomain, thirdParty, sitekey)
   {
     if (this.regexp.test(location) &&
         (RegExpFilter.typeMap[contentType] & this.contentType) != 0 &&
         (this.thirdParty == null || this.thirdParty == thirdParty) &&
-        this.isActiveOnDomain(docDomain))
+        this.isActiveOnDomain(docDomain, sitekey))
     {
       return true;
     }
 
     return false;
   }
 };
 
 // Required to optimize Matcher, see also RegExpFilter.prototype.length
 Object.defineProperty(RegExpFilter.prototype, "0",
@@ skipping 11 matching lines @@
   let origText = text;
   if (text.indexOf("@@") == 0)
   {
     blocking = false;
     text = text.substr(2);
   }
 
   let contentType = null;
   let matchCase = null;
   let domains = null;
-  let siteKeys = null;
+  let sitekeys = null;
   let thirdParty = null;
   let collapse = null;
   let options;
   let match = (text.indexOf("$") >= 0 ? Filter.optionsRegExp.exec(text) : null);
   if (match)
   {
     options = match[1].toUpperCase().split(",");
     text = match.input.substr(0, match.index);
     for (let option of options)
     {
@@ skipping 25 matching lines @@
         domains = value;
       else if (option == "THIRD_PARTY")
         thirdParty = true;
       else if (option == "~THIRD_PARTY")
         thirdParty = false;
       else if (option == "COLLAPSE")
         collapse = true;
       else if (option == "~COLLAPSE")
         collapse = false;
       else if (option == "SITEKEY" && typeof value != "undefined")
-        siteKeys = value.split(/\|/);
+        sitekeys = value;
       else
         return new InvalidFilter(origText, "Unknown option " + option.toLowerCase());
     }
   }
 
   if (!blocking && (contentType == null || (contentType & RegExpFilter.typeMap.DOCUMENT)) &&
       (!options || options.indexOf("DOCUMENT") < 0) && !/^\|?[\w\-]+:/.test(text))
   {
     // Exception filters shouldn't apply to pages by default unless they start with a protocol name
     if (contentType == null)
       contentType = RegExpFilter.prototype.contentType;
     contentType &= ~RegExpFilter.typeMap.DOCUMENT;
   }
-  if (!blocking && siteKeys)
-    contentType = RegExpFilter.typeMap.DOCUMENT;
 
   try
   {
     if (blocking)
-      return new BlockingFilter(origText, text, contentType, matchCase, domains, thirdParty, collapse);
+      return new BlockingFilter(origText, text, contentType, matchCase, domains, thirdParty, sitekeys, collapse);
     else
-      return new WhitelistFilter(origText, text, contentType, matchCase, domains, thirdParty, siteKeys);
+      return new WhitelistFilter(origText, text, contentType, matchCase, domains, thirdParty, sitekeys);
   }
   catch (e)
   {
     return new InvalidFilter(origText, e);
   }
 }
 
 /**
  * Maps type strings like "SCRIPT" or "OBJECT" to bit masks
  */
@@ skipping 23 matching lines @@
 RegExpFilter.prototype.contentType &= ~(RegExpFilter.typeMap.ELEMHIDE | RegExpFilter.typeMap.POPUP);
 
 /**
  * Class for blocking filters
  * @param {String} text see Filter()
  * @param {String} regexpSource see RegExpFilter()
  * @param {Number} contentType see RegExpFilter()
  * @param {Boolean} matchCase see RegExpFilter()
  * @param {String} domains see RegExpFilter()
  * @param {Boolean} thirdParty see RegExpFilter()
+ * @param {String} sitekeys see RegExpFilter()
  * @param {Boolean} collapse  defines whether the filter should collapse blocked content, can be null
  * @constructor
  * @augments RegExpFilter
  */
-function BlockingFilter(text, regexpSource, contentType, matchCase, domains, thirdParty, collapse)
+function BlockingFilter(text, regexpSource, contentType, matchCase, domains, thirdParty, sitekeys, collapse)
 {
-  RegExpFilter.call(this, text, regexpSource, contentType, matchCase, domains, thirdParty);
+  RegExpFilter.call(this, text, regexpSource, contentType, matchCase, domains, thirdParty, sitekeys);
 
   this.collapse = collapse;
 }
 exports.BlockingFilter = BlockingFilter;
 
 BlockingFilter.prototype =
 {
   __proto__: RegExpFilter.prototype,
 
   /**
    * Defines whether the filter should collapse blocked content. Can be null (use the global preference).
    * @type Boolean
    */
   collapse: null
 };
 
 /**
  * Class for whitelist filters
  * @param {String} text see Filter()
  * @param {String} regexpSource see RegExpFilter()
  * @param {Number} contentType see RegExpFilter()
  * @param {Boolean} matchCase see RegExpFilter()
  * @param {String} domains see RegExpFilter()
  * @param {Boolean} thirdParty see RegExpFilter()
- * @param {String[]} siteKeys public keys of websites that this filter should apply to
+ * @param {String} sitekeys see RegExpFilter()
  * @constructor
  * @augments RegExpFilter
  */
-function WhitelistFilter(text, regexpSource, contentType, matchCase, domains, thirdParty, siteKeys)
+function WhitelistFilter(text, regexpSource, contentType, matchCase, domains, thirdParty, sitekeys)
 {
-  RegExpFilter.call(this, text, regexpSource, contentType, matchCase, domains, thirdParty);
-
-  if (siteKeys != null)
-    this.siteKeys = siteKeys;
+  RegExpFilter.call(this, text, regexpSource, contentType, matchCase, domains, thirdParty, sitekeys);
 }
 exports.WhitelistFilter = WhitelistFilter;
 
 WhitelistFilter.prototype =
 {
-  __proto__: RegExpFilter.prototype,
-
-  /**
-   * List of public keys of websites that this filter should apply to
-   * @type String[]
-   */
-  siteKeys: null
+  __proto__: RegExpFilter.prototype
 }
 
 /**
  * Base class for element hiding filters
  * @param {String} text see Filter()
- * @param {String} domains    (optional) Host names or domains the filter should be restricted to
+ * @param {String} [domains] Host names or domains the filter should be restricted to
  * @param {String} selector   CSS selector for the HTML elements that should be hidden
  * @constructor
  * @augments ActiveFilter
  */
 function ElemHideBase(text, domains, selector)
 {
   ActiveFilter.call(this, text, domains || null);
 
   if (domains)
     this.selectorDomain = domains.replace(/,~[^,]+/g, "").replace(/^~[^,]+,?/, "").toLowerCase();
@@ skipping 113 matching lines @@
 function ElemHideException(text, domains, selector)
 {
   ElemHideBase.call(this, text, domains, selector);
 }
 exports.ElemHideException = ElemHideException;
 
 ElemHideException.prototype =
 {
   __proto__: ElemHideBase.prototype
 };