[Python-checkins] bpo-42782: Fail fast for permission errors in shutil.move() (GH-24001)
orsenthil
webhook-mailer at python.org
Tue Mar 2 15:53:28 EST 2021
https://github.com/python/cpython/commit/132131b404e06ee1a19b040a1f96cd1118abed0c
commit: 132131b404e06ee1a19b040a1f96cd1118abed0c
branch: master
author: Winson Luk <winson.luk at gmail.com>
committer: orsenthil <skumaran at gatech.edu>
date: 2021-03-02T12:53:15-08:00
summary:
bpo-42782: Fail fast for permission errors in shutil.move() (GH-24001)
* Fail fast in shutil.move() to avoid creating destination directories on failure.
Co-authored-by: Zackery Spytz <zspytz at gmail.com>
files:
A Misc/NEWS.d/next/Library/2020-12-29-13-46-57.bpo-42782.3r0HFY.rst
M Lib/shutil.py
M Lib/test/test_shutil.py
diff --git a/Lib/shutil.py b/Lib/shutil.py
index f0e833dc979b7..89d924dec8aa4 100644
--- a/Lib/shutil.py
+++ b/Lib/shutil.py
@@ -813,6 +813,12 @@ def move(src, dst, copy_function=copy2):
if _destinsrc(src, dst):
raise Error("Cannot move a directory '%s' into itself"
" '%s'." % (src, dst))
+ if (_is_immutable(src)
+ or (not os.access(src, os.W_OK) and os.listdir(src)
+ and sys.platform == 'darwin')):
+ raise PermissionError("Cannot move the non-empty directory "
+ "'%s': Lacking write permission to '%s'."
+ % (src, src))
copytree(src, real_dst, copy_function=copy_function,
symlinks=True)
rmtree(src)
@@ -830,6 +836,11 @@ def _destinsrc(src, dst):
dst += os.path.sep
return dst.startswith(src)
+def _is_immutable(src):
+ st = _stat(src)
+ immutable_states = [stat.UF_IMMUTABLE, stat.SF_IMMUTABLE]
+ return hasattr(st, 'st_flags') and st.st_flags in immutable_states
+
def _get_gid(name):
"""Returns a gid, given a group name."""
if getgrnam is None or name is None:
diff --git a/Lib/test/test_shutil.py b/Lib/test/test_shutil.py
index df8dcdcce6091..4bcad51509d9c 100644
--- a/Lib/test/test_shutil.py
+++ b/Lib/test/test_shutil.py
@@ -34,6 +34,8 @@
from test.support.os_helper import TESTFN, FakePath
TESTFN2 = TESTFN + "2"
+TESTFN_SRC = TESTFN + "_SRC"
+TESTFN_DST = TESTFN + "_DST"
MACOS = sys.platform.startswith("darwin")
AIX = sys.platform[:3] == 'aix'
try:
@@ -2085,6 +2087,41 @@ def test_move_dir_caseinsensitive(self):
os.rmdir(dst_dir)
+ @unittest.skipUnless(hasattr(os, 'geteuid') and os.geteuid() == 0
+ and hasattr(os, 'lchflags')
+ and hasattr(stat, 'SF_IMMUTABLE')
+ and hasattr(stat, 'UF_OPAQUE'),
+ 'root privileges required')
+ def test_move_dir_permission_denied(self):
+ # bpo-42782: shutil.move should not create destination directories
+ # if the source directory cannot be removed.
+ try:
+ os.mkdir(TESTFN_SRC)
+ os.lchflags(TESTFN_SRC, stat.SF_IMMUTABLE)
+
+ # Testing on an empty immutable directory
+ # TESTFN_DST should not exist if shutil.move failed
+ self.assertRaises(PermissionError, shutil.move, TESTFN_SRC, TESTFN_DST)
+ self.assertFalse(TESTFN_DST in os.listdir())
+
+ # Create a file and keep the directory immutable
+ os.lchflags(TESTFN_SRC, stat.UF_OPAQUE)
+ os_helper.create_empty_file(os.path.join(TESTFN_SRC, 'child'))
+ os.lchflags(TESTFN_SRC, stat.SF_IMMUTABLE)
+
+ # Testing on a non-empty immutable directory
+ # TESTFN_DST should not exist if shutil.move failed
+ self.assertRaises(PermissionError, shutil.move, TESTFN_SRC, TESTFN_DST)
+ self.assertFalse(TESTFN_DST in os.listdir())
+ finally:
+ if os.path.exists(TESTFN_SRC):
+ os.lchflags(TESTFN_SRC, stat.UF_OPAQUE)
+ os_helper.rmtree(TESTFN_SRC)
+ if os.path.exists(TESTFN_DST):
+ os.lchflags(TESTFN_DST, stat.UF_OPAQUE)
+ os_helper.rmtree(TESTFN_DST)
+
+
class TestCopyFile(unittest.TestCase):
class Faux(object):
diff --git a/Misc/NEWS.d/next/Library/2020-12-29-13-46-57.bpo-42782.3r0HFY.rst b/Misc/NEWS.d/next/Library/2020-12-29-13-46-57.bpo-42782.3r0HFY.rst
new file mode 100644
index 0000000000000..065df9bf0cf42
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2020-12-29-13-46-57.bpo-42782.3r0HFY.rst
@@ -0,0 +1,2 @@
+Fail fast in :func:`shutil.move()` to avoid creating destination directories on
+failure.
More information about the Python-checkins
mailing list