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

lib/requestBlocker.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 55 matching lines @@
   // WEBRTC gets addressed through a workaround, even if the webRequest API is
   // lacking support to block this kind of a request.
   yield "WEBRTC";
 
   // POPUP, CSP and ELEMHIDE filters aren't mapped to resource types.
   yield "POPUP";
   yield "ELEMHIDE";
   yield "CSP";
 }());
 
-function onBeforeRequestAsync(tabId, url, type, docDomain,
-                              thirdParty, sitekey,
-                              specificOnly, filter)
+function getRelatedTabIds(details)
 {
-  let tabIds = tabId != -1 ? [tabId] : [];
+  // This is the common case, the request is associated with a single tab.
+  // If tabId is -1, its not (e.g. the request was sent by
+  // a Service/Shared Worker) and we have to identify the related tabs.
+  if (details.tabId != -1)
+    return Promise.resolve([details.tabId]);
 
-  if (filter)
-    FilterNotifier.emit("filter.hitCount", filter, 0, 0, tabIds);
+  // Firefox >=48 provides "originUrl" indicating the URL of the tab
+  // that caused this request. In case of Service/Shared Worker,
+  // this is URL of the tab that caused the worker to be spawned.
+  if (details.originUrl)

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

Nit: Please use braces since the body spans multiple lines. (same below)

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

No longer relevant (the block has only one line now, same below).

+    return browser.tabs.query({url: details.originUrl}).then(
+      tabs => tabs.map(tab => tab.id)
+    );
 
-  devtools.logRequest(
-    tabIds, url, type, docDomain,
-    thirdParty, sitekey,
-    specificOnly, filter
-  );
+  // Chromium >=63 provides "intiator" which is equivalent to "originUrl" on
+  // Firefox except that its not a full URL but just an origin (proto + host).
+  // So we have to find each tab with an URL of the same origin.
+  if (details.initiator)
+    return browser.tabs.query({}).then(tabs =>

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

Couldn't we pass something like `details.initiator + "*" ` for the url part of
the query? I figure we could avoid filtering the URLs manually later that way.

https://developer.chrome.com/extensions/tabs#method-query
https://developer.chrome.com/extensions/match_patterns

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

In fact we can. I wasn't aware that tab.query() accepts URL patterns. Done.

[kzar, 2018/04/05 10:31:51]

Cool, I assumed I was wrong somehow :p. This looks much cleaner now.

+    {
+      let tabIds = [];
+      for (let tab of tabs)
+      {
+        if (new URL(tab.url).origin == details.initiator)
+          tabIds.push(tab.id);

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

Wouldn't this mean we sometimes log a request for a tab incorrectly, say if the
user has two tabs open for the same domain?

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

This usually means that the request was sent by a Service Worker controlling
multiple tabs, in which case logging the request for all those tabs is correct.
Theoretically we can get false positives, e.g. if the request was sent by a
Service Worker controlling https://example.com/foo while there is another tab
for https://example.com/bar which isn't controlled by that Service Worker.
However, by default ServiceWorkers have a domain-wide scope and limiting the
scope requires to jump through some hoops. Also given the limited information,
Chrome provides us with, this seems to be the best we can do. Also FWIW,
currently, those requests are logged to all tabs. So we don't degrade here.

[kzar, 2018/04/05 10:31:51]

Acknowledged.

+      }
+      return tabIds;
+    });
+
+  return Promise.resolve([]);
+}
+
+function logRequest(details, url, type, docDomain, thirdParty,
+                    sitekey, specificOnly, filter)
+{
+  getRelatedTabIds(details).then(tabIds =>
+  {
+    if (filter)
+      FilterNotifier.emit("filter.hitCount", filter, 0, 0, tabIds);
+
+    devtools.logRequest(
+      tabIds, url, type, docDomain,
+      thirdParty, sitekey,
+      specificOnly, filter
+    );
+  });
 }
 
 browser.webRequest.onBeforeRequest.addListener(details =>
 {
   // Never block top-level documents.
   if (details.type == "main_frame")
     return;
 
   // Filter out requests from non web protocols. Ideally, we'd explicitly
   // specify the protocols we are interested in (i.e. http://, https://,
   // ws:// and wss://) with the url patterns, given below, when adding this
   // listener. But unfortunately, Chrome <=57 doesn't support the WebSocket
   // protocol and is causing an error if it is given.
   let url = new URL(details.url);
   if (url.protocol != "http:" && url.protocol != "https:" &&
       url.protocol != "ws:" && url.protocol != "wss:")
     return;
 
-  // Firefox (only) allows to intercept requests sent by the browser
-  // and other extensions. We don't want to block these.
+  let originUrl = null;
   if (details.originUrl)
   {
-    let originUrl = new URL(details.originUrl);
+    originUrl = new URL(details.originUrl);
+
+    // Firefox (only) allows to intercept requests sent by the browser
+    // and other extensions. We don't want to block these.
     if (originUrl.protocol == "chrome:" ||
         originUrl.protocol == "moz-extension:")
       return;
   }
+  // Fallback to "initiator" on Chrome >=63. It doesn't include the
+  // path (unlike "originUrl" on Firefox), but is still good enough
+  // (in case the tab/frame is unknown) for the $domain filter option
+  // and most document exception rules which only match the domain part.
+  else if (details.initiator)
+    originUrl = new URL(details.initiator);
 
-  let frame = ext.getFrame(
-    details.tabId,
-    // We are looking for the frame that contains the element which
-    // has triggered this request. For most requests (e.g. images) we
-    // can just use the request's frame ID, but for subdocument requests
-    // (e.g. iframes) we must instead use the request's parent frame ID.
-    details.type == "sub_frame" ? details.parentFrameId : details.frameId
-  );
+  let page = null;
+  let frame = null;
+  if (details.tabId != -1)
+  {
+    page = new ext.Page({id: details.tabId});
+    frame = ext.getFrame(
+      details.tabId,
+      // We are looking for the frame that contains the element which
+      // has triggered this request. For most requests (e.g. images) we
+      // can just use the request's frame ID, but for subdocument requests
+      // (e.g. iframes) we must instead use the request's parent frame ID.
+      details.type == "sub_frame" ? details.parentFrameId : details.frameId
+    );
+  }
 
-  let docDomain = null;
-  let sitekey = null;
-  let thirdParty = false;
-  let specificOnly = false;
-
-  if (frame)
-  {
-    let page = new ext.Page({id: details.tabId});
-
-    if (checkWhitelisted(page, frame))
-      return;
-
-    docDomain = extractHostFromFrame(frame);
-    sitekey = getKey(page, frame);
-    thirdParty = isThirdParty(url, docDomain);
-    specificOnly = !!checkWhitelisted(page, frame,
-                                      RegExpFilter.typeMap.GENERICBLOCK);
-  }
+  if (checkWhitelisted(page, frame, originUrl))
+    return;
 
   let urlString = stringifyURL(url);
   let type = resourceTypes.get(details.type) || "OTHER";
+  let docDomain = extractHostFromFrame(frame, originUrl);
+  let thirdParty = isThirdParty(url, docDomain);
+  let sitekey = getKey(page, frame, originUrl);
+  let specificOnly = !!checkWhitelisted(page, frame, originUrl,
+                                        RegExpFilter.typeMap.GENERICBLOCK);
+
   let filter = defaultMatcher.matchesAny(
     urlString, RegExpFilter.typeMap[type],
     docDomain, thirdParty, sitekey, specificOnly
   );
 
-  setTimeout(onBeforeRequestAsync, 0, details.tabId, urlString,
-                                      type, docDomain,
-                                      thirdParty, sitekey,
-                                      specificOnly, filter);
+  logRequest(details, urlString, type, docDomain,
+             thirdParty, sitekey, specificOnly, filter);
 
   if (filter instanceof BlockingFilter)
     return {cancel: true};
 }, {urls: ["<all_urls>"]}, ["blocking"]);
 
 port.on("filters.collapse", (message, sender) =>
 {
   if (checkWhitelisted(sender.page, sender.frame))
     return false;
 
   let typeMask = RegExpFilter.typeMap[message.mediatype];
   let documentHost = extractHostFromFrame(sender.frame);
   let sitekey = getKey(sender.page, sender.frame);
   let blocked = false;
 
   let specificOnly = checkWhitelisted(
-    sender.page, sender.frame,
+    sender.page, sender.frame, null,
     RegExpFilter.typeMap.GENERICBLOCK
   );
 
   for (let url of message.urls)
   {
     let urlObj = new URL(url, message.baseURL);
     let filter = defaultMatcher.matchesAny(
       stringifyURL(urlObj),
       typeMask, documentHost,
       isThirdParty(urlObj, documentHost),
@@ skipping 55 matching lines @@
 
 port.on("request.blockedByRTCWrapper", (msg, sender) =>
 {
   return ext.webRequest.onBeforeRequest._dispatch(
      new URL(msg.url),
      "webrtc",
      sender.page,
      sender.frame
   ).includes(false);
 });