Opened 6 days ago

Closed 4 days ago

Last modified 4 days ago

#36191 closed Bug (fixed)

FileSystemStorage with allow_overwrite=True does not truncate previous file

Reported by: Gaël UTARD Owned by: Gaël UTARD
Component: File uploads/storage Version: 5.1
Severity: Release blocker Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

The new allow_overwrite parameter of FileSystemStorage class allows to overwrite an existing file. However, the previous file is not truncated. So, if the previous file was bigger than the new, the file gets corrupted.

It is possible to workaround with :

storage.OS_OPEN_FLAGS = storage.OS_OPEN_FLAGS & ~os.O_EXCL | os.O_TRUNC

Change History (8)

comment:1 by Jacob Walls, 6 days ago

Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

Thanks for the report, reproduced by adjusting this existing test case:

  • tests/file_storage/tests.py

    diff --git a/tests/file_storage/tests.py b/tests/file_storage/tests.py
    index c048b8f071..fcbf633bb4 100644
    a b class OverwritingStorageTests(FileStorageTests):  
    616616        """Saving to same file name twice overwrites the first file."""
    617617        name = "test.file"
    618618        self.assertFalse(self.storage.exists(name))
    619         content_1 = b"content one"
    620         content_2 = b"second content"
     619        content_1 = b"content one: this part should be truncated."
     620        content_2 = b"content two"
    621621        f_1 = ContentFile(content_1)
    622622        f_2 = ContentFile(content_2)
    623623        stored_name_1 = self.storage.save(name, f_1)

======================================================================
FAIL: test_save_overwrite_behavior (file_storage.tests.OverwritingStorageTests.test_save_overwrite_behavior)
Saving to same file name twice overwrites the first file.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/.../django/tests/file_storage/tests.py", line 633, in test_save_overwrite_behavior
    self.assertEqual(fp.read(), content_2)
AssertionError: b'content two: this part should be truncated.' != b'content two'

----------------------------------------------------------------------
Ran 2 tests in 0.002s

FAILED (failures=1)

Test passes with the suggested patch:

diff --git a/django/core/files/storage/filesystem.py b/django/core/files/storage/filesystem.py
index b8de9b0a58..54c31e536a 100644
--- a/django/core/files/storage/filesystem.py
+++ b/django/core/files/storage/filesystem.py
@@ -113,7 +113,7 @@ class FileSystemStorage(Storage, StorageSettingsMixin):
                         | getattr(os, "O_BINARY", 0)
                     )
                     if self._allow_overwrite:
-                        open_flags = open_flags & ~os.O_EXCL
+                        open_flags = open_flags & ~os.O_EXCL | os.O_TRUNC
                     fd = os.open(full_path, open_flags, 0o666)
                     _file = None
                     try:

Would you like to submit a PR?

Last edited 6 days ago by Jacob Walls (previous) (diff)

comment:2 by Gaël UTARD, 6 days ago

Owner: set to Gaël UTARD
Status: newassigned

comment:3 by Gaël UTARD, 6 days ago

Has patch: set

comment:4 by Gaël UTARD, 6 days ago

Jacob, thanks for your confirmation of the bug, and confirmation of the fix I had in mind.
I pushed a PR, including your idea to assert the length of both contents in the test.
It's my first contribution to Django. I read the docs. I hope I did it right.

comment:5 by Sarah Boyce, 4 days ago

Triage Stage: AcceptedReady for checkin

comment:6 by Sarah Boyce <42296566+sarahboyce@…>, 4 days ago

Resolution: fixed
Status: assignedclosed

In 0d1dd6bb:

Fixed #36191 -- Truncated the overwritten file content in FileSystemStorage.

comment:7 by Sarah Boyce <42296566+sarahboyce@…>, 4 days ago

In ae391ca3:

[5.2.x] Fixed #36191 -- Truncated the overwritten file content in FileSystemStorage.

Backport of 0d1dd6bba0c18b7feb6caa5cbd8df80fbac54afd from main.

comment:8 by Sarah Boyce <42296566+sarahboyce@…>, 4 days ago

In a9d03c4:

[5.1.x] Fixed #36191 -- Truncated the overwritten file content in FileSystemStorage.

Backport of 0d1dd6bba0c18b7feb6caa5cbd8df80fbac54afd from main.

Note: See TracTickets for help on using tickets.
Back to Top