The Delete Room API should use the http DELETE method, not POST

[PaarthShah]

With roots in this suggestion by @richvdh, the current Delete Room API uses POST /_synapse/admin/v1/rooms/<room_id>/delete instead of DELETE /_synapse/admin/v1/rooms/<room_id>, on the logic that DELETE should not have a request body.

This logic is incorrect, as better detailed by this stackoverflow answer, but in short, the spec supports it, and the matrix APIs already have other instances of DELETE being used with a request body.
https://matrix.org/docs/spec/client_server/latest#delete-matrix-client-r0-devices-deviceid

[richvdh]

While you're probably right that the reasoning for making it POST rather than DELETE was bogus, is it a significant problem in practice? Changing it would mean breaking any applications that are using the API.

[PaarthShah]

I agree that it's not worth breaking any applications using the API, however:

• Element (web/desktop/mobile all) does not yet make use of this API at all, so I question whether it would break anything. I certainly don't see the feature to delete a room within any of the apps.
• I find it unlikely that any user-made scripts would depend on this API considering its nature (why would anyone need to systemically delete a room so regularly?)
• It would seem to be fairly trivial to just deprecate (but not disable) the API temporarily if either of the above were a major concern anyway.
• It is better to remain consistent with previously-established Matrix behavior even if it is wrong for ease of learning; quite frankly, it makes Synapse/Matrix look really unpolished when different API paradigms are being used in this fashion. If the POST /delete style were consistently used, then I probably wouldn't have even brought it up. And considering that this is a _synapse endpoint rather than a _matrix common one, it seems that the burden should be on Synapse to adjust to the greater-established API style, for v1.25 (where the old room purge etc APIs are being deprecated as well).

[richvdh]

I'm don't have strong opinions here. cc @dklimpel

[dklimpel]

I have no opinion on this.
I think that there should only be a consistent approach for Synapse, which is then documented.
There are much more delete APIs: group, devices, media, etc.

[ptman]

allow DELETE and deprecate POST?

[PaarthShah]

allow DELETE and deprecate POST?

Would definitely be my suggestion.

[clokep]

allow DELETE and deprecate POST?

This was going to be my suggestion as well.

Note also that this is an admin API so the spec is somewhat...unrelated.

We would accept a PR for this if it was backwards compatible, but the core team likely will not have a chance to work on this.

[PaarthShah]

I could definitely work on this sometime in this coming week, most likely, though I've never contributed to this particular project before :)

[ShadowJonathan]

Maybe allow DELETE and POST side-by-side for a while, making on_POST call on_DELETE, and then decide on deprecation later.

[shadowcat-mst]

Per the HTTP spec this is undefined behaviour and existing implementations - including proxies - are explicitly called out as potentially rejecting such a DELETE request.

See #11698 (comment) for expansion on that /cc @PaarthShah