From 420180a69ddd25d44b1c7c048d809b05683978f4 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 3 Mar 2020 12:30:19 -0500 Subject: [PATCH 1/5] MSC: Remove query_auth federation endpoint. --- .gitignore | 1 + ...x-remove-query_auth-federation-endpoint.md | 31 +++++++++++++++++++ 2 files changed, 32 insertions(+) create mode 100644 proposals/xxxx-remove-query_auth-federation-endpoint.md diff --git a/.gitignore b/.gitignore index 800be2fa..9cc27b85 100644 --- a/.gitignore +++ b/.gitignore @@ -12,3 +12,4 @@ *.swp _rendered.rst /.vscode/ +/.idea/ diff --git a/proposals/xxxx-remove-query_auth-federation-endpoint.md b/proposals/xxxx-remove-query_auth-federation-endpoint.md new file mode 100644 index 00000000..7930c5f8 --- /dev/null +++ b/proposals/xxxx-remove-query_auth-federation-endpoint.md @@ -0,0 +1,31 @@ +# MSCxxxx: Remove the `query_auth` federation endpoint + +The `query_auth` federation endpoint is unused by Synapse and should be removed. +The current implementation in Synapse is broken and will return a 500 error in +some situations. + +## Proposal + +Remove: + +* [POST `/_matrix/federation/v1/query_auth/{roomId}/{eventId}`](https://matrix.org/docs/spec/server_server/r0.1.3#post-matrix-federation-v1-query-auth-roomid-eventid) + +## Potential issues + +Removing this endpoint impacts backwards compatibility. + +In practice, removing this endpoint should have minimal impact. Since 1.5.0rc1 +of Synapse this endpoint is not called (see [#6214](https://github.com/matrix-org/synapse/pull/6214)). +During removal it was noted that the code to call this endpoint was already +unreachable. + +Note that it seems like this was initially supported in Synapse v0.7.0. It is +not clear at what point it became unused. + +## Alternatives + +The endpoint could be deprecated in removed in a future version of the specification. + +## Security considerations + +None. From c3420770ad3a559217ee5ae7a5998b1c78960055 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 3 Mar 2020 13:15:32 -0500 Subject: [PATCH 2/5] Clarify history of endpoint. --- ...x-remove-query_auth-federation-endpoint.md | 33 +++++++++++++------ 1 file changed, 23 insertions(+), 10 deletions(-) diff --git a/proposals/xxxx-remove-query_auth-federation-endpoint.md b/proposals/xxxx-remove-query_auth-federation-endpoint.md index 7930c5f8..387f2efa 100644 --- a/proposals/xxxx-remove-query_auth-federation-endpoint.md +++ b/proposals/xxxx-remove-query_auth-federation-endpoint.md @@ -1,26 +1,39 @@ # MSCxxxx: Remove the `query_auth` federation endpoint The `query_auth` federation endpoint is unused by Synapse and should be removed. -The current implementation in Synapse is broken and will return a 500 error in -some situations. +The current implementation in Synapse is not robust and will return a 500 error +in some situations. ## Proposal -Remove: +Remove the following endpoint: * [POST `/_matrix/federation/v1/query_auth/{roomId}/{eventId}`](https://matrix.org/docs/spec/server_server/r0.1.3#post-matrix-federation-v1-query-auth-roomid-eventid) ## Potential issues -Removing this endpoint impacts backwards compatibility. +Removing this endpoint impacts backwards compatibility, in practice removing +this endpoint should have minimal impact as it was an unused error path in +Synapse. The federation client code to call this endpoint was removed in Synapse +v1.5.0rc1. + +Note that dendrite has never implemented this federation endpoint. + +### History + +This endpoint (and the federation client code) to call it was initially +added in Synapse v0.7.0 (see [#43](https://github.com/matrix-org/synapse/pull/43)). +The federation client code was heavily modified for v1.0.0rc1 (see +[#5227](https://github.com/matrix-org/synapse/pull/5227/)), + +The federation client code to call this endpoint was removed in v1.5.0rc1 of +Synapse (see [#6214](https://github.com/matrix-org/synapse/pull/6214). After +that point this endpoint is not called). -In practice, removing this endpoint should have minimal impact. Since 1.5.0rc1 -of Synapse this endpoint is not called (see [#6214](https://github.com/matrix-org/synapse/pull/6214)). During removal it was noted that the code to call this endpoint was already -unreachable. - -Note that it seems like this was initially supported in Synapse v0.7.0. It is -not clear at what point it became unused. +unreachable. It seems that this code was never reachable and was meant for an +error situation which was never built out (see `git log -S NOT_ANCESTOR`, the +error condition is never assigned). ## Alternatives From 6754d5ba5f4e98ed67c5771a7176cd7d3ecba582 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 3 Mar 2020 16:18:59 -0500 Subject: [PATCH 3/5] Move filename based on MSC #. --- ...ndpoint.md => 2451-remove-query_auth-federation-endpoint.md} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename proposals/{xxxx-remove-query_auth-federation-endpoint.md => 2451-remove-query_auth-federation-endpoint.md} (96%) diff --git a/proposals/xxxx-remove-query_auth-federation-endpoint.md b/proposals/2451-remove-query_auth-federation-endpoint.md similarity index 96% rename from proposals/xxxx-remove-query_auth-federation-endpoint.md rename to proposals/2451-remove-query_auth-federation-endpoint.md index 387f2efa..d5c2af4d 100644 --- a/proposals/xxxx-remove-query_auth-federation-endpoint.md +++ b/proposals/2451-remove-query_auth-federation-endpoint.md @@ -1,4 +1,4 @@ -# MSCxxxx: Remove the `query_auth` federation endpoint +# MSC2451: Remove the `query_auth` federation endpoint The `query_auth` federation endpoint is unused by Synapse and should be removed. The current implementation in Synapse is not robust and will return a 500 error From 68357a7d23420b069308cfbe99244168307230d6 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 4 Mar 2020 07:31:38 -0500 Subject: [PATCH 4/5] Fix a typo in -> and. Co-Authored-By: Matthew Hodgson --- proposals/2451-remove-query_auth-federation-endpoint.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proposals/2451-remove-query_auth-federation-endpoint.md b/proposals/2451-remove-query_auth-federation-endpoint.md index d5c2af4d..0d84069e 100644 --- a/proposals/2451-remove-query_auth-federation-endpoint.md +++ b/proposals/2451-remove-query_auth-federation-endpoint.md @@ -37,7 +37,7 @@ error condition is never assigned). ## Alternatives -The endpoint could be deprecated in removed in a future version of the specification. +The endpoint could be deprecated and removed in a future version of the specification. ## Security considerations From 61715f6452bfda06b273d1fc8a94b9412f4910b6 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Wed, 4 Mar 2020 08:07:05 -0500 Subject: [PATCH 5/5] Update and expand the proposal based on feedback and additional info. --- ...1-remove-query_auth-federation-endpoint.md | 20 +++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/proposals/2451-remove-query_auth-federation-endpoint.md b/proposals/2451-remove-query_auth-federation-endpoint.md index 0d84069e..b2d8bc9f 100644 --- a/proposals/2451-remove-query_auth-federation-endpoint.md +++ b/proposals/2451-remove-query_auth-federation-endpoint.md @@ -1,8 +1,19 @@ # MSC2451: Remove the `query_auth` federation endpoint -The `query_auth` federation endpoint is unused by Synapse and should be removed. -The current implementation in Synapse is not robust and will return a 500 error -in some situations. +This API was added without sufficient thought nor testing. The endpoint isn't +used in any known implementations, and we do not believe it to be necessary +for federation to work. The only known implementation (in Synapse) was not fully +fleshed out and is broken. + +For background, the idea behind this endpoint was that two homeservers would be +able to share state events with the hope of filling in missing state from one +of homeservers allowing state resolution to complete. This was to protect +against a joining server not providing the full (or providing stale) state. + +In addition to the ideas above not coming to fruition, it is unclear whether the +current design of this endpoint would be sufficient. If this state negotiation +feature is needed in the future it should be redesigned from scratch via the MSC +proposal process. ## Proposal @@ -17,7 +28,8 @@ this endpoint should have minimal impact as it was an unused error path in Synapse. The federation client code to call this endpoint was removed in Synapse v1.5.0rc1. -Note that dendrite has never implemented this federation endpoint. +There is no evidence of other homeserver implementations having implemented this +endpoint. ### History