[Django] #36298: file_move_safe with allow_overwrite=True does not truncate previous file when there is a permission error on rename

[Django]

#36298: file_move_safe with allow_overwrite=True does not truncate previous file
when there is a permission error on rename
-------------------------------------+-------------------------------------
Reporter: Sarah | Owner: Sarah Boyce
Boyce |
Type: Bug | Status: assigned
Component: File | Version: 5.2
uploads/storage |
Severity: Release | Keywords:
blocker |
Triage Stage: | Has patch: 0
Unreviewed |
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-------------------------------------+-------------------------------------
Very similar to #36191

We are missing `os.O_TRUNC` in `django.core.files.move.file_move_safe()`

Regression test:
{{{#!diff
--- a/tests/files/tests.py
+++ b/tests/files/tests.py
@@ -496,6 +496,28 @@ class FileMoveSafeTests(unittest.TestCase):
os.close(handle_b)
os.close(handle_c)

+ def test_file_move_permission_error_truncate(self):
+ with tempfile.NamedTemporaryFile(delete=False) as src:
+ src.write(b"content")
+ src_name = src.name
+
+ with tempfile.NamedTemporaryFile(delete=False) as dest:
+ dest.write(b"This is a longer content.")
+ dest_name = dest.name
+
+ try:
+ 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")
+ finally:
+ os.remove(dest_name)
+ if os.path.exists(src_name):
+ os.remove(src_name)
+

class SpooledTempTests(unittest.TestCase):
}}}

Technically this issue has existed for a while. However:

> Security fixes and data loss bugs will be applied to the current main
branch, the last two feature release branches, and any other supported
long-term support release branches

https://docs.djangoproject.com/en/5.2/internals/release-process
/#supported-versions

I think there is an angle that this is a "Data loss" bug as the integrity
of uploads is impacted (in which case, we should fix in main, 5.2, 5.1,
4.2). Hence marking as a release blocker.
--
Ticket URL: <https://code.djangoproject.com/ticket/36298>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

#36298: file_move_safe with allow_overwrite=True does not truncate previous file
when there is a permission error on rename
-------------------------------------+-------------------------------------

Reporter: Sarah Boyce | Owner: Sarah

| Boyce
Type: Bug | Status: assigned
Component: File | Version: 5.2
uploads/storage |

Severity: Release blocker | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Sarah Boyce):

* has_patch: 0 => 1

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