mozilla-services / screenshots

Firefox Screenshots: the best way to take screenshots on the web.
https://screenshots.firefox.com
Mozilla Public License 2.0
620 stars 128 forks source link

Limit upload size at the server #2229

Closed jaredhirsch closed 7 years ago

ianb commented 7 years ago

I think that body-parser might have some maximum size in server.js.

Ideally the client would get a reasonable error message on failure.

pdehaan commented 7 years ago

Yeah, I think our current JSON upload limit is 100MB of JSON: /server/src/server.js:221

app.use(bodyParser.json({limit: '100mb'}));

https://github.com/expressjs/body-parser#limit

ckprice commented 7 years ago

What do we think about a 15mb limit? @relud is there a recommendation here?

relud commented 7 years ago

i don't have a recommendation, but the default client_max_body_size in nginx is 1m, so it could end up causing you issues if you are expecting more and it isn't changed or disabled (I can set it to 0 to disable it).

ianb commented 7 years ago

I'm pretty sure it's been raised in our Docker config, as I remember people hit that limit frequently when one of our instances had the 1mb limit in place. We do have information in Google Analytics about upload size, but I only know how to see average (670kb) and I don't know how to see a histogram of values.

jvehent commented 7 years ago

I took a shot of a very large high def image on a 4k screen, and the resulting PNG is 21MB. That's probably a bit much. Could we somehow limit to 10MB?

relud commented 7 years ago

ah, i see that the nginx default client_max_body_size for cloudops using dockerflow has been changed to 256m in nginx, which would be why you haven't hit that limit. client_max_body_sizeis set in nginx, so your docker config would not have affected it.

as long as the app continues to have a setting less than nginx (so anything less than 256m), and it doesn't cause servers to die, I don't care what this gets set to.

ghost commented 7 years ago

GA doesn't have a way to show us the bounds of what it has recorded so far, but Gareth is going to look at dumping it into BigQuery and getting them for us. If we want to keep measuring, this would normally be a custom dimension in GA and then we could do the calculation there.

ghost commented 7 years ago

Thanks to Gareth, an analysis of the month of March is available at https://docs.google.com/spreadsheets/d/1Tc0ZYnK5wwzq7EyXqCmM0yymmOtara3vPB1ED7MfLD8/edit#gid=462581354

tl;dr 99.23% of files uploaded were less than 10mb. That said, there were still 322 files that were larger than 10mb.