Validate that integers are in the proper range while decoding JSON.

[clokep]

Per the Matrix specification, canonical JSON needs to limit integers in JSON to the range of [-2 ^ 53 + 1, 2 ^ 53 + 1] to match RFC 7159.

This PR enforces this restriction in Synapse for all "incoming" JSON data that is deserialized by Synapse, including:

• The client-server API.
• The server-server API.
• The identity server API.
• The push gateway API.

This is mostly done via modifying the core areas that parse JSON from HTTP queries and HTTP responses.

This does NOT make any changes to:

• The replication API (which uses JSON internally within Synapse workers).
• Data that is sent via Synapse.
• Loading of JSON data from the database.

My hope is that this will mean nothing in Synapse will break if a database already has bad data in it.

Note that I do not believe this is really part of the "canonicaljson" package since the APIs should not be using these invalid values anyway. After this change, anytime we generate a canonicaljson value it should be after the JSON has been validated, so it should already abide by these rules.

[clokep]

SyTest PR up at matrix-org/sytest#860

[clokep]

I believe this hunk is not necessary anymore due to Synapse no longer supporting Python 2.

[richvdh]

The main problem with this is the existence of historical events (and hence rooms) which do not honour the spec. We'll essentially be breaking the ability to join those rooms, or to backfill in them if the homeserver is already joined.

This may be acceptable collateral... but I'm not completely sure it is. We should probably at least check what the failure modes are?

Also... does this mean that we end up using the pure-python json parsing implementation in simplejson rather than the optimised C version? that would probably be prohibitive in terms of performance overhead.

[clokep]

The main problem with this is the existence of historical events (and hence rooms) which do not honour the spec. We'll essentially be breaking the ability to join those rooms, or to backfill in them if the homeserver is already joined.

This may be acceptable collateral... but I'm not completely sure it is. We should probably at least check what the failure modes are?

I'll have to check more into this, I had only considered being able to load events that have already been stored in your database. I'll test making some broken rooms and see what happens.

Also... does this mean that we end up using the pure-python json parsing implementation in simplejson rather than the optimised C version? that would probably be prohibitive in terms of performance overhead.

I did a little bit of benchmarking and this seems to have a negligible effect on performance, but I am not sure how representative my dataset is. It still uses the C-speedups, but would cause it to bail on a fast-path of converting the string to the int (see this portion of _match_number_str). The overhead here is essentially a function call from C back out to Python + the overhead of the new method.

Looking at the specification and some sample events it seems like the vast majority of JSON values are strings, not integers, so I'm hoping the effect of this change would be minimized.

[clokep]

@richvdh and I discussed this a bit in #synapse-dev:matrix.org and there's a few major issues with it:

1. We need to ensure that different servers agree upon what data is "valid" in a room, so this will need to take into account the room version to avoid split-brain rooms. (This also links us more concretely to canonicaljson.)
2. It is probably OK to always enforce this on the client-server API.
   • Although there is some question of a client feature which relies on sticking floats into account data for the ordering of rooms in the room list. This is currently disabled in Riot. Floats shouldn't be a problem though, so this likely OK (?).
3. I missed a code path and this isn't actually being applied to responses to federation requests right now (oops).
4. I'll need to further investigate other uses where Synapse makes JSON requests.

[clokep]

I'm going to close this PR and attempt a different approach.