Fix endpoints that only differ by a trailing variable

[anoadragon453]

Some of the endpoints are differentiated simply by lack of a variable on the end. This is bad because you get into the question of what to do when you have the following endpoint definitions:

• /_matrix/some/endpoint
• /_matrix/some/endpoint/$var

and you get a request that looks like:

/_matrix/some/endpoint/

Does this match the first definition? Or the second definition with $var = ""? To make matters worse sometimes "" is a valid value!

This is currently noticed in the following endpoints, but there may be more:

Specced (as of this issue):

• GET /_matrix/client/r0/rooms/{roomId}/state/{eventType}/{stateKey}
• GET /_matrix/client/r0/rooms/{roomId}/state/{eventType}

---

• GET /_matrix/client/r0/user/{userId}/filter/
• GET /_matrix/client/r0/user/{userId}/filter/{filter}

Unspecced (as of this issue):

• GET /_matrix/federation/v1/groups/{groupId}/categories/
• GET /_matrix/federation/v1/groups/{groupId}/categories/{categoryId}

---

• GET /_matrix/federation/v1/groups/{groupId}/roles/
• GET /_matrix/federation/v1/groups/{groupId}/roles/{roleId}

[turt2live]

Considering we're a RESTful API, I think standard rules apply. /endpoint and /endpoint/ would always be treated the same unless otherwise documented.

[anoadragon453]

Ideally, but we should just really be changing things to /endpoint/{someId} and /endpoints to avoid confusion.

[turt2live]

Sure, but we decided that an empty string is a valid state key so that rule doesn't apply in many cases.

[anoadragon453]

ftr the context for this is matrix-org/synapse#3622 where we're trying to remove trailing slashes from some endpoints (including /groups/{groupId}/categories) but in the meantime we want to support both .../categories and .../categories/.

However should .../categories/ match .../categories or .../categories/{id} where {id} is an empty string?

[turt2live]

So after a bunch of research, people are divided on the issue.

It ends up being a contextual judgement call. The recommendation with a 51% majority is:

• If you're following true REST, /objects/ and /objects are the same, unless an applicable ID is given (/objects/1).
• Endpoints without IDs at the end should affect the whole set (list all, modify all, create new, delete all, etc), however we already don't do this for things like PUT /state.

So following the principles of common sense:

• Where an ID can't possibly be an empty string (/categories), assume that trailing slashes don't matter
• where an ID can be an empty string, the spec should be clear. A good case of this is state events: /state/m.room.example in both PUT and GET is clearly defined as a empty state key whereas /state/m.room.example/whatever has "whatever" as the state key.

I'm not really sure how much it makes sense to put this into the spec though. Technically speaking, the routes are all defined without trailing slashes so there's arguably no expectation for implementations to consider /endpoint the same as /endpoint/, however it is not exactly in their best interest to be sticky about this point.

[uhoreg]

In the case of GET /_matrix/client/r0/rooms/{roomId}/state/{eventType}, having the trailing slash there or not doesn't make a difference in the current spec, since GET /_matrix/client/r0/rooms/{roomId}/state/{eventType} returns the same thing as GET /_matrix/client/r0/rooms/{roomId}/state/{eventType}/{stateKey} with {stateKey} = ""