Opened 14 years ago

Closed 11 years ago

Last modified 7 years ago

#26 closed Uncategorized (wontfix)

Admin validation errors cause FileUploadFields to be reset

Reported by: Adrian Holovaty Owned by: nobody
Component: contrib.admin Version: newforms-admin
Severity: minor Keywords: nfa-someday
Cc: tbarta@… Triage Stage: Design decision needed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


Phil said:

<pcauthon> if, say, the turnpike episode, event, etc admin kicks
back with an error, you have to rebrowse to upload the pics.
only really aggravating on turnpike episodes where there are 6 pics
to browse for

JavaScript form validation could be a clean fix for this.

Change History (10)

comment:1 Changed 13 years ago by Link

Type: defect

comment:2 Changed 12 years ago by Gary Wilson <gary.wilson@…>

Triage Stage: UnreviewedDesign decision needed

So the file path is not remembered? Is the full path sent in the POST data? If so maybe we can store this to redisplay when there are errors. Or the javascript validation as you mention. Gmail is an example, the attachments are uploaded shortly after selecting the file while you compose your email.

comment:3 Changed 12 years ago by anonymous

You can't set a default value for <input type="file">, for security reasons. See some details here. I believe that Javascript can be used to access the path information (and then store it in a hidden input), but that the value attribute of a file input is read-only, making this a one-way street.

You could temporarily store the file and change the widget to "Select a file, or use <path-to-my-original-file-submission> again", but you run the risk of garbage collection issues:

  • Temporary files may not be destroyed if the user abandons the process
  • If a cron or at job is used to destroy the file, it is possible that the deletion will occur before the user resubmits, thus causing an awful UI bug of "I know I said you could use that file again, but I just lost it. Sorry. Tell me again."

Furthermore, any client-facing apps that store temporary files for an indeterminate amount of time have a higher risk of attacks (someone can fill up your hard disk / partition by re-submitting a failed attempt over and over again). Maybe something like CachedFileUploadField would provide this convenience without leaving existing FileUploadFields as vulnerable.

comment:4 Changed 12 years ago by tbarta@…

Cc: tbarta@… added

(adding myself to the cc list, i was the previous commenter)

comment:5 Changed 12 years ago by James Bennett

Is this still an issue on trunk?

comment:6 Changed 11 years ago by Jacob

Version: newforms-admin

comment:7 Changed 11 years ago by Brian Rosner

Keywords: nfa-someday added

Anyone else for just closing this ticket? The solution for something like this would just be too messy for the framework level. Too many different conditions to deal with. newforms and newforms-admin should be enough to allow a user to choose how they might want to implement this if they need this type of functionality.

comment:8 Changed 11 years ago by anonymous

+1 for this ticket. it is very frustrating to have to find your files again.

i would deal with this by writing the file into the /tmp directory. Responsible sys-admins will have a quota on /tmp, to prevent a Denial Of Service.

You might have to watch out for races, so this patch would need a good review for security purposes.

comment:9 Changed 11 years ago by James Bennett

Resolution: wontfix
Status: newclosed

I'm going to wontfix this; trying to work around browsers' security restrictions on setting the value of a file upload field would require far too much fragile hacking around on the Django side, so anybody who wants to do that hacking around can do it in their own code.

comment:10 Changed 7 years ago by Chris Wilson

Easy pickings: unset
Type: Uncategorized
UI/UX: unset

Please can we have an UploadHandler or something that handles this on the server side, by keeping the temporarily uploaded file until the session times out, even if it's not enabled by default? This is pretty tricky to implement and for developers to have to reimplement it multiple times is just a waste.

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