[Django] #20486: file_move_safe doesn't respect it's docstring
Django
--------------------------------------+--------------------
Reporter: kux | Owner: nobody
Type: Bug | Status: new
Component: File uploads/storage | Version: 1.4
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+--------------------
django.core.files.move.file_move_safe's docstring states the following:
If the destination file exists and ``allow_overwrite`` is ``False``,
this
function will throw an ``IOError``.
However, if the destination file exists and movement is performed using
os.rename (which is the case most of the times) then no exception is being
thrown. The exception is only being thrown if streaming manually from one
file to another.
This could be fixed by doing one of the following:
1. update the docstring to match the actual behavior
2. make the function raise IOError regardless of how the movement is being
performed
--
Ticket URL: <https://code.djangoproject.com/ticket/20486>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: kux | Owner: nobody
Type: Bug | Status: new
uploads/storage | Resolution:
Severity: Normal | Triage Stage:
Keywords: | Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by kux):
* needs_better_patch: => 0
* version: 1.4 => master
* needs_tests: => 0
* needs_docs: => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/20486#comment:1>
Django
Reporter: kux | Owner: nobody
Type: Bug | Status: new
uploads/storage | Resolution:
Severity: Normal | Triage Stage:
Keywords: | Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by kux):
Note. This might be platform dependent.
I reproduced this on Linux 3.2.0-23-generic-pae #36-Ubuntu SMP
--
Ticket URL: <https://code.djangoproject.com/ticket/20486#comment:2>
Django
Reporter: kux | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
--------------------------------------+------------------------------------
Changes (by Kamu):
* easy: 0 => 1
* stage: Unreviewed => Accepted
Comment:
Looking over the code and
http://docs.python.org/2/library/os.html#os.rename it looks like this is
quite true.
An interesting note is os.rename will not replace the file on Windows: "On
Windows, if dst already exists, OSError will be raised even if it is a
file; there may be no way to implement an atomic rename when dst names an
existing file." So this seems to be unix systems only.
--
Ticket URL: <https://code.djangoproject.com/ticket/20486#comment:3>
Django
Reporter: kux | Owner: Kamu
Type: Bug | Status: assigned
Component: File | Version: master
uploads/storage | Resolution:
Keywords: | checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Kamu):
* owner: nobody => Kamu
* status: new => assigned
* has_patch: 0 => 1
* stage: Accepted => Ready for checkin
Comment:
I had a stab at fixing it. Please see:
https://github.com/django/django/pull/1253
--
Ticket URL: <https://code.djangoproject.com/ticket/20486#comment:4>
Django
Reporter: kux | Owner: Kamu
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by Kamu):
* stage: Ready for checkin => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/20486#comment:5>
Django
Reporter: kux | Owner: Kamu
Type: Bug | Status: assigned
Component: File uploads/storage | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
--------------------------------------+------------------------------------
Changes (by Kamu):
* cc: Kamu (added)
--
Ticket URL: <https://code.djangoproject.com/ticket/20486#comment:6>
Django
Reporter: kux | Owner: Kamu
Component: File uploads/storage | Version: master
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
--------------------------------------+------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"18e79f1425fa87f2f38df25c65203ec4d311f499"]:
{{{
#!CommitTicketReference repository=""
revision="18e79f1425fa87f2f38df25c65203ec4d311f499"
Fixed #20486 -- Ensure that file_move_safe raises an error if the
destination already exists.
Thanks to kux for the report, and Russ Webber for the patch.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/20486#comment:7>