1770473 - `sendStandalonePing` in `TelemetrySend.jsm` does not correctly encode UTF-8, leading to `com.fasterxml.jackson.core.JsonParseException: Illegal unquoted character` in the ingestion pipeline

Status: RESOLVED FIXED

(Toolkit :: Telemetry, defect, P1)

Description

[Nick Alexander :nalexander [he/him] (PTO August 22-September 12)]

In TelemetrySend.jsm, _doPingRequest takes an object, stringifies it, and then UTF-8 encodes it: https://searchfox.org/mozilla-central/rev/24c1cdc33ccce692612276cd0d3e9a44f6c22fd3/toolkit/components/telemetry/app/TelemetrySend.jsm#1311-1339.

However, sendStandalonePing takes a string and does not UTF-8 encode it: https://searchfox.org/mozilla-central/rev/24c1cdc33ccce692612276cd0d3e9a44f6c22fd3/toolkit/components/telemetry/app/TelemetrySend.jsm#201-233.

All consumers of sendStandalonePing are via sendStructuredIngestionPing (
https://searchfox.org/mozilla-central/rev/9f95c41a962c9228f569f8a6b2c30edbb50b65ae/browser/modules/PingCentre.jsm#126-145) which stringifies its input object but does not UTF-8 encode.

This leads to JSON parsing exceptions in the ingestion pipeline like

com.fasterxml.jackson.core.JsonParseException: Illegal unquoted character ...

When pingsender2 was re-enabled in the second landing of Bug 1746983, essentially all main pings sent at shutdown witnessed this error because default theme names contain the string System theme —, and the Unicode dash variant trips this encoding error. See this STMO query.

This is also, I think, the underlying cause of the difficult to diagnose Bug 1769013 and even the earlier Bug 1736524 which was, I believe, mis-diagnosed as being due to the test harness only.

Comment 1

[Nick Alexander :nalexander [he/him] (PTO August 22-September 12)]

Attachment: Bug 1770473 - Pre: Add shutdown ping tests; assert existing `pingsender` Unicode encoding. r?chutten

When enabling pingsender2, I witnessed Unicode encoding differences
that were not caught by testing. In addition, the resultant 500
status codes when submitting shutdown pings were not caught.

Comment 2

[Nick Alexander :nalexander [he/him] (PTO August 22-September 12)]

Attachment: Bug 1770473 - Encode Unicode as UTF-8 in `TelemetrySend.sendStandalonePing`. r?chutten

The tests in the Pre: part will exercise this functionality once we
cut over to pingsender2.

Depends on D147611

Comment 3

[BugBot [:suhaib / :marco]]

The severity field is not set for this bug.
:chutten, could you have a look please?

For more information, please visit auto_nag documentation.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/25ce814e40b9
Pre: Add shutdown ping tests; assert existing `pingsender` Unicode encoding. r=chutten
https://hg.mozilla.org/integration/autoland/rev/d62480e627da
Encode Unicode as UTF-8 in `TelemetrySend.sendStandalonePing`. r=chutten

Comment 5

[Sandor Molnar[:smolnar]]

https://hg.mozilla.org/mozilla-central/rev/25ce814e40b9
https://hg.mozilla.org/mozilla-central/rev/d62480e627da