File upload size limit logic is broken (SYN-785)

[matrixbot]

Submitted by @​matthew:matrix.org
It checks only content-length, which may lie. And it does so after the file has been uploaded, which is too late.

(Imported from https://matrix.org/jira/browse/SYN-785)

[PC-Admin]

I have been experiencing this bug since i first set up synapse.

Value is set to: max_upload_size: "10M"

Interestingly a @:matrix.org account can post up to 10MB files on a #:perthchat.org rooms, yet a perthchat user cannot post more then a 1MB file without experiencing:

The file ‘2m’ exceeds this home server’s size limit for uploads.

Any help fixing this issue is greatly appreciated, Matrix sucks when you can't upload files to it, and worse, the entire file tries to upload itself BEFORE telling you it cannot be done.

[grafenhofer]

I can confirm this bug. I followed https://www.digitalocean.com/community/tutorials/how-to-install-matrix-synapse-on-ubuntu-16-04. I even tried to increase max_upload_size to "100M" - no success.

[grafenhofer]

Crap, I should have done more research: it is an nginx "issue". The server section of the nginx site config should in include the following (or a similar) line:

client_max_body_size 20M;

[PC-Admin]

/etc/nginx/nginx.conf

then add the above line under "Basic Settings", works well

[pabelenda]

Is there any short/mid term plan for this issue?

[lampholder]

Is the fix for this:

1. check content-length at the start of upload, assume it's not lying, and fail fast if content-length > max_upload_size
2. check the actual uploaded content size after upload has completed and before persisting it to the media repo
3. give better feedback to the user when their upload fails due to either 1. or 2.

[richvdh]

check the actual uploaded content size after upload has completed and before persisting it to the media repo

better to bail out during the upload rather than waiting for the upload to fill our disk

give better feedback to the user when their upload fails due to either 1. or 2.

the problem is that it's hard to give good feedback over the API without accepting the entire body (which may take hours for a large file). Other problems include the fact that there may be things other than the synapse upload size limit (notably, the reverse-proxy's limit) which cause a failure, and I don't know how they behave.

[lampholder]

better to bail out during the upload rather than waiting for the upload to fill our disk

Makes sense

the problem is that it's hard to give good feedback over the API without accepting the entire body...

I guess for the good feedback component the client would be better asking the server for a maximum upload size before it even gets started? And yes it is hard to account for all the myriad other ways in which the upload could fail

[richvdh]

I guess for the good feedback component the client would be better asking the server for a maximum upload size before it even gets started?

I kinda thought we had that in the capabilities API (I'm sure there was at least an MSC about it). But we probably also ought to do some testing with various reverse proxies and browsers to see if there is a more reliable way to do it. (Maybe the client can do an Expect: 100-Continue or whatever the header is called which asks the server to confirm it's ok to proceed before uploading the body?)

[ptman]

https://matrix.org/docs/spec/client_server/r0.6.1#get-matrix-media-r0-config - yes, clients can get the upload limit