Bug 1292226 - Reject non-object ping payloads submitted to Telemetry. r=gfritzsche, data-r=bsmedberg
authorrthyberg <robertthyberg@gmail.com>
Thu, 22 Sep 2016 18:29:34 +0100
changeset 314891 46645a6bb7dc88603beb3ce3e55fd63a1d2fc75b
parent 314890 d6103dea7601d9bae093847ef2b234697d923552
child 314946 052d4d77cbcae9d168754a62e6f71fd9946743b0
push id20595
push usergeorg.fritzsche@googlemail.com
push dateThu, 22 Sep 2016 17:29:43 +0000
treeherderfx-team@46645a6bb7dc [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersgfritzsche
bugs1292226
milestone52.0a1
Bug 1292226 - Reject non-object ping payloads submitted to Telemetry. r=gfritzsche, data-r=bsmedberg
toolkit/components/telemetry/Histograms.json
toolkit/components/telemetry/TelemetryController.jsm
toolkit/components/telemetry/tests/unit/test_PingAPI.js
toolkit/components/telemetry/tests/unit/test_TelemetrySend.js
--- a/toolkit/components/telemetry/Histograms.json
+++ b/toolkit/components/telemetry/Histograms.json
@@ -5508,16 +5508,23 @@
   },
   "TELEMETRY_INVALID_PING_TYPE_SUBMITTED": {
     "alert_emails": ["telemetry-client-dev@mozilla.com"],
     "expires_in_version": "never",
     "kind": "count",
     "keyed": true,
     "description": "Count of individual invalid ping types that were submitted to Telemetry."
   },
+  "TELEMETRY_INVALID_PAYLOAD_SUBMITTED": {
+    "alert_emails": ["telemetry-client-dev@mozilla.com"],
+    "expires_in_version": "never",
+    "bug_numbers": [1292226],
+    "kind": "count",
+    "description": "Count of individual invalid payloads that were submitted to Telemetry."
+  },
   "TELEMETRY_PING_EVICTED_FOR_SERVER_ERRORS": {
     "alert_emails": ["telemetry-client-dev@mozilla.com"],
     "expires_in_version": "never",
     "kind": "count",
     "description": "Number of Telemetry ping files evicted due to server errors (4XX HTTP code received)"
   },
   "TELEMETRY_SESSIONDATA_FAILED_LOAD": {
     "alert_emails": ["telemetry-client-dev@mozilla.com"],
--- a/toolkit/components/telemetry/TelemetryController.jsm
+++ b/toolkit/components/telemetry/TelemetryController.jsm
@@ -496,16 +496,23 @@ var Impl = {
     // Enforce the type string to only contain sane characters.
     const typeUuid = /^[a-z0-9][a-z0-9-]+[a-z0-9]$/i;
     if (!typeUuid.test(aType)) {
       this._log.error("submitExternalPing - invalid ping type: " + aType);
       let histogram = Telemetry.getKeyedHistogramById("TELEMETRY_INVALID_PING_TYPE_SUBMITTED");
       histogram.add(aType, 1);
       return Promise.reject(new Error("Invalid type string submitted."));
     }
+    // Enforce that the payload is an object.
+    if (aPayload === null || typeof aPayload !== 'object' || Array.isArray(aPayload)) {
+      this._log.error("submitExternalPing - invalid payload type: " + typeof aPayload);
+      let histogram = Telemetry.getHistogramById("TELEMETRY_INVALID_PAYLOAD_SUBMITTED");
+      histogram.add(1);
+      return Promise.reject(new Error("Invalid payload type submitted."));
+    }
 
     let promise = this._submitPingLogic(aType, aPayload, aOptions);
     this._trackPendingPingTask(promise);
     return promise;
   },
 
   /**
    * Save a ping to disk.
--- a/toolkit/components/telemetry/tests/unit/test_PingAPI.js
+++ b/toolkit/components/telemetry/tests/unit/test_PingAPI.js
@@ -442,16 +442,37 @@ add_task(function* test_InvalidPingType(
                  "Should not have counted this invalid ping yet: " + type);
     Assert.ok(promiseRejects(TelemetryController.submitExternalPing(type, {})),
               "Ping type should have been rejected.");
     Assert.equal(histogram.snapshot(type).sum, 1,
                  "Should have counted this as an invalid ping type.");
   }
 });
 
+add_task(function* test_InvalidPayloadType() {
+  const PAYLOAD_TYPES = [
+    19,
+    "string",
+    [1, 2, 3, 4],
+    null,
+    undefined,
+  ];
+
+  let histogram = Telemetry.getHistogramById("TELEMETRY_INVALID_PAYLOAD_SUBMITTED");
+  for (let i = 0; i < PAYLOAD_TYPES.length; i++) {
+    histogram.clear();
+    Assert.equal(histogram.snapshot().sum, 0,
+      "Should not have counted this invalid payload yet: " + JSON.stringify(PAYLOAD_TYPES[i]));
+    Assert.ok(yield promiseRejects(TelemetryController.submitExternalPing("payload-test", PAYLOAD_TYPES[i])),
+      "Payload type should have been rejected.");
+    Assert.equal(histogram.snapshot().sum, 1,
+      "Should have counted this as an invalid payload type.");
+  }
+});
+
 add_task(function* test_currentPingData() {
   yield TelemetryController.testSetup();
 
   // Setup test data.
   let h = Telemetry.getHistogramById("TELEMETRY_TEST_RELEASE_OPTOUT");
   h.clear();
   h.add(1);
   let k = Telemetry.getKeyedHistogramById("TELEMETRY_TEST_KEYED_RELEASE_OPTOUT");
--- a/toolkit/components/telemetry/tests/unit/test_TelemetrySend.js
+++ b/toolkit/components/telemetry/tests/unit/test_TelemetrySend.js
@@ -250,17 +250,17 @@ add_task(function* test_backoffTimeout()
   yield TelemetrySend.testWaitOnOutgoingPings();
   Assert.equal(TelemetrySend.pendingPingCount, 0, "Should have no pending pings left");
 });
 
 add_task(function* test_discardBigPings() {
   const TEST_PING_TYPE = "test-ping-type";
 
   // Generate a 2MB string and create an oversized payload.
-  const OVERSIZED_PAYLOAD = generateRandomString(2 * 1024 * 1024);
+  const OVERSIZED_PAYLOAD = {"data": generateRandomString(2 * 1024 * 1024)};
 
   // Reset the histograms.
   Telemetry.getHistogramById("TELEMETRY_PING_SIZE_EXCEEDED_SEND").clear();
   Telemetry.getHistogramById("TELEMETRY_DISCARDED_SEND_PINGS_SIZE_MB").clear();
 
   // Submit a ping of a normal size and check that we don't count it in the histogram.
   yield TelemetryController.submitExternalPing(TEST_PING_TYPE, { test: "test" });
   yield TelemetrySend.testWaitOnOutgoingPings();