Bug 1638007 - Check windowId/tabId before urls r=mixedpuppy
authorRob Wu <rob@robwu.nl>
Fri, 29 May 2020 20:22:30 +0000
changeset 533061 009021c1beb7f0e90517b65fff661d82762caa79
parent 533060 7018853b1370d7f64ed5c6e36fec3de64818418e
child 533062 435776ca8a2dd31bfdb5eef8b46382136083b8f3
push id37462
push usermalexandru@mozilla.com
push dateSat, 30 May 2020 09:46:43 +0000
treeherdermozilla-central@8aaca63ec5c6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmixedpuppy
bugs1638007
milestone78.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1638007 - Check windowId/tabId before urls r=mixedpuppy Differential Revision: https://phabricator.services.mozilla.com/D77174
toolkit/components/extensions/parent/ext-webRequest.js
toolkit/components/extensions/test/mochitest/test_ext_activityLog.html
toolkit/components/extensions/test/xpcshell/test_webRequest_ancestors.js
toolkit/components/extensions/test/xpcshell/test_webRequest_cookies.js
toolkit/components/extensions/test/xpcshell/test_webRequest_filtering.js
toolkit/components/extensions/webrequest/WebRequest.jsm
--- a/toolkit/components/extensions/parent/ext-webRequest.js
+++ b/toolkit/components/extensions/parent/ext-webRequest.js
@@ -1,18 +1,14 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 "use strict";
 
-// This file expects tabTracker to be defined in the global scope (e.g.
-// by ext-utils.js).
-/* global tabTracker */
-
 ChromeUtils.defineModuleGetter(
   this,
   "WebRequest",
   "resource://gre/modules/WebRequest.jsm"
 );
 
 // The guts of a WebRequest event handler.  Takes care of converting
 // |details| parameter when invoking listeners.
@@ -20,37 +16,17 @@ function registerEvent(
   extension,
   eventName,
   fire,
   filter,
   info,
   remoteTab = null
 ) {
   let listener = async data => {
-    let browserData = { tabId: -1, windowId: -1 };
-    if (data.browser) {
-      browserData = tabTracker.getBrowserData(data.browser);
-    }
-    if (filter.tabId != null && browserData.tabId != filter.tabId) {
-      return;
-    }
-    if (filter.windowId != null && browserData.windowId != filter.windowId) {
-      return;
-    }
-
-    let event = data.serialize(eventName);
-    event.tabId = browserData.tabId;
-    if (data.originAttributes) {
-      event.incognito = data.originAttributes.privateBrowsingId > 0;
-      if (extension.hasPermission("cookies")) {
-        event.cookieStoreId = getCookieStoreIdForOriginAttributes(
-          data.originAttributes
-        );
-      }
-    }
+    let event = data.serialize(eventName, extension);
     if (data.registerTraceableChannel) {
       // If this is a primed listener, no tabParent was passed in here,
       // but the convert() callback later in this function will be called
       // when the background page is started.  Force that to happen here
       // after which we'll have a valid tabParent.
       if (fire.wakeup) {
         await fire.wakeup();
       }
@@ -73,20 +49,20 @@ function registerEvent(
       Cu.reportError(
         "The webRequest.addListener filter doesn't overlap with host permissions."
       );
     }
   }
   if (filter.types) {
     filter2.types = filter.types;
   }
-  if (filter.tabId) {
+  if (filter.tabId !== undefined) {
     filter2.tabId = filter.tabId;
   }
-  if (filter.windowId) {
+  if (filter.windowId !== undefined) {
     filter2.windowId = filter.windowId;
   }
   if (filter.incognito !== undefined) {
     filter2.incognito = filter.incognito;
   }
 
   let blockingAllowed = extension.hasPermission("webRequestBlocking");
 
--- a/toolkit/components/extensions/test/mochitest/test_ext_activityLog.html
+++ b/toolkit/components/extensions/test/mochitest/test_ext_activityLog.html
@@ -222,23 +222,23 @@ add_task(async function test_api() {
         data: {
           args: [
             {
               url: expectedUrl,
               method: "GET",
               type: "main_frame",
               frameId: 0,
               parentFrameId: -1,
+              incognito: false,
               thirdParty: false,
               ip: null,
               frameAncestors: [],
               urlClassification: { firstParty: [], thirdParty: [] },
               requestSize: 0,
               responseSize: 0,
-              incognito: false,
             },
           ],
           result: {
             cancel: false,
           },
         },
       },
       // Test manifest based content script.
--- a/toolkit/components/extensions/test/xpcshell/test_webRequest_ancestors.js
+++ b/toolkit/components/extensions/test/xpcshell/test_webRequest_ancestors.js
@@ -1,20 +1,29 @@
 "use strict";
 
 var { WebRequest } = ChromeUtils.import(
   "resource://gre/modules/WebRequest.jsm"
 );
 var { PromiseUtils } = ChromeUtils.import(
   "resource://gre/modules/PromiseUtils.jsm"
 );
+var { ExtensionParent } = ChromeUtils.import(
+  "resource://gre/modules/ExtensionParent.jsm"
+);
 
 const server = createHttpServer({ hosts: ["example.com"] });
 server.registerDirectory("/data/", do_get_file("data"));
 
+add_task(async function setup() {
+  // When WebRequest.jsm is used directly instead of through ext-webRequest.js,
+  // ExtensionParent.apiManager is not automatically initialized. Do it here.
+  await ExtensionParent.apiManager.lazyInit();
+});
+
 add_task(async function test_ancestors_exist() {
   let deferred = PromiseUtils.defer();
   function onBeforeRequest(details) {
     info(`onBeforeRequest ${details.url}`);
     ok(
       typeof details.frameAncestors === "object",
       `ancestors exists [${typeof details.frameAncestors}]`
     );
--- a/toolkit/components/extensions/test/xpcshell/test_webRequest_cookies.js
+++ b/toolkit/components/extensions/test/xpcshell/test_webRequest_cookies.js
@@ -1,14 +1,18 @@
 "use strict";
 
 var { WebRequest } = ChromeUtils.import(
   "resource://gre/modules/WebRequest.jsm"
 );
 
+var { ExtensionParent } = ChromeUtils.import(
+  "resource://gre/modules/ExtensionParent.jsm"
+);
+
 const server = createHttpServer({ hosts: ["example.com"] });
 server.registerPathHandler("/", (request, response) => {
   response.setStatusLine(request.httpVersion, 200, "OK");
   if (request.hasHeader("Cookie")) {
     let value = request.getHeader("Cookie");
     if (value == "blinky=1") {
       response.setHeader("Set-Cookie", "dinky=1", false);
     }
@@ -65,16 +69,22 @@ function onResponseStarted(details) {
       equal(value, "dinky=1", "Cookie is correct");
       found = true;
     }
   }
   ok(found, "Saw cookie header");
   equal(countAfter, 1, "onResponseStarted hit once");
 }
 
+add_task(async function setup() {
+  // When WebRequest.jsm is used directly instead of through ext-webRequest.js,
+  // ExtensionParent.apiManager is not automatically initialized. Do it here.
+  await ExtensionParent.apiManager.lazyInit();
+});
+
 add_task(async function filter_urls() {
   // First load the URL so that we set cookie foopy=1.
   let contentPage = await ExtensionTestUtils.loadContentPage(URL);
   await contentPage.close();
 
   // Now load with WebRequest set up.
   WebRequest.onBeforeSendHeaders.addListener(onBeforeSendHeaders, null, [
     "blocking",
--- a/toolkit/components/extensions/test/xpcshell/test_webRequest_filtering.js
+++ b/toolkit/components/extensions/test/xpcshell/test_webRequest_filtering.js
@@ -1,14 +1,18 @@
 "use strict";
 
 var { WebRequest } = ChromeUtils.import(
   "resource://gre/modules/WebRequest.jsm"
 );
 
+var { ExtensionParent } = ChromeUtils.import(
+  "resource://gre/modules/ExtensionParent.jsm"
+);
+
 const server = createHttpServer({ hosts: ["example.com"] });
 server.registerDirectory("/data/", do_get_file("data"));
 
 const BASE = "http://example.com/data/";
 const URL = BASE + "/file_WebRequest_page2.html";
 
 var requested = [];
 
@@ -37,16 +41,22 @@ function onResponseStarted(details) {
 }
 
 const expected_urls = [
   BASE + "/file_style_good.css",
   BASE + "/file_style_bad.css",
   BASE + "/file_style_redirect.css",
 ];
 
+function resetExpectations() {
+  requested.length = 0;
+  sendHeaders.length = 0;
+  completed.length = 0;
+}
+
 function removeDupes(list) {
   let j = 0;
   for (let i = 1; i < list.length; i++) {
     if (list[i] != list[j]) {
       j++;
       if (i != j) {
         list[j] = list[i];
       }
@@ -61,16 +71,20 @@ function compareLists(list1, list2, kind
   list2.sort();
   removeDupes(list2);
   equal(String(list1), String(list2), `${kind} URLs correct`);
 }
 
 add_task(async function setup() {
   // Disable rcwn to make cache behavior deterministic.
   Services.prefs.setBoolPref("network.http.rcwn.enabled", false);
+
+  // When WebRequest.jsm is used directly instead of through ext-webRequest.js,
+  // ExtensionParent.apiManager is not automatically initialized. Do it here.
+  await ExtensionParent.apiManager.lazyInit();
 });
 
 add_task(async function filter_urls() {
   let filter = { urls: new MatchPatternSet(["*://*/*_style_*"]) };
 
   WebRequest.onBeforeRequest.addListener(onBeforeRequest, filter, ["blocking"]);
   WebRequest.onBeforeSendHeaders.addListener(onBeforeSendHeaders, filter, [
     "blocking",
@@ -85,16 +99,17 @@ add_task(async function filter_urls() {
   compareLists(completed, expected_urls, "completed");
 
   WebRequest.onBeforeRequest.removeListener(onBeforeRequest);
   WebRequest.onBeforeSendHeaders.removeListener(onBeforeSendHeaders);
   WebRequest.onResponseStarted.removeListener(onResponseStarted);
 });
 
 add_task(async function filter_types() {
+  resetExpectations();
   let filter = { types: ["stylesheet"] };
 
   WebRequest.onBeforeRequest.addListener(onBeforeRequest, filter, ["blocking"]);
   WebRequest.onBeforeSendHeaders.addListener(onBeforeSendHeaders, filter, [
     "blocking",
   ]);
   WebRequest.onResponseStarted.addListener(onResponseStarted, filter);
 
@@ -104,8 +119,56 @@ add_task(async function filter_types() {
   compareLists(requested, expected_urls, "requested");
   compareLists(sendHeaders, expected_urls, "sendHeaders");
   compareLists(completed, expected_urls, "completed");
 
   WebRequest.onBeforeRequest.removeListener(onBeforeRequest);
   WebRequest.onBeforeSendHeaders.removeListener(onBeforeSendHeaders);
   WebRequest.onResponseStarted.removeListener(onResponseStarted);
 });
+
+add_task(async function filter_windowId() {
+  resetExpectations();
+  // Check that adding windowId will exclude non-matching requests.
+  // test_ext_webrequest_filter.html provides coverage for matching requests.
+  let filter = { urls: new MatchPatternSet(["*://*/*_style_*"]), windowId: 0 };
+
+  WebRequest.onBeforeRequest.addListener(onBeforeRequest, filter, ["blocking"]);
+  WebRequest.onBeforeSendHeaders.addListener(onBeforeSendHeaders, filter, [
+    "blocking",
+  ]);
+  WebRequest.onResponseStarted.addListener(onResponseStarted, filter);
+
+  let contentPage = await ExtensionTestUtils.loadContentPage(URL);
+  await contentPage.close();
+
+  compareLists(requested, [], "requested");
+  compareLists(sendHeaders, [], "sendHeaders");
+  compareLists(completed, [], "completed");
+
+  WebRequest.onBeforeRequest.removeListener(onBeforeRequest);
+  WebRequest.onBeforeSendHeaders.removeListener(onBeforeSendHeaders);
+  WebRequest.onResponseStarted.removeListener(onResponseStarted);
+});
+
+add_task(async function filter_tabId() {
+  resetExpectations();
+  // Check that adding tabId will exclude non-matching requests.
+  // test_ext_webrequest_filter.html provides coverage for matching requests.
+  let filter = { urls: new MatchPatternSet(["*://*/*_style_*"]), tabId: 0 };
+
+  WebRequest.onBeforeRequest.addListener(onBeforeRequest, filter, ["blocking"]);
+  WebRequest.onBeforeSendHeaders.addListener(onBeforeSendHeaders, filter, [
+    "blocking",
+  ]);
+  WebRequest.onResponseStarted.addListener(onResponseStarted, filter);
+
+  let contentPage = await ExtensionTestUtils.loadContentPage(URL);
+  await contentPage.close();
+
+  compareLists(requested, [], "requested");
+  compareLists(sendHeaders, [], "sendHeaders");
+  compareLists(completed, [], "completed");
+
+  WebRequest.onBeforeRequest.removeListener(onBeforeRequest);
+  WebRequest.onBeforeSendHeaders.removeListener(onBeforeSendHeaders);
+  WebRequest.onResponseStarted.removeListener(onResponseStarted);
+});
--- a/toolkit/components/extensions/webrequest/WebRequest.jsm
+++ b/toolkit/components/extensions/webrequest/WebRequest.jsm
@@ -13,35 +13,46 @@ const EXPORTED_SYMBOLS = ["WebRequest"];
 const { nsIHttpActivityObserver, nsISocketTransport } = Ci;
 
 const { Services } = ChromeUtils.import("resource://gre/modules/Services.jsm");
 const { XPCOMUtils } = ChromeUtils.import(
   "resource://gre/modules/XPCOMUtils.jsm"
 );
 
 XPCOMUtils.defineLazyModuleGetters(this, {
+  ExtensionParent: "resource://gre/modules/ExtensionParent.jsm",
   ExtensionUtils: "resource://gre/modules/ExtensionUtils.jsm",
   WebRequestUpload: "resource://gre/modules/WebRequestUpload.jsm",
   SecurityInfo: "resource://gre/modules/SecurityInfo.jsm",
 });
 
+// WebRequest.jsm's only consumer is ext-webRequest.js, so we can depend on
+// the apiManager.global being initialized.
+XPCOMUtils.defineLazyGetter(this, "tabTracker", () => {
+  return ExtensionParent.apiManager.global.tabTracker;
+});
+XPCOMUtils.defineLazyGetter(this, "getCookieStoreIdForOriginAttributes", () => {
+  return ExtensionParent.apiManager.global.getCookieStoreIdForOriginAttributes;
+});
+
 function runLater(job) {
   Services.tm.dispatchToMainThread(job);
 }
 
 function parseFilter(filter) {
   if (!filter) {
     filter = {};
   }
 
-  // FIXME: Support windowId filtering.
   return {
     urls: filter.urls || null,
     types: filter.types || null,
-    incognito: filter.incognito !== undefined ? filter.incognito : null,
+    tabId: filter.tabId ?? null,
+    windowId: filter.windowId ?? null,
+    incognito: filter.incognito ?? null,
   };
 }
 
 function parseExtra(extra, allowed = [], optionsObj = {}) {
   if (extra) {
     for (let ex of extra) {
       if (!allowed.includes(ex)) {
         throw new ExtensionUtils.ExtensionError(`Invalid option ${ex}`);
@@ -220,30 +231,38 @@ const OPTIONAL_PROPERTIES = [
   "proxyInfo",
   "ip",
   "frameAncestors",
   "urlClassification",
   "requestSize",
   "responseSize",
 ];
 
-function serializeRequestData(eventName) {
+function serializeRequestData(eventName, extension) {
   let data = {
     requestId: this.requestId,
     url: this.url,
     originUrl: this.originUrl,
     documentUrl: this.documentUrl,
     method: this.method,
     type: this.type,
     timeStamp: Date.now(),
-    frameId: this.windowId,
+    tabId: this.tabId,
+    frameId: this.frameId,
     parentFrameId: this.parentWindowId,
+    incognito: this.incognito,
     thirdParty: this.thirdParty,
   };
 
+  if (extension) {
+    if (extension.hasPermission("cookies")) {
+      data.cookieStoreId = this.cookieStoreId;
+    }
+  }
+
   if (MAYBE_CACHED_EVENTS.has(eventName)) {
     data.fromCache = !!this.fromCache;
   }
 
   for (let opt of OPTIONAL_PROPERTIES) {
     if (typeof this[opt] !== "undefined") {
       data[opt] = this[opt];
     }
@@ -698,46 +717,51 @@ HttpObserverManager = {
       lastActivity !==
         nsIHttpActivityObserver.ACTIVITY_SUBTYPE_TRANSACTION_CLOSE
     ) {
       channel.lastActivity = activitySubtype;
     }
   },
 
   getRequestData(channel, extraData) {
-    let originAttributes =
-      channel.loadInfo && channel.loadInfo.originAttributes;
+    let originAttributes = channel.loadInfo?.originAttributes;
     let data = {
       requestId: String(channel.id),
       url: channel.finalURL,
       method: channel.method,
-      browser: channel.browserElement,
       type: channel.type,
       fromCache: channel.fromCache,
-      originAttributes,
+      incognito: originAttributes?.privateBrowsingId > 0,
       thirdParty: channel.thirdParty,
 
       originUrl: channel.originURL || undefined,
       documentUrl: channel.documentURL || undefined,
 
-      windowId: channel.windowId,
+      tabId: this.getBrowserData(channel).tabId,
+      frameId: channel.windowId,
       parentWindowId: channel.parentWindowId,
 
       frameAncestors: channel.frameAncestors || undefined,
 
       ip: channel.remoteAddress,
 
       proxyInfo: channel.proxyInfo,
 
       serialize: serializeRequestData,
       requestSize: channel.requestSize,
       responseSize: channel.responseSize,
       urlClassification: channel.urlClassification,
     };
 
+    if (originAttributes) {
+      data.cookieStoreId = getCookieStoreIdForOriginAttributes(
+        originAttributes
+      );
+    }
+
     return Object.assign(data, extraData);
   },
 
   handleEvent(event) {
     let channel = event.currentTarget;
     switch (event.type) {
       case "error":
         this.runChannelListener(channel, "onErrorOccurred", {
@@ -764,30 +788,52 @@ HttpObserverManager = {
     "onBeforeRequest",
     "onBeforeSendHeaders",
     "onSendHeaders",
     "onHeadersReceived",
     "onAuthRequired",
     "onBeforeRedirect",
   ]),
 
+  getBrowserData(wrapper) {
+    let browserData = wrapper._browserData;
+    if (!browserData) {
+      if (wrapper.browserElement) {
+        browserData = tabTracker.getBrowserData(wrapper.browserElement);
+      } else {
+        browserData = { tabId: -1, windowId: -1 };
+      }
+      wrapper._browserData = browserData;
+    }
+    return browserData;
+  },
+
   runChannelListener(channel, kind, extraData = null) {
     let handlerResults = [];
     let requestHeaders;
     let responseHeaders;
 
     try {
       if (kind !== "onErrorOccurred" && channel.errorString) {
         return;
       }
 
       let registerFilter = this.FILTER_TYPES.has(kind);
       let commonData = null;
       let requestBody;
       this.listeners[kind].forEach((opts, callback) => {
+        if (opts.filter.tabId !== null || opts.filter.windowId !== null) {
+          const { tabId, windowId } = this.getBrowserData(channel);
+          if (
+            (opts.filter.tabId !== null && tabId != opts.filter.tabId) ||
+            (opts.filter.windowId !== null && windowId != opts.filter.windowId)
+          ) {
+            return;
+          }
+        }
         if (!channel.matches(opts.filter, opts.policy, extraData)) {
           return;
         }
 
         if (!commonData) {
           commonData = this.getRequestData(channel, extraData);
           if (this.STATUS_TYPES.has(kind)) {
             commonData.statusCode = channel.statusCode;