1334513 - Make sure Keyed Scalars APIs don't allow empty keys

Status: RESOLVED FIXED

(Toolkit :: Telemetry, defect, P1)

Description

[Alessio Placitelli [:Dexter]]

We have to make sure that we're not allowing to store keyed scalrs with empty keys.

Comment 1

[Alessio Placitelli [:Dexter]]

Attachment: bug1334513.patch

This patch makes the scalars print an error if an empty key is provided when changng keyed scalars.

Comment 2

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8831622 [details] [diff] [review]
bug1334513.patch

Review of attachment 8831622 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/components/telemetry/TelemetryScalar.cpp
@@ +819,5 @@
>      case ScalarResult::StringTooLong:
>        errorMessage.Append(NS_LITERAL_STRING(" - Truncating scalar value to 50 characters."));
>        break;
> +    case ScalarResult::InvalidKeyLength:
> +      errorMessage.Append(NS_LITERAL_STRING(" - The key must not be empty and must be limited to 70 characters."));

I think we should have two distinct error messages here.

Comment 3

[Alessio Placitelli [:Dexter]]

Attachment: bug1334513.patch

Comment 4

[Alessio Placitelli [:Dexter]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=91a3b51912a5

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/fbe43964eb8d
Make sure Keyed Scalars APIs don't allow empty keys. r=gfritzsche

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/fbe43964eb8d