Issue 3860 - Move request blocking logic to a seperate module

lib/whitelisting.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-2016 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/>.
  */
 
 /** @module whitelisting */
 
 "use strict";
 
 let {defaultMatcher} = require("matcher");
 let {RegExpFilter} = require("filterClasses");
 let {DownloadableSubscription} = require("subscriptionClasses");
 let {FilterNotifier} = require("filterNotifier");
 let {stringifyURL, getDecodedHostname, extractHostFromFrame, isThirdParty} = require("url");
 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)
 {
   let thirdParty = !!docDomain && isThirdParty(url, docDomain);
   let urlString = stringifyURL(url);
 
   if (!docDomain)
     docDomain = getDecodedHostname(url);
 
   let filter = defaultMatcher.whitelist.matchesAny(
@@ skipping 36 matching lines @@
       filter = match(page, frame.url, typeMask, docDomain, sitekey);
       frame = parent;
     }
 
     return filter;
   }
 
   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(
     "page.WhitelistingStateRevalidate",
     page, checkWhitelisted(page)
   );
 }
 
-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);
 
 let getKey =
 /**
  * Gets the public key, previously recorded for the given page
  * and frame, to be considered for the $sitekey filter option.
  *
  * @param {Page}  page
  * @param {Frame} frame
  * @return {string}
  */
 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;
   }
 
   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"]
+  );