Bug 1451702: Mangle file: and jar: paths in browser error reports. r=Gijs
☠☠ backed out by ba5746164cfc ☠ ☠
authorMichael Kelly <mkelly@mozilla.com>
Fri, 18 May 2018 15:34:11 -0700
changeset 423041 71156adbd18ecb9f5b4d09638881a3714c71244b
parent 423040 0e32ff604fc61d6231e56fa944296a9bf86fd42a
child 423042 b1c9becdad64833c5e4d9c73591a40614a387ec3
push id65342
push userapavel@mozilla.com
push dateWed, 20 Jun 2018 11:30:38 +0000
treeherderautoland@dadc58a65c2e [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/D1319
browser/modules/BrowserErrorReporter.jsm
browser/modules/test/browser/browser_BrowserErrorReporter.js
--- a/browser/modules/BrowserErrorReporter.jsm
+++ b/browser/modules/BrowserErrorReporter.jsm
@@ -148,16 +148,34 @@ class BrowserErrorReporter {
     );
     XPCOMUtils.defineLazyPreferenceGetter(
       this,
       "sampleRatePref",
       PREF_SAMPLE_RATE,
       "0.0",
       this.handleSampleRatePrefChanged.bind(this),
     );
+
+    // Prefix mappings for the mangleFilePaths transform.
+    this.manglePrefixes = options.manglePrefixes || {
+      greDir: Services.dirsvc.get("GreD", Ci.nsIFile),
+      profileDir: Services.dirsvc.get("ProfD", Ci.nsIFile),
+    };
+    // File paths are encoded by nsIURI, so let's do the same for the prefixes
+    // we're comparing them to.
+    for (const [name, prefixFile] of Object.entries(this.manglePrefixes)) {
+      let filePath = Services.io.newFileURI(prefixFile).filePath;
+
+      // filePath might not have a trailing slash in some cases
+      if (!filePath.endsWith("/")) {
+        filePath += "/";
+      }
+
+      this.manglePrefixes[name] = filePath;
+    }
   }
 
   /**
    * Lazily-created logger
    */
   get logger() {
     const logger = Log.repository.getLogger("BrowserErrorReporter");
     logger.addAppender(new Log.ConsoleAppender(new Log.BasicFormatter()));
@@ -271,16 +289,17 @@ class BrowserErrorReporter {
       },
     };
 
     const transforms = [
       addErrorMessage,
       addStacktrace,
       addModule,
       mangleExtensionUrls,
+      this.mangleFilePaths.bind(this),
       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}`);
@@ -294,22 +313,58 @@ 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}`);
+    }
+  }
+
+  /**
+   * Alters file: and jar: paths to remove leading file paths that may contain
+   * user-identifying or platform-specific paths.
+   *
+   * prefixes is a mapping of replacementName -> filePath, where filePath is a
+   * path on the filesystem that should be replaced, and replacementName is the
+   * text that will replace it.
+   */
+  mangleFilePaths(message, exceptionValue) {
+    exceptionValue.module = this._transformFilePath(exceptionValue.module);
+    for (const frame of exceptionValue.stacktrace.frames) {
+      frame.module = this._transformFilePath(frame.module);
     }
   }
+
+  _transformFilePath(path) {
+    try {
+      const uri = Services.io.newURI(path);
+      if (uri.schemeIs("jar")) {
+        return uri.filePath;
+      }
+      if (uri.schemeIs("file")) {
+        for (const [name, prefix] of Object.entries(this.manglePrefixes)) {
+          console.log(`Evaluating prefix: ${name}: ${prefix} against ${uri.filePath}`);
+          if (uri.filePath.startsWith(prefix)) {
+            return uri.filePath.replace(prefix, `[${name}]/`);
+          }
+        }
+
+        return "[UNKNOWN_LOCAL_FILEPATH]";
+      }
+    } catch (err) {}
+
+    return path;
+  }
 }
 
 function defaultFetch(...args) {
   // Do not make network requests while running in automation
   if (Cu.isInAutomation) {
     return null;
   }
 
@@ -390,20 +445,19 @@ 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);
--- 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("");
@@ -549,22 +576,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.handleMessage(message);
   }
 
   await SpecialPowers.pushPrefEnv({set: [[PREF_SAMPLE_RATE, "0.0"]]});
@@ -667,8 +689,77 @@ 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 = Services.dirsvc.get("GreD", Ci.nsIFile).path;
+  const profileDir = Services.dirsvc.get("ProfD", Ci.nsIFile).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",
+  );
+});
+
+add_task(async function testFilePathMangleWhitespace() {
+  const fetchSpy = sinon.spy();
+  const manglePrefixes = {
+    whitespace: new FileUtils.File("/fake/GreD/with whitespace/"),
+  };
+  const reporter = new BrowserErrorReporter({fetch: fetchSpy, manglePrefixes});
+  await SpecialPowers.pushPrefEnv({set: [
+    [PREF_ENABLED, true],
+    [PREF_SAMPLE_RATE, "1.0"],
+  ]});
+
+  const message = createScriptError({
+    message: "Whatever",
+    sourceName: "file:///fake/GreD/with whitespace/remaining/file.jsm",
+  });
+  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, "[whitespace]/remaining/file.jsm",
+    "Prefixes with whitespace are correctly mangled",
+  );
+});