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.

Sign up for GitHub

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

Jump to bottom

Remove spurious event_id from federation APIs, and convert to POST? #555

Open
richvdh opened this issue Oct 25, 2019 · 3 comments
Open

Remove spurious event_id from federation APIs, and convert to POST? #555

richvdh opened this issue Oct 25, 2019 · 3 comments
Labels
A-S2S Server-to-Server API (federation) wart A point where the protocol is inconsistent or inelegant

Comments

@richvdh
Copy link
Member

richvdh commented Oct 25, 2019 •
edited

The following federation APIs have roomIds and eventIds in the path which are completely redundant:

  • PUT /_matrix/federation/vN/send_join/{roomId}/{eventId}
  • PUT /_matrix/federation/vN/invite/{roomId}/{eventId}
  • PUT /_matrix/federation/vN/send_leave/{roomId}/{eventId}
  • PUT /_matrix/federation/v1/exchange_third_party_invite/{roomId}

The room_id is given in the body, and the event_id is either in the body or calculated from the hash. Deduplication takes place based on that event id, not on the path params.

The implication of all this is that we could remove the roomId and eventId path params, and convert the request to a POST.

One counter-argument is that it would make sharding by room harder if the roomId was omitted. So maybe we should leave that there (but still remove eventId and switch to POST).

While I'm here: why is PUT /_matrix/federation/v1/3pid/onbind a PUT ?
@turt2live
Copy link
Member

tbh the solution here depends on which RESTful guidelines you end up reading.

@turt2live turt2live added A-S2S Server-to-Server API (federation) wart A point where the protocol is inconsistent or inelegant labels Oct 25, 2019
@richvdh
Copy link
Member Author

richvdh commented Nov 17, 2020

these go beyond a wart: they are foot-guns because you can end up using the path param in one place and then the body param in another.

@richvdh
Copy link
Member Author

richvdh commented Nov 17, 2020

if we keep both sets of params, what is the correct behaviour when there is a mismatch? a 400 error of some kind?

@richvdh richvdh mentioned this issue Nov 17, 2020
Closed
@richvdh richvdh mentioned this issue Nov 18, 2020
Merged
richvdh referenced this issue in matrix-org/synapse Nov 18, 2020
@richvdh
Consistently use room_id from federation request body
c186fda 
Some federation APIs have a redundant `room_id` path param (see
https://github.com/matrix-org/matrix-doc/issues/2330). We should make sure we
consistently use either the path param or the body param, and the body param is
easier.
gnieto referenced this issue in gnieto/ruma Nov 18, 2020
@gnieto
Use Pdu on create_join_event endpoints
f15b9d7 
Sending requests to this endpoints to Synapse/Dendrite homeservers leads
to some deserialization errors.

After claryfing it
(matrix-org/matrix-spec-proposals#2856), `room_id` and
`event_id` fields are expected to appear on request's body and also on
path params. It seems that there's some initiative, in any case, to
remove the parameters from path:
https://github.com/matrix-org/matrix-doc/issues/2330
@gnieto gnieto mentioned this issue Nov 18, 2020
Merged
gnieto referenced this issue in gnieto/ruma Nov 18, 2020
@gnieto
Use Pdu on create_join_event endpoints
1485882 
Sending requests to this endpoints to Synapse/Dendrite homeservers leads
to some deserialization errors.

After claryfing it
(matrix-org/matrix-spec-proposals#2856), `room_id` and
`event_id` fields are expected to appear on request's body and also on
path params. It seems that there's some initiative, in any case, to
remove the parameters from path:
https://github.com/matrix-org/matrix-doc/issues/2330
jplatte referenced this issue in ruma/ruma Nov 18, 2020
@gnieto @jplatte
Use Pdu on create_join_event endpoints
b168c38 
Sending requests to this endpoints to Synapse/Dendrite homeservers leads
to some deserialization errors.

After claryfing it
(matrix-org/matrix-spec-proposals#2856), `room_id` and
`event_id` fields are expected to appear on request's body and also on
path params. It seems that there's some initiative, in any case, to
remove the parameters from path:
https://github.com/matrix-org/matrix-doc/issues/2330
richvdh referenced this issue in matrix-org/synapse Nov 19, 2020
@richvdh
Consistently use room_id from federation request body (#8776)
950bb03 
* Consistently use room_id from federation request body

Some federation APIs have a redundant `room_id` path param (see
https://github.com/matrix-org/matrix-doc/issues/2330). We should make sure we
consistently use either the path param or the body param, and the body param is
easier.

* Kill off some references to "context"

Once upon a time, "rooms" were known as "contexts". I think this kills of the
last references to "contexts".
erikjohnston referenced this issue in matrix-org/synapse Dec 9, 2020
@richvdh @erikjohnston
Consistently use room_id from federation request body (#8776)
3ce2f30 
* Consistently use room_id from federation request body

Some federation APIs have a redundant `room_id` path param (see
https://github.com/matrix-org/matrix-doc/issues/2330). We should make sure we
consistently use either the path param or the body param, and the body param is
easier.

* Kill off some references to "context"

Once upon a time, "rooms" were known as "contexts". I think this kills of the
last references to "contexts".
@richvdh richvdh transferred this issue from matrix-org/matrix-spec-proposals Mar 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-S2S Server-to-Server API (federation) wart A point where the protocol is inconsistent or inelegant
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@turt2live @richvdh