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
  * 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/>.
  */
 
 /**
  * @fileOverview Definition of Filter class and its subclasses.
  */
 
 let {FilterNotifier} = require("filterNotifier");
+let {Utils} = require("utils");

[Wladimir Palant, 2014/08/01 10:50:21]

This is a pretty self-contained module, it shouldn't have a dependency on
utils.js.

[Thomas Greiner, 2014/08/11 17:18:15]

Done.

 
 /**
  * Abstract base class for filters
  *
  * @param {String} text   string representation of the filter
  * @constructor
  */
 function Filter(text)
 {
   this.text = text;
@@ skipping 176 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} siteKeys (optional) Public keys of websites that this filter should apply to, e.g. "foo|bar|~baz"

[Wladimir Palant, 2014/08/01 10:50:21]

I don't really see how exceptions are useful here, original implementation
didn't have any. Keep it simple?

[Thomas Greiner, 2014/08/11 17:18:15]

The issue stated that $sitekey should be handled exactly the same way as the
domain option. However, if you don't see any concerns in regard to backwards
compatibility when implementing it at a later point if needed, I don't mind
removing it.

  * @constructor
  * @augments Filter
  */
-function ActiveFilter(text, domains)
+function ActiveFilter(text, domains, siteKeys)
 {
   Filter.call(this, text);
 
   this.domainSource = domains;
+  this.siteKeySource = siteKeys;

[Thomas Greiner, 2014/08/11 17:18:15]

Done.

 }
 exports.ActiveFilter = ActiveFilter;
 
 ActiveFilter.prototype =
 {
   __proto__: Filter.prototype,
 
   _disabled: false,
   _hitCount: 0,
   _lastHit: 0,
@@ skipping 137 matching lines @@
       }
 
       this.domainSource = null;
     }
 
     Object.defineProperty(this, "domains", {value: domains, enumerable: true});
     return this.domains;
   },
 
   /**
+   * String that the siteKey property should be generated from
+   * @type String
+   */
+  siteKeySource: null,
+
+  /**
+   * Map containing public keys of websites that this filter should apply to
+   * @type Object
+   */
+  get siteKeys()
+  {
+    let siteKeys = null;
+
+    if (this.siteKeySource)
+    {
+      let source = this.siteKeySource;
+      let list = source.split("|");
+      if (list.length == 1 && list[0][0] != "~")
+      {
+        // Fast track for the common one-sitekey scenario
+        siteKeys = {__proto__: null, "": false};
+        siteKeys[list[0]] = true;
+      }
+      else
+      {
+        let hasIncludes = false;
+        for (let i = 0; i < list.length; i++)
+        {
+          let siteKey = list[i];
+          if (siteKey == "")
+            continue;
+
+          let include;
+          if (siteKey[0] == "~")
+          {
+            include = false;
+            siteKey = siteKey.substr(1);
+          }
+          else
+          {
+            include = true;
+            hasIncludes = true;
+          }
+
+          if (!siteKeys)
+            siteKeys = Object.create(null);
+
+          siteKeys[siteKey] = include;
+        }
+        siteKeys[""] = !hasIncludes;
+      }
+
+      this.siteKeySource = null;
+    }
+
+    Object.defineProperty(this, "siteKeys", {value: siteKeys, enumerable: true});
+    return this.siteKeys;
+  },
+
+  /**
    * Checks whether this filter is active on a domain.
    */
   isActiveOnDomain: function(/**String*/ docDomain) /**Boolean*/
   {
     // 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)
@@ skipping 29 matching lines @@
     docDomain = docDomain.toUpperCase();
 
     for (let domain in this.domains)
       if (this.domains[domain] && domain != docDomain && (domain.length <= docDomain.length || domain.indexOf("." + docDomain) != domain.length - docDomain.length - 1))
         return false;
 
     return true;
   },
 
   /**
+   * Checks whether the provided site key is valid and active on a page
+   */
+  isActiveOnPage: function(/**String*/ docDomain, /**String*/ docLocation, /**String*/ siteKey) /**Boolean*/

