Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pre-room version 6 handling of IEEE 754 in canonical JSON #1244

Open
neilalexander opened this issue Sep 22, 2022 · 2 comments
Open

Pre-room version 6 handling of IEEE 754 in canonical JSON #1244

neilalexander opened this issue Sep 22, 2022 · 2 comments
Labels
clarification An area where the expected behaviour is understood, but the spec could do with being more explicit

Comments

@neilalexander
Copy link
Contributor

After room version 6, "strong" enforcement of canonical JSON was specced for event JSONs, which outlaws float values, NaN, Infinity and -Infinity.

In room versions 5 and below, these are unclear, so it's possible that round-tripping the JSON might result in:

  • a change of precision — IEEE 754 is not mandated by the JSON spec, so some implementations may do something different
  • a change of formatting — Python for example will reformat to scientific notation in some cases (12345678901234567890.0 -> 1.2345678901234567e+19) or convert some scientific notation into floats

It's also not clear how to handle special values that Python has been known to produce but strictly aren't JSON: NaN, Infinity and -Infinity.

(created from #1232)

@neilalexander neilalexander added the clarification An area where the expected behaviour is understood, but the spec could do with being more explicit label Sep 22, 2022
@richvdh
Copy link
Member

richvdh commented Sep 22, 2022

For the record, the synapse PR that changed the behaviour here was matrix-org/synapse#7381, and the MSC was MSC2540.

AFAICT the real issue here is that there is no attempt in the spec to document the pre-v6 "canonical" json. If we were to make that attempt, the most reasonable thing to do would be to spec what Synapse did in practice (and hope that it is at least constant, and not dependent on python version/machine architecture/etc). So can we just start doing that?

Of course, the point remains that mirroring that behaviour accurately may be a massive PITA for non-Python implementations. But I'm not sure what we can reasonably do about that now, other than accept that rooms v1-5 are just too much like hard work for it to be worth implementing?

@turt2live
Copy link
Member

I'm not sure what we can reasonably do about that now, other than accept that rooms v1-5 are just too much like hard work for it to be worth implementing?

Worth noting that once room upgrades don't suck there's a vague plan to mark ancient versions like this as "unstable", allowing server implementations to not even bother having support for them (alongside encouraging rooms in the wild to upgrade to something modern). This should hopefully help put the specifics behind us a bit, though we should still make an effort to document what we can, imo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clarification An area where the expected behaviour is understood, but the spec could do with being more explicit
Projects
None yet
Development

No branches or pull requests

3 participants