Backed out 5 changesets (bug 1734262) for causing failures in test_HealthPing.js CLOSED TREE

Backed out changeset c3b0edc896e9 (bug 1734262)
Backed out changeset a6ecb9d9697a (bug 1734262)
Backed out changeset 1e4cafb39d62 (bug 1734262)
Backed out changeset 19eb67d15f2a (bug 1734262)
Backed out changeset 845c7749c2c3 (bug 1734262)

browser/modules/PingCentre.jsm

@@ -21,14 +21,15 @@ ChromeUtils.defineModuleGetter(
 );
 ChromeUtils.defineModuleGetter(
   this,
-  "TelemetrySend",
-  "resource://gre/modules/TelemetrySend.jsm"
+  "ServiceRequest",
+  "resource://gre/modules/ServiceRequest.jsm"
 );
 
 const PREF_BRANCH = "browser.ping-centre.";
 
 const TELEMETRY_PREF = `${PREF_BRANCH}telemetry`;
 const LOGGING_PREF = `${PREF_BRANCH}log`;
+const STRUCTURED_INGESTION_SEND_TIMEOUT = 30 * 1000; // 30 seconds
 
 const FHR_UPLOAD_ENABLED_PREF = "datareporting.healthreport.uploadEnabled";
 
@@ -107,9 +108,69 @@ class PingCentre {
     return payload;
   }
 
-  // We route through this helper because it gets hooked in testing.
-  static _sendStandalonePing(endpoint, payload) {
-    return TelemetrySend.sendStandalonePing(endpoint, payload);
+  static _gzipCompressString(string) {
+    let observer = {
+      buffer: "",
+      onStreamComplete(loader, context, status, length, result) {
+        this.buffer = String.fromCharCode(...result);
+      },
+    };
+
+    let scs = Cc["@mozilla.org/streamConverters;1"].getService(
+      Ci.nsIStreamConverterService
+    );
+    let listener = Cc["@mozilla.org/network/stream-loader;1"].createInstance(
+      Ci.nsIStreamLoader
+    );
+    listener.init(observer);
+    let converter = scs.asyncConvertData(
+      "uncompressed",
+      "gzip",
+      listener,
+      null
+    );
+    let stringStream = Cc[
+      "@mozilla.org/io/string-input-stream;1"
+    ].createInstance(Ci.nsIStringInputStream);
+    stringStream.data = string;
+    converter.onStartRequest(null, null);
+    converter.onDataAvailable(null, stringStream, 0, string.length);
+    converter.onStopRequest(null, null, null);
+    return observer.buffer;
+  }
+
+  static _sendInGzip(endpoint, payload) {
+    return new Promise((resolve, reject) => {
+      let request = new ServiceRequest({ mozAnon: true });
+      request.mozBackgroundRequest = true;
+      request.timeout = STRUCTURED_INGESTION_SEND_TIMEOUT;
+
+      request.open("POST", endpoint, true);
+      request.overrideMimeType("text/plain");
+      request.setRequestHeader(
+        "Content-Type",
+        "application/json; charset=UTF-8"
+      );
+      request.setRequestHeader("Content-Encoding", "gzip");
+      request.setRequestHeader("Date", new Date().toUTCString());
+
+      request.onload = event => {
+        if (request.status !== 200) {
+          reject(event);
+        } else {
+          resolve(event);
+        }
+      };
+      request.onerror = reject;
+      request.onabort = reject;
+      request.ontimeout = reject;
+
+      let payloadStream = Cc[
+        "@mozilla.org/io/string-input-stream;1"
+      ].createInstance(Ci.nsIStringInputStream);
+      payloadStream.data = PingCentre._gzipCompressString(payload);
+      request.sendInputStream(payloadStream);
+    });
   }
 
   /**
@@ -137,7 +198,7 @@ class PingCentre {
       );
     }
 
-    return PingCentre._sendStandalonePing(endpoint, payload).catch(event => {
+    return PingCentre._sendInGzip(endpoint, payload).catch(event => {
       Cu.reportError(
         `Structured Ingestion ping failure with error: ${event.type}`
       );

browser/modules/test/unit/test_PingCentre.js

@@ -128,17 +128,17 @@ add_task(function test_createStructuredIngestionPing() {
 
 add_task(function test_sendStructuredIngestionPing_disabled() {
   _setUp();
-  sandbox.stub(PingCentre, "_sendStandalonePing").resolves();
+  sandbox.stub(PingCentre, "_sendInGzip").resolves();
   Services.prefs.setBoolPref(PingCentreConstants.TELEMETRY_PREF, false);
   pingCentre.sendStructuredIngestionPing(FAKE_PING, FAKE_ENDPOINT);
 
-  Assert.ok(PingCentre._sendStandalonePing.notCalled, "Should not be sent");
+  Assert.ok(PingCentre._sendInGzip.notCalled, "Should not be sent");
 });
 
 add_task(function test_sendStructuredIngestionPing_success() {
   _setUp();
-  sandbox.stub(PingCentre, "_sendStandalonePing").resolves();
+  sandbox.stub(PingCentre, "_sendInGzip").resolves();
   pingCentre.sendStructuredIngestionPing(FAKE_PING, FAKE_ENDPOINT);
 
-  Assert.equal(PingCentre._sendStandalonePing.callCount, 1, "Should be sent");
+  Assert.equal(PingCentre._sendInGzip.callCount, 1, "Should be sent");
 });

toolkit/components/telemetry/app/TelemetrySend.jsm

@@ -17,7 +17,6 @@ var EXPORTED_SYMBOLS = [
   "SendScheduler",
   "TelemetrySendImpl",
   "PING_SUBMIT_TIMEOUT_MS",
-  "sendStandalonePing",
   "gzipCompressString",
 ];
 
@@ -196,42 +195,6 @@ function gzipCompressString(string) {
   return observer.buffer;
 }
 
-const STANDALONE_PING_TIMEOUT = 30 * 1000; // 30 seconds
-
-function sendStandalonePing(endpoint, payload, extraHeaders = {}) {
-  return new Promise((resolve, reject) => {
-    let request = new ServiceRequest({ mozAnon: true });
-    request.mozBackgroundRequest = true;
-    request.timeout = STANDALONE_PING_TIMEOUT;
-
-    request.open("POST", endpoint, true);
-    request.overrideMimeType("text/plain");
-    request.setRequestHeader("Content-Type", "application/json; charset=UTF-8");
-    request.setRequestHeader("Content-Encoding", "gzip");
-    request.setRequestHeader("Date", new Date().toUTCString());
-    for (let header in extraHeaders) {
-      request.setRequestHeader(header, extraHeaders[header]);
-    }
-
-    request.onload = event => {
-      if (request.status !== 200) {
-        reject(event);
-      } else {
-        resolve(event);
-      }
-    };
-    request.onerror = reject;
-    request.onabort = reject;
-    request.ontimeout = reject;
-
-    let payloadStream = Cc[
-      "@mozilla.org/io/string-input-stream;1"
-    ].createInstance(Ci.nsIStringInputStream);
-    payloadStream.data = gzipCompressString(payload);
-    request.sendInputStream(payloadStream);
-  });
-}
-
 var TelemetrySend = {
   get pendingPingCount() {
     return TelemetrySendImpl.pendingPingCount;
@@ -1638,14 +1601,12 @@ var TelemetrySendImpl = {
     }
 
     const exeName =
-      AppConstants.MOZ_APP_NAME +
-      (AppConstants.platform === "win" ? ".exe" : "");
+      AppConstants.platform === "win" ? "pingsender.exe" : "pingsender";
 
     let exe = Services.dirsvc.get("GreBinD", Ci.nsIFile);
     exe.append(exeName);
 
-    let params = ["--backgroundtask", "pingsender"];
-    params.push(...pings.flatMap(ping => [ping.url, ping.path]));
+    let params = pings.flatMap(ping => [ping.url, ping.path]);
     let process = Cc["@mozilla.org/process/util;1"].createInstance(
       Ci.nsIProcess
     );

toolkit/components/telemetry/moz.build

@@ -116,10 +116,6 @@ EXTRA_JS_MODULES += [
     "TelemetryStartup.jsm",
 ]
 
-EXTRA_JS_MODULES.backgroundtasks += [
-    "pings/BackgroundTask_pingsender.jsm",
-]
-
 if CONFIG["OS_ARCH"] == "WINNT":
     EXTRA_JS_MODULES += [
         "pings/UninstallPing.jsm",

toolkit/components/telemetry/pings/BackgroundTask_pingsender.jsm

@@ -1,48 +0,0 @@
-/* -*- indent-tabs-mode: nil; js-indent-level: 2 -*-
- * 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";
-
-const { sendStandalonePing } = ChromeUtils.import(
-  "resource://gre/modules/TelemetrySend.jsm"
-);
-
-var EXPORTED_SYMBOLS = ["runBackgroundTask"];
-async function runBackgroundTask(commandLine) {
-  let sends = [];
-  for (let i = 0; i < commandLine.length; i += 2) {
-    sends.push(
-      sendPing(commandLine.getArgument(i), commandLine.getArgument(i + 1))
-    );
-  }
-
-  return Promise.all(sends);
-}
-
-// The standalone pingsender utility had an allowlist of endpoints, which was
-// added to prevent it from being used as a generic exfiltration utility by
-// unrelated malware running on the same system. It's unclear whether a gecko-
-// based pingsender would be similarly desirable for that use-case, but it's
-// easy enough to implement an allowlist here as well.
-const ALLOWED_ENDPOINTS = ["localhost", "incoming.telemetry.mozilla.org"];
-
-async function sendPing(endpoint, path) {
-  console.log(`pingsender: sending ${path} to ${endpoint}`);
-
-  let hostname = new URL(endpoint).hostname;
-  if (!ALLOWED_ENDPOINTS.includes(hostname)) {
-    throw new Error(`pingsender: Endpoint ${endpoint} is not allowed`);
-  }
-
-  let json = await IOUtils.readUTF8(path);
-  console.log(`pingsender: read payload ${json}`);
-
-  await sendStandalonePing(endpoint, json, {
-    "User-Agent": "pingsender/2.0",
-    "X-PingSender-Version": "2.0",
-  });
-
-  return IOUtils.remove(path);
-}

toolkit/components/telemetry/tests/unit/test_HealthPing.js

@@ -376,12 +376,12 @@ add_task(async function test_usePingSenderOnShutdown() {
   // Check that the health ping is sent at shutdown using the pingsender.
   Assert.equal(
     request.getHeader("User-Agent"),
-    "pingsender/2.0",
+    "pingsender/1.0",
     "Should have received the correct user agent string."
   );
   Assert.equal(
     request.getHeader("X-PingSender-Version"),
-    "2.0",
+    "1.0",
     "Should have received the correct PingSender version string."
   );
 });

toolkit/components/telemetry/tests/unit/test_PingSender.js

@@ -135,12 +135,12 @@ add_task(async function test_pingSender() {
 
   Assert.equal(
     req.getHeader("User-Agent"),
-    "pingsender/2.0",
+    "pingsender/1.0",
     "Should have received the correct user agent string."
   );
   Assert.equal(
     req.getHeader("X-PingSender-Version"),
-    "2.0",
+    "1.0",
     "Should have received the correct PingSender version string."
   );
   Assert.equal(
@@ -164,18 +164,6 @@ add_task(async function test_pingSender() {
   // Check that the PingSender removed the pending ping.
   await waitForPingDeletion(data.id);
 
-  // Shut down the failing server.
-  await new Promise(r => failingServer.stop(r));
-});
-
-add_task(async function test_bannedDomains() {
-  // Generate a new ping and save it among the pending pings.
-  const data = generateTestPingData();
-  await TelemetryStorage.savePing(data, true);
-
-  // Get the local path of the saved ping.
-  const pingPath = OS.Path.join(TelemetryStorage.pingDirectoryPath, data.id);
-
   // Confirm we can't send a ping to another destination url
   let bannedUris = [
     "https://example.com",
@@ -185,25 +173,35 @@ add_task(async function test_bannedDomains() {
     "http://localhost:bob@example.com",
     "http://localhost:localhost@localhost.example.com",
   ];
-  for (let url of bannedUris) {
-    let result = await new Promise(resolve =>
-      TelemetrySend.testRunPingSender(
-        [{ url, path: pingPath }],
-        (_, topic, __) => {
-          switch (topic) {
-            case "process-finished": // finished indicates an exit code of 0
-            case "process-failed": // failed indicates an exit code != 0
-              resolve(topic);
-          }
+  for (let indx in bannedUris) {
+    TelemetrySend.testRunPingSender(
+      [{ url: bannedUris[indx], path: pingPath }],
+      (_, topic, __) => {
+        switch (topic) {
+          case "process-finished": // finished indicates an exit code of 0
+            Assert.equal(
+              false,
+              true,
+              "Pingsender should not be able to post to any banned urls: " +
+                bannedUris[indx]
+            );
+            break;
+          case "process-failed": // failed indicates an exit code != 0
+            Assert.equal(
+              true,
+              true,
+              "Pingsender should not be able to post to any banned urls: " +
+                bannedUris[indx]
+            );
+            break;
         }
-      )
-    );
-    Assert.equal(
-      result,
-      "process-failed",
-      `Pingsender should not be able to post to ${url}`
+      }
     );
   }
+
+  // Shut down the failing server. We do this now, and not right after using it,
+  // to make sure we're not interfering with the test.
+  await new Promise(r => failingServer.stop(r));
 });
 
 add_task(async function test_pingSender_multiple_pings() {

toolkit/components/telemetry/tests/unit/test_TelemetryController.js

@@ -810,12 +810,12 @@ add_task(async function test_sendNewProfile() {
   // Check that the new-profile ping is sent at shutdown using the pingsender.
   Assert.equal(
     req.getHeader("User-Agent"),
-    "pingsender/2.0",
+    "pingsender/1.0",
     "Should have received the correct user agent string."
   );
   Assert.equal(
     req.getHeader("X-PingSender-Version"),
-    "2.0",
+    "1.0",
     "Should have received the correct PingSender version string."
   );
 

toolkit/components/telemetry/tests/unit/xpcshell.ini

@@ -94,7 +94,7 @@ skip-if =
   apple_silicon  # bug 1707747
   apple_catalina # Bug 1713329
 [test_PingSender.js]
-skip-if = os == "android" # pingsender is disabled on Android in TelemetrySend.jsm
+skip-if = (os == "android") || (os == "linux" && bits == 32)
 [test_TelemetryAndroidEnvironment.js]
 [test_TelemetryUtils.js]
 [test_ThirdPartyModulesPing.js]