-
Notifications
You must be signed in to change notification settings - Fork 28
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
Disambiguate "size" #583
base: main
Are you sure you want to change the base?
Disambiguate "size" #583
Conversation
Sounds good. My only qualm here is that "whatever the current maximum file size limit is" should not be a guess and reflect the actual size limit. @gasman and/or @glennlunder could help here? |
The limit (set at the nginx level) is indeed 10Mb. I think this change mostly makes sense, although any time there's a proposal to make a change that requires people to read more stuff, I'm keen to look for alternatives. The 10Mb limit was supposed to be "big enough that nobody should reasonably run into it", so I'd like to probe a bit further into whether 14Mb screenshots are really something we're going to encounter regularly...
|
Just two cents from an actual user perspective - first of all, the 10Mb limit is not per file, it's for that "bunch of files" - hence if I multi-select a few files that are each under 10 but collectively over, it fails. This has led to a lot of working around by just adding four and four files until done and such over the years. Now, most screenshots from actual moving productions should not exceed the limit, unless as @gasman says you've added a raw png from a 4k capture. Which is the type of shenanigans I did try a lot of back in the day, but as time has progressed I've got more pragmatic, and just jpg it if it's above 1080p. Where I most commonly run into the size limit is with photo competitions and the like, where some re-compressing and even shrinking has been needed to get stuff in at times. |
As I suspected. Would it be possible to make that configurable in an environment variable or similar, so it could be used both by NGINX and Django, keeping the HTTP configuration and UI consistent upon change?
Yes, it was this screenshot taken on an iMac 5k. It's of course an image where JPEG would be a much more sensible format than PNG, but PNG is the format in which screenshots are taken on macOS, and as the upload UI states that I could upload any size I wanted, I didn't think much about neither format nor file size.
It's not per file as such, but per request. 10 MB is the HTTP request body limit set in the HTTP server to mitigate DDoS attacks and similar.
Yes, that too. When you bunch files together, they are, with the default
It also fails if only a single file above 10 MB is uploaded.
Yeah, converting to JPEG, AVIF or WebP is probably the most sensible thing to do. If we implemented HTML drag and drop, we could augment the upload experience by using JavaScript to upload each file separately (in parallel if we want to) as well as check the file size, format, etc., client-side. Without JavaScript, we would still offer the current upload experience. With HTML drag and drop, we could even offer a client-side preview of each image that is about to be uploaded, and for images above 10 MB, offer to automatically convert them to JPEG before they are uploaded.
An easy short-term fix would be to bump the limit from 10 MB to 20 MB, which I believe should be enough for most images given today's screen sizes (for screenshots) and image sensors (for photographs). |
I just tried to upload a screenshot of around 14MB and got a
413 Payload Too Large
response. When uploading screenshots and artwork, the current wording is:The word "size" here is imho a bit ambiguous. This PR changes that to "resolution" and adds a note to keep the file size below 10MB (or whatever the current maximum file size limit is set to in the HTTP server).