[Wladimir Palant, 2014/08/01 10:50:21]

I don't think that this is the right place to perform sitekey validation, by the
time the matcher is called the sitekey should already be validated. This also
means that you don't need to pass document URL around everywhere and no new
function, you can simply extend isActiveOnDomain() in RegExpFilter to consider
one more parameter.

[Thomas Greiner, 2014/08/11 17:18:15]

Done.

+  {
+    // If no site keys are set the rule matches everywhere
+    if (!this.siteKeys)
+      return true;
+
+    // If the document has not provided a sitekey, match only if the filter isn't restricted by sitekeys
+    if (!docDomain || !siteKey)
+      return this.siteKeys[""];
+
+    // If the document has provided an invalid sitekey the filter should not match
+    if (siteKey.indexOf("_") < 0)
+      return false;
+
+    let [key, signature] = siteKey.split("_", 2);
+    let formattedKey = key.replace(/=/g, "").toUpperCase();
+    if (!(formattedKey in this.siteKeys) || !Utils.crypto)
+      return this.siteKeys[""];
+
+    // Website specifies a key that we know but is the signature valid?
+    let uri = Services.io.newURI(docLocation, null, null);
+    let host = uri.asciiHost;
+    if (uri.port > 0)
+      host += ":" + uri.port;
+    let params = [
+      uri.path.replace(/#.*/, ""),  // REQUEST_URI
+      host,                         // HTTP_HOST
+      Utils.httpProtocol.userAgent  // HTTP_USER_AGENT
+    ];
+    if (Utils.verifySignature(key, signature, params.join("\0")))
+      return this.siteKeys[formattedKey];
+
+    return !this.siteKeys[formattedKey];
+  },
+
+  /**
    * See Filter.serialize()
    */
   serialize: function(buffer)
   {
     if (this._disabled || this._hitCount || this._lastHit)
     {
       Filter.prototype.serialize.call(this, buffer);
       if (this._disabled)
         buffer.push("disabled=true");
       if (this._hitCount)
         buffer.push("hitCount=" + this._hitCount);
       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 {String} siteKeys     (optional) Public keys of websites that this filter should apply to, e.g. "foo|bar|~baz"
  * @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 (regexpSource.length >= 2 && regexpSource[0] == "/" && regexpSource[regexpSource.length - 1] == "/")
   {
@@ skipping 73 matching lines @@
    * @type Boolean
    */
   thirdParty: null,
 
   /**
    * 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} docLocation location of the document that loads the URL
+   * @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, docLocation, siteKey)
   {
     if (this.regexp.test(location) &&
         (RegExpFilter.typeMap[contentType] & this.contentType) != 0 &&
         (this.thirdParty == null || this.thirdParty == thirdParty) &&
-        this.isActiveOnDomain(docDomain))
+        this.isActiveOnDomain(docDomain) &&
+        this.isActiveOnPage(docDomain, docLocation, siteKey))
     {
       return true;
     }
 
     return false;
   }
 };
 
 // Required to optimize Matcher, see also RegExpFilter.prototype.length
 Object.defineProperty(RegExpFilter.prototype, "0",
@@ skipping 57 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);
   }
   catch (e)
   {
     return new InvalidFilter(origText, e);
   }
 }
 
 /**
@@ skipping 25 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)
 {
-  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} selector   CSS selector for the HTML elements that should be hidden
  * @constructor
  * @augments ActiveFilter
  */
@@ skipping 119 matching lines @@
 function ElemHideException(text, domains, selector)
 {
   ElemHideBase.call(this, text, domains, selector);
 }
 exports.ElemHideException = ElemHideException;
 
 ElemHideException.prototype =
 {
   __proto__: ElemHideBase.prototype
 };