From 8ad3e80e88201f4c557f6fa79fcfc0f8a0961830 Mon Sep 17 00:00:00 2001 From: Sarah Boyce <42296566+sarahboyce@users.noreply.github.com> Date: Fri, 4 Apr 2025 09:52:22 +0200 Subject: [PATCH] Fixed #36298 -- Truncated the overwritten file content in file_move_safe(). Regression in 58cd4902a71a3695dd6c21dc957f59c333db364c. Thanks Baptiste Mispelon for the report. --- django/core/files/move.py | 1 + docs/releases/4.2.21.txt | 15 +++++++++++++++ docs/releases/5.1.9.txt | 15 +++++++++++++++ docs/releases/5.2.1.txt | 5 +++++ docs/releases/index.txt | 2 ++ tests/files/tests.py | 21 +++++++++++++++++++++ 6 files changed, 59 insertions(+) create mode 100644 docs/releases/4.2.21.txt create mode 100644 docs/releases/5.1.9.txt diff --git a/django/core/files/move.py b/django/core/files/move.py index d7a9c7026e..57508fab82 100644 --- a/django/core/files/move.py +++ b/django/core/files/move.py @@ -55,6 +55,7 @@ def file_move_safe( | os.O_CREAT | getattr(os, "O_BINARY", 0) | (os.O_EXCL if not allow_overwrite else 0) + | os.O_TRUNC ), ) try: diff --git a/docs/releases/4.2.21.txt b/docs/releases/4.2.21.txt new file mode 100644 index 0000000000..36e24df12f --- /dev/null +++ b/docs/releases/4.2.21.txt @@ -0,0 +1,15 @@ +=========================== +Django 4.2.21 release notes +=========================== + +*Expected May 7, 2025* + +Django 4.2.21 fixes a data loss bug in 4.2.20. + +Bugfixes +======== + +* Fixed a data corruption possibility in ``file_move_safe()`` when + ``allow_overwrite=True``, where leftover content from a previously larger + file could remain after overwriting with a smaller one due to lack of + truncation (:ticket:`36298`). diff --git a/docs/releases/5.1.9.txt b/docs/releases/5.1.9.txt new file mode 100644 index 0000000000..6847aa9a2c --- /dev/null +++ b/docs/releases/5.1.9.txt @@ -0,0 +1,15 @@ +========================== +Django 5.1.9 release notes +========================== + +*Expected May 7, 2025* + +Django 5.1.9 fixes a data loss bug in 5.1.8. + +Bugfixes +======== + +* Fixed a data corruption possibility in ``file_move_safe()`` when + ``allow_overwrite=True``, where leftover content from a previously larger + file could remain after overwriting with a smaller one due to lack of + truncation (:ticket:`36298`). diff --git a/docs/releases/5.2.1.txt b/docs/releases/5.2.1.txt index 53f18152ae..b2ff7c22d4 100644 --- a/docs/releases/5.2.1.txt +++ b/docs/releases/5.2.1.txt @@ -23,3 +23,8 @@ Bugfixes * Fixed a regression in Django 5.2 that caused fields to be incorrectly selected when using ``QuerySet.alias()`` after ``values()`` (:ticket:`36299`). + +* Fixed a data corruption possibility in ``file_move_safe()`` when + ``allow_overwrite=True``, where leftover content from a previously larger + file could remain after overwriting with a smaller one due to lack of + truncation (:ticket:`36298`). diff --git a/docs/releases/index.txt b/docs/releases/index.txt index dd308c8a08..7b88a42d2b 100644 --- a/docs/releases/index.txt +++ b/docs/releases/index.txt @@ -40,6 +40,7 @@ versions of the documentation contain the release notes for any later releases. .. toctree:: :maxdepth: 1 + 5.1.9 5.1.8 5.1.7 5.1.6 @@ -77,6 +78,7 @@ versions of the documentation contain the release notes for any later releases. .. toctree:: :maxdepth: 1 + 4.2.21 4.2.20 4.2.19 4.2.18 diff --git a/tests/files/tests.py b/tests/files/tests.py index 4f6d1fa74b..7e365aae39 100644 --- a/tests/files/tests.py +++ b/tests/files/tests.py @@ -496,6 +496,27 @@ class FileMoveSafeTests(unittest.TestCase): os.close(handle_b) os.close(handle_c) + def test_file_move_ensure_truncation(self): + with tempfile.NamedTemporaryFile(delete=False) as src: + src.write(b"content") + src_name = src.name + self.addCleanup( + lambda: os.remove(src_name) if os.path.exists(src_name) else None + ) + + with tempfile.NamedTemporaryFile(delete=False) as dest: + dest.write(b"This is a longer content.") + dest_name = dest.name + self.addCleanup(os.remove, dest_name) + + with mock.patch("django.core.files.move.os.rename", side_effect=OSError()): + file_move_safe(src_name, dest_name, allow_overwrite=True) + + with open(dest_name, "rb") as f: + content = f.read() + + self.assertEqual(content, b"content") + class SpooledTempTests(unittest.TestCase): def test_in_memory_spooled_temp(self):