Bug 1357798 - Ensure URLs don't end up in sync error telemetry. r=markh

MozReview-Commit-ID: AxFzysv0ks3

--HG--
extra : rebase_source : 6b2aa9354ed9540c5cdb8f537eaca7080c97a4eb

services/sync/modules/telemetry.js

@@ -402,6 +402,22 @@ class TelemetryRecord {
   }
 }
 
+function cleanErrorMessage(error) {
+  // There's a chance the profiledir is in the error string which is PII we
+  // want to avoid including in the ping.
+  error = error.replace(reProfileDir, "[profileDir]");
+  // MSG_INVALID_URL from /dom/bindings/Errors.msg -- no way to access this
+  // directly from JS.
+  if (error.endsWith("is not a valid URL.")) {
+    error = "<URL> is not a valid URL.";
+  }
+  // Try to filter things that look somewhat like a URL (in that they contain a
+  // colon in the middle of non-whitespace), in case anything else is including
+  // these in error messages.
+  error = error.replace(/\S+:\S+/g, "<URL>");
+  return error;
+}
+
 class SyncTelemetryImpl {
   constructor(allowedEngines) {
     log.level = Log.Level[Svc.Prefs.get("log.logger.telemetry", "Trace")];
@@ -666,9 +682,7 @@ class SyncTelemetryImpl {
         // This is hacky, but I can't imagine that it's not also accurate.
         return { name: "othererror", error };
       }
-      // There's a chance the profiledir is in the error string which is PII we
+      error = cleanErrorMessage(error);
-      // want to avoid including in the ping.
-      error = error.replace(reProfileDir, "[profileDir]");
       return { name: "unexpectederror", error };
     }
 
@@ -698,9 +712,8 @@ class SyncTelemetryImpl {
 
     return {
       name: "unexpectederror",
-      // as above, remove the profile dir value.
+      error: cleanErrorMessage(String(error))
-      error: String(error).replace(reProfileDir, "[profileDir]")
+    };
-    }
   }
 
 }

services/sync/tests/unit/test_telemetry.js

@@ -384,6 +384,38 @@ add_task(async function test_engine_fail_ioerror() {
   }
 });
 
+add_task(async function test_clean_urls() {
+  enableValidationPrefs();
+
+  Service.engineManager.register(SteamEngine);
+  let engine = Service.engineManager.get("steam");
+  engine.enabled = true;
+  let server = serverForFoo(engine);
+  await SyncTestingInfrastructure(server);
+  engine._errToThrow = new TypeError("http://www.google .com is not a valid URL.");
+
+  try {
+    _(`test_clean_urls: Steam tracker contents: ${
+      JSON.stringify(engine._tracker.changedIDs)}`);
+    let ping = await sync_and_validate_telem(true);
+    equal(ping.status.service, SYNC_FAILED_PARTIAL);
+    let failureReason = ping.engines.find(e => e.name === "steam").failureReason;
+    equal(failureReason.name, "unexpectederror");
+    equal(failureReason.error, "<URL> is not a valid URL.");
+    // Handle other errors that include urls.
+    engine._errToThrow = "Other error message that includes some:url/foo/bar/ in it.";
+    ping = await sync_and_validate_telem(true);
+    equal(ping.status.service, SYNC_FAILED_PARTIAL);
+    failureReason = ping.engines.find(e => e.name === "steam").failureReason;
+    equal(failureReason.name, "unexpectederror");
+    equal(failureReason.error, "Other error message that includes <URL> in it.");
+  } finally {
+    await cleanAndGo(engine, server);
+    Service.engineManager.unregister(engine);
+  }
+});
+
+
 add_task(async function test_initial_sync_engines() {
   enableValidationPrefs();