Skip to content

Commit

Permalink
Fixed #35326 -- handled temporary uploaded files for OverwritingStorage.
Browse files Browse the repository at this point in the history
  • Loading branch information
bcail committed Mar 28, 2024
1 parent d658a31 commit e9653cd
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 8 deletions.
16 changes: 10 additions & 6 deletions django/core/files/storage/filesystem.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,7 @@ class FileSystemStorage(Storage, StorageSettingsMixin):
Standard filesystem storage
"""

# The combination of O_CREAT and O_EXCL makes os.open() raise OSError if
# the file already exists before it's opened.
OS_OPEN_FLAGS = os.O_WRONLY | os.O_CREAT | os.O_EXCL | getattr(os, "O_BINARY", 0)
ALLOW_OVERWRITE = False

def __init__(
self,
Expand Down Expand Up @@ -98,12 +96,18 @@ def _save(self, name, content):
try:
# This file has a file path that we can move.
if hasattr(content, "temporary_file_path"):
file_move_safe(content.temporary_file_path(), full_path)
file_move_safe(
content.temporary_file_path(),
full_path,
allow_overwrite=self.ALLOW_OVERWRITE,
)

# This is a normal uploadedfile that we can stream.
else:
# The current umask value is masked out by os.open!
fd = os.open(full_path, self.OS_OPEN_FLAGS, 0o666)
open_flags = os.O_WRONLY | os.O_CREAT | getattr(os, "O_BINARY", 0)
if not self.ALLOW_OVERWRITE:
open_flags |= os.O_EXCL
fd = os.open(full_path, open_flags, 0o666)
_file = None
try:
locks.lock(fd, locks.LOCK_EX)
Expand Down
31 changes: 29 additions & 2 deletions tests/file_storage/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -613,8 +613,7 @@ class OverwritingStorage(FileSystemStorage):
unused name.
"""

# Mask out O_EXCL so os.open() doesn't raise OSError if the file exists.
OS_OPEN_FLAGS = FileSystemStorage.OS_OPEN_FLAGS & ~os.O_EXCL
ALLOW_OVERWRITE = True

def get_available_name(self, name, max_length=None):
"""Override the effort to find an used name."""
Expand Down Expand Up @@ -648,6 +647,34 @@ def test_save_overwrite_behavior(self):
finally:
self.storage.delete(name)

def test_save_overwrite_behavior_temp_file(self):
"""Saving to same file name twice overwrites the first file."""
name = "test.file"
self.assertFalse(self.storage.exists(name))
content_1 = b"content one"
content_2 = b"second content"
f_1 = TemporaryUploadedFile("tmp1", "text/plain", 11, "utf8")
f_1.write(content_1)
f_1.seek(0)
f_2 = TemporaryUploadedFile("tmp2", "text/plain", 14, "utf8")
f_2.write(content_2)
f_2.seek(0)
stored_name_1 = self.storage.save(name, f_1)
try:
self.assertEqual(stored_name_1, name)
self.assertTrue(self.storage.exists(name))
self.assertTrue(os.path.exists(os.path.join(self.temp_dir, name)))
with self.storage.open(name) as fp:
self.assertEqual(fp.read(), content_1)
stored_name_2 = self.storage.save(name, f_2)
self.assertEqual(stored_name_2, name)
self.assertTrue(self.storage.exists(name))
self.assertTrue(os.path.exists(os.path.join(self.temp_dir, name)))
with self.storage.open(name) as fp:
self.assertEqual(fp.read(), content_2)
finally:
self.storage.delete(name)


class DiscardingFalseContentStorage(FileSystemStorage):
def _save(self, name, content):
Expand Down

0 comments on commit e9653cd

Please sign in to comment.