Closed Bug 1659419 Opened 1 year ago Closed 1 year ago

Forbid usage of floats in Remote Settings data

Categories

(Cloud Services :: Server: Remote Settings, enhancement, P3)

enhancement

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: leplatrem, Assigned: leplatrem)

References

(Blocks 1 open bug)

Details

Our Canonical JSON implementation precedes some standardization efforts, and our float representation differs from the most polished one (the only one with proper spec, tests suite, and reference implementation in Go).

We are looking into replacing our JavaScript and Python implementations with a single one in Rust. We could take advantage of this rewrite to align our implementation with a specification, instead of having our own (matrix.org also has a different one, etc.).

If no float has ever been published among the Remote Settings data, then we're good. If there are floats (or were recently), then we should investigate with data authors if their data could hit one of the divergences (namely scientific notation with exponents), and/or if there is a migration path.

But for now, if we forbid them (ie. server returns HTTP 400), then we make sure that the problem does not get worse. For new use-cases, floats can be transported as string (or integer + denominator)

The only collection with floats was main/cfr. After talking with the team, we figured out that the float value is used to compute a CSS rule that shows a percentage of 5 stars (eg. percent = rating * 100 / 5). Since JavaScript has implicit casting, the field could be changed to string without consequences ("4.1" * 100 / 5 == 81.9999 🎉).
Andrei published the data changes already.

That basically means that in theory we could proceed with the Canonical JSON code migration, with floats forbidden.

The plan could look like this:

  1. Release and publish the canonical_json crate and Python bindings.
  2. Replace the serializer of Poucave, it will be the first to alert if there is signature divergence
  3. Communicate about the breaking change, and start forbidding floats on the server. The code will return a HTTP 400
  4. If signature validation never fails during a period, it will indirectly make sure that we could replace it in the server
  5. Once the serializer is replaced in the server, we could start replacing it in the client too :)
  6. When all Firefox clients run the latest serializer, allow floats again :)

Notes: leveraging Rust in Python and JavaScript https://blog.mathieu-leplatre.info/leveraging-rust-in-python-and-javascript.html

Blocks: 1665331

Mike, Kate, is there any situation where a recipe/experiment would have a field with a float value (eg. a preference) ?

Flags: needinfo?(mcooper)
Flags: needinfo?(khudson)

Firefox preferences are not allowed to be floats. The types are integer, boolean, and string. Any float values in prefs are stored as strings.

Normandy has a few fields where a float value would semantically make sense, but we don't practically use them that way. I'd be ok with not having floats anywhere.

Flags: needinfo?(mcooper)

This seems reasonable as a short-term measure – I can't think of any immediate use cases. The only thing I would be concerned about is unexpected validation requirements on the client ahead of actually publishing to remote settings, although assuming we checked schemas were valid for remote settings ahead of time, I think this will be fine.

Flags: needinfo?(khudson)

Floats were disabled in STAGE/PROD (step 3).

I will now replace the serializer of Poucave (step 2) and next month the one of kinto-signer (step 4).

Status: NEW → RESOLVED
Closed: 1 year ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.