10000 GH-73991: Use same signature for `shutil._rmtree_[un]safe()`. (#120517) · python/cpython@69058e2 · GitHub
[go: up one dir, main page]
Skip to content

Commit 69058e2

Browse files
barneygalebarneygale
authored
GH-73991: Use same signature for shutil._rmtree_[un]safe(). (#120517)
Preparatory work for moving `_rmtree_unsafe()` and `_rmtree_safe_fd()` to `pathlib._os` so that they can be used from both `shutil` and `pathlib`. Move implementation-specific setup from `rmtree()` into the safe/unsafe functions, and give them the same signature `(path, dir_fd, onexc)`. In the tests, mock `os.open` rather than `_rmtree_safe_fd()` to ensure the FD-based walk is used, and replace a couple references to `shutil._use_fd_functions` with `shutil.rmtree.avoids_symlink_attacks` (which has the same value). No change of behaviour.
1 parent 49f51de commit 69058e2

File tree

2 files changed

+44
-45
lines changed

2 files changed

+44
-45
lines changed

‎Lib/shutil.py

Lines changed: 38 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -605,7 +605,22 @@ def _rmtree_islink(st):
605605
return stat.S_ISLNK(st.st_mode)
606606

607607
# version vulnerable to race conditions
608-
def _rmtree_unsafe(path, onexc):
608+
def _rmtree_unsafe(path, dir_fd, onexc):
609+
if dir_fd is not None:
610+
raise NotImplementedError("dir_fd unavailable on this platform")
611+
try:
612+
st = os.lstat(path)
613+
except OSError as err:
614+
onexc(os.lstat, path, err)
615+
return
616+
try:
617+
if _rmtree_islink(st):
618+
# symlinks to directories are forbidden, see bug #1669
619+
raise OSError("Cannot call rmtree on a symbolic link")
620+
except OSError as err:
621+
onexc(os.path.islink, path, err)
622+
# can't continue even if onexc hook returns
623+
return
609624
def onerror(err):
610625
if not isinstance(err, FileNotFoundError):
611626
onexc(os.scandir, err.filename, err)
@@ -635,7 +650,26 @@ def onerror(err):
635650
onexc(os.rmdir, path, err)
636651

637652
# Version using fd-based APIs to protect against races
638-
def _rmtree_safe_fd(stack, onexc):
653+
def _rmtree_safe_fd(path, dir_fd, onexc):
654+
# While the unsafe rmtree works fine on bytes, the fd based does not.
655+
if isinstance(path, bytes):
656+
path = os.fsdecode(path)
657+
stack = [(os.lstat, dir_fd, path, None)]
658+
try:
659+
while stack:
660+
_rmtree_safe_fd_step(stack, onexc)
661+
finally:
662+
# Close any file descriptors still on the stack.
663+
while stack:
664+
func, fd, path, entry = stack.pop()
665+
if func is not os.close:
666+
continue
667+
try:
668+
os.close(fd)
669+
except OSError as err:
670+
onexc(os.close, path, err)
671+
672+
def _rmtree_safe_fd_step(stack, onexc):
639673
# Each stack item has four elements:
640674
# * func: The first operation to perform: os.lstat, os.close or os.rmdir.
641675
# Walking a directory starts with an os.lstat() to detect symlinks; in
@@ -710,6 +744,7 @@ def _rmtree_safe_fd(stack, onexc):
710744
os.supports_dir_fd and
711745
os.scandir in os.supports_fd and
712746
os.stat in os.supports_follow_symlinks)
747+
_rmtree_impl = _rmtree_safe_fd if _use_fd_functions else _rmtree_unsafe
713748

714749
def rmtree(path, ignore_errors=False, onerror=None, *, onexc=None, dir_fd=None):
715750
"""Recursively delete a directory tree.
@@ -753,41 +788,7 @@ def onexc(*args):
753788
exc_info = type(exc), exc, exc.__traceback__
754789
return onerror(func, path, exc_info)
755790

756-
if _use_fd_functions:
757-
# While the unsafe rmtree works fine on bytes, the fd based does not.
758-
if isinstance(path, bytes):
759-
path = os.fsdecode(path)
760-
stack = [(os.lstat, dir_fd, path, None)]
761-
try:
762-
while stack:
763-
_rmtree_safe_fd(stack, onexc)
764-
finally:
765-
# Close any file descriptors still on the stack.
766-
while stack:
767-
func, fd, path, entry = stack.pop()
768-
if func is not os.close:
769-
continue
770-
try:
771-
os.close(fd)
772-
except OSError as err:
773-
onexc(os.close, path, err)
774-
else:
775-
if dir_fd is not None:
776-
raise NotImplementedError("dir_fd unavailable on this platform")
777-
try:
778-
st = os.lstat(path)
779-
except OSError as err:
780-
onexc(os.lstat, path, err)
781-
return
782-
try:
783-
if _rmtree_islink(st):
784-
# symlinks to directories are forbidden, see bug #1669
785-
raise OSError("Cannot call rmtree on a symbolic link")
786-
except OSError as err:
787-
onexc(os.path.islink, path, err)
788-
# can't continue even if onexc hook returns
789-
return
790-
return _rmtree_unsafe(path, onexc)
791+
_rmtree_impl(path, dir_fd, onexc)
791792

792793
# Allow introspection of whether or not the hardening against symlink
793794
# attacks is supported on the current platform

‎Lib/test/test_shutil.py

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -558,25 +558,23 @@ def test_rmtree_uses_safe_fd_version_if_available(self):
558558
os.listdir in os.supports_fd and
559559
os.stat in os.supports_follow_symlinks)
560560
if _use_fd_functions:
561-
self.assertTrue(shutil._use_fd_functions)
562561
self.assertTrue(shutil.rmtree.avoids_symlink_attacks)
563562
tmp_dir = self.mkdtemp()
564563
d = os.path.join(tmp_dir, 'a')
565564
os.mkdir(d)
566565
try:
567-
real_rmtree = shutil._rmtree_safe_fd
566+
real_open = os. 1099A open
568567
class Called(Exception): pass
569568
def _raiser(*args, **kwargs):
570569
raise Called
571-
shutil._rmtree_safe_fd = _raiser
570+
os.open = _raiser
572571
self.assertRaises(Called, shutil.rmtree, d)
573572
finally:
574-
shutil._rmtree_safe_fd = real_rmtree
573+
os.open = real_open
575574
else:
576-
self.assertFalse(shutil._use_fd_functions)
577575
self.assertFalse(shutil.rmtree.avoids_symlink_attacks)
578576

579-
@unittest.skipUnless(shutil._use_fd_functions, "requires safe rmtree")
577+
@unittest.skipUnless(shutil.rmtree.avoids_symlink_attacks, "requires safe rmtree")
580578
def test_rmtree_fails_on_close(self):
581579
# Test that the error handler is called for failed os.close() and that
582580
# os.close() is only called once for a file descriptor.
@@ -611,7 +609,7 @@ def onexc(*args):
611609
self.assertEqual(errors[1][1], dir1)
612610
self.assertEqual(close_count, 2)
613611

614-
@unittest.skipUnless(shutil._use_fd_functions, "dir_fd is not supported")
612+
@unittest.skipUnless(shutil.rmtree.avoids_symlink_attacks, "dir_fd is not supported")
615613
def test_rmtree_with_dir_fd(self):
616614
tmp_dir = self.mkdtemp()
617615
victim = 'killme'
@@ -625,7 +623,7 @@ def test_rmtree_with_dir_fd(self):
625623
shutil.rmtree(victim, dir_fd=dir_fd)
626624
self.assertFalse(os.path.exists(fullname))
627625

628-
@unittest.skipIf(shutil._use_fd_functions, "dir_fd is supported")
626+
@unittest.skipIf(shutil.rmtree.avoids_symlink_attacks, "dir_fd is supported")
629627
def test_rmtree_with_dir_fd_unsupported(self):
630628
tmp_dir = self.mkdtemp()
631629
with self.assertRaises(NotImplementedError):

0 commit comments

Comments
 (0)
0