Opened 7 days ago

Closed 5 days ago

Last modified 5 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, 7 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..b1dafd7cb9 100644
    a b class OverwritingStorageTests(FileStorageTests):  
    617617        name = "test.file"
    618618        self.assertFalse(self.storage.exists(name))
    619619        content_1 = b"content one"
    620         content_2 = b"second content"
     620        content_2 = b"overwrite"
     621        assert len(content_1) > len(content_2), "Ensure truncation is tested."
    621622        f_1 = ContentFile(content_1)
    622623        f_2 = ContentFile(content_2)
    623624        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'overwritene' != b'overwrite'

----------------------------------------------------------------------
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?

Version 0, edited 7 days ago by Jacob Walls (next)

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

Owner: set to Gaël UTARD
Status: newassigned

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

Has patch: set

comment:4 by Gaël UTARD, 7 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, 5 days ago

Triage Stage: AcceptedReady for checkin

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

Resolution: fixed
Status: assignedclosed

In 0d1dd6bb:

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

comment:7 by Sarah Boyce <42296566+sarahboyce@…>, 5 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@…>, 5 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