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

.m.rule.tombstone and .m.rule.room.server_acl pushrules are in the wrong order #1406

Closed
richvdh opened this issue Jan 18, 2023 · 0 comments · Fixed by #1421
Closed

.m.rule.tombstone and .m.rule.room.server_acl pushrules are in the wrong order #1406

richvdh opened this issue Jan 18, 2023 · 0 comments · Fixed by #1421
Labels
A-Push spec-bug Something which is in the spec, but is wrong

Comments

@richvdh
Copy link
Member

richvdh commented Jan 18, 2023

Link to problem area:

https://spec.matrix.org/v1.5/client-server-api/#default-override-rules

Issue

.m.rule.tombstone and .m.rule.room.server_acl are listed as happening before .m.rule.roomnotif; in practice (see synapse implementation, previously in python) they take place after .m.rule.roomnotif.

In practice, it's unlikely that a given event will match .m.rule.roomnotif as well as one of those two rules, so it doesn't matter that much, but it's an unnecessary source of confusion.

The problem appears to have arisen from matrix-org/matrix-spec-proposals#2020, which inexplicably inserted .m.rule.tombstone into the middle of the list, in contrast to the implementation (linked above) and MSC1930 (which didn't explicitly mention an order, so "the end" was the most plausible interpretation).

This was then compounded when .m.rule.room.server_acl was added by MSC3786, saying "immediately after .m.rule.tombstone". The interpretation of that obviously depended on where .m.rule.tombstone appeared in the list.

I believe the spec has always been wrong here, and .m.rule.tombstone and .m.rule.room.server_acl should be moved after .m.rule.roomnotif.

(Note also that Synapse implements an .m.rule.reaction, from MSC2677).

@richvdh richvdh added spec-bug Something which is in the spec, but is wrong A-Push labels Jan 18, 2023
richvdh added a commit that referenced this issue Jan 31, 2023
See #1406 for the reasoning on
this. TL;DR: the spec has always been wrong here.
richvdh added a commit that referenced this issue Feb 10, 2023
See #1406 for the reasoning on
this. TL;DR: the spec has always been wrong here.
clokep pushed a commit to clokep/matrix-spec that referenced this issue May 3, 2023
See matrix-org#1406 for the reasoning on
this. TL;DR: the spec has always been wrong here.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Push spec-bug Something which is in the spec, but is wrong
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant