Tatoeba / tatoeba2

Tatoeba is a platform whose purpose is to create a collaborative and open dataset of sentences and their translations.
https://tatoeba.org
GNU Affero General Public License v3.0
678 stars 131 forks source link

Uploading too large of an image as an avatar shouldn't redirect to a 413 "Request Entity Too Large" page #3100

Open vxern opened 4 months ago

vxern commented 4 months ago

I tried uploading a new avatar for my profile, which was a 5 MB image file, which threw me out onto this page: image

I would propose that instead of this, Tatoeba wouldn't allow files that are larger than a predefined size limit, and if that limit is crossed, would show a simple dialog or error message telling the user the file they tried to upload is too large.

cblanken commented 3 months ago

Currently any upload requests over 1MB won't reach the application due to the current nginx configuration. The default Nginx client_max_body_size is 1MB per the docs. Thus the nginx 413 error page.

The application actually already rejects any images over 1MB in size and displays an error message (see image below) provided Nginx is configured to allow them. image

The simplest solution would be to increase the client_max_body_size within reason. So probably somewhere between 5MB and 10MB so most "too large" images will trigger the default alert message.

In this case users will still see the Nginx error page if their image exceeds the client_max_body_size, but I don't see any way to avoid that besides allowing unlimited size uploads or adding in client-side upload size verification.

If that sounds like an acceptable solution I can work on a PR to update the Nginx config setup in Ansible for TatoVM, but I assume an admin will have to update the configuration on the official deployment.

Yorwba commented 3 months ago

adding in client-side upload size verification

I would prefer that solution. It's future-proof (no need to bump the limit as camera resolution and images sizes increase), saves users' bandwidth from uselessly uploading a file only to have it rejected, and also prevents our server from using too much memory to handle a request.

I found a StackOverflow question with answers suggesting that checking the file size client-side in JavaScript was already supported 13 years ago, though the particulars might be outdated by now.

An alternative solution that came to mind is to replace the 413 error page by something custom that explains more clearly what's going on.

cblanken commented 3 months ago

It's future-proof (no need to bump the limit as camera resolution and images sizes increase), saves users' bandwidth from uselessly uploading a file only to have it rejected

That's a good point, I hadn't really thought that far ahead. Client-side verification is probably the way to go then for a more permanent solution.

An alternative solution that came to mind is to replace the 413 error page by something custom that explains more clearly what's going on.

I was also considering that, but, correct me if I'm wrong, I think you'd have to configure Nginx to serve a static error page instead of going through the application which would be okay I guess. Just a bit strange and hard to find since it wouldn't go through the rest of the application routing.

Either way, I'll look into adding some client-side verification with the File API.