Skip to content

Commit 45587df

Browse files
authored
[3.12] GH-89727: Partially fix shutil.rmtree() recursion error on deep trees (GH-119634) (#119749)
* GH-89727: Partially fix `shutil.rmtree()` recursion error on deep trees (#119634) Make `shutil._rmtree_unsafe()` call `os.walk()`, which is implemented without recursion. `shutil._rmtree_safe_fd()` is not affected and can still raise a recursion error. Co-authored-by: Jelle Zijlstra <[email protected]> (cherry picked from commit a150679)
1 parent e902503 commit 45587df

File tree

4 files changed

+32
-24
lines changed

4 files changed

+32
-24
lines changed

Lib/os.py

+8-1
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,10 @@ def renames(old, new):
279279

280280
__all__.extend(["makedirs", "removedirs", "renames"])
281281

282+
# Private sentinel that makes walk() classify all symlinks and junctions as
283+
# regular files.
284+
_walk_symlinks_as_files = object()
285+
282286
def walk(top, topdown=True, onerror=None, followlinks=False):
283287
"""Directory tree generator.
284288
@@ -380,7 +384,10 @@ def walk(top, topdown=True, onerror=None, followlinks=False):
380384
break
381385

382386
try:
383-
is_dir = entry.is_dir()
387+
if followlinks is _walk_symlinks_as_files:
388+
is_dir = entry.is_dir(follow_symlinks=False) and not entry.is_junction()
389+
else:
390+
is_dir = entry.is_dir()
384391
except OSError:
385392
# If is_dir() raises an OSError, consider the entry not to
386393
# be a directory, same behaviour as os.path.isdir().

Lib/shutil.py

+10-23
Original file line numberDiff line numberDiff line change
@@ -617,31 +617,18 @@ def _rmtree_islink(path):
617617

618618
# version vulnerable to race conditions
619619
def _rmtree_unsafe(path, onexc):
620-
try:
621-
with os.scandir(path) as scandir_it:
622-
entries = list(scandir_it)
623-
except OSError as err:
624-
onexc(os.scandir, path, err)
625-
entries = []
626-
for entry in entries:
627-
fullname = entry.path
628-
try:
629-
is_dir = entry.is_dir(follow_symlinks=False)
630-
except OSError:
631-
is_dir = False
632-
633-
if is_dir and not entry.is_junction():
620+
def onerror(err):
621+
onexc(os.scandir, err.filename, err)
622+
results = os.walk(path, topdown=False, onerror=onerror, followlinks=os._walk_symlinks_as_files)
623+
for dirpath, dirnames, filenames in results:
624+
for name in dirnames:
625+
fullname = os.path.join(dirpath, name)
634626
try:
635-
if entry.is_symlink():
636-
# This can only happen if someone replaces
637-
# a directory with a symlink after the call to
638-
# os.scandir or entry.is_dir above.
639-
raise OSError("Cannot call rmtree on a symbolic link")
627+
os.rmdir(fullname)
640628
except OSError as err:
641-
onexc(os.path.islink, fullname, err)
642-
continue
643-
_rmtree_unsafe(fullname, onexc)
644-
else:
629+
onexc(os.rmdir, fullname, err)
630+
for name in filenames:
631+
fullname = os.path.join(dirpath, name)
645632
try:
646633
os.unlink(fullname)
647634
except OSError as err:

Lib/test/test_shutil.py

+11
Original file line numberDiff line numberDiff line change
@@ -686,6 +686,17 @@ def test_rmtree_on_named_pipe(self):
686686
shutil.rmtree(TESTFN)
687687
self.assertFalse(os.path.exists(TESTFN))
688688

689+
@unittest.skipIf(shutil._use_fd_functions, "fd-based functions remain unfixed (GH-89727)")
690+
def test_rmtree_above_recursion_limit(self):
691+
recursion_limit = 40
692+
# directory_depth > recursion_limit
693+
directory_depth = recursion_limit + 10
694+
base = os.path.join(TESTFN, *(['d'] * directory_depth))
695+
os.makedirs(base)
696+
697+
with support.infinite_recursion(recursion_limit):
698+
shutil.rmtree(TESTFN)
699+
689700

690701
class TestCopyTree(BaseTest, unittest.TestCase):
691702

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Partially fix issue with :func:`shutil.rmtree` where a :exc:`RecursionError`
2+
is raised on deep directory trees. A recursion error is no longer raised
3+
when :data:`!rmtree.avoids_symlink_attacks` is false.

0 commit comments

Comments
 (0)