Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Overly strict validation of /sync filters #14365

Closed
squahtx opened this issue Nov 4, 2022 · 0 comments · Fixed by #14369
Closed

Overly strict validation of /sync filters #14365

squahtx opened this issue Nov 4, 2022 · 0 comments · Fixed by #14369
Assignees
Labels
A-Push Issues related to push/notifications A-Sync defects related to /sync A-Threads Threaded messages O-Frequent Affects or can be seen by most users regularly or impacts most users' first experience S-Major Major functionality / product severely impaired, no satisfactory workaround. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.

Comments

@squahtx
Copy link
Contributor

squahtx commented Nov 4, 2022

This caused a nasty Element Android bug where initial syncs could not complete: element-hq/element-android#7516

For forward compatibility reasons, it's important for homeservers to ignore unrecognized fields in requests. Synapse does not do this for /sync filters, and returns an error to clients when there are unrecognized fields present.

Note that on the client side, clients are generally supposed to determine whether new fields are supported, based on the supported spec versions and unstable features in the /_matrix/client/versions response.

(thanks to richvdh and babolivier for clarifying how forward compatibility is supposed to work)


We'll want to set "additionalProperties": True in all of our filter schemas.

ROOM_EVENT_FILTER_SCHEMA = {
"additionalProperties": False,
"type": "object",
"properties": {
"limit": {"type": "number"},
"senders": {"$ref": "#/definitions/user_id_array"},
"not_senders": {"$ref": "#/definitions/user_id_array"},
"types": {"type": "array", "items": {"type": "string"}},
"not_types": {"type": "array", "items": {"type": "string"}},
"rooms": {"$ref": "#/definitions/room_id_array"},
"not_rooms": {"$ref": "#/definitions/room_id_array"},
"contains_url": {"type": "boolean"},
"lazy_load_members": {"type": "boolean"},
"include_redundant_members": {"type": "boolean"},
# Include or exclude events with the provided labels.
# cf https://github.com/matrix-org/matrix-doc/pull/2326
"org.matrix.labels": {"type": "array", "items": {"type": "string"}},
"org.matrix.not_labels": {"type": "array", "items": {"type": "string"}},
# MSC3440, filtering by event relations.
"related_by_senders": {"type": "array", "items": {"type": "string"}},
"related_by_rel_types": {"type": "array", "items": {"type": "string"}},
},
}

@squahtx squahtx added A-Push Issues related to push/notifications A-Sync defects related to /sync S-Major Major functionality / product severely impaired, no satisfactory workaround. A-Threads Threaded messages O-Frequent Affects or can be seen by most users regularly or impacts most users' first experience labels Nov 4, 2022
squahtx pushed a commit that referenced this issue Nov 4, 2022
For forward compatibility, Synapse needs to ignore fields it does not
recognise instead of raising an error.

Fixes #14365.

Signed-off-by: Sean Quah <[email protected]>
@squahtx squahtx self-assigned this Nov 4, 2022
squahtx pushed a commit that referenced this issue Nov 4, 2022
For forward compatibility, Synapse needs to ignore fields it does not
recognise instead of raising an error.

Fixes #14365.

Signed-off-by: Sean Quah <[email protected]>
@squahtx squahtx added the T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. label Nov 4, 2022
squahtx added a commit that referenced this issue Nov 7, 2022
For forward compatibility, Synapse needs to ignore fields it does not
recognise instead of raising an error.

Fixes #14365.

Signed-off-by: Sean Quah <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Push Issues related to push/notifications A-Sync defects related to /sync A-Threads Threaded messages O-Frequent Affects or can be seen by most users regularly or impacts most users' first experience S-Major Major functionality / product severely impaired, no satisfactory workaround. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant