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.

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

MSC3743: Standardized error response for unknown endpoints #3743

Merged
merged 10 commits into from
Oct 31, 2022
Merged
189 changes: 189 additions & 0 deletions proposals/3743-errors-for-unknown-endpoints.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,189 @@
# MSC3743: Standardized error response for unknown endpoints

Matrix does not define how a server should treat unknown endpoints. This makes it
difficult to determine whether an endpoint is responding with a legitimate error
(e.g. a `404` for an object not being found) or because it does not support the
endpoint.

This has impacted clients [wishing to support stable features](https://github.com/vector-im/element-web/issues/19738),
as well as requiring servers to [implement workarounds](https://github.com/matrix-org/synapse/blob/a711ae78a8f8ba406ff122035c8bf096fac9a26c/synapse/federation/federation_client.py#L602-L622)
based on heuristics.


## Proposal

The Client-Server API, Server-Server API, Application Service API, Identity Service API,
and Push Gateway shall respond with the following for all paths under the `/_matrix` prefix:

* `404` HTTP error response with an error code of `M_UNRECOGNIZED` if a request to an unknown
path is received.
* `405` HTTP error response with an error code of `M_UNRECOGNIZED` if an invalid method for a
known path is received.

Both of these can be treated the same from the client making the request and represent that
the request cannot be routed to business logic.

Note that the major homeserver implementations have recently settled on using `M_UNRECOGNIZED`,
so it seems reasonable to specify this formally.

## Potential issues

Servers and clients may still need to rely on heuristics until this is widely
available, but this proposal should not cause any additional issues.

A `M_UNRECOGNIZED` error code with a status of `400` has more specific definition
in the Identity Server API:

> The request contained an unrecognised value, such as an unknown token or medium.

## Alternatives

[MSC3723](https://github.com/matrix-org/matrix-doc/pull/3723) could be an alternative
to this, but it has the downside of servers needing to track the version state
of each other server it is interacting with.
clokep marked this conversation as resolved.
Show resolved Hide resolved

It is seen as being more narrowly applicable (only to the Server-Server API),
while also being more complicated to implement.
turt2live marked this conversation as resolved.
Show resolved Hide resolved

It could be proposed to only handle the known prefixes under `/_matrix`:

* `/_matrix/client/`
* `/_matrix/federation/`
* `/_matrix/media/`
* `/_matrix/keys/`
* `/_matrix/push/`
* `/_matrix/identity/`
* `/_matrix/app/`

But it is more future-proof to include the entire `/_matrix` prefix in case
additional services are added to the Matrix ecosystem.

## Security considerations

None.

## Unstable prefix

None (as this is about unknown endpoints!)

## Current status

[Issue #1492](https://github.com/matrix-org/matrix-doc/issues/1492) discusses this
problem a bit, but does not propose a concrete solution. [^0]

### Homeserver API

Tested by querying for:

* Unprefixed: `GET /_matrix/unknown`
* Client-Server: `GET /_matrix/client/v4/login`,
* Server-Server: `GET /_matrix/federation/unknown`.
* Media: `GET /_matrix/media/unknown`
* Keys: `GET /_matrix/keys/unknown`

Tested incorrect method via:

* `PUT /_matrix/client/v3/login`
* `PUT /_matrix/federation/v1/version`
* `PUT /_matrix/media/v3/upload`
* `PUT /_matrix/v3/keys/upload`

If the below doesn't include differences for an incorrect method, assume it is
identical to the behavior of an unknown endpoint.

----

#### Synapse:

* Unprefixed, and Keys: 404 error with an HTML body
* Client-Server:
* < 1.53.0: 404 error with an HTML body
* \>= 1.53.0: 400 error with a JSON body [^1]
```json
{"errcode": "M_UNRECOGNIZED", "error": "Unrecognized request"}
```
* Server-Server: 400 error with a JSON body
```json
{"errcode": "M_UNRECOGNIZED", "error": "Unrecognized request"}
```
* Media:
* 404 error with an HTML body
* Incorrect method: 400 error with a JSON body:
```json
{"errcode": "M_UNRECOGNIZED", "error": "Unrecognized request"}
```

#### Dendrite:

* Unprefixed and Server-Server:
* 404 error with a text body of `404 page not found`
* Incorrect method: 405 error with no body
* Client-Server:
* < 0.10.0: 404 error with a text body of `404 page not found`
* \>= 0.10.0: 404 error with a JSON body (but a `text/plain` content type) [^2]
```json
{"errcode": "M_UNRECOGNIZED", "error": "Unrecognized request"}
```
* Media:
* 404 error with a text body of `404 page not found`
* Incorrect method: 405 error with a text body of `405 PUT not allowed on this endpoint`
* Keys: 404 error with a text body of `404 page not found`

#### Conduit:

* Unprefied, Client-Server, Server-Server, and Media:
* < 0.4.0: 404 error with no body
* == 0.4.0: 404 error with a JSON body [^3]
```json
{"errcode": "M_NOT_FOUND", "error": "M_NOT_FOUND: Unknown or unimplemented route"}
```
* \> 0.4.0: 404 error with a JSON body [^4]
```json
{"errcode": "M_UNRECOGNIZED", "error": "M_UNRECOGNIZED: Unrecognized request"}
```
* Incorrect method: 405 error with no body
* Keys: handled as unknown endpoints as above -- unimplemented?

#### matrix-media-repo (Media-only):

* 404 error with a JSON body:
```json
{"errcode": "M_NOT_FOUND", "error": "Not found", "mr_errcode": "M_NOT_FOUND"}
```
* Incorrect method: 405 error with a JSON body:
```json
{"errcode": "M_UNKNOWN", "error": "Method Not Allowed", "mr_errcode": "M_METHOD_NOT_ALLOWED"}
```

### Application Service API
clokep marked this conversation as resolved.
Show resolved Hide resolved

Tested by querying for `GET /_matrix/app/unknown`

Untested, but anecdotally been told this is poorly handled.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to @Half-Shot matrix-bot-sdk just times out.

I'm not sure it is terribly important to have this for application services, but I don't see why we wouldn't if everywhere else in the ecosystem has a consistent error code for unknown endpoints.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some http router libraries are just bad at things, but arguably that's a reason to find one which supports proper 404 errors.

Appservices are generally heavily tied to server support though, so it's less important for them to match the behaviour of this MSC. Usually people ask if a given appservice works with a given version of Synapse, for instance (rather than just installing and hoping for the best).

This is all in dramatic contrast to regular clients which just fire off requests and hope for the best.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(for clarity: that doesn't mean we should exclude appservices, just that at the spec compliance level we're less likely to fuss about it)


### Identity Service API,

Tested by querying for `GET /_matrix/identity/unknown`.

* Sydent:
* 404 with an HTML body
* Incorrect method: 405 with an HTML body

### Push Gateway

Tested by querying for `GET /_matrix/push/unknown`.

* Sygnal:
* 404 with an HTML body
* Incorrect method: 405 with an HTML body

## Dependencies

None.

[^0]: Tests were run against matrix.org (1.69.0rc3 (b=matrix-org-hotfixes,aca3a117a9));
dendrite.matrix.org (0.10.2); and conduit.rs (0.4.0-next).
[^1]: https://github.com/matrix-org/synapse/issues/11600
[^2]: https://github.com/matrix-org/dendrite/issues/2739
[^3]: https://gitlab.com/famedly/conduit/-/merge_requests/306
[^4]: https://gitlab.com/famedly/conduit/-/merge_requests/388