[Django] #36975: SimpleUploadedFile cannot be re-opened

[Django]

#36975: SimpleUploadedFile cannot be re-opened
-------------------------------------+-------------------------------------
Reporter: Denis Washington | Type:
| Uncategorized
Status: new | Component:
| Uncategorized
Version: 6.0 | 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
-------------------------------------+-------------------------------------
=== Summary

When a `SimpleUploadedFile` is closed, it calls `close()` on the
underlying `BytesIO`, which means that the next call to `open()` fails
(`ValueError: I/O operation on closed file.`). This is unlike the
conceptually similar `ContentFile`, which explicitly overrides `close()`
to do nothing, avoiding this issue.

=== How to Reproduce

The following script reproduces the issue:

{{{#!python
from django.core.files.base import ContentFile
from django.core.files.uploadedfile import SimpleUploadedFile


def read_twice(file):
with file.open() as f:
print(f.read())
with file.open() as f:
print(f.read())


# Works as expected
read_twice(ContentFile(b"test data"))

# ValueError: I/O operation on closed file.
read_twice(SimpleUploadedFile("test.txt", b"test data"))
}}}

=== Expected Behavior

`SimpleUploadedFile` should follow `ContentFile` in making `close()` a no-
op.

Ideally, the same should be done by its superclass `InMemoryUploadedFile`,
at least if its `file` is a `BytesIO` or `StringIO`. (Skipping closing
unconditionally could be risky here because the init method accepts any
file-like object in principle.)
--
Ticket URL: <https://code.djangoproject.com/ticket/36975>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#36975: SimpleUploadedFile cannot be re-opened
-------------------------------------+-------------------------------------

Reporter: Denis Washington | Owner: Dinesh
| Thumma
Type: Uncategorized | Status: assigned
Component: Uncategorized | Version: 6.0
Severity: Normal | Resolution:

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 Dinesh Thumma):

* owner: (none) => Dinesh Thumma
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/36975#comment:1>

[Django]

#36975: SimpleUploadedFile cannot be re-opened
-------------------------------------+-------------------------------------
Reporter: Denis Washington | Owner: Dinesh
| Thumma
Type: Uncategorized | Status: assigned
Component: Uncategorized | Version: 6.0
Severity: Normal | Resolution:
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 Vishy Algo):

This appears to be a fundamental file handling problem: once a
{{{BytesIO}}} object is created and passed to parent classes, it cannot be
reconstructed if abruptly closed. Since it's an
{{{InMemoryUploadedFile}}}, we could save the content and recreate the
file object via {{{open()}}}, allowing the context manager to clean it up
automatically.

But, currently {{{InMemoryUploadedFile}}} accepts file object while
construction, we have to add it to the {{{SimpleUploadedFile}}} somewhat
similar to this,


{{{#!diff
diff --git a/django/core/files/uploadedfile.py
b/django/core/files/uploadedfile.py
index 1d006ede4f..8ff3c0a08a 100644
--- a/django/core/files/uploadedfile.py
+++ b/django/core/files/uploadedfile.py
@@ -113,6 +113,8 @@ class InMemoryUploadedFile(UploadedFile):
self.field_name = field_name

def open(self, mode=None):
+ if self.closed:
+ self.file = BytesIO(self.content)
self.file.seek(0)
return self

@@ -132,9 +134,9 @@ class SimpleUploadedFile(InMemoryUploadedFile):
"""

def __init__(self, name, content, content_type="text/plain"):
- content = content or b""
+ self.content = content or b""
super().__init__(
- BytesIO(content), None, name, content_type, len(content),
None, None
+ BytesIO(self.content), None, name, content_type,
len(self.content), None, None
)
}}}


I've verified this change with a unit test, and it works as expected. I
believe using {{{InMemoryUploadedFile}}} would have been more appropriate
than {{{SimpleUploadedFile}}}, as it enables easy reconstruction of the
file from its content throughout the request lifecycle for all in-memory
file wrappers.

I'd appreciate any second opinions or suggestions on this approach.
--
Ticket URL: <https://code.djangoproject.com/ticket/36975#comment:2>

[Django]

#36975: SimpleUploadedFile cannot be re-opened

----------------------------------+--------------------------------------
Reporter: Denis Washington | Owner: Vishy Algo

Type: Uncategorized | Status: assigned
Component: Uncategorized | Version: 6.0
Severity: Normal | Resolution:
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 Vishy Algo):

* owner: Dinesh Thumma => Vishy Algo

--
Ticket URL: <https://code.djangoproject.com/ticket/36975#comment:3>

[Django]

#36975: SimpleUploadedFile cannot be re-opened
----------------------------------+--------------------------------------
Reporter: Denis Washington | Owner: Vishy Algo
Type: Uncategorized | Status: assigned

Component: Core (Other) | Version: 6.0

Severity: Normal | Resolution:
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 Vishy Algo):

* component: Uncategorized => Core (Other)

--
Ticket URL: <https://code.djangoproject.com/ticket/36975#comment:4>

[Django]

#36975: SimpleUploadedFile cannot be re-opened

-------------------------------------+-------------------------------------

Reporter: Denis Washington | Owner: Vishy
| Algo

Type: Bug | Status: closed
Component: File | Version: 6.0
uploads/storage |
Severity: Normal | Resolution: duplicate

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 Jacob Walls):

* component: Core (Other) => File uploads/storage
* resolution: => duplicate
* status: assigned => closed
* type: Uncategorized => Bug

Comment:

Thanks, I'm a little sympathetic to your report, since it seems like a
straightforward violation of the docs for `File.open()`, which claim it
handles re-opening, and shouldn't be complicated to fix, but I'm going to
respect the prior triage decision here in #33023. Feel free to start a
forum discussion if you'd like to gather consensus to revisit.
--
Ticket URL: <https://code.djangoproject.com/ticket/36975#comment:5>