From 7250393f31cf8000833312e381501b4575fdb1f1 Mon Sep 17 00:00:00 2001 From: Derrick Jackson Date: Wed, 10 May 2017 11:29:12 -0400 Subject: [PATCH] [1.11.x] Fixed #28170 -- Fixed file_move_safe() crash when moving files to a CIFS mount. Backport of 789c290150a0a5e7312e152df281dbcaf4ec174e from master --- django/core/files/move.py | 11 ++++++++++- docs/releases/1.11.2.txt | 3 +++ tests/files/tests.py | 26 ++++++++++++++++++++++++++ 3 files changed, 39 insertions(+), 1 deletion(-) diff --git a/django/core/files/move.py b/django/core/files/move.py index f3972c5f984..8be62ff6d80 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 @@ -67,7 +68,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 OSError 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 getattr(e, 'errno', 0) != 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 a01ecc33423..fa23f1ddf02 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 72a121fcfec..02d6d43b3ce 100644 --- a/tests/files/tests.py +++ b/tests/files/tests.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- from __future__ import unicode_literals +import errno import gzip import os import struct @@ -317,6 +318,31 @@ 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 = OSError(errno.EACCES, 'msg') + copystat_EPERM_error = OSError(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(OSError) as e: + file_move_safe(self.file_a, self.file_b, allow_overwrite=True) + self.assertEqual(e.exception.errno, errno.EACCES) + # 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):