Opened 6 years ago

Closed 2 years ago

#14787 closed New feature (wontfix)

Upload handler should pass errors on to forms.FileField

Reported by: Marti Owned by: ANUBHAV JOSHI
Component: File uploads/storage Version: master
Severity: Normal Keywords:
Cc: anubhav9042@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


As far as I can tell, currently no mechanism exists for file upload handlers to pass errors forward to form fields.

The result is that any upload errors result in "This field is required" or "The submitted file is empty" messages.

Other errors worth handling:

  • File upload size limits
  • Unicode decode error in filename


Attachments (1)

14787.diff (2.3 KB) - added by ANUBHAV JOSHI 2 years ago.
max_size for FileField

Download all attachments as: .zip

Change History (19)

comment:1 Changed 6 years ago by Gabriel Hurley

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset
Triage Stage: UnreviewedAccepted

This seems reasonable to me and I don't see any obvious mechanism in place with which to do this currently. Marking accepted.

comment:2 Changed 6 years ago by anonymous

Severity: Normal
Type: New feature

comment:3 Changed 5 years ago by Aymeric Augustin

UI/UX: unset

Change UI/UX from NULL to False.

comment:4 Changed 5 years ago by Aymeric Augustin

Easy pickings: unset

Change Easy pickings from NULL to False.

comment:5 Changed 3 years ago by ANUBHAV JOSHI

Cc: anubhav9042@… added
Owner: changed from nobody to ANUBHAV JOSHI
Status: newassigned

There have been some changes since past years. But I am in favour of this.
I hope/intend to do this in GSoC this year. I think the following can also be added along with those suggested above:

File not uploaded completely
Write failure

comment:6 Changed 2 years ago by ANUBHAV JOSHI

Version: 1.2master

comment:7 Changed 2 years ago by ANUBHAV JOSHI

for upload limit we now have max_length.
We can have a check unicode error in filename and also of possible the two I mentioned above.

EDITS: max_length error is for filename.

Last edited 2 years ago by ANUBHAV JOSHI (previous) (diff)

comment:8 Changed 2 years ago by Tim Graham

In general we should be careful about exposing detailed error messages to users. e.g. I believe the unicode error is a server configuration issue (see #17686). Similarly, "file not uploaded completely" and "write error" are not issues that the user is in a position to resolve so if anything, those errors would need to be surfaced as something like "The file upload was unsuccessful. Contact the site admin if the error persists". The issue of upload size seems like a valid one though. Do you have a proposal for propagating errors from upload handlers to forms?

comment:9 Changed 2 years ago by ANUBHAV JOSHI

I think checks for exceeding_upload_size and for encoding_type, we could add a check in forms.FileField.to_python(). We already have several checks there, so it seems reasonable enough to add there.
Thought on this?

Changed 2 years ago by ANUBHAV JOSHI

Attachment: 14787.diff added

max_size for FileField

comment:10 Changed 2 years ago by Tim Graham

Although the scope of this ticket isn't well defined, I think what you've implemented is somewhat tangential to what the summary describes "Upload handler should pass errors on to forms.FileField" -- although, I'm not sure how/if file size limit errors would be part of the upload handler. Btw, max_upload_size already exists as a third party package: django-validated-file. We'd probably want a mailing list thread to figure out if we'd want to bring it into core. I don't feel strongly either way on doing so.

comment:12 Changed 2 years ago by Tim Graham

A couple comments on the post:

  1. It's better to make a proposal than ask "what to do" or "how to solve a ticket." What do you think should be done? If you don't care, then find I'd find something else to work on.
  1. A subject that includes a description of the item instead of "Decision for ticket #14787" is more likely to get interested people to read it.

comment:13 Changed 2 years ago by ANUBHAV JOSHI

I did mention what I want to do, i.e. including the new attribute max_size, though I will pay more attention to the subject from next time onwards.

comment:14 Changed 2 years ago by Tim Graham

What's the file name encoding issue you mentioned in the mailing list post? Is it different from #17686?

comment:15 Changed 2 years ago by ANUBHAV JOSHI

What I wanted to say is we can inform the user about the problem in a better way than just adding in the docs by raising a proper error in to_python() itself as we have file_name there so we could check the encoding.

comment:16 Changed 2 years ago by Tim Graham

It's a server configuration issue that should be fixed by the person deploying the site, not something an end user should get an error about.

comment:17 Changed 2 years ago by ANUBHAV JOSHI

OK. So we are down to that maximum size issue itself.
Can you please post your replies on both the points on ML, so that when someone sees he knows about these things as well.

comment:18 Changed 2 years ago by Tim Graham

Resolution: wontfix
Status: assignedclosed

After looking into this, Anubhav and I don't see any easy way to pass errors from upload handlers to forms since at the time the handler exception is raised, you aren't inside a form to catch it.

One thing I thought of would be to allow an exception raised during file upload parsing to be stored in request.FILES so it would be re-raised when you accessed the file's key later, but I'm wary of this complexity and not even sure it would work.

Regarding max_size on FileField, it really should be the subject of a separate ticket and given it's possible to implement this yourself and available in 3rd party packages, I don't think there's an urgent need for it.

Note: See TracTickets for help on using tickets.
Back to Top