Issue 6543 - Match requests without tabId/frameId in their originating context

lib/whitelisting.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-present 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
@@ skipping 25 matching lines @@
   let thirdParty = !!docDomain && isThirdParty(url, docDomain);
   let urlString = stringifyURL(url);
 
   if (!docDomain)
     docDomain = getDecodedHostname(url);
 
   let filter = defaultMatcher.whitelist.matchesAny(
     urlString, typeMask, docDomain, thirdParty, sitekey
   );
 
-  if (filter)
+  if (filter && page)
     logWhitelistedDocument(page.id, urlString, typeMask, docDomain, filter);
 
   return filter;
 }
 
 let checkWhitelisted =
 /**
  * Gets the active whitelisting filter for the document associated
  * with the given page/frame, or null if it's not whitelisted.
  *
- * @param {Page}   page
- * @param {Frame}  [frame]
+ * @param {?Page}   page
+ * @param {?Frame} [frame]
+ * @param {?URL}   [originUrl]
  * @param {number} [typeMask=RegExpFilter.typeMap.DOCUMENT]
  * @return {?WhitelistFilter}
  */
-exports.checkWhitelisted = (page, frame, typeMask) =>
+exports.checkWhitelisted = (page, frame, originUrl,
+                            typeMask = RegExpFilter.typeMap.DOCUMENT) =>
 {
-  if (typeof typeMask == "undefined")
-    typeMask = RegExpFilter.typeMap.DOCUMENT;
+  if (frame || originUrl)
+  {
+    while (frame)
+    {
+      let parentFrame = frame.parent;
+      let filter = match(page, frame.url, typeMask,
+                         extractHostFromFrame(parentFrame, originUrl),
+                         getKey(page, frame, originUrl));
 
-  if (frame)
-  {
-    let filter = null;
+      if (filter)
+        return filter;
 
-    while (frame && !filter)
-    {
-      let {parent} = frame;
-      let docDomain = extractHostFromFrame(parent);
-      let sitekey = getKey(page, frame);
-
-      filter = match(page, frame.url, typeMask, docDomain, sitekey);
-      frame = parent;
+      frame = parentFrame;
     }
 
-    return filter;
+    return originUrl && match(page, originUrl, typeMask, null,

[kzar, 2018/04/03 12:49:30]

How come you changed the logic to `originUrl && ...` here?

[Sebastian Noack, 2018/04/04 01:04:45]

It's the same semantics for checkWhitelisted(), getKey() and
extractHostnameFromFrame(): We first try to retrieve information for the given
frame, falling back to its ancestors, and only if no information were found
(either because no frame was given in the first place or no frame in the
hierarchy was whitelisted), we fall back to the originUrl. In other words, we
give priority to information more local to the current document.

Therefore any whitelisting filter found in the frame hierarchy is now
immediately returned from inside the loop, and the code here is only reached in
case neither the frame nor any of its ancestors were whitelisted (or if no frame
was given in the first place), where we fall back to originUrl (if any).

+                              getKey(null, null, originUrl));
   }
 
-  return match(page, page.url, typeMask);
+  return page && match(page, page.url, typeMask);
 };
 
 port.on("filters.isWhitelisted", message =>
 {
   return !!checkWhitelisted(new ext.Page(message.tab));
 });
 
 port.on("filters.whitelist", message =>
 {
   let page = new ext.Page(message.tab);
@@ skipping 41 matching lines @@
   });
 });
 
 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
+ * @param {?Page}   page
+ * @param {?Frame}  frame
+ * @param {URL}    [originUrl]
  * @return {string}
  */
-exports.getKey = (page, frame) =>
+exports.getKey = (page, frame, originUrl) =>
 {
-  let keys = sitekeys.get(page);
-  if (!keys)
-    return null;
+  if (page)
+  {
+    let keys = sitekeys.get(page);
+    if (keys)
+    {
+      for (; frame; frame = frame.parent)
+      {
+        let key = keys.get(stringifyURL(frame.url));
+        if (key)
+          return key;
+      }
+    }
+  }
 
-  for (; frame != null; frame = frame.parent)
+  if (originUrl)
   {
-    let key = keys.get(stringifyURL(frame.url));
-    if (key)
-      return key;
+    for (let keys of sitekeys._map.values())
+    {
+      let key = keys.get(stringifyURL(originUrl));
+      if (key)
+        return key;
+    }
   }
 
   return null;
 };
 
 function checkKey(token, url)
 {
   let parts = token.split("_");
   if (parts.length < 2)
     return false;
@@ skipping 50 matching lines @@
 {
   browser.webRequest.onHeadersReceived.addListener(
     onHeadersReceived,
     {
       urls: ["http://*/*", "https://*/*"],
       types: ["main_frame", "sub_frame"]
     },
     ["responseHeaders"]
   );
 }