Bug 1334513 - Make sure Keyed Scalars APIs don't allow empty keys. r=gfritzsche
authorAlessio Placitelli <alessio.placitelli@gmail.com>
Tue, 31 Jan 2017 05:46:00 -0500
changeset 468864 fbe43964eb8d1595bb67fd679da69053de3a2015
parent 468863 ac3294db3ea4f3836031d008fc7d787ec3487904
child 468865 8d2a9160dcafe13d28099a643212f784b5124e8c
push id43551
push userbmo:kgilbert@mozilla.com
push dateTue, 31 Jan 2017 23:27:06 +0000
reviewersgfritzsche
bugs1334513
milestone54.0a1
Bug 1334513 - Make sure Keyed Scalars APIs don't allow empty keys. r=gfritzsche
toolkit/components/telemetry/TelemetryScalar.cpp
toolkit/components/telemetry/tests/unit/test_TelemetryScalars.js
--- a/toolkit/components/telemetry/TelemetryScalar.cpp
+++ b/toolkit/components/telemetry/TelemetryScalar.cpp
@@ -89,16 +89,17 @@ enum class ScalarResult : uint8_t {
   CannotRecordInProcess,
   CannotRecordDataset,
   KeyedTypeMismatch,
   UnknownScalar,
   OperationNotSupported,
   InvalidType,
   InvalidValue,
   // Keyed Scalar Errors
+  KeyIsEmpty,
   KeyTooLong,
   TooManyKeys,
   // String Scalar Errors
   StringTooLong,
   // Unsigned Scalar Errors
   UnsignedNegativeValue,
   UnsignedTruncatedValue
 };
@@ -688,16 +689,20 @@ KeyedScalar::GetValue(nsTArray<KeyValueP
 /**
  * Get the scalar for the referenced key.
  * If there's no such key, instantiate a new Scalar object with the
  * same type of the Keyed scalar and create the key.
  */
 ScalarResult
 KeyedScalar::GetScalarForKey(const nsAString& aKey, ScalarBase** aRet)
 {
+  if (aKey.IsEmpty()) {
+    return ScalarResult::KeyIsEmpty;
+  }
+
   if (aKey.Length() >= kMaximumKeyStringLength) {
     return ScalarResult::KeyTooLong;
   }
 
   if (mScalarKeys.Count() >= kMaximumNumberOfKeys) {
     return ScalarResult::TooManyKeys;
   }
 
@@ -816,16 +821,19 @@ internal_LogScalarError(const nsACString
       errorMessage.Append(NS_LITERAL_STRING(" - Attempted to set the scalar to an invalid data type."));
       break;
     case ScalarResult::InvalidValue:
       errorMessage.Append(NS_LITERAL_STRING(" - Attempted to set the scalar to an incompatible value."));
       break;
     case ScalarResult::StringTooLong:
       errorMessage.Append(NS_LITERAL_STRING(" - Truncating scalar value to 50 characters."));
       break;
+    case ScalarResult::KeyIsEmpty:
+      errorMessage.Append(NS_LITERAL_STRING(" - The key must not be empty."));
+      break;
     case ScalarResult::KeyTooLong:
       errorMessage.Append(NS_LITERAL_STRING(" - The key length must be limited to 70 characters."));
       break;
     case ScalarResult::TooManyKeys:
       errorMessage.Append(NS_LITERAL_STRING(" - Keyed scalars cannot have more than 100 keys."));
       break;
     case ScalarResult::UnsignedNegativeValue:
       errorMessage.Append(NS_LITERAL_STRING(" - Trying to set an unsigned scalar to a negative number."));
--- a/toolkit/components/telemetry/tests/unit/test_TelemetryScalars.js
+++ b/toolkit/components/telemetry/tests/unit/test_TelemetryScalars.js
@@ -468,27 +468,32 @@ add_task(function* test_keyed_keys_lengt
   // Set the value for a key within the length limits.
   Telemetry.keyedScalarSet(KEYED_UINT_SCALAR, NORMAL_KEY, 1);
 
   // Now try to set and modify the value for a very long key (must not throw).
   Telemetry.keyedScalarAdd(KEYED_UINT_SCALAR, LONG_KEY_STRING, 10);
   Telemetry.keyedScalarSet(KEYED_UINT_SCALAR, LONG_KEY_STRING, 1);
   Telemetry.keyedScalarSetMaximum(KEYED_UINT_SCALAR, LONG_KEY_STRING, 10);
 
+  // Also attempt to set the value for an empty key.
+  Telemetry.keyedScalarSet(KEYED_UINT_SCALAR, "", 1);
+
   // Make sure the key with the right length contains the expected value.
   let keyedScalars =
     getParentProcessScalars(Ci.nsITelemetry.DATASET_RELEASE_CHANNEL_OPTIN, true);
   Assert.equal(Object.keys(keyedScalars[KEYED_UINT_SCALAR]).length, 1,
                "The keyed scalar must contain exactly 1 key.");
   Assert.ok(NORMAL_KEY in keyedScalars[KEYED_UINT_SCALAR],
             "The keyed scalar must contain the expected key.");
   Assert.equal(keyedScalars[KEYED_UINT_SCALAR][NORMAL_KEY], 1,
                "The key must contain the expected value.");
   Assert.ok(!(LONG_KEY_STRING in keyedScalars[KEYED_UINT_SCALAR]),
             "The data for the long key should not have been recorded.");
+  Assert.ok(!("" in keyedScalars[KEYED_UINT_SCALAR]),
+            "The data for the empty key should not have been recorded.");
 });
 
 add_task(function* test_keyed_max_keys() {
   Telemetry.clearScalars();
 
   // Generate the names for the first 100 keys.
   let keyNamesSet = new Set();
   for (let k = 0; k < 100; k++) {