From 74eb064c8ed04831f534236756ed2115e54d7da2 Mon Sep 17 00:00:00 2001
From: Tom Ritter <tom@mozilla.com>
Date: Tue, 10 Sep 2019 18:01:26 +0000
Subject: [PATCH] Bug 1575974 - Add tests for pingsender url parsing r=gsvelto

Differential Revision: https://phabricator.services.mozilla.com/D44706

--HG--
extra : moz-landing-system : lando
---
 .../telemetry/app/TelemetrySend.jsm           | 11 ++--
 .../telemetry/tests/unit/test_PingSender.js   | 55 ++++++++++++++++++-
 2 files changed, 61 insertions(+), 5 deletions(-)

diff --git a/toolkit/components/telemetry/app/TelemetrySend.jsm b/toolkit/components/telemetry/app/TelemetrySend.jsm
index e1c38ce022c6..9d5131058f2d 100644
--- a/toolkit/components/telemetry/app/TelemetrySend.jsm
+++ b/toolkit/components/telemetry/app/TelemetrySend.jsm
@@ -315,14 +315,17 @@ var TelemetrySend = {
    * @param {String} aUrl The telemetry server URL
    * @param {String} aPingFilePath The path to the file holding the ping
    *        contents, if if sent successfully the pingsender will delete it.
+   * @param {callback} observer A function called with parameters
+            (subject, topic, data) and a topic of "process-finished" or
+            "process-failed" after pingsender completion.
    *
    * @throws NS_ERROR_FAILURE if we couldn't find or run the pingsender
    *         executable.
    * @throws NS_ERROR_NOT_IMPLEMENTED on Android as the pingsender is not
    *         available.
    */
-  testRunPingSender(url, pingPath) {
-    TelemetrySendImpl.runPingSender(url, pingPath);
+  testRunPingSender(url, pingPath, observer) {
+    return TelemetrySendImpl.runPingSender(url, pingPath, observer);
   },
 };
 
@@ -1553,7 +1556,7 @@ var TelemetrySendImpl = {
     };
   },
 
-  runPingSender(url, pingPath) {
+  runPingSender(url, pingPath, observer) {
     if (AppConstants.platform === "android") {
       throw Cr.NS_ERROR_NOT_IMPLEMENTED;
     }
@@ -1570,6 +1573,6 @@ var TelemetrySendImpl = {
     process.init(exe);
     process.startHidden = true;
     process.noShell = true;
-    process.run(/* blocking */ false, [url, pingPath], 2);
+    process.runAsync([url, pingPath], 2, observer);
   },
 };
diff --git a/toolkit/components/telemetry/tests/unit/test_PingSender.js b/toolkit/components/telemetry/tests/unit/test_PingSender.js
index c48fef71d354..0c62e020ff71 100644
--- a/toolkit/components/telemetry/tests/unit/test_PingSender.js
+++ b/toolkit/components/telemetry/tests/unit/test_PingSender.js
@@ -96,7 +96,24 @@ add_task(async function test_pingSender() {
 
   // Try to send it using the pingsender.
   const url = "http://localhost:" + PingServer.port + "/submit/telemetry/";
-  TelemetrySend.testRunPingSender(url, pingPath);
+  TelemetrySend.testRunPingSender(url, pingPath, (_, topic, __) => {
+    switch (topic) {
+      case "process-finished": // finished indicates an exit code of 0
+        Assert.equal(
+          true,
+          true,
+          "Pingsender should be able to post to localhost"
+        );
+        break;
+      case "process-failed": // failed indicates an exit code != 0
+        Assert.equal(
+          true,
+          false,
+          "Pingsender should be able to post to localhost"
+        );
+        break;
+    }
+  });
 
   let req = await PingServer.promiseNextRequest();
   let ping = decodeRequestPayload(req);
@@ -132,6 +149,42 @@ add_task(async function test_pingSender() {
   // Check that the PingSender removed the pending ping.
   await waitForPingDeletion(data.id);
 
+  // Confirm we can't send a ping to another destination url
+  let bannedUris = [
+    "https://example.com",
+    "http://localhost.com",
+    "http://localHOST.com",
+    "http://localhost@example.com",
+    "http://localhost:bob@example.com",
+    "http://localhost:localhost@localhost.example.com",
+  ];
+  for (let indx in bannedUris) {
+    TelemetrySend.testRunPingSender(
+      bannedUris[indx],
+      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;
+        }
+      }
+    );
+  }
+
   // 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));