element-hq / synapse

Synapse: Matrix homeserver written in Python/Twisted.
https://element-hq.github.io/synapse
GNU Affero General Public License v3.0
1.62k stars 203 forks source link

Correct docs for max file size with reverse proxy #17053

Open HarHarLinks opened 7 months ago

HarHarLinks commented 7 months ago

(reporting this with my Nordeck hat on)

Description

Synapse docs recommend to configure a potential reverse proxy with the same max body size as the synapse max upload size, so the allowed size is not blocked by the proxy/ingress even though synapse would accept it.

The issue with that is the following: For example synapse is set to its default 50M (Mebibytes) and an nginx reverse proxy/ingress is also set to 50m (also Mebibytes, per reviewing its code - its docs are not clear about that). then a file as large as 50MiB+1B will be blocked by the reverse proxy, and with the indicated config for nginx that means it will return a HTTP 413 with its default 413 HTML status page. This diverges from a strict interpretation from the spec, wherein

Any errors which occur at the Matrix API level MUST return a “standard error response”.

(though the code itself is correct.)

Hence, a client implementing a strict interpretation of the spec will not be able to properly understand the API response.

This is (in this specific setting) not a problem with e.g. the element web client, which implements an optional check via the /_matrix/media/v3/config API to not try uploading files that would be rejected (by synapse) in the first place. I have not checked how element behaves if the reverse proxy is set to a smaller size than allowed by synapse (and hence m.upload.size), but clients might still be fine if they understand (the not matrix spec conforming) response by nginx based on solely the HTTP status code.

It is possible to disable size checking in nginx by setting the limit to 0, which however is not necessarily recommended either, I'm not sure.

Perhaps the correct way to handle this would be to coax the reverse proxy into returning a matrix “standard error response” instead of the default page (at least on the relevant paths), in which case an example should be added to the docs.

Steps to reproduce

Homeserver

a synapse instance with an nginx ingress in k8s

Synapse Version

1.93.0

Installation Method

Docker (matrixdotorg/synapse)

Database

psql/not relevant

Workers

Single process

Platform

k8s/rancher

Configuration

No response

Relevant log output

-

Anything else that would be useful to know?

No response

devonh commented 5 months ago

Hmm. This does indeed seem like it could lead to issues for new client developers. Thanks for raising this.

Some thought will be needed for how best to approach updating the recommended setup.

devonh commented 5 months ago

After speaking with the team, the solution here should be to update the documentation examples to include returning an appropriate standard Matrix error. There should also be a general disclaimer added to the documentation for reverse proxy setup that any valid matrix request which is rejected by a proxy should return a standard Matrix error.