Issue 433 - Made adjustments for $sitekey-related changes to the Matcher API

lib/whitelisting.js

Index: lib/whitelisting.js
===================================================================
--- a/lib/whitelisting.js
+++ b/lib/whitelisting.js
@@ -18,15 +18,16 @@
 let {defaultMatcher} = require("matcher");
 let {WhitelistFilter} = require("filterClasses");
 
-let pagesWithKeyException = new ext.PageMap();
+let pagesWithKey = new ext.PageMap();
 
-let isWhitelisted = exports.isWhitelisted = function(url, parentUrl, type)
+let isWhitelisted = exports.isWhitelisted = function(url, parentUrl, type, key)
 {
   let filter = defaultMatcher.matchesAny(
     stripFragmentFromURL(url),
     type || "DOCUMENT",
     extractHostFromURL(parentUrl || url),
-    false
+    false,
+    key
   );
 
   return (filter instanceof WhitelistFilter ? filter : null);
@@ -34,29 +35,36 @@
 
 let isFrameWhitelisted = exports.isFrameWhitelisted = function(page, frame, type)
 {
-  let urlsWithKeyException = pagesWithKeyException.get(page);
-
   for (; frame != null; frame = frame.parent)
   {
-    if (urlsWithKeyException && stripFragmentFromURL(frame.url) in urlsWithKeyException)
-      return true;
-    if (isWhitelisted(frame.url, (frame.parent || {}).url, type))
+    let key = getKey(page, frame);
+    if (isWhitelisted(frame.url, (frame.parent || {}).url, type, key))
       return true;
   }
 
   return false;
 };
 
-let verifyKeyException = function(token, url, docDomain)
+let getKey = exports.getKey = function(page, frame)
 {
-  let match = token.match(/((.*?)=*)_(.*)/);
-  if (!match)
-    return false;  // invalid format
+  let urlsWithKey = pagesWithKey.get(page);
+  if (!urlsWithKey)
+    return null;
 
-  let strippedKey = match[2];
-  if (!defaultMatcher.matchesByKey(url, strippedKey, docDomain))
-    return false;  // unknown key
+  while (true)
+  {
+    if (urlsWithKey[frame.url])
+      return urlsWithKey[frame.url];
 
+    if (frame === frame.parent)

[Wladimir Palant, 2014/09/10 18:27:04]

This isn't the DOM, it's our own structure - frame and frame.parent cannot be
equal, frame.parent can be null however.

The correct iterating solution is right above your code:

  for (; frame != null; frame = frame.parent)

[Thomas Greiner, 2014/09/15 09:24:59]

Done.

+      return null;
+
+    frame = frame.parent;
+  }
+}
+
+let verifyKey = function(key, signature, url, docDomain)
+{
   let uri = new URI(url);
   let params = [
     uri.path,                                               // REQUEST_URI
@@ -64,29 +72,33 @@
     window.navigator.userAgent                              // HTTP_USER_AGENT
   ];
 
-  let key = match[1];
-  let signature = match[3];
   return verifySignature(key, signature, params.join("\0"));
 };
 
-let recordKeyException = function(page, url)
+let recordKey = function(page, url, key)
 {
-  let urlsWithKeyException = pagesWithKeyException.get(page);
+  let urlsWithKey = pagesWithKey.get(page);
 
-  if (!urlsWithKeyException)
+  if (!urlsWithKey)
   {
-    urlsWithKeyException = {__proto__: null};
-    pagesWithKeyException.set(page, urlsWithKeyException);
+    urlsWithKey = {__proto__: null};
+    pagesWithKey.set(page, urlsWithKey);
   }
 
-  urlsWithKeyException[url] = null;
+  urlsWithKey[url] = key;
 };
 
-let processKeyException = exports.processKeyException = function(token, page, frame)
+let processKey = exports.processKey = function(sitekey, page, frame)
 {
   let url = stripFragmentFromURL(frame.url);
   let docDomain = extractHostFromURL((frame.parent || frame).url);
 
-  if (verifyKeyException(token, url, docDomain))
-    recordKeyException(page, url);
+  let keydata = sitekey.match(/((.*?)=*)_(.*)/);
+  if (!keydata)
+    return;
+
+  let key = keydata[1].replace(/=/g, "");
+  let signature = keydata[3];

[Wladimir Palant, 2014/09/10 18:27:04]

Why not use the same approach as in Firefox?

  if (token.indexOf("_") < 0)
    return;

  let [key, signature] = token.split("_", 2);
  key = key.replace(/=/g, "");
 
Implementing two different approaches for the same thing means twice as many
bugs. In particular, the regular expression above doesn't look like it would
always work correctly (I'd definitely include ^ and $) and removing trailing =
signs from the key explicitly is unnecessary as the regexp should have done it
already.

Looked up who introduced this code: apparently, Sebastian has hidden this change
in a much larger and unrelated review
(http://codereview.adblockplus.org/6346177440120832/). For reference: please
never do this...

[Thomas Greiner, 2014/09/15 09:24:59]

Done.

+  if (verifyKey(key, signature, url, docDomain))
+    recordKey(page, url, key);
 };