Re: [Django] #13809: FileField open method is only accepting 'rb' modes
Django
------------------------------------+--------------------------------------
Reporter: canassa | Owner: nobody
Type: Bug | Status: reopened
Milestone: | Component: File uploads/storage
Version: SVN | Severity: Normal
Resolution: | Keywords:
Triage Stage: Accepted | Has patch: 0
Needs documentation: 0 | Needs tests: 1
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------+--------------------------------------
Changes (by mrmachine):
* cc: real.human@… (added)
* version: 1.2 => SVN
* ui_ux: => 0
* needs_tests: 0 => 1
* easy: => 0
Comment:
The docs say that `File.open()` will "re-open" the file (and as a side
effect, do `File.seek(0)`. In reality, `File.open()` will only do
`File.seek(0)` if the file is already open, and will ignore the newly
specified mode.
A separate but related issue is that even if the file is explicitly
closed, and `File.open()` is called with a new mode, `File.mode` will
still return the original mode. I'm not sure if the intent of the docs was
that re-opening a file would use the original mode if not specified, or
the current mode. Either way, I think that `File.mode` should mirror the
current file mode.
The problem with `FieldFile.open()` (a subclass of `File`) is that it
calls `FieldFile.file.open()`, when `FieldFile.file` will open the file
implicitly with a mode of "rb", and then `.open()` will just do
`.seek(0)`, regardless of the mode that was passed in.
I'm fine with the idea that `FieldFile` should be open in "rb" mode when
accessed by default, but users shouldn't have to explicitly access it
first so that they can insert an explicit call to `.close()` before
calling `.open()` with a different mode.
I think the fix here involves a couple of steps.
1. `File.open()` should call `.close()` if the file is already open
(instead of `.seek(0)`), and then actually re-open it with the new mode
(or the mode that was originally used, if not specified), as per the docs.
2. `FieldFile.open()` should not implicitly open the file in "rb" mode
when a mode is explicitly set. `FieldFile.open()`.
Cheers.
--
Ticket URL: <https://code.djangoproject.com/ticket/13809#comment:6>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Type: Bug | Status: reopened
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by davidfstr):
* cc: davidfstr (added)
Comment:
I confirm this issue as still present in Django 1.3.
In short, if you call {{{FieldFile.open(mode=NOT_RB)}}}, it will always
open in mode {{{rb}}}. This is due to {{{FieldFile.open}}} calling the
{{{self.file}}} property, which auto-opens in 'rb' mode.
{{{FieldFile.open}}} attempts to subsequently call {{{open}}} on this new
file, but that does not reopen the file again.
My suggested workaround for:
* {{{field_file.open(mode='w+b')}}}
is:
* {{{field_file.file = field_file.storage.open(field_file.name, 'w+b')}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/13809#comment:7>
Django
--------------------------------------+------------------------------------
Reporter: canassa | Owner:
Component: File uploads/storage | Version: master
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
--------------------------------------+------------------------------------
* owner: nobody =>
* status: reopened => new
* has_patch: 0 => 1
* needs_tests: 1 => 0
Comment:
The patch is still not optimal, because the file is opened in 'rb' mode
upon access, then closed and reopened in the specified mode. Seems to
work, though.
--
Ticket URL: <https://code.djangoproject.com/ticket/13809#comment:8>
Django
--------------------------------------+------------------------------------
Reporter: canassa | Owner:
Type: Bug | Status: new
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: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by claudep):
#16964 was a duplicate.
--
Ticket URL: <https://code.djangoproject.com/ticket/13809#comment:9>
Django
--------------------------------------+------------------------------------
Reporter: canassa | Owner:
Type: Bug | Status: new
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: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by anonymous):
I confirm this issue as still present in Django 1.5.
--
Ticket URL: <https://code.djangoproject.com/ticket/13809#comment:10>
Django
--------------------------------------+------------------------------------
Reporter: canassa | Owner:
Type: Bug | Status: new
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: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by jetfix):
I confirm this that this issue is still present in 1.5 =(
--
Ticket URL: <https://code.djangoproject.com/ticket/13809#comment:11>
Django
--------------------------------------+------------------------------------
Reporter: canassa | Owner:
Type: Bug | Status: new
Component: File uploads/storage | Version: master
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
* needs_better_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/13809#comment:12>
Django
--------------------------------------+------------------------------------
Reporter: canassa | Owner:
Type: Bug | Status: new
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: 1
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Comment (by synotna):
Still present in 1.7 :(
My workaround is always close the file before opening it again
--
Ticket URL: <https://code.djangoproject.com/ticket/13809#comment:13>
Django
Reporter: canassa | Owner:
Type: Bug | Status: closed
Component: File | Version: master
uploads/storage |
Severity: Normal | Resolution: duplicate
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* status: new => closed
* resolution: => duplicate
Comment:
I think #26398 may have at least partially addressed this but I haven't
verified completely.
--
Ticket URL: <https://code.djangoproject.com/ticket/13809#comment:14>
Django
Reporter: canassa | Owner:
Type: Bug | Status: new
Component: File uploads/storage | Version: master
Severity: Normal | Resolution:
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* status: closed => new
* resolution: duplicate =>
--
Ticket URL: <https://code.djangoproject.com/ticket/13809#comment:15>
Django
--------------------------------------+------------------------------------
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: 1
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
* owner: => csinchok
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/13809#comment:16>
Django
Reporter: canassa | Owner: csinchok
Type: Bug | Status: assigned
uploads/storage |
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: 1 => 0
* stage: Accepted => Ready for checkin
Comment:
[https://github.com/django/django/pull/6938 PR]. I marked #26398 as a
duplicate and will include the test from the PR for that ticket here.
--
Ticket URL: <https://code.djangoproject.com/ticket/13809#comment:17>
Django
-------------------------------------+-------------------------------------
Reporter: canassa | Owner: csinchok
Component: File | Version: master
uploads/storage |
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"ac1975b18b5a33234284bec86e5a5bb44a4af976" ac1975b1]:
{{{
#!CommitTicketReference repository=""
revision="ac1975b18b5a33234284bec86e5a5bb44a4af976"
Fixed #13809 -- Made FieldFile.open() respect its mode argument.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/13809#comment:18>