[Django] #33023: InMemoryUploadedFile - opening with context manager multiple time is throwing error
Django
throwing error
------------------------------------------+------------------------
Reporter: Harsh Bhikadia | Owner: nobody
Type: Bug | Status: new
Component: Core (Other) | Version: 3.2
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------+------------------------
**What is the issue:**
when "opening" the "uploaded file" with "context manager (like `with
file.open('rb') as f:`) multiple times it is throwing `` exception
**Explaination**
Digging into the code, I realised that `InMemoryUploadedFile` overrides
the `open` method from `File` instance - the implementation just "seeks to
0". Since the context manager will call the `close` method on "exit". When
"opening" the file again (with or without context manager) it fails with
"cannot seek(0) of already closed file"
** Potential solution**
- Not sure why the "open" method was overridden - if not required then
just should remove the "overridden method"
- or making sure the file is "re-opened" (like in `File`) if closed
**To conclude**
Since
[https://docs.djangoproject.com/en/3.2/ref/files/file/#django.core.files.File.open
the doc on `File`] mentions that it could be used with "context manager"
- I think this is unexpected behaviour and should be fixed.
I have been using Django for many years but new to contribution/issue
reporting, so forgive me when I am being ignorant.
I am interested in fixing this myself if someone here gives me a green
signal for it.
--
Ticket URL: <https://code.djangoproject.com/ticket/33023>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
throwing error
Reporter: Harsh Bhikadia | Owner: nobody
Component: Core (Other) | Version: 3.2
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Harsh Bhikadia):
* status: new => closed
* resolution: => fixed
Old description:
> **What is the issue:**
> when "opening" the "uploaded file" with "context manager (like `with
> file.open('rb') as f:`) multiple times it is throwing `` exception
>
> **Explaination**
> Digging into the code, I realised that `InMemoryUploadedFile` overrides
> the `open` method from `File` instance - the implementation just "seeks
> to 0". Since the context manager will call the `close` method on "exit".
> When "opening" the file again (with or without context manager) it fails
> with "cannot seek(0) of already closed file"
>
> ** Potential solution**
> - Not sure why the "open" method was overridden - if not required then
> just should remove the "overridden method"
> - or making sure the file is "re-opened" (like in `File`) if closed
>
> **To conclude**
>
> Since
> [https://docs.djangoproject.com/en/3.2/ref/files/file/#django.core.files.File.open
> the doc on `File`] mentions that it could be used with "context manager"
> - I think this is unexpected behaviour and should be fixed.
>
> I have been using Django for many years but new to contribution/issue
> reporting, so forgive me when I am being ignorant.
>
> I am interested in fixing this myself if someone here gives me a green
> signal for it.
New description:
**What is the issue:**
when "opening" the "uploaded file" with "context manager (like `with
file.open('rb') as f:`) multiple times it is throwing `ValueError: I/O
operation on closed file.` exception
**Explaination**
Digging into the code, I realised that `InMemoryUploadedFile` overrides
the `open` method from `File` instance - the implementation just "seeks to
0". Since the context manager will call the `close` method on "exit". When
"opening" the file again (with or without context manager) it fails with
"cannot seek(0) of already closed file"
** Potential solution**
- Not sure why the "open" method was overridden - if not required then
just should remove the "overridden method"
- or making sure the file is "re-opened" (like in `File`) if closed
**To conclude**
Since
[https://docs.djangoproject.com/en/3.2/ref/files/file/#django.core.files.File.open
the doc on `File`] mentions that it could be used with "context manager"
- I think this is unexpected behaviour and should be fixed.
I have been using Django for many years but new to contribution/issue
reporting, so forgive me when I am being ignorant.
I am interested in fixing this myself if someone here gives me a green
signal for it.
--
--
Ticket URL: <https://code.djangoproject.com/ticket/33023#comment:1>
Django
throwing error
Reporter: Harsh Bhikadia | Owner: nobody
Component: Core (Other) | Version: 3.2
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Harsh Bhikadia):
Also realised the implementation is not respecting the `mode` passed in
arg when calling `open`.
The current implementation (`master` branch as of now) looks like the
following:
{{{
class InMemoryUploadedFile(UploadedFile):
"""
A file uploaded into memory (i.e. stream-to-memory).
"""
def __init__(self, file, field_name, name, content_type, size,
charset, content_type_extra=None):
super().__init__(file, name, content_type, size, charset,
content_type_extra)
self.field_name = field_name
def open(self, mode=None):
self.file.seek(0)
return self
def chunks(self, chunk_size=None):
self.file.seek(0)
yield self.read()
def multiple_chunks(self, chunk_size=None):
# Since it's in memory, we'll never have multiple chunks.
return False
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/33023#comment:2>
Django
throwing error
Reporter: Harsh Bhikadia | Owner: nobody
Type: Bug | Status: new
Component: Core (Other) | Version: 3.2
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Harsh Bhikadia):
* status: closed => new
* resolution: fixed =>
--
Ticket URL: <https://code.djangoproject.com/ticket/33023#comment:3>
Django
throwing error
Reporter: Harsh Bhikadia | Owner: nobody
Component: Core (Other) | Version: 3.2
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Carlton Gibson):
* status: new => closed
* resolution: => wontfix
Comment:
Hi. Thanks for the report.
I don't think it's worth complicating the `UploadedFile` code here. It's a
way of Django passing the file data to your application. If you need to
process that data multiple times you should save it to a suitable location
(which may be in memory) and then use that, which is fully under your
control. (That the file will be closed on exiting the context manager
allows the memory to be released. That's the right behaviour, that we'd
want to maintain.)
> Not sure why the "open" method was overridden
It's required to ensure the `BytesIO` instance is ready to read. It's been
there for many years, at least since the current file upload structure was
introduced. See #2070 (d725cc9734272f867d41f7236235c28b3931a1b2).
--
Ticket URL: <https://code.djangoproject.com/ticket/33023#comment:4>