Skip to content

Commit a150679

Browse files
pythonGH-89727: Partially fix shutil.rmtree() recursion error on deep trees (python#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 <jelle.zijlstra@gmail.com>
1 parent c22323c commit a150679

File tree

4 files changed

+33
-28
lines changed

4 files changed

+33
-28
lines changed

Lib/os.py

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

282282
__all__.extend(["makedirs", "removedirs", "renames"])
283283

284+
# Private sentinel that makes walk() classify all symlinks and junctions as
285+
# regular files.
286+
_walk_symlinks_as_files = object()
287+
284288
def walk(top, topdown=True, onerror=None, followlinks=False):
285289
"""Directory tree generator.
286290
@@ -382,7 +386,10 @@ def walk(top, topdown=True, onerror=None, followlinks=False):
382386
break
383387

384388
try:
385-
is_dir = entry.is_dir()
389+
if followlinks is _walk_symlinks_as_files:
390+
is_dir = entry.is_dir(follow_symlinks=False) and not entry.is_junction()
391+
else:
392+
is_dir = entry.is_dir()
386393
except OSError:
387394
# If is_dir() raises an OSError, consider the entry not to
388395
# be a directory, same behaviour as os.path.isdir().

Lib/shutil.py

+11-27
Original file line numberDiff line numberDiff line change
@@ -606,37 +606,21 @@ def _rmtree_islink(st):
606606

607607
# version vulnerable to race conditions
608608
def _rmtree_unsafe(path, onexc):
609-
try:
610-
with os.scandir(path) as scandir_it:
611-
entries = list(scandir_it)
612-
except FileNotFoundError:
613-
return
614-
except OSError as err:
615-
onexc(os.scandir, path, err)
616-
entries = []
617-
for entry in entries:
618-
fullname = entry.path
619-
try:
620-
is_dir = entry.is_dir(follow_symlinks=False)
621-
except FileNotFoundError:
622-
continue
623-
except OSError:
624-
is_dir = False
625-
626-
if is_dir and not entry.is_junction():
609+
def onerror(err):
610+
if not isinstance(err, FileNotFoundError):
611+
onexc(os.scandir, err.filename, err)
612+
results = os.walk(path, topdown=False, onerror=onerror, followlinks=os._walk_symlinks_as_files)
613+
for dirpath, dirnames, filenames in results:
614+
for name in dirnames:
615+
fullname = os.path.join(dirpath, name)
627616
try:
628-
if entry.is_symlink():
629-
# This can only happen if someone replaces
630-
# a directory with a symlink after the call to
631-
# os.scandir or entry.is_dir above.
632-
raise OSError("Cannot call rmtree on a symbolic link")
617+
os.rmdir(fullname)
633618
except FileNotFoundError:
634619
continue
635620
except OSError as err:
636-
onexc(os.path.islink, fullname, err)
637-
continue
638-
_rmtree_unsafe(fullname, onexc)
639-
else:
621+
onexc(os.rmdir, fullname, err)
622+
for name in filenames:
623+
fullname = os.path.join(dirpath, name)
640624
try:
641625
os.unlink(fullname)
642626
except FileNotFoundError:

Lib/test/test_shutil.py

+11
Original file line numberDiff line numberDiff line change
@@ -741,6 +741,17 @@ def _onexc(fn, path, exc):
741741
shutil.rmtree(TESTFN)
742742
raise
743743

744+
@unittest.skipIf(shutil._use_fd_functions, "fd-based functions remain unfixed (GH-89727)")
745+
def test_rmtree_above_recursion_limit(self):
746+
recursion_limit = 40
747+
# directory_depth > recursion_limit
748+
directory_depth = recursion_limit + 10
749+
base = os.path.join(TESTFN, *(['d'] * directory_depth))
750+
os.makedirs(base)
751+
752+
with support.infinite_recursion(recursion_limit):
753+
shutil.rmtree(TESTFN)
754+
744755

745756
class TestCopyTree(BaseTest, unittest.TestCase):
746757

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)