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):