Code

Opened 9 years ago

Closed 6 years ago

Last modified 2 years ago

#26 closed Uncategorized (wontfix)

Admin validation errors cause FileUploadFields to be reset

Reported by: adrian 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

Description

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.

Attachments (0)

Change History (10)

comment:1 Changed 8 years ago by Link

  • Type defect deleted

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

  • Triage Stage changed from Unreviewed to Design 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 7 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 7 years ago by tbarta@…

  • Cc tbarta@… added

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

comment:5 Changed 7 years ago by ubernostrum

Is this still an issue on trunk?

comment:6 Changed 7 years ago by jacob

  • Version set to newforms-admin

comment:7 Changed 6 years ago by brosner

  • 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 6 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 6 years ago by ubernostrum

  • Resolution set to wontfix
  • Status changed from new to closed

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 2 years ago by gcc

  • Easy pickings unset
  • Type set to 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.

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.