Strictly enforce canonicaljson requirements in a new room version#7381
Strictly enforce canonicaljson requirements in a new room version#7381
Conversation
|
Requesting review on this to get some thoughts on the approach. |
|
I did some testing with two different clients:
Are there other scenarios to test? |
|
It's not a canonicaljson thing, but, while we're adding a new room version, might be good to check that power levels have to be ints rather than strings (and perhaps sanitycheck other type safety in synapse) |
that is the thin end of a very large wedge. I think we're better off keeping this constrained for now. |
| raise SynapseError(400, "JSON integer out of range", Codes.BAD_JSON) | ||
|
|
||
| elif isinstance(value, float): | ||
| # Note that Infinity, -Infinity, and NaN are also considered floats. |
There was a problem hiding this comment.
CanonicalJSON indeed does aim to represent all floats as ints, just in case anyone was unsure about blocking all floats: https://matrix.org/docs/spec/appendices#canonical-json
There was a problem hiding this comment.
Correct, as the description says all floats need to be rejected. I didn't find the specification super clear here unless you read into the grammar though. I think we could improve that.
richvdh
left a comment
There was a problem hiding this comment.
this looks like generally the right plan to me.
I wouldn't be too worried about performance here: event_from_pdu_json is called only once for each event, and it tends to happen on workers rather than on the master.
This PR does the following:
Replaces #7356
Implements MSC2540: matrix-org/matrix-spec-proposals#2540.
To Do
event_from_pdu_json.)