diff --git a/Doc/library/pathlib.rst b/Doc/library/pathlib.rst index 60c099af928..3df446a39ea 100644 --- a/Doc/library/pathlib.rst +++ b/Doc/library/pathlib.rst @@ -1636,7 +1636,7 @@ Copying, renaming and deleting .. method:: Path.unlink(missing_ok=False) Remove this file or symbolic link. If the path points to a directory, - use :func:`Path.rmdir` instead. + use :func:`Path.rmdir` or :func:`Path.delete` instead. If *missing_ok* is false (the default), :exc:`FileNotFoundError` is raised if the path does not exist. @@ -1650,33 +1650,40 @@ Copying, renaming and deleting .. method:: Path.rmdir() - Remove this directory. The directory must be empty. + Remove this directory. The directory must be empty; use + :meth:`Path.delete` to remove a non-empty directory. -.. method:: Path.rmtree(ignore_errors=False, on_error=None) +.. method:: Path.delete(ignore_errors=False, on_error=None) - Recursively delete this entire directory tree. The path must not refer to a symlink. + Delete this file or directory. If this path refers to a non-empty + directory, its files and sub-directories are deleted recursively. - If *ignore_errors* is true, errors resulting from failed removals will be - ignored. If *ignore_errors* is false or omitted, and a function is given to - *on_error*, it will be called each time an exception is raised. If neither - *ignore_errors* nor *on_error* are supplied, exceptions are propagated to - the caller. + If *ignore_errors* is true, errors resulting from failed deletions will be + ignored. If *ignore_errors* is false or omitted, and a callable is given as + the optional *on_error* argument, it will be called with one argument of + type :exc:`OSError` each time an exception is raised. The callable can + handle the error to continue the deletion process or re-raise it to stop. + Note that the filename is available as the :attr:`~OSError.filename` + attribute of the exception object. If neither *ignore_errors* nor + *on_error* are supplied, exceptions are propagated to the caller. .. note:: - On platforms that support the necessary fd-based functions, a symlink - attack-resistant version of :meth:`~Path.rmtree` is used by default. On - other platforms, the :func:`~Path.rmtree` implementation is susceptible - to a symlink attack: given proper timing and circumstances, attackers - can manipulate symlinks on the filesystem to delete files they would not - be able to access otherwise. + When deleting non-empty directories on platforms that lack the necessary + file descriptor-based functions, the :meth:`~Path.delete` implementation + is susceptible to a symlink attack: given proper timing and + circumstances, attackers can manipulate symlinks on the filesystem to + delete files they would not be able to access otherwise. Applications + can use the :data:`~Path.delete.avoids_symlink_attacks` method attribute + to determine whether the implementation is immune to this attack. - If the optional argument *on_error* is specified, it should be a callable; - it will be called with one argument of type :exc:`OSError`. The - callable can handle the error to continue the deletion process or re-raise - it to stop. Note that the filename is available as the :attr:`~OSError.filename` - attribute of the exception object. + .. attribute:: delete.avoids_symlink_attacks + + Indicates whether the current platform and implementation provides a + symlink attack resistant version of :meth:`~Path.delete`. Currently + this is only true for platforms supporting fd-based directory access + functions. .. versionadded:: 3.14 diff --git a/Doc/whatsnew/3.14.rst b/Doc/whatsnew/3.14.rst index aecc7cabd0d..c989de26fd4 100644 --- a/Doc/whatsnew/3.14.rst +++ b/Doc/whatsnew/3.14.rst @@ -141,8 +141,7 @@ pathlib :func:`shutil.copyfile`. * :meth:`~pathlib.Path.copytree` copies one directory tree to another, like :func:`shutil.copytree`. - * :meth:`~pathlib.Path.rmtree` recursively removes a directory tree, like - :func:`shutil.rmtree`. + * :meth:`~pathlib.Path.delete` removes a file or directory tree. (Contributed by Barney Gale in :gh:`73991`.) diff --git a/Lib/pathlib/_abc.py b/Lib/pathlib/_abc.py index ee903177aa1..8c799196e47 100644 --- a/Lib/pathlib/_abc.py +++ b/Lib/pathlib/_abc.py @@ -919,15 +919,15 @@ class PathBase(PurePathBase): """ raise UnsupportedOperation(self._unsupported_msg('rmdir()')) - def rmtree(self, ignore_errors=False, on_error=None): + def delete(self, ignore_errors=False, on_error=None): """ - Recursively delete this directory tree. + Delete this file or directory (including all sub-directories). - If *ignore_errors* is true, exceptions raised from scanning the tree - and removing files and directories are ignored. Otherwise, if - *on_error* is set, it will be called to handle the error. If neither - *ignore_errors* nor *on_error* are set, exceptions are propagated to - the caller. + If *ignore_errors* is true, exceptions raised from scanning the + filesystem and removing files and directories are ignored. Otherwise, + if *on_error* is set, it will be called to handle the error. If + neither *ignore_errors* nor *on_error* are set, exceptions are + propagated to the caller. """ if ignore_errors: def on_error(err): @@ -935,14 +935,10 @@ class PathBase(PurePathBase): elif on_error is None: def on_error(err): raise err - try: - if self.is_symlink(): - raise OSError("Cannot call rmtree on a symbolic link") - elif self.is_junction(): - raise OSError("Cannot call rmtree on a junction") + if self.is_dir(follow_symlinks=False): results = self.walk( on_error=on_error, - top_down=False, # Bottom-up so we rmdir() empty directories. + top_down=False, # So we rmdir() empty directories. follow_symlinks=False) for dirpath, dirnames, filenames in results: for name in filenames: @@ -955,10 +951,15 @@ class PathBase(PurePathBase): dirpath.joinpath(name).rmdir() except OSError as err: on_error(err) - self.rmdir() + delete_self = self.rmdir + else: + delete_self = self.unlink + try: + delete_self() except OSError as err: err.filename = str(self) on_error(err) + delete.avoids_symlink_attacks = False def owner(self, *, follow_symlinks=True): """ diff --git a/Lib/pathlib/_local.py b/Lib/pathlib/_local.py index 4fd5279f9fe..6e2f88c9342 100644 --- a/Lib/pathlib/_local.py +++ b/Lib/pathlib/_local.py @@ -3,6 +3,7 @@ import ntpath import operator import os import posixpath +import shutil import sys from glob import _StringGlobber from itertools import chain @@ -830,24 +831,34 @@ class Path(PathBase, PurePath): """ os.rmdir(self) - def rmtree(self, ignore_errors=False, on_error=None): + def delete(self, ignore_errors=False, on_error=None): """ - Recursively delete this directory tree. + Delete this file or directory (including all sub-directories). - If *ignore_errors* is true, exceptions raised from scanning the tree - and removing files and directories are ignored. Otherwise, if - *on_error* is set, it will be called to handle the error. If neither - *ignore_errors* nor *on_error* are set, exceptions are propagated to - the caller. + If *ignore_errors* is true, exceptions raised from scanning the + filesystem and removing files and directories are ignored. Otherwise, + if *on_error* is set, it will be called to handle the error. If + neither *ignore_errors* nor *on_error* are set, exceptions are + propagated to the caller. """ - if on_error: - def onexc(func, filename, err): - err.filename = filename - on_error(err) - else: + if self.is_dir(follow_symlinks=False): onexc = None - import shutil - shutil.rmtree(str(self), ignore_errors, onexc=onexc) + if on_error: + def onexc(func, filename, err): + err.filename = filename + on_error(err) + shutil.rmtree(str(self), ignore_errors, onexc=onexc) + else: + try: + self.unlink() + except OSError as err: + if not ignore_errors: + if on_error: + on_error(err) + else: + raise + + delete.avoids_symlink_attacks = shutil.rmtree.avoids_symlink_attacks def rename(self, target): """ diff --git a/Lib/test/test_pathlib/test_pathlib.py b/Lib/test/test_pathlib/test_pathlib.py index 7160e764dfb..9e922259cba 100644 --- a/Lib/test/test_pathlib/test_pathlib.py +++ b/Lib/test/test_pathlib/test_pathlib.py @@ -32,7 +32,7 @@ root_in_posix = False if hasattr(os, 'geteuid'): root_in_posix = (os.geteuid() == 0) -rmtree_use_fd_functions = ( +delete_use_fd_functions = ( {os.open, os.stat, os.unlink, os.rmdir} <= os.supports_dir_fd and os.listdir in os.supports_fd and os.stat in os.supports_follow_symlinks) @@ -862,8 +862,9 @@ class PathTest(test_pathlib_abc.DummyPathTest, PurePathTest): self.assertEqual(expected_gid, gid_2) self.assertEqual(expected_name, link.group(follow_symlinks=False)) - def test_rmtree_uses_safe_fd_version_if_available(self): - if rmtree_use_fd_functions: + def test_delete_uses_safe_fd_version_if_available(self): + if delete_use_fd_functions: + self.assertTrue(self.cls.delete.avoids_symlink_attacks) d = self.cls(self.base, 'a') d.mkdir() try: @@ -876,16 +877,18 @@ class PathTest(test_pathlib_abc.DummyPathTest, PurePathTest): raise Called os.open = _raiser - self.assertRaises(Called, d.rmtree) + self.assertRaises(Called, d.delete) finally: os.open = real_open + else: + self.assertFalse(self.cls.delete.avoids_symlink_attacks) @unittest.skipIf(sys.platform[:6] == 'cygwin', "This test can't be run on Cygwin (issue #1071513).") @os_helper.skip_if_dac_override @os_helper.skip_unless_working_chmod - def test_rmtree_unwritable(self): - tmp = self.cls(self.base, 'rmtree') + def test_delete_unwritable(self): + tmp = self.cls(self.base, 'delete') tmp.mkdir() child_file_path = tmp / 'a' child_dir_path = tmp / 'b' @@ -902,7 +905,7 @@ class PathTest(test_pathlib_abc.DummyPathTest, PurePathTest): tmp.chmod(new_mode) errors = [] - tmp.rmtree(on_error=errors.append) + tmp.delete(on_error=errors.append) # Test whether onerror has actually been called. self.assertEqual(len(errors), 3) finally: @@ -911,9 +914,9 @@ class PathTest(test_pathlib_abc.DummyPathTest, PurePathTest): child_dir_path.chmod(old_child_dir_mode) @needs_windows - def test_rmtree_inner_junction(self): + def test_delete_inner_junction(self): import _winapi - tmp = self.cls(self.base, 'rmtree') + tmp = self.cls(self.base, 'delete') tmp.mkdir() dir1 = tmp / 'dir1' dir2 = dir1 / 'dir2' @@ -929,15 +932,15 @@ class PathTest(test_pathlib_abc.DummyPathTest, PurePathTest): link3 = dir1 / 'link3' _winapi.CreateJunction(str(file1), str(link3)) # make sure junctions are removed but not followed - dir1.rmtree() + dir1.delete() self.assertFalse(dir1.exists()) self.assertTrue(dir3.exists()) self.assertTrue(file1.exists()) @needs_windows - def test_rmtree_outer_junction(self): + def test_delete_outer_junction(self): import _winapi - tmp = self.cls(self.base, 'rmtree') + tmp = self.cls(self.base, 'delete') tmp.mkdir() try: src = tmp / 'cheese' @@ -946,22 +949,22 @@ class PathTest(test_pathlib_abc.DummyPathTest, PurePathTest): spam = src / 'spam' spam.write_text('') _winapi.CreateJunction(str(src), str(dst)) - self.assertRaises(OSError, dst.rmtree) - dst.rmtree(ignore_errors=True) + self.assertRaises(OSError, dst.delete) + dst.delete(ignore_errors=True) finally: - tmp.rmtree(ignore_errors=True) + tmp.delete(ignore_errors=True) @needs_windows - def test_rmtree_outer_junction_on_error(self): + def test_delete_outer_junction_on_error(self): import _winapi - tmp = self.cls(self.base, 'rmtree') + tmp = self.cls(self.base, 'delete') tmp.mkdir() dir_ = tmp / 'dir' dir_.mkdir() link = tmp / 'link' _winapi.CreateJunction(str(dir_), str(link)) try: - self.assertRaises(OSError, link.rmtree) + self.assertRaises(OSError, link.delete) self.assertTrue(dir_.exists()) self.assertTrue(link.exists(follow_symlinks=False)) errors = [] @@ -969,18 +972,18 @@ class PathTest(test_pathlib_abc.DummyPathTest, PurePathTest): def on_error(error): errors.append(error) - link.rmtree(on_error=on_error) + link.delete(on_error=on_error) self.assertEqual(len(errors), 1) self.assertIsInstance(errors[0], OSError) self.assertEqual(errors[0].filename, str(link)) finally: os.unlink(str(link)) - @unittest.skipUnless(rmtree_use_fd_functions, "requires safe rmtree") - def test_rmtree_fails_on_close(self): + @unittest.skipUnless(delete_use_fd_functions, "requires safe delete") + def test_delete_fails_on_close(self): # Test that the error handler is called for failed os.close() and that # os.close() is only called once for a file descriptor. - tmp = self.cls(self.base, 'rmtree') + tmp = self.cls(self.base, 'delete') tmp.mkdir() dir1 = tmp / 'dir1' dir1.mkdir() @@ -996,7 +999,7 @@ class PathTest(test_pathlib_abc.DummyPathTest, PurePathTest): close_count = 0 with swap_attr(os, 'close', close) as orig_close: with self.assertRaises(OSError): - dir1.rmtree() + dir1.delete() self.assertTrue(dir2.is_dir()) self.assertEqual(close_count, 2) @@ -1004,7 +1007,7 @@ class PathTest(test_pathlib_abc.DummyPathTest, PurePathTest): errors = [] with swap_attr(os, 'close', close) as orig_close: - dir1.rmtree(on_error=errors.append) + dir1.delete(on_error=errors.append) self.assertEqual(len(errors), 2) self.assertEqual(errors[0].filename, str(dir2)) self.assertEqual(errors[1].filename, str(dir1)) @@ -1013,27 +1016,23 @@ class PathTest(test_pathlib_abc.DummyPathTest, PurePathTest): @unittest.skipUnless(hasattr(os, "mkfifo"), 'requires os.mkfifo()') @unittest.skipIf(sys.platform == "vxworks", "fifo requires special path on VxWorks") - def test_rmtree_on_named_pipe(self): + def test_delete_on_named_pipe(self): p = self.cls(self.base, 'pipe') os.mkfifo(p) - try: - with self.assertRaises(NotADirectoryError): - p.rmtree() - self.assertTrue(p.exists()) - finally: - p.unlink() + p.delete() + self.assertFalse(p.exists()) p = self.cls(self.base, 'dir') p.mkdir() os.mkfifo(p / 'mypipe') - p.rmtree() + p.delete() self.assertFalse(p.exists()) @unittest.skipIf(sys.platform[:6] == 'cygwin', "This test can't be run on Cygwin (issue #1071513).") @os_helper.skip_if_dac_override @os_helper.skip_unless_working_chmod - def test_rmtree_deleted_race_condition(self): + def test_delete_deleted_race_condition(self): # bpo-37260 # # Test that a file or a directory deleted after it is enumerated @@ -1057,7 +1056,7 @@ class PathTest(test_pathlib_abc.DummyPathTest, PurePathTest): if p != keep: p.unlink() - tmp = self.cls(self.base, 'rmtree') + tmp = self.cls(self.base, 'delete') tmp.mkdir() paths = [tmp] + [tmp / f'child{i}' for i in range(6)] dirs = paths[1::2] @@ -1075,7 +1074,7 @@ class PathTest(test_pathlib_abc.DummyPathTest, PurePathTest): path.chmod(new_mode) try: - tmp.rmtree(on_error=on_error) + tmp.delete(on_error=on_error) except: # Test failed, so cleanup artifacts. for path, mode in zip(paths, old_modes): @@ -1083,13 +1082,13 @@ class PathTest(test_pathlib_abc.DummyPathTest, PurePathTest): path.chmod(mode) except OSError: pass - tmp.rmtree() + tmp.delete() raise - def test_rmtree_does_not_choke_on_failing_lstat(self): + def test_delete_does_not_choke_on_failing_lstat(self): try: orig_lstat = os.lstat - tmp = self.cls(self.base, 'rmtree') + tmp = self.cls(self.base, 'delete') def raiser(fn, *args, **kwargs): if fn != str(tmp): @@ -1102,7 +1101,7 @@ class PathTest(test_pathlib_abc.DummyPathTest, PurePathTest): tmp.mkdir() foo = tmp / 'foo' foo.write_text('') - tmp.rmtree() + tmp.delete() finally: os.lstat = orig_lstat diff --git a/Lib/test/test_pathlib/test_pathlib_abc.py b/Lib/test/test_pathlib/test_pathlib_abc.py index 37678c5d799..443a4e989fb 100644 --- a/Lib/test/test_pathlib/test_pathlib_abc.py +++ b/Lib/test/test_pathlib/test_pathlib_abc.py @@ -2641,85 +2641,43 @@ class DummyPathTest(DummyPurePathTest): self.assertFileNotFound(p.stat) self.assertFileNotFound(p.unlink) - def test_rmtree(self): + def test_delete_file(self): + p = self.cls(self.base) / 'fileA' + p.delete() + self.assertFileNotFound(p.stat) + self.assertFileNotFound(p.unlink) + + def test_delete_dir(self): base = self.cls(self.base) - base.joinpath('dirA').rmtree() + base.joinpath('dirA').delete() self.assertRaises(FileNotFoundError, base.joinpath('dirA').stat) self.assertRaises(FileNotFoundError, base.joinpath('dirA', 'linkC').lstat) - base.joinpath('dirB').rmtree() + base.joinpath('dirB').delete() self.assertRaises(FileNotFoundError, base.joinpath('dirB').stat) self.assertRaises(FileNotFoundError, base.joinpath('dirB', 'fileB').stat) self.assertRaises(FileNotFoundError, base.joinpath('dirB', 'linkD').lstat) - base.joinpath('dirC').rmtree() + base.joinpath('dirC').delete() self.assertRaises(FileNotFoundError, base.joinpath('dirC').stat) self.assertRaises(FileNotFoundError, base.joinpath('dirC', 'dirD').stat) self.assertRaises(FileNotFoundError, base.joinpath('dirC', 'dirD', 'fileD').stat) self.assertRaises(FileNotFoundError, base.joinpath('dirC', 'fileC').stat) self.assertRaises(FileNotFoundError, base.joinpath('dirC', 'novel.txt').stat) - def test_rmtree_errors(self): - tmp = self.cls(self.base, 'rmtree') - tmp.mkdir() - # filename is guaranteed not to exist - filename = tmp / 'foo' - self.assertRaises(FileNotFoundError, filename.rmtree) - # test that ignore_errors option is honored - filename.rmtree(ignore_errors=True) - - # existing file - filename = tmp / "tstfile" - filename.write_text("") - with self.assertRaises(NotADirectoryError) as cm: - filename.rmtree() - self.assertEqual(cm.exception.filename, str(filename)) - self.assertTrue(filename.exists()) - # test that ignore_errors option is honored - filename.rmtree(ignore_errors=True) - self.assertTrue(filename.exists()) - - def test_rmtree_on_error(self): - tmp = self.cls(self.base, 'rmtree') - tmp.mkdir() - filename = tmp / "tstfile" - filename.write_text("") - errors = [] - - def on_error(error): - errors.append(error) - - filename.rmtree(on_error=on_error) - self.assertEqual(len(errors), 2) - # First from scandir() - self.assertIsInstance(errors[0], NotADirectoryError) - self.assertEqual(errors[0].filename, str(filename)) - # Then from munlink() - self.assertIsInstance(errors[1], NotADirectoryError) - self.assertEqual(errors[1].filename, str(filename)) - @needs_symlinks - def test_rmtree_outer_symlink(self): - tmp = self.cls(self.base, 'rmtree') + def test_delete_symlink(self): + tmp = self.cls(self.base, 'delete') tmp.mkdir() dir_ = tmp / 'dir' dir_.mkdir() link = tmp / 'link' link.symlink_to(dir_) - self.assertRaises(OSError, link.rmtree) + link.delete() self.assertTrue(dir_.exists()) - self.assertTrue(link.exists(follow_symlinks=False)) - errors = [] - - def on_error(error): - errors.append(error) - - link.rmtree(on_error=on_error) - self.assertEqual(len(errors), 1) - self.assertIsInstance(errors[0], OSError) - self.assertEqual(errors[0].filename, str(link)) + self.assertFalse(link.exists(follow_symlinks=False)) @needs_symlinks - def test_rmtree_inner_symlink(self): - tmp = self.cls(self.base, 'rmtree') + def test_delete_inner_symlink(self): + tmp = self.cls(self.base, 'delete') tmp.mkdir() dir1 = tmp / 'dir1' dir2 = dir1 / 'dir2' @@ -2735,11 +2693,26 @@ class DummyPathTest(DummyPurePathTest): link3 = dir1 / 'link3' link3.symlink_to(file1) # make sure symlinks are removed but not followed - dir1.rmtree() + dir1.delete() self.assertFalse(dir1.exists()) self.assertTrue(dir3.exists()) self.assertTrue(file1.exists()) + def test_delete_missing(self): + tmp = self.cls(self.base, 'delete') + tmp.mkdir() + # filename is guaranteed not to exist + filename = tmp / 'foo' + self.assertRaises(FileNotFoundError, filename.delete) + # test that ignore_errors option is honored + filename.delete(ignore_errors=True) + # test on_error + errors = [] + filename.delete(on_error=errors.append) + self.assertEqual(len(errors), 1) + self.assertIsInstance(errors[0], FileNotFoundError) + self.assertEqual(errors[0].filename, str(filename)) + def setUpWalk(self): # Build: # TESTFN/ diff --git a/Misc/NEWS.d/next/Library/2024-05-15-01-21-44.gh-issue-73991.bNDqQN.rst b/Misc/NEWS.d/next/Library/2024-05-15-01-21-44.gh-issue-73991.bNDqQN.rst index 9aa7a7dba66..5806fed91c7 100644 --- a/Misc/NEWS.d/next/Library/2024-05-15-01-21-44.gh-issue-73991.bNDqQN.rst +++ b/Misc/NEWS.d/next/Library/2024-05-15-01-21-44.gh-issue-73991.bNDqQN.rst @@ -1 +1 @@ -Add :meth:`pathlib.Path.rmtree`, which recursively removes a directory. +Add :meth:`pathlib.Path.delete`, which recursively removes a file or directory.