search

matrix-org / synapse

Synapse: Matrix homeserver written in Python/Twisted.
https://matrix-org.github.io/synapse
Apache License 2.0
11.8k stars 2.13k forks source link

Synapse accepts a PUT /createRoom request #14535

Open DMRobertson opened 1 year ago

DMRobertson commented 1 year ago

But the spec says it's only to be POSTed.

https://spec.matrix.org/v1.5/client-server-api/#creation

In particular, /createRoom is not idempotent, so it shouldn't be a PUT.

DMRobertson commented 1 year ago

https://github.com/matrix-org/matrix-spec/issues/222 confuses the matter somewhat

DMRobertson commented 1 year ago

Pinged clients here'; no requests to PUT /_matrix/client/[^/]*/createRoom HTTP in the last day. Let's bin it.

DMRobertson commented 1 year ago

Hmm. On reflection it looks like the implementation is handling a transaction id: https://github.com/matrix-org/synapse/blob/86c5a710d8b4212f8a8a668d7d4a79c0bb371508/synapse/rest/client/room.py#L153-L158

which means it's doing exactly what https://github.com/matrix-org/matrix-spec/issues/222 asks for. Maybe we just add this to the spec?

tulir commented 1 year ago

How is it handling a transaction ID without a transaction ID parameter in the path? Also, is it actually idempotent or does it just eat a transaction ID and do nothing with it?

I'd say it's safe to remove either way and re-add properly once there's a MSC.

DMRobertson commented 1 year ago

How is it handling a transaction ID without a transaction ID parameter in the path?

See https://github.com/matrix-org/synapse/blob/86c5a710d8b4212f8a8a668d7d4a79c0bb371508/synapse/rest/client/room.py#L149-L151

and the implementation of register_txn_path.

Also, is it actually idempotent or does it just eat a transaction ID and do nothing with it?

It looks idempotent enough to me. The transaction id is included here in the key we use to deduplicate requests:

https://github.com/matrix-org/synapse/blob/fa0eab9c8e159b698a31fc7cfaafed643f47e284/synapse/rest/client/transactions.py#L70-L85

https://github.com/matrix-org/synapse/blob/fa0eab9c8e159b698a31fc7cfaafed643f47e284/synapse/rest/client/transactions.py#L53-L68

A comment suggests that we guarantee deduplication within a 30 minute range. https://github.com/matrix-org/synapse/blob/fa0eab9c8e159b698a31fc7cfaafed643f47e284/synapse/rest/client/transactions.py#L46-L50

tulir commented 1 year ago

Huh, so synapse also has undocumented PUT versions of all the membership and join endpoints 🤔

I guess that just needs a MSC for PUT /createRoom then, although it'd be nice to switch to /create_room too