Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Synapse supports unspecced DELETE /directory/list/room/<room ID> endpoint #13111

Closed
DMRobertson opened this issue Jun 22, 2022 · 6 comments · Fixed by #13123
Closed

Synapse supports unspecced DELETE /directory/list/room/<room ID> endpoint #13111

DMRobertson opened this issue Jun 22, 2022 · 6 comments · Fixed by #13123
Labels
A-Spec-Compliance places where synapse does not conform to the spec P4 (OBSOLETE: use S- labels.) Okay backlog: will not schedule, will accept patches T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks.

Comments

@DMRobertson
Copy link
Contributor

Synapse supports a DELETE /directory/list/room/<room ID> endpoint which appears to be unspecced. (It's specced with GET and PUT).

async def on_DELETE(
self, request: SynapseRequest, room_id: str
) -> Tuple[int, JsonDict]:
requester = await self.auth.get_user_by_req(request)
await self.directory_handler.edit_published_room_list(
requester, room_id, "private"
)
return 200, {}

Originally posted by @DMRobertson in #13102 (comment)

DELETE is a shortcut for PUTting private visibility. I'd like to either spec it or remove it.

@DMRobertson DMRobertson changed the title I think there are a few things going on here; I don't think your diagnosis is completely correct. Synapse supports unspecced DELETE /directory/list/room/<room ID> endpoint Jun 22, 2022
@DMRobertson DMRobertson added A-Spec-Compliance places where synapse does not conform to the spec T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks. P4 (OBSOLETE: use S- labels.) Okay backlog: will not schedule, will accept patches labels Jun 22, 2022
@santhoshivan23
Copy link
Contributor

santhoshivan23 commented Jun 24, 2022

@DMRobertson The right place to spec it is in matrix-spec repo right? I think its better to spec it. What say?

@clokep
Copy link
Member

clokep commented Jun 27, 2022

Part of #8334.

@richvdh
Copy link
Member

richvdh commented Jun 27, 2022

I think its better to spec it

I don't like having more than one way to do something: it leads to incompatible implementations. If you can do the equivalent with PUT, better to remove DELETE.

@DMRobertson
Copy link
Contributor Author

I don't like having more than one way to do something: it leads to incompatible implementations. If you can do the equivalent with PUT, better to remove DELETE.

Agreed entirely. What I should have said above: we should check to see if any clients use this (i.e. if matrix.org has seen a significant number of requests to this endpoint with method DELETE). I hope not, so we can rip this out.

@richvdh
Copy link
Member

richvdh commented Jun 27, 2022

element-web uses PUT. No DELETE requests to matrix.org in the past week. Let's rip it out.

@DMRobertson
Copy link
Contributor Author

There is also an unspecced DELETE on the application-service version of this endpoint.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Spec-Compliance places where synapse does not conform to the spec P4 (OBSOLETE: use S- labels.) Okay backlog: will not schedule, will accept patches T-Task Refactoring, removal, replacement, enabling or disabling functionality, other engineering tasks.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants