Bug 1441366 Use internal add-on IDs in browser error stacktrace reports. draft
authorMichael Kelly <mkelly@mozilla.com>
Mon, 26 Feb 2018 16:12:08 -0800
changeset 767595 935e19ff56b7850622beb0bc6c85e07c2e1ff297
parent 767347 80b4777a6421d8df4bb27ac23fb607c318a3006c
child 767596 663efcfec8a20464f60f725396f2da18e9af2318
child 767732 173b41380a2631680528cc52f1992a19ea080a28
push id102646
push userbmo:mkelly@mozilla.com
push dateWed, 14 Mar 2018 20:47:54 +0000
bugs1441366
milestone61.0a1
Bug 1441366 Use internal add-on IDs in browser error stacktrace reports. MozReview-Commit-ID: 7OC4utHLgXC
browser/modules/BrowserErrorReporter.jsm
browser/modules/test/browser/browser_BrowserErrorReporter.js
--- a/browser/modules/BrowserErrorReporter.jsm
+++ b/browser/modules/BrowserErrorReporter.jsm
@@ -50,19 +50,20 @@ const REPORTED_CATEGORIES = new Set([
  * The outgoing requests are designed to be compatible with Sentry. See
  * https://docs.sentry.io/clientdev/ for details on the data format that Sentry
  * expects.
  *
  * Errors may contain PII, such as in messages or local file paths in stack
  * traces; see bug 1426482 for privacy review and server-side mitigation.
  */
 class BrowserErrorReporter {
-  constructor(fetchMethod = this._defaultFetch) {
-    // A fake fetch is passed by the tests to avoid network connections
+  constructor(fetchMethod = this._defaultFetch, chromeOnly = true) {
+    // Test arguments for mocks and changing behavior
     this.fetch = fetchMethod;
+    this.chromeOnly = chromeOnly;
 
     // Values that don't change between error reports.
     this.requestBodyTemplate = {
       logger: "javascript",
       platform: "javascript",
       release: Services.appinfo.version,
       environment: UpdateUtils.getUpdateChannel(false),
       tags: {
@@ -128,53 +129,71 @@ class BrowserErrorReporter {
     try {
       message.QueryInterface(Ci.nsIScriptError);
     } catch (err) {
       return; // Not an error
     }
 
     const isWarning = message.flags & message.warningFlag;
     const isFromChrome = REPORTED_CATEGORIES.has(message.category);
-    if (!isFromChrome || isWarning) {
+    if ((this.chromeOnly && !isFromChrome) || isWarning) {
       return;
     }
 
     // Sample the amount of errors we send out
     const sampleRate = Number.parseFloat(Services.prefs.getCharPref(PREF_SAMPLE_RATE));
     if (!Number.isFinite(sampleRate) || (Math.random() >= sampleRate)) {
       return;
     }
 
+    const extensions = new Map();
+    for (let extension of WebExtensionPolicy.getActiveExtensions()) {
+      extensions.set(extension.mozExtensionHostname, extension);
+    }
+
+    // Replaces any instances of moz-extension:// URLs with internal UUIDs to use
+    // the add-on ID instead.
+    function mangleExtURL(string, anchored = true) {
+      let re = new RegExp(`${anchored ? "^" : ""}moz-extension://([^/]+)/`, "g");
+
+      return string.replace(re, (m0, m1) => {
+        let id = extensions.has(m1) ? extensions.get(m1).id : m1;
+        return `moz-extension://${id}/`;
+      });
+    }
+
     // Parse the error type from the message if present (e.g. "TypeError: Whoops").
     let errorMessage = message.errorMessage;
     let errorName = "Error";
     if (message.errorMessage.match(ERROR_PREFIX_RE)) {
       const parts = message.errorMessage.split(":");
       errorName = parts[0];
       errorMessage = parts.slice(1).join(":").trim();
     }
 
     const frames = [];
     let frame = message.stack;
     // Avoid an infinite loop by limiting traces to 100 frames.
     while (frame && frames.length < 100) {
-      frames.push(await this.normalizeStackFrame(frame));
+      const normalizedFrame = await this.normalizeStackFrame(frame);
+      normalizedFrame.module = mangleExtURL(normalizedFrame.module, false);
+      frames.push(normalizedFrame);
       frame = frame.parent;
     }
     // Frames are sent in order from oldest to newest.
     frames.reverse();
 
     const requestBody = Object.assign({}, this.requestBodyTemplate, {
       timestamp: new Date().toISOString().slice(0, -1), // Remove trailing "Z"
       project: Services.prefs.getCharPref(PREF_PROJECT_ID),
       exception: {
         values: [
           {
             type: errorName,
-            value: errorMessage,
+            value: mangleExtURL(errorMessage),
             stacktrace: {
               frames,
             }
           },
         ],
       },
       culprit: message.sourceName,
     });
--- a/browser/modules/test/browser/browser_BrowserErrorReporter.js
+++ b/browser/modules/test/browser/browser_BrowserErrorReporter.js
@@ -1,12 +1,13 @@
 /* Any copyright is dedicated to the Public Domain.
  * http://creativecommons.org/publicdomain/zero/1.0/ */
 "use strict";
 
+ChromeUtils.import("resource://testing-common/AddonTestUtils.jsm", this);
 ChromeUtils.import("resource:///modules/BrowserErrorReporter.jsm", this);
 
 Cu.importGlobalProperties(["fetch"]);
 
 /* global sinon */
 Services.scriptloader.loadSubScript("resource://testing-common/sinon-2.3.2.js");
 registerCleanupFunction(function() {
   delete window.sinon;
@@ -402,8 +403,50 @@ add_task(async function testFetchArgumen
       },
       "Reporter builds stack trace from scriptError correctly.",
     );
   });
 
   reporter.uninit();
   resetConsole();
 });
+
+add_task(async function testAddonIDMangle() {
+  const fetchSpy = sinon.spy();
+  // Passing false here disables category checks on errors, which would
+  // otherwise block errors directly from extensions.
+  const reporter = new BrowserErrorReporter(fetchSpy, false);
+  await SpecialPowers.pushPrefEnv({set: [
+    [PREF_ENABLED, true],
+    [PREF_SAMPLE_RATE, "1.0"],
+  ]});
+  reporter.init();
+
+  // Create and install test add-on
+  const id = "browsererrorcollection@example.com";
+  const extension = ExtensionTestUtils.loadExtension({
+    manifest: {
+      applications: {
+        gecko: { id },
+      },
+    },
+    background() {
+      throw new Error("testAddonIDMangle error");
+    },
+  });
+  await extension.startup();
+
+  // Just in case the error hasn't been thrown before add-on startup.
+  const call = await TestUtils.waitForCondition(
+    () => fetchCallForMessage(fetchSpy, "testAddonIDMangle error"),
+    `Wait for error from ${id} to be logged`,
+  );
+  const body = JSON.parse(call.args[1].body);
+  const stackFrame = body.exception.values[0].stacktrace.frames[0];
+  ok(
+    stackFrame.module.startsWith(`moz-extension://${id}/`),
+    "Stack frame filenames use the proper add-on ID instead of internal UUIDs.",
+  );
+
+  await extension.unload();
+  reporter.uninit();
+  resetConsole();
+});