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

lib/contentPolicy.js

Index: lib/contentPolicy.js
===================================================================
--- a/lib/contentPolicy.js
+++ b/lib/contentPolicy.js
@@ -170,29 +170,9 @@
       {
         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;
-            }
-          }
-        }
+        let siteKey = getSiteKey(node, testWnd);
+        match = Policy.isWhitelisted(testWndLocation, parentWndLocation, siteKey);

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

The logic here seems wrong to me. I've extended the description in
https://issues.adblockplus.org/ticket/431 to explain what I mean - the sitekey
shouldn't just apply to the current frame but also all frames below it.

What this boils down IMHO:

  let [sitekey, sitekeyWnd] = getSitekey(wnd);

This function should walk up from wnd, retrieve the sitekeys and validate them.
If a valid one is found it should be returned along with the window it belongs
to.

The whitelisting check then should create its own copies of these variables and
call getSitekey() again whenever it arrives at the frame belonging to the
sitekey (only after processing this frame and only if it isn't the top frame
already of course).

Note that this approach also makes sure the sitekey isn't retrieved twice
(currently you do it for the whitelisting check and actual request check).

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

Done.

 
         if (match instanceof WhitelistFilter)
         {
@@ -260,7 +240,9 @@
 
     if (!match && Prefs.enabled)
     {
-      match = defaultMatcher.matchesAny(locationText, Policy.typeDescr[contentType] || "", docDomain, thirdParty);
+      let docLocation = getWindowLocation(wnd);
+      let siteKey = getSiteKey(node, wnd);
+      match = defaultMatcher.matchesAny(locationText, Policy.typeDescr[contentType] || "", docDomain, thirdParty, docLocation, siteKey);
       if (match instanceof BlockingFilter && node.ownerDocument && !(contentType in Policy.nonVisual))
       {
         let prefCollapse = (match.collapse != null ? match.collapse : !Prefs.fastcollapse);
@@ -297,10 +279,11 @@
   /**
    * Checks whether a page is whitelisted.
    * @param {String} url
-   * @param {String} [parentUrl] location of the parent page
+   * @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)

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

If you change the parameters here you probably want to adjust the other callers:
isWindowWhitelisted() here, appSupport.js, ui.js.

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

Done.

   {
     if (!url)
       return null;
@@ -318,7 +301,7 @@
     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, parentUrl, siteKey);
     return (result instanceof WhitelistFilter ? result : null);
   },
 
@@ -691,6 +674,17 @@
 }
 
 /**
+ * Retrieves the sitekey of a window.
+ */
+function getSiteKey(node, wnd)

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

Why pass in node parameter that isn't used?

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

Done.

+{
+  if (wnd.document && wnd.document.documentElement)
+    return wnd.document.documentElement.getAttribute("data-adblockkey");
+
+  return null;
+}
+
+/**
  * Retrieves the location of a window.
  * @param wnd {nsIDOMWindow}
  * @return {String} window location or null on failure