Code

Opened 5 years ago

Closed 5 years ago

Last modified 3 years ago

#10788 closed (wontfix)

Actual name of uploaded file available later than it was in 1.0

Reported by: kmtracey Owned by: jacob
Component: File uploads/storage Version: master
Severity: Keywords:
Cc: mitsuhiko Triage Stage: Design decision needed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Originally reported here: http://groups.google.com/group/django-developers/browse_thread/thread/ce20f196f5e85296#

Prior to r9766 an uploaded file was saved during model form save (even when commit=False) by a field save_form_data override. Thus the actual name (including upload directory and any appended underscores) used for a FileField was available to pre-save signal handlers and any code that ran after a ModelForm.save(commit=False). r9766 delayed the actual file save to much later in the model save process (a field pre_save routine), so some code that used to be able to use the actual file name no longer can.

(There are other side-effects of r9766 still open: #10404, #10300, #10249 but these are all slightly different issues. In the end we might choose to decide to deal with them all en masse somehow, but just to make sure this particular wrinkle isn't forgotten I figured it should have its own ticket.)

Attachments (0)

Change History (12)

comment:1 Changed 5 years ago by jacob

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

comment:2 Changed 5 years ago by dc

Is this is really in scope of 1.1? Maybe better move this ticket to 1.2 as this is not a bugfix.

comment:3 Changed 5 years ago by kmtracey

How could this be out of scope for 1.1? We've changed things such that application level code that used to be able to access the actual file name at a certain point in the processing path no longer can. That's a regression. We either need to restore the old behavior or at a minimum document the change.

comment:4 Changed 5 years ago by dc

You are right - this is in scope of 1.1. Sorry for the false alert.

comment:5 Changed 5 years ago by mitsuhiko

  • Owner changed from nobody to mitsuhiko

comment:6 follow-up: Changed 5 years ago by mitsuhiko

  • Cc mitsuhiko added
  • Owner changed from mitsuhiko to jacob
  • Triage Stage changed from Accepted to Design decision needed

The core no longer depends on that and I stronly suggest designing custom upload handlers in a way that they don't depend on the final filename of the file on the filesystem. This however is slighty backwards incomaptible as it seems so it requires a decision by a main developer.

comment:7 in reply to: ↑ 6 ; follow-up: Changed 5 years ago by anonymous

Replying to mitsuhiko:

The core no longer depends on that and I stronly suggest designing custom upload handlers in a way that they don't depend on the final filename of the file on the filesystem. This however is slighty backwards incomaptible as it seems so it requires a decision by a main developer.

It's totally incompatible with every code I could see that needs to rename (uploaded) files. To rename the file, you have know its name first, but in the current status, this is not possible anymore (especially in pre_save() signals).

comment:8 in reply to: ↑ 7 ; follow-up: Changed 5 years ago by mitsuhiko

Replying to anonymous:

Replying to mitsuhiko:

The core no longer depends on that and I stronly suggest designing custom upload handlers in a way that they don't depend on the final filename of the file on the filesystem. This however is slighty backwards incomaptible as it seems so it requires a decision by a main developer.

It's totally incompatible with every code I could see that needs to rename (uploaded) files. To rename the file, you have know its name first, but in the current status, this is not possible anymore (especially in pre_save() signals).

Why would you need to rename a file? Can't see how this is useful. Just set the filename before the file is saved by the model then you don't have to rename the file, it would be stored with the correct name.

comment:9 in reply to: ↑ 8 Changed 5 years ago by lstep

Replying to mitsuhiko:

Replying to anonymous:

Replying to mitsuhiko:

The core no longer depends on that and I stronly suggest designing custom upload handlers in a way that they don't depend on the final filename of the file on the filesystem. This however is slighty backwards incomaptible as it seems so it requires a decision by a main developer.

It's totally incompatible with every code I could see that needs to rename (uploaded) files. To rename the file, you have know its name first, but in the current status, this is not possible anymore (especially in pre_save() signals).

Why would you need to rename a file? Can't see how this is useful. Just set the filename before the file is saved by the model then you don't have to rename the file, it would be stored with the correct name.

Well, not necessarily renaming a file, in fact it's just accessing it. We don't have the path to access it. Just look at the example in the original django dev thread (http://groups.google.com/group/django-developers/browse_thread/thread/ce20f196f5e85296 ).

As a real use-case (simple) example which is often used, for an uploaded image (put in an ImageField), generate a thumbnail for it, using the pre_save() signal.

comment:10 follow-up: Changed 5 years ago by jacob

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

Given Armin's notes here, and Marty's concurrence on django-developers (http://groups.google.com/group/django-developers/browse_thread/thread/560b3d7cad9bd491), I'm going to mark this wontifx and note in the 1.1 release notes that the final file name isn't available until the file has actually been saved to disk. It's of dubious use anyway to allow someone to try to "predict" the final file name; better to use the storage APIs always to access the given object.

comment:11 in reply to: ↑ 10 Changed 5 years ago by anonymous

Replying to jacob:

Given Armin's notes here, and Marty's concurrence on django-developers (http://groups.google.com/group/django-developers/browse_thread/thread/560b3d7cad9bd491), I'm going to mark this wontifx and note in the 1.1 release notes that the final file name isn't available until the file has actually been saved to disk. It's of dubious use anyway to allow someone to try to "predict" the final file name; better to use the storage APIs always to access the given object.

I find it really disturbing for someone that comes for the first time and *thinks* that's it's possible (especially when it was possible in 1.0, and you'll get a lot of references on the web on 'tips' and similar usage) to access the filenames in a pre_save signal.

The problem is that *if* you don't know that it's impossible now, you'll try to access instance.attribute.path (which contained the correct path to the file in Django 1.0) and get a result, but a wrong one! Isn't it possible to prevent access to the attribute or at least set it to something that will not be interpreted as "strange"?

comment:12 Changed 3 years ago by jacob

  • milestone 1.1 deleted

Milestone 1.1 deleted

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.