Bug 1451702: Mangle file: and jar: paths in browser error reports. r=gijs
☠☠ backed out by 94415f8067ec ☠ ☠
authorMichael Kelly <mkelly@mozilla.com>
Thu, 17 May 2018 15:05:18 -0700
changeset 418788 8ad87d070ec9
parent 418787 fb806e16195a
child 418789 272880e5ca08
push id34013
push userdluca@mozilla.com
push date2018-05-18 09:56 +0000
treeherdermozilla-central@11ee70f24ea5 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgijs
bugs1451702
milestone62.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 1451702: Mangle file: and jar: paths in browser error reports. r=gijs Differential Revision: https://phabricator.services.mozilla.com/D1292 MozReview-Commit-ID: 1Yfz1UfIFae
browser/modules/BrowserErrorReporter.jsm
browser/modules/test/browser/browser_BrowserErrorReporter.js
--- a/browser/modules/BrowserErrorReporter.jsm
+++ b/browser/modules/BrowserErrorReporter.jsm
@@ -2,16 +2,17 @@
  * 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/. */
 
 ChromeUtils.import("resource://gre/modules/AppConstants.jsm");
 ChromeUtils.import("resource://gre/modules/Services.jsm");
 ChromeUtils.import("resource://gre/modules/Timer.jsm");
 ChromeUtils.import("resource://gre/modules/XPCOMUtils.jsm");
 
+ChromeUtils.defineModuleGetter(this, "FileUtils", "resource://gre/modules/FileUtils.jsm");
 ChromeUtils.defineModuleGetter(this, "Log", "resource://gre/modules/Log.jsm");
 ChromeUtils.defineModuleGetter(this, "UpdateUtils", "resource://gre/modules/UpdateUtils.jsm");
 
 Cu.importGlobalProperties(["fetch", "URL"]);
 
 var EXPORTED_SYMBOLS = ["BrowserErrorReporter"];
 
 const CONTEXT_LINES = 5;
@@ -243,16 +244,17 @@ class BrowserErrorReporter {
       },
     };
 
     const transforms = [
       addErrorMessage,
       addStacktrace,
       addModule,
       mangleExtensionUrls,
+      mangleFilePaths,
       tagExtensionErrors,
     ];
     for (const transform of transforms) {
       await transform(message, exceptionValue, requestBody);
     }
 
     const url = new URL(Services.prefs.getCharPref(PREF_SUBMIT_URL));
     url.searchParams.set("sentry_client", `${SDK_NAME}/${SDK_VERSION}`);
@@ -266,20 +268,20 @@ class BrowserErrorReporter {
           "Content-Type": "application/json",
           "Accept": "application/json",
         },
         // Sentry throws an auth error without a referrer specified.
         referrer: "https://fake.mozilla.org",
         body: JSON.stringify(requestBody)
       });
       Services.telemetry.scalarAdd(TELEMETRY_ERROR_REPORTED, 1);
-      this.logger.debug("Sent error successfully.");
+      this.logger.debug(`Sent error "${message.errorMessage}" successfully.`);
     } catch (error) {
       Services.telemetry.scalarAdd(TELEMETRY_ERROR_REPORTED_FAIL, 1);
-      this.logger.warn(`Failed to send error: ${error}`);
+      this.logger.warn(`Failed to send error "${message.errorMessage}": ${error}`);
     }
   }
 }
 
 function defaultFetch(...args) {
   // Do not make network requests while running in automation
   if (Cu.isInAutomation) {
     return null;
@@ -362,28 +364,62 @@ function mangleExtensionUrls(message, ex
 
   // Replaces any instances of moz-extension:// URLs with internal UUIDs to use
   // the add-on ID instead.
   function mangleExtURL(string, anchored = true) {
     if (!string) {
       return string;
     }
 
-    let re = new RegExp(`${anchored ? "^" : ""}moz-extension://([^/]+)/`, "g");
-
+    const re = new RegExp(`${anchored ? "^" : ""}moz-extension://([^/]+)/`, "g");
     return string.replace(re, (m0, m1) => {
-      let id = extensions.has(m1) ? extensions.get(m1).id : m1;
+      const id = extensions.has(m1) ? extensions.get(m1).id : m1;
       return `moz-extension://${id}/`;
     });
   }
 
   exceptionValue.value = mangleExtURL(exceptionValue.value, false);
   exceptionValue.module = mangleExtURL(exceptionValue.module);
   for (const frame of exceptionValue.stacktrace.frames) {
     frame.module = mangleExtURL(frame.module);
   }
 }
 
 function tagExtensionErrors(message, exceptionValue, requestBody) {
   requestBody.tags.isExtensionError = !!(
       exceptionValue.module && exceptionValue.module.startsWith("moz-extension://")
   );
 }
+
+/**
+ * Alters file: and jar: paths to remove leading file paths that may contain
+ * user-identifying or platform-specific paths.
+ */
+function mangleFilePaths(message, exceptionValue) {
+  function transformFilePath(path) {
+    try {
+      const uri = Services.io.newURI(path);
+      if (uri.schemeIs("jar")) {
+        return uri.filePath;
+      }
+      if (uri.schemeIs("file")) {
+        const prefixes = {
+          greDir: FileUtils.getDir("GreD", []).path,
+          profileDir: FileUtils.getDir("ProfD", []).path,
+        };
+        for (const [name, prefix] of Object.entries(prefixes)) {
+          if (uri.filePath.startsWith(prefix)) {
+            return uri.filePath.replace(prefix, `[${name}]`);
+          }
+        }
+
+        return "[UNKNOWN_LOCAL_FILEPATH]";
+      }
+    } catch (err) {}
+
+    return path;
+  }
+
+  exceptionValue.module = transformFilePath(exceptionValue.module);
+  for (const frame of exceptionValue.stacktrace.frames) {
+    frame.module = transformFilePath(frame.module);
+  }
+}
--- a/browser/modules/test/browser/browser_BrowserErrorReporter.js
+++ b/browser/modules/test/browser/browser_BrowserErrorReporter.js
@@ -1,15 +1,16 @@
 /* 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);
 ChromeUtils.import("resource://gre/modules/AppConstants.jsm", this);
+ChromeUtils.import("resource://gre/modules/FileUtils.jsm", this);
+ChromeUtils.import("resource://testing-common/AddonTestUtils.jsm", this);
 
 /* global sinon */
 Services.scriptloader.loadSubScript("resource://testing-common/sinon-2.3.2.js");
 registerCleanupFunction(function() {
   delete window.sinon;
 });
 
 const PREF_ENABLED = "browser.chrome.errorReporter.enabled";
@@ -20,29 +21,55 @@ const PREF_SUBMIT_URL = "browser.chrome.
 const TELEMETRY_ERROR_COLLECTED = "browser.errors.collected_count";
 const TELEMETRY_ERROR_COLLECTED_FILENAME = "browser.errors.collected_count_by_filename";
 const TELEMETRY_ERROR_COLLECTED_STACK = "browser.errors.collected_with_stack_count";
 const TELEMETRY_ERROR_REPORTED = "browser.errors.reported_success_count";
 const TELEMETRY_ERROR_REPORTED_FAIL = "browser.errors.reported_failure_count";
 const TELEMETRY_ERROR_SAMPLE_RATE = "browser.errors.sample_rate";
 
 function createScriptError(options = {}) {
-  const scriptError = Cc["@mozilla.org/scripterror;1"].createInstance(Ci.nsIScriptError);
+  let scriptError = Cc["@mozilla.org/scripterror;1"].createInstance(Ci.nsIScriptError);
   scriptError.init(
     options.message || "",
     "sourceName" in options ? options.sourceName : null,
     options.sourceLine || null,
     options.lineNumber || null,
     options.columnNumber || null,
     options.flags || Ci.nsIScriptError.errorFlag,
     options.category || "chrome javascript",
   );
+
+  // You can't really set the stack of a scriptError in JS, so we shadow it instead.
+  if (options.stack) {
+    scriptError = Object.create(scriptError, {
+      stack: {
+        value: createStack(options.stack),
+      },
+    });
+  }
+
   return scriptError;
 }
 
+function createStack(frames) {
+  for (let k = 0; k < frames.length - 1; k++) {
+    frames[k].parent = frames[k + 1];
+  }
+  return frames[0];
+}
+
+function frame(options = {}) {
+  return Object.assign({
+    functionDisplayName: "fooFunction",
+    source: "resource://modules/BrowserErrorReporter.jsm",
+    line: 5,
+    column: 10,
+  }, options);
+}
+
 function noop() {
   // Do nothing
 }
 
 // Clears the console of any previous messages. Should be called at the end of
 // each test that logs to the console.
 function resetConsole() {
   Services.console.logStringMessage("");
@@ -502,22 +529,17 @@ add_task(async function testScalars() {
     createScriptError({message: "More no name", sourceName: "resource://gre/modules/Bar.jsm"}),
     createScriptError({message: "Yeah sures", sourceName: "unsafe://gre/modules/Bar.jsm"}),
     createScriptError({message: "Addon", sourceName: "moz-extension://foo/Bar.jsm"}),
     createScriptError({
       message: "long",
       sourceName: "resource://gre/modules/long/long/long/long/long/long/long/long/long/long/",
     }),
     {message: "Not a scripterror instance."},
-
-    // No easy way to create an nsIScriptError with a stack, so let's pretend.
-    Object.create(
-      createScriptError({message: "Whatever"}),
-      {stack: {value: new Error().stack}},
-    ),
+    createScriptError({message: "Whatever", stack: [frame()]}),
   ];
 
   // Use observe to avoid errors from other code messing up our counts.
   for (const message of messages) {
     await reporter.observe(message);
   }
 
   await SpecialPowers.pushPrefEnv({set: [[PREF_SAMPLE_RATE, "0.0"]]});
@@ -617,8 +639,51 @@ add_task(async function testCollectedFil
       shouldMatch
         ? `${TELEMETRY_ERROR_COLLECTED_FILENAME} logs a key for ${filename}.`
         : `${TELEMETRY_ERROR_COLLECTED_FILENAME} logs a FILTERED key for ${filename}.`,
     );
   }
 
   resetConsole();
 });
+
+add_task(async function testFilePathMangle() {
+  const fetchSpy = sinon.spy();
+  const reporter = new BrowserErrorReporter({fetch: fetchSpy});
+  await SpecialPowers.pushPrefEnv({set: [
+    [PREF_ENABLED, true],
+    [PREF_SAMPLE_RATE, "1.0"],
+  ]});
+
+  const greDir = FileUtils.getDir("GreD", []).path;
+  const profileDir = FileUtils.getDir("ProfD", []).path;
+
+  const message = createScriptError({
+    message: "Whatever",
+    sourceName: "file:///path/to/main.jsm",
+    stack: [
+      frame({source: "jar:file:///path/to/jar!/inside/jar.jsm"}),
+      frame({source: `file://${greDir}/defaults/prefs/channel-prefs.js`}),
+      frame({source: `file://${profileDir}/prefs.js`}),
+    ],
+  });
+  await reporter.observe(message);
+
+  const call = fetchCallForMessage(fetchSpy, "Whatever");
+  const body = JSON.parse(call.args[1].body);
+  const exception = body.exception.values[0];
+  is(exception.module, "[UNKNOWN_LOCAL_FILEPATH]", "Unrecognized local file paths are mangled");
+
+  // Stackframe order is reversed from what is in the message.
+  const stackFrames = exception.stacktrace.frames;
+  is(
+    stackFrames[0].module, "[profileDir]/prefs.js",
+    "Paths within the profile directory are preserved but mangled",
+  );
+  is(
+    stackFrames[1].module, "[greDir]/defaults/prefs/channel-prefs.js",
+    "Paths within the GRE directory are preserved but mangled",
+  );
+  is(
+    stackFrames[2].module, "/inside/jar.jsm",
+    "Paths within jarfiles are extracted from the full jar: URL",
+  );
+});