[Pkg-mozext-commits] [adblock-plus] 10/28: Issue 431/432 - Remove special handling for the $sitekey option

David Prévot taffit at moszumanska.debian.org
Wed Nov 12 02:09:49 UTC 2014


This is an automated email from the git hooks/post-receive script.

taffit pushed a commit to branch master
in repository adblock-plus.

commit 68e89996be0a2e6bc08a329cf888eb8de4ca87d9
Author: Thomas Greiner <thomas at adblockplus.org>
Date:   Tue Sep 2 10:36:38 2014 +0200

    Issue 431/432 - Remove special handling for the $sitekey option
---
 lib/contentPolicy.js |  84 +++++++++++++++++++++++++++---------------
 lib/filterClasses.js | 102 +++++++++++++++++++++++++++++++++++----------------
 lib/matcher.js       |  67 +++++++--------------------------
 3 files changed, 137 insertions(+), 116 deletions(-)

diff --git a/lib/contentPolicy.js b/lib/contentPolicy.js
index a06e6de..d21a2cd 100644
--- a/lib/contentPolicy.js
+++ b/lib/contentPolicy.js
@@ -162,37 +162,18 @@ let Policy = exports.Policy =
     let wndLocation = originWindow.location.href;
     let docDomain = getHostname(wndLocation);
     let match = null;
+    let [sitekey, sitekeyWnd] = getSitekey(wnd);
     if (!match && Prefs.enabled)
     {
       let testWnd = wnd;
+      let testSitekey = sitekey;
+      let testSitekeyWnd = sitekeyWnd;
       let parentWndLocation = getWindowLocation(testWnd);
       while (true)
       {
         let testWndLocation = parentWndLocation;
         parentWndLocation = (testWnd == testWnd.parent ? testWndLocation : getWindowLocation(testWnd.parent));
-        match = Policy.isWhitelisted(testWndLocation, parentWndLocation);
-
-        if (!(match instanceof WhitelistFilter))
-        {
-          let keydata = (testWnd.document && testWnd.document.documentElement ? testWnd.document.documentElement.getAttribute("data-adblockkey") : null);
-          if (keydata && keydata.indexOf("_") >= 0)
-          {
-            let [key, signature] = keydata.split("_", 2);
-            let keyMatch = defaultMatcher.matchesByKey(testWndLocation, key.replace(/=/g, ""), docDomain);
-            if (keyMatch && Utils.crypto)
-            {
-              // Website specifies a key that we know but is the signature valid?
-              let uri = Services.io.newURI(testWndLocation, null, null);
-              let params = [
-                uri.path.replace(/#.*/, ""),  // REQUEST_URI
-                uri.asciiHost,                // HTTP_HOST
-                Utils.httpProtocol.userAgent  // HTTP_USER_AGENT
-              ];
-              if (Utils.verifySignature(key, signature, params.join("\0")))
-                match = keyMatch;
-            }
-          }
-        }
+        match = Policy.isWhitelisted(testWndLocation, parentWndLocation, testSitekey);
 
         if (match instanceof WhitelistFilter)
         {
@@ -203,8 +184,10 @@ let Policy = exports.Policy =
 
         if (testWnd.parent == testWnd)
           break;
-        else
-          testWnd = testWnd.parent;
+
+        if (testWnd == testSitekeyWnd)
+          [testSitekey, testSitekeyWnd] = getSitekey(testWnd.parent);
+        testWnd = testWnd.parent;
       }
     }
 
@@ -260,7 +243,7 @@ let Policy = exports.Policy =
 
     if (!match && Prefs.enabled)
     {
-      match = defaultMatcher.matchesAny(locationText, Policy.typeDescr[contentType] || "", docDomain, thirdParty);
+      match = defaultMatcher.matchesAny(locationText, Policy.typeDescr[contentType] || "", docDomain, thirdParty, sitekey);
       if (match instanceof BlockingFilter && node.ownerDocument && !(contentType in Policy.nonVisual))
       {
         let prefCollapse = (match.collapse != null ? match.collapse : !Prefs.fastcollapse);
@@ -298,9 +281,10 @@ let Policy = exports.Policy =
    * Checks whether a page is whitelisted.
    * @param {String} url
    * @param {String} [parentUrl] location of the parent page
+   * @param {String} [sitekey] public key provided on the page
    * @return {Filter} filter that matched the URL or null if not whitelisted
    */
-  isWhitelisted: function(url, parentUrl)
+  isWhitelisted: function(url, parentUrl, sitekey)
   {
     if (!url)
       return null;
@@ -318,12 +302,12 @@ let Policy = exports.Policy =
     if (index >= 0)
       url = url.substring(0, index);
 
-    let result = defaultMatcher.matchesAny(url, "DOCUMENT", getHostname(parentUrl), false);
+    let result = defaultMatcher.matchesAny(url, "DOCUMENT", getHostname(parentUrl), false, sitekey);
     return (result instanceof WhitelistFilter ? result : null);
   },
 
   /**
-   * Checks whether the page loaded in a window is whitelisted.
+   * Checks whether the page loaded in a window is whitelisted for indication in the UI.
    * @param wnd {nsIDOMWindow}
    * @return {Filter} matching exception rule or null if not whitelisted
    */
@@ -332,7 +316,6 @@ let Policy = exports.Policy =
     return Policy.isWhitelisted(getWindowLocation(wnd));
   },
 
-
   /**
    * Asynchronously re-checks filters for given nodes.
    */
@@ -691,6 +674,47 @@ function getHostname(/**String*/ url) /**String*/
 }
 
 /**
+ * Retrieves the sitekey of a window.
+ */
+function getSitekey(wnd)
+{
+  let sitekey = null;
+
+  while (true)
+  {
+    if (wnd.document && wnd.document.documentElement)
+    {
+      let keydata = wnd.document.documentElement.getAttribute("data-adblockkey");
+      if (keydata && keydata.indexOf("_") >= 0)
+      {
+        let [key, signature] = keydata.split("_", 2);
+        key = key.replace(/=/g, "");
+
+        // Website specifies a key but is the signature valid?
+        let uri = Services.io.newURI(getWindowLocation(wnd), 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 [key, wnd];
+      }
+    }
+
+    if (wnd === wnd.parent)
+      break;
+
+    wnd = wnd.parent;
+  }
+
+  return [sitekey, wnd];
+}
+
+/**
  * Retrieves the location of a window.
  * @param wnd {nsIDOMWindow}
  * @return {String} window location or null on failure
diff --git a/lib/filterClasses.js b/lib/filterClasses.js
index c120938..3e067de 100644
--- a/lib/filterClasses.js
+++ b/lib/filterClasses.js
@@ -215,7 +215,7 @@ CommentFilter.prototype =
 /**
  * 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
  */
@@ -387,10 +387,24 @@ ActiveFilter.prototype =
   },
 
   /**
+   * 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)
   {
+    // Sitekeys are case-sensitive so we shouldn't convert them to upper-case to avoid false
+    // positives here. Instead we need to change the way filter options are parsed.
+    if (this.sitekeys && (!sitekey || this.sitekeys.indexOf(sitekey.toUpperCase()) < 0))
+      return false;
+
     // If no domains are set the rule matches everywhere
     if (!this.domains)
       return true;
@@ -457,16 +471,17 @@ ActiveFilter.prototype =
  * 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;
@@ -474,6 +489,8 @@ function RegExpFilter(text, regexpSource, contentType, matchCase, domains, third
     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] == "/")
   {
@@ -561,19 +578,50 @@ RegExpFilter.prototype =
   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()
+  {
+    // Despite this property being cached, the getter is called
+    // several times on Safari, due to WebKit bug 132872
+    let prop = Object.getOwnPropertyDescriptor(this, "sitekeys");
+    if (prop)
+      return prop.value;
+
+    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;
     }
@@ -605,7 +653,7 @@ RegExpFilter.fromText = function(text)
   let contentType = null;
   let matchCase = null;
   let domains = null;
-  let siteKeys = null;
+  let sitekeys = null;
   let thirdParty = null;
   let collapse = null;
   let options;
@@ -651,7 +699,7 @@ RegExpFilter.fromText = function(text)
       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());
     }
@@ -665,15 +713,13 @@ RegExpFilter.fromText = function(text)
       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)
   {
@@ -717,13 +763,14 @@ RegExpFilter.prototype.contentType &= ~(RegExpFilter.typeMap.ELEMHIDE | RegExpFi
  * @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;
 }
@@ -748,34 +795,25 @@ BlockingFilter.prototype =
  * @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
diff --git a/lib/matcher.js b/lib/matcher.js
index 40289d0..b76a721 100644
--- a/lib/matcher.js
+++ b/lib/matcher.js
@@ -165,13 +165,13 @@ Matcher.prototype = {
   /**
    * Checks whether the entries for a particular keyword match a URL
    */
-  _checkEntryMatch: function(keyword, location, contentType, docDomain, thirdParty)
+  _checkEntryMatch: function(keyword, location, contentType, docDomain, thirdParty, sitekey)
   {
     let list = this.filterByKeyword[keyword];
     for (let i = 0; i < list.length; i++)
     {
       let filter = list[i];
-      if (filter.matches(location, contentType, docDomain, thirdParty))
+      if (filter.matches(location, contentType, docDomain, thirdParty, sitekey))
         return filter;
     }
     return null;
@@ -183,9 +183,10 @@ Matcher.prototype = {
    * @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 {RegExpFilter} matching filter or null
    */
-  matchesAny: function(location, contentType, docDomain, thirdParty)
+  matchesAny: function(location, contentType, docDomain, thirdParty, sitekey)
   {
     let candidates = location.toLowerCase().match(/[a-z0-9%]{3,}/g);
     if (candidates === null)
@@ -196,7 +197,7 @@ Matcher.prototype = {
       let substr = candidates[i];
       if (substr in this.filterByKeyword)
       {
-        let result = this._checkEntryMatch(substr, location, contentType, docDomain, thirdParty);
+        let result = this._checkEntryMatch(substr, location, contentType, docDomain, thirdParty, sitekey);
         if (result)
           return result;
       }
@@ -215,7 +216,6 @@ function CombinedMatcher()
 {
   this.blacklist = new Matcher();
   this.whitelist = new Matcher();
-  this.keys = Object.create(null);
   this.resultCache = Object.create(null);
 }
 exports.CombinedMatcher = CombinedMatcher;
@@ -241,12 +241,6 @@ CombinedMatcher.prototype =
   whitelist: null,
 
   /**
-   * Exception rules that are limited by public keys, mapped by the corresponding keys.
-   * @type Object
-   */
-  keys: null,
-
-  /**
    * Lookup table of previous matchesAny results
    * @type Object
    */
@@ -265,7 +259,6 @@ CombinedMatcher.prototype =
   {
     this.blacklist.clear();
     this.whitelist.clear();
-    this.keys = Object.create(null);
     this.resultCache = Object.create(null);
     this.cacheEntries = 0;
   },
@@ -276,15 +269,7 @@ CombinedMatcher.prototype =
   add: function(filter)
   {
     if (filter instanceof WhitelistFilter)
-    {
-      if (filter.siteKeys)
-      {
-        for (let i = 0; i < filter.siteKeys.length; i++)
-          this.keys[filter.siteKeys[i]] = filter.text;
-      }
-      else
-        this.whitelist.add(filter);
-    }
+      this.whitelist.add(filter);
     else
       this.blacklist.add(filter);
 
@@ -301,15 +286,7 @@ CombinedMatcher.prototype =
   remove: function(filter)
   {
     if (filter instanceof WhitelistFilter)
-    {
-      if (filter.siteKeys)
-      {
-        for (let i = 0; i < filter.siteKeys.length; i++)
-          delete this.keys[filter.siteKeys[i]];
-      }
-      else
-        this.whitelist.remove(filter);
-    }
+      this.whitelist.remove(filter);
     else
       this.blacklist.remove(filter);
 
@@ -370,7 +347,7 @@ CombinedMatcher.prototype =
    * simultaneously. For parameters see Matcher.matchesAny().
    * @see Matcher#matchesAny
    */
-  matchesAnyInternal: function(location, contentType, docDomain, thirdParty)
+  matchesAnyInternal: function(location, contentType, docDomain, thirdParty, sitekey)
   {
     let candidates = location.toLowerCase().match(/[a-z0-9%]{3,}/g);
     if (candidates === null)
@@ -383,12 +360,12 @@ CombinedMatcher.prototype =
       let substr = candidates[i];
       if (substr in this.whitelist.filterByKeyword)
       {
-        let result = this.whitelist._checkEntryMatch(substr, location, contentType, docDomain, thirdParty);
+        let result = this.whitelist._checkEntryMatch(substr, location, contentType, docDomain, thirdParty, sitekey);
         if (result)
           return result;
       }
       if (substr in this.blacklist.filterByKeyword && blacklistHit === null)
-        blacklistHit = this.blacklist._checkEntryMatch(substr, location, contentType, docDomain, thirdParty);
+        blacklistHit = this.blacklist._checkEntryMatch(substr, location, contentType, docDomain, thirdParty, sitekey);
     }
     return blacklistHit;
   },
@@ -396,13 +373,13 @@ CombinedMatcher.prototype =
   /**
    * @see Matcher#matchesAny
    */
-  matchesAny: function(location, contentType, docDomain, thirdParty)
+  matchesAny: function(location, contentType, docDomain, thirdParty, sitekey)
   {
-    let key = location + " " + contentType + " " + docDomain + " " + thirdParty;
+    let key = location + " " + contentType + " " + docDomain + " " + thirdParty + " " + sitekey;
     if (key in this.resultCache)
       return this.resultCache[key];
 
-    let result = this.matchesAnyInternal(location, contentType, docDomain, thirdParty);
+    let result = this.matchesAnyInternal(location, contentType, docDomain, thirdParty, sitekey);
 
     if (this.cacheEntries >= CombinedMatcher.maxCacheEntries)
     {
@@ -414,24 +391,6 @@ CombinedMatcher.prototype =
     this.cacheEntries++;
 
     return result;
-  },
-
-  /**
-   * Looks up whether any filters match the given website key.
-   */
-  matchesByKey: function(/**String*/ location, /**String*/ key, /**String*/ docDomain)
-  {
-    key = key.toUpperCase();
-    if (key in this.keys)
-    {
-      let filter = Filter.knownFilters[this.keys[key]];
-      if (filter && filter.matches(location, "DOCUMENT", docDomain, false))
-        return filter;
-      else
-        return null;
-    }
-    else
-      return null;
   }
 }
 

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-mozext/adblock-plus.git



More information about the Pkg-mozext-commits mailing list