Fixed #28170 -- Fixed file_move_safe() crash when moving files to a CIFS mount.

3 files changed, 38 insertions, 1 deletions

diff --git a/django/core/files/move.py b/django/core/files/move.py
index 23491d6573..bf0f1fe4e8 100644
--- a/django/core/files/move.py
+++ b/django/core/files/move.py
@@ -5,6 +5,7 @@ Move a file in the safest way possible::
     >>> file_move_safe("/tmp/old_file", "/tmp/new_file")
 """
 
+import errno
 import os
 from shutil import copystat
 
@@ -66,7 +67,15 @@ def file_move_safe(old_file_name, new_file_name, chunk_size=1024 * 64, allow_ove
         finally:
             locks.unlock(fd)
             os.close(fd)
-    copystat(old_file_name, new_file_name)
+
+    try:
+        copystat(old_file_name, new_file_name)
+    except PermissionError as e:
+        # Certain filesystems (e.g. CIFS) fail to copy the file's metadata if
+        # the type of the destination filesystem isn't the same as the source
+        # filesystem; ignore that.
+        if e.errno != errno.EPERM:
+            raise
 
     try:
         os.remove(old_file_name)
diff --git a/docs/releases/1.11.2.txt b/docs/releases/1.11.2.txt
index a01ecc3342..fa23f1ddf0 100644
--- a/docs/releases/1.11.2.txt
+++ b/docs/releases/1.11.2.txt
@@ -54,3 +54,6 @@ Bugfixes
 
 * Made date-based generic views return a 404 rather than crash when given an
   out of range date (:ticket:`28209`).
+
+* Fixed a regression where ``file_move_safe()`` crashed when moving files to a
+  CIFS mount (:ticket:`28170`).
diff --git a/tests/files/tests.py b/tests/files/tests.py
index 1a383090f9..5e165a998f 100644
--- a/tests/files/tests.py
+++ b/tests/files/tests.py
@@ -1,3 +1,4 @@
+import errno
 import gzip
 import os
 import struct
@@ -340,6 +341,30 @@ class FileMoveSafeTests(unittest.TestCase):
         os.close(handle_a)
         os.close(handle_b)
 
+    def test_file_move_copystat_cifs(self):
+        """
+        file_move_safe() ignores a copystat() EPERM PermissionError. This
+        happens when the destination filesystem is CIFS, for example.
+        """
+        copystat_EACCES_error = PermissionError(errno.EACCES, 'msg')
+        copystat_EPERM_error = PermissionError(errno.EPERM, 'msg')
+        handle_a, self.file_a = tempfile.mkstemp()
+        handle_b, self.file_b = tempfile.mkstemp()
+        try:
+            # This exception is required to reach the copystat() call in
+            # file_safe_move().
+            with mock.patch('django.core.files.move.os.rename', side_effect=OSError()):
+                # An error besides EPERM isn't ignored.
+                with mock.patch('django.core.files.move.copystat', side_effect=copystat_EACCES_error):
+                    with self.assertRaises(PermissionError):
+                        file_move_safe(self.file_a, self.file_b, allow_overwrite=True)
+                # EPERM is ignored.
+                with mock.patch('django.core.files.move.copystat', side_effect=copystat_EPERM_error):
+                    self.assertIsNone(file_move_safe(self.file_a, self.file_b, allow_overwrite=True))
+        finally:
+            os.close(handle_a)
+            os.close(handle_b)
+
 
 class SpooledTempTests(unittest.TestCase):
     def test_in_memory_spooled_temp(self):