Code

Opened 10 months ago

Closed 10 months ago

Last modified 10 months ago

#20660 closed Bug (fixed)

Filefield.delete() on empty field delete MEDIA_ROOT

Reported by: stanislas.guerra@… Owned by: claudep
Component: File uploads/storage Version: master
Severity: Normal Keywords: FileField delete
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Maybe this is because I use a symlink in my Apache configuration to serve the media :

$ ls -l /var/www/mysite.domain.com/documents/
-rw-r--r-- 1 www-data www-data  474  1 févr. 15:27 maintenance.html
lrwxrwxrwx 1 root     root       20 26 juin  15:02 media -> /data/media_mysite
drwxr-xr-x 6 myuser www-data 4096 16 mai   12:01 static



$ ls -l /data/media_mysite
total 8
drwxrwsr-x 3 myuser www-data 4096 13 févr. 10:13 stuffs
drwxrwsr-x 3 myuser www-data 4096 13 févr. 10:00 other_stuffs

But when I call delete() on an ImageField, if that very field is empty, the symlink is wipped out!

$ python manage.py shell
In [1]: from myproject.myapp.models import MyModel
In [2]: obj = MyModel.objects.get(euid="21439011")
In [3]: obj.my_file
Out[3]: <FieldFile: None>
In [4]: obj.my_file.delete()


$ ls -l /var/www/mysite.domain.com/documents/
-rw-r--r-- 1 www-data www-data  474  1 févr. 15:27 maintenance.html
drwxr-xr-x 6 myproject www-data 4096 16 mai   12:01 static

Should not an exception be raised here instead ?

Attachments (0)

Change History (9)

comment:1 Changed 10 months ago by stanislas.guerra@…

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

I should have mention that MEDIA_ROOT is not deleted (only the symlink is.)

comment:2 Changed 10 months ago by claudep

  • Triage Stage changed from Unreviewed to Accepted
  • Version changed from 1.4 to master

I could reproduce this. And yes, the deletion succeeds only when MEDIA_ROOT is a symlink (when it's a real directory, it will fail with OSError: [Errno 21] Is a directory: ... at least on Linux).

I tend to think there are two bugs:

  • FieldFile should not call storage.delete when self._file is None
  • FileSystemStorage should not try to remove a path when its delete method receives '' as argument.

comment:3 Changed 10 months ago by claudep

  • Owner changed from nobody to claudep
  • Status changed from new to assigned

comment:5 Changed 10 months ago by bmispelon

  • Triage Stage changed from Accepted to Ready for checkin

comment:6 Changed 10 months ago by Claude Paroz <claude@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In ea3fe78a9d742904f6902cdc353a11d795418105:

Fixed #20660 -- Do not try to delete an unset FieldFile

Thanks stanislas.guerra at gmail.com for the report and
Baptiste Mispelon for the review.

comment:7 Changed 10 months ago by Claude Paroz <claude@…>

In 7fbab3ebaf8b60bbe847b772f895df47067a60d3:

Do not allow FileSystemStorage.delete to receive an empty name

Refs #20660.

comment:8 Changed 10 months ago by Claude Paroz <claude@…>

In b6aed803b20cc7898a82fd65845e97676276f3fa:

[1.6.x] Fixed #20660 -- Do not try to delete an unset FieldFile

Thanks stanislas.guerra at gmail.com for the report and
Baptiste Mispelon for the review.
Backport of ea3fe78a9d from master.

comment:9 Changed 10 months ago by Claude Paroz <claude@…>

In a9b5a1e506a9e8492407399b8bec1c2a8420be60:

[1.6.x] Do not allow FileSystemStorage.delete to receive an empty name

Refs #20660.
Backport of 7fbab3eba from master.

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.