mirror of
				https://github.com/python/cpython.git
				synced 2025-11-04 07:31:38 +00:00 
			
		
		
		
	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 <jelle.zijlstra@gmail.com>
This commit is contained in:
		
							parent
							
								
									c22323cd1c
								
							
						
					
					
						commit
						a150679f90
					
				
					 4 changed files with 33 additions and 28 deletions
				
			
		| 
						 | 
					@ -281,6 +281,10 @@ def renames(old, new):
 | 
				
			||||||
 | 
					
 | 
				
			||||||
__all__.extend(["makedirs", "removedirs", "renames"])
 | 
					__all__.extend(["makedirs", "removedirs", "renames"])
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					# Private sentinel that makes walk() classify all symlinks and junctions as
 | 
				
			||||||
 | 
					# regular files.
 | 
				
			||||||
 | 
					_walk_symlinks_as_files = object()
 | 
				
			||||||
 | 
					
 | 
				
			||||||
def walk(top, topdown=True, onerror=None, followlinks=False):
 | 
					def walk(top, topdown=True, onerror=None, followlinks=False):
 | 
				
			||||||
    """Directory tree generator.
 | 
					    """Directory tree generator.
 | 
				
			||||||
 | 
					
 | 
				
			||||||
| 
						 | 
					@ -382,6 +386,9 @@ def walk(top, topdown=True, onerror=None, followlinks=False):
 | 
				
			||||||
                    break
 | 
					                    break
 | 
				
			||||||
 | 
					
 | 
				
			||||||
                try:
 | 
					                try:
 | 
				
			||||||
 | 
					                    if followlinks is _walk_symlinks_as_files:
 | 
				
			||||||
 | 
					                        is_dir = entry.is_dir(follow_symlinks=False) and not entry.is_junction()
 | 
				
			||||||
 | 
					                    else:
 | 
				
			||||||
                        is_dir = entry.is_dir()
 | 
					                        is_dir = entry.is_dir()
 | 
				
			||||||
                except OSError:
 | 
					                except OSError:
 | 
				
			||||||
                    # If is_dir() raises an OSError, consider the entry not to
 | 
					                    # If is_dir() raises an OSError, consider the entry not to
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -606,37 +606,21 @@ def _rmtree_islink(st):
 | 
				
			||||||
 | 
					
 | 
				
			||||||
# version vulnerable to race conditions
 | 
					# version vulnerable to race conditions
 | 
				
			||||||
def _rmtree_unsafe(path, onexc):
 | 
					def _rmtree_unsafe(path, onexc):
 | 
				
			||||||
 | 
					    def onerror(err):
 | 
				
			||||||
 | 
					        if not isinstance(err, FileNotFoundError):
 | 
				
			||||||
 | 
					            onexc(os.scandir, err.filename, err)
 | 
				
			||||||
 | 
					    results = os.walk(path, topdown=False, onerror=onerror, followlinks=os._walk_symlinks_as_files)
 | 
				
			||||||
 | 
					    for dirpath, dirnames, filenames in results:
 | 
				
			||||||
 | 
					        for name in dirnames:
 | 
				
			||||||
 | 
					            fullname = os.path.join(dirpath, name)
 | 
				
			||||||
            try:
 | 
					            try:
 | 
				
			||||||
        with os.scandir(path) as scandir_it:
 | 
					                os.rmdir(fullname)
 | 
				
			||||||
            entries = list(scandir_it)
 | 
					 | 
				
			||||||
    except FileNotFoundError:
 | 
					 | 
				
			||||||
        return
 | 
					 | 
				
			||||||
    except OSError as err:
 | 
					 | 
				
			||||||
        onexc(os.scandir, path, err)
 | 
					 | 
				
			||||||
        entries = []
 | 
					 | 
				
			||||||
    for entry in entries:
 | 
					 | 
				
			||||||
        fullname = entry.path
 | 
					 | 
				
			||||||
        try:
 | 
					 | 
				
			||||||
            is_dir = entry.is_dir(follow_symlinks=False)
 | 
					 | 
				
			||||||
        except FileNotFoundError:
 | 
					 | 
				
			||||||
            continue
 | 
					 | 
				
			||||||
        except OSError:
 | 
					 | 
				
			||||||
            is_dir = False
 | 
					 | 
				
			||||||
 | 
					 | 
				
			||||||
        if is_dir and not entry.is_junction():
 | 
					 | 
				
			||||||
            try:
 | 
					 | 
				
			||||||
                if entry.is_symlink():
 | 
					 | 
				
			||||||
                    # This can only happen if someone replaces
 | 
					 | 
				
			||||||
                    # a directory with a symlink after the call to
 | 
					 | 
				
			||||||
                    # os.scandir or entry.is_dir above.
 | 
					 | 
				
			||||||
                    raise OSError("Cannot call rmtree on a symbolic link")
 | 
					 | 
				
			||||||
            except FileNotFoundError:
 | 
					            except FileNotFoundError:
 | 
				
			||||||
                continue
 | 
					                continue
 | 
				
			||||||
            except OSError as err:
 | 
					            except OSError as err:
 | 
				
			||||||
                onexc(os.path.islink, fullname, err)
 | 
					                onexc(os.rmdir, fullname, err)
 | 
				
			||||||
                continue
 | 
					        for name in filenames:
 | 
				
			||||||
            _rmtree_unsafe(fullname, onexc)
 | 
					            fullname = os.path.join(dirpath, name)
 | 
				
			||||||
        else:
 | 
					 | 
				
			||||||
            try:
 | 
					            try:
 | 
				
			||||||
                os.unlink(fullname)
 | 
					                os.unlink(fullname)
 | 
				
			||||||
            except FileNotFoundError:
 | 
					            except FileNotFoundError:
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -741,6 +741,17 @@ def _onexc(fn, path, exc):
 | 
				
			||||||
            shutil.rmtree(TESTFN)
 | 
					            shutil.rmtree(TESTFN)
 | 
				
			||||||
            raise
 | 
					            raise
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					    @unittest.skipIf(shutil._use_fd_functions, "fd-based functions remain unfixed (GH-89727)")
 | 
				
			||||||
 | 
					    def test_rmtree_above_recursion_limit(self):
 | 
				
			||||||
 | 
					        recursion_limit = 40
 | 
				
			||||||
 | 
					        # directory_depth > recursion_limit
 | 
				
			||||||
 | 
					        directory_depth = recursion_limit + 10
 | 
				
			||||||
 | 
					        base = os.path.join(TESTFN, *(['d'] * directory_depth))
 | 
				
			||||||
 | 
					        os.makedirs(base)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					        with support.infinite_recursion(recursion_limit):
 | 
				
			||||||
 | 
					            shutil.rmtree(TESTFN)
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					
 | 
				
			||||||
class TestCopyTree(BaseTest, unittest.TestCase):
 | 
					class TestCopyTree(BaseTest, unittest.TestCase):
 | 
				
			||||||
 | 
					
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
| 
						 | 
					@ -0,0 +1,3 @@
 | 
				
			||||||
 | 
					Partially fix issue with :func:`shutil.rmtree` where a :exc:`RecursionError`
 | 
				
			||||||
 | 
					is raised on deep directory trees. A recursion error is no longer raised
 | 
				
			||||||
 | 
					when :data:`!rmtree.avoids_symlink_attacks` is false.
 | 
				
			||||||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue