Issue 3860 - Move request blocking logic to a seperate module

lib/whitelisting.js

Index: lib/whitelisting.js
===================================================================
--- a/lib/whitelisting.js
+++ b/lib/whitelisting.js
@@ -27,7 +27,7 @@
 let {port} = require("messaging");
 let devtools = require("devtools");
 
-let pagesWithKey = new ext.PageMap();
+let sitekeys = new ext.PageMap();
 
 function match(page, url, typeMask, docDomain, sitekey)
 {
@@ -84,6 +84,11 @@
   return match(page, page.url, typeMask);
 };
 
+port.on("filters.isPageWhitelisted", (message, sender) =>

[Sebastian Noack, 2016/03/23 14:02:06]

This is unrelated, but I moved this function up here just below the function
that is called, in order to improve the code locality,

[kzar, 2016/03/23 15:36:00]

Acknowledged. I can also see an argument for keeping all message listeners
together at the bottom of modules though. I leave it up to you anyway.

+{
+  return !!checkWhitelisted(sender.page);
+});
+
 function revalidateWhitelistingState(page)
 {
   FilterNotifier.triggerListeners(
@@ -92,32 +97,14 @@
   );
 }
 
-let revalidating = false;
-FilterNotifier.addListener((action, arg) =>
+FilterNotifier.addListener(action =>
 {
-  switch (action)
-  {
-    case "subscription.added":
-      if (arg instanceof DownloadableSubscription && !arg.lastDownload)
-        break;
-    case "subscription.removed":
-    case "subscription.disabled":
-    case "subscription.updated":
-    case "filter.added":
-    case "filter.removed":
-    case "filter.disabled":
-    case "load":
-      if (!revalidating)
-      {
-        revalidating = true;
-        ext.pages.query({}, pages =>
-        {
-          revalidating = false;
-          for (let page of pages)
-            revalidateWhitelistingState(page);
-        });
-      }
-  }
+  if (action == "filter.behaviorChanged")
+    ext.pages.query({}, pages =>
+    {
+      for (let page of pages)
+        revalidateWhitelistingState(page);
+    });
 });
 
 ext.pages.onLoading.addListener(revalidateWhitelistingState);
@@ -133,13 +120,13 @@
  */
 exports.getKey = function(page, frame)
 {
-  let urlsWithKey = pagesWithKey.get(page);
-  if (!urlsWithKey)
+  let keys = sitekeys.get(page);
+  if (!keys)
     return null;
 
   for (; frame != null; frame = frame.parent)
   {
-    let key = urlsWithKey[stringifyURL(frame.url)];
+    let key = keys[stringifyURL(frame.url)];
     if (key)
       return key;
   }
@@ -147,60 +134,49 @@
   return null;
 };
 
-function verifyKey(key, signature, url)
+function recordKey(token, page, url)
 {
-  let params = [
-    url.pathname + url.search, // REQUEST_URI
-    url.host,                  // HTTP_HOST
-    window.navigator.userAgent // HTTP_USER_AGENT
-  ];
-
-  return verifySignature(key, signature, params.join("\0"));
-}
-
-function recordKey(page, url, key)
-{
-  let urlsWithKey = pagesWithKey.get(page);
-
-  if (!urlsWithKey)
-  {
-    urlsWithKey = Object.create(null);
-    pagesWithKey.set(page, urlsWithKey);
-  }
-
-  urlsWithKey[stringifyURL(url)] = key;
-}
-
-let processKey =
-/**
- * Validates signatures given by the "X-Adblock-Key" response
- * header or the "data-adblockkey" attribute of the document
- * element. If the signature is valid, the public key will be
- * recorded and considered for the $sitekey filter option.
- *
- * @param {string} token  The base64-encoded public key and
- *                        signature separated by an underscrore.
- * @param {Page}   page
- * @param {Frame}  frame
- */
-exports.processKey = function(token, page, frame)
-{
-  if (token.indexOf("_") < 0)
+  let parts = token.split("_");
+  if (parts.length < 2)
     return;
 
-  let [key, signature] = token.split("_", 2);
-  key = key.replace(/=/g, "");
+  let key = parts[0].replace(/=/g, "");

[kzar, 2016/03/23 15:36:00]

Don't we rather want to trim the "="s from the start of the key than remove them
throughout the first part?

[Sebastian Noack, 2016/03/23 16:44:48]

We want to trim them from the end. However, they can only appear at the end in
base64 anyway. So this simpler regexp is saved. Note that we also used it
before.

[kzar, 2016/03/23 17:18:22]

Acknowledged.

+  let signature = parts[1];
+  let data = url.pathname + url.search + "\0" +
+             url.host + "\0" +
+             window.navigator.userAgent;
+  if (!verifySignature(key, signature, data))
+    return;
 
-  if (verifyKey(key, signature, frame.url))
-    recordKey(page, frame.url, key);
-};
+  let keys = sitekeys.get(page);
+  if (!keys)
+  {
+    keys = Object.create(null);
+    sitekeys.set(page, keys);
+  }
+  keys[stringifyURL(url)] = key;
+}
 
 port.on("filters.addKey", (message, sender) =>
 {
-  processKey(message.token, sender.page, sender.frame);
+  recordKey(message.token, sender.page, sender.frame.url);
 });
 
-port.on("filters.isPageWhitelisted", (message, sender) =>
+function onHeadersReceived(details)
 {
-  return !!checkWhitelisted(sender.page);
-});
+  let page = new ext.Page({id: details.tabId});
+
+  for (let header of details.responseHeaders)
+    if (header.name.toLowerCase() == "x-adblock-key" && header.value)
+      recordKey(header.value, page, new URL(details.url));
+}
+
+if (typeof chrome == "object")
+  chrome.webRequest.onHeadersReceived.addListener(

[kzar, 2016/03/23 15:36:00]

I guess this part should be in ext/chrome somewhere?

[Sebastian Noack, 2016/03/23 16:44:48]

Not really. There is no counterpart for Safari. And we generally don't add APIs
that only exist on one platform to the abstraction layer.

[kzar, 2016/03/23 17:18:22]

Acknowledged.

+    onHeadersReceived,
+    {
+      urls: ["http://*/*", "https://*/*"],
+      types: ["main_frame", "sub_frame"]
+    },
+    ["responseHeaders"]
+  );