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

Correct docs for max file size with reverse proxy #17053

Open
HarHarLinks opened this issue Apr 5, 2024 · 2 comments
Open

Correct docs for max file size with reverse proxy #17053

HarHarLinks opened this issue Apr 5, 2024 · 2 comments
Labels
good first issue This is a fix that might be an easy place for someone to start for their first contribution O-Uncommon S-Minor T-Defect

Comments

@HarHarLinks
Copy link
Contributor

HarHarLinks commented Apr 5, 2024

(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

  • implement docs recommendation
  • see above

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
Copy link
Member

devonh commented Jun 21, 2024

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
Copy link
Member

devonh commented Jun 24, 2024

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.

@devonh devonh added the good first issue This is a fix that might be an easy place for someone to start for their first contribution label Jun 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue This is a fix that might be an easy place for someone to start for their first contribution O-Uncommon S-Minor T-Defect
Projects
None yet
Development

No branches or pull requests

2 participants