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

Clarify that MSC3827 also affects federation endpoints #3858

Merged
merged 1 commit into from
Aug 4, 2022

Conversation

turt2live
Copy link
Member

MSC: #3827

matrix-org/synapse#13031 originally added support for the feature to Synapse, which although doesn't include an obvious federation route it does end up sending the field over federation.

Here the server copies the search filter just before it goes over the wire, which is supplied by through a chain of function calls originating here.

Additionally, it is clear that this sort of feature would have included federation given the filtering is able to be proxied directly like this (as demonstrated by Synapse above).

As such, this is determined to be a clarification/minor edit to the MSC, not requiring a second MSC to add the functionality.

matrix-org/synapse#13031 originally added support for the feature to Synapse, which although doesn't include an obvious federation route it does end up sending the field over federation.

[Here](https://github.com/matrix-org/synapse/blob/a6895dd576f96d7fd086fb4128d48ac8a3f098c5/synapse/federation/transport/client.py#L481) the server copies the search filter just before it goes over the wire, which is supplied by through a chain of function calls originating [here](https://github.com/matrix-org/synapse/blob/c6d617641186221829c644204f24654430858826/synapse/rest/client/room.py#L456). 

Additionally, it is clear that this sort of feature would have included federation given the filtering is able to be proxied directly like this (as demonstrated by Synapse above).

As such, this is determined to be a clarification/minor edit to the MSC, not requiring a second MSC to add the functionality.
@turt2live turt2live requested a review from a team August 3, 2022 00:10
@turt2live turt2live marked this pull request as ready for review August 3, 2022 00:10
turt2live added a commit to matrix-org/matrix-spec that referenced this pull request Aug 3, 2022
Copy link
Contributor

@SimonBrandner SimonBrandner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!!

@turt2live turt2live merged commit f139eee into main Aug 4, 2022
@turt2live turt2live deleted the travis/fix-msc/publicrooms-s2s branch August 4, 2022 17:40
@turt2live turt2live moved this from Requires spec review (post-FCP) to Done to some definition in Spec Core Team Backlog Aug 4, 2022
turt2live added a commit to matrix-org/matrix-spec that referenced this pull request Aug 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Spec Core Team Backlog
  
Done to some definition
Development

Successfully merging this pull request may close these issues.

None yet

3 participants