mirrors/cpython
1
0
Fork 0
mirror of https://github.com/python/cpython.git synced 2025-07-09 20:35:26 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 11

gh-102828: add onexc arg to shutil.rmtree. Deprecate onerror. (#102829)

Browse source
This commit is contained in:
Irit Katriel 2023-03-19 18:33:51 +00:00 • committed by GitHub
parent 4d1f033986
commit d51a6dc28e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 256 additions and 56 deletions
Download patch file Download diff file Expand all files Collapse all files

24
Doc/library/shutil.rst
View file

@ -292,15 +292,15 @@ Directory and files operations
.. versionadded:: 3.8
The *dirs_exist_ok* parameter.
.. function:: rmtree(path, ignore_errors=False, onerror=None, *, dir_fd=None)
.. function:: rmtree(path, ignore_errors=False, onerror=None, *, onexc=None, dir_fd=None)
.. index:: single: directory; deleting
Delete an entire directory tree; *path* must point to a directory (but not a
symbolic link to a directory). If *ignore_errors* is true, errors resulting
from failed removals will be ignored; if false or omitted, such errors are
handled by calling a handler specified by *onerror* or, if that is omitted,
they raise an exception.
handled by calling a handler specified by *onexc* or *onerror* or, if both
are omitted, exceptions are propagated to the caller.
This function can support :ref:`paths relative to directory descriptors
<dir_fd>`.
@ -315,14 +315,17 @@ Directory and files operations
otherwise. Applications can use the :data:`rmtree.avoids_symlink_attacks`
function attribute to determine which case applies.
If *onerror* is provided, it must be a callable that accepts three
parameters: *function*, *path*, and *excinfo*.
If *onexc* is provided, it must be a callable that accepts three parameters:
*function*, *path*, and *excinfo*.
The first parameter, *function*, is the function which raised the exception;
it depends on the platform and implementation. The second parameter,
*path*, will be the path name passed to *function*. The third parameter,
*excinfo*, will be the exception information returned by
:func:`sys.exc_info`. Exceptions raised by *onerror* will not be caught.
*excinfo*, is the exception that was raised. Exceptions raised by *onexc*
will not be caught.
The deprecated *onerror* is similar to *onexc*, except that the third
parameter it receives is the tuple returned from :func:`sys.exc_info`.
.. audit-event:: shutil.rmtree path,dir_fd shutil.rmtree
@ -337,6 +340,9 @@ Directory and files operations
.. versionchanged:: 3.11
The *dir_fd* parameter.
.. versionchanged:: 3.12
Added the *onexc* parameter, deprecated *onerror*.
.. attribute:: rmtree.avoids_symlink_attacks
Indicates whether the current platform and implementation provides a
@ -509,7 +515,7 @@ rmtree example
~~~~~~~~~~~~~~
This example shows how to remove a directory tree on Windows where some
of the files have their read-only bit set. It uses the onerror callback
of the files have their read-only bit set. It uses the onexc callback
to clear the readonly bit and reattempt the remove. Any subsequent failure
will propagate. ::
@ -521,7 +527,7 @@ will propagate. ::
os.chmod(path, stat.S_IWRITE)
func(path)
shutil.rmtree(directory, onerror=remove_readonly)
shutil.rmtree(directory, onexc=remove_readonly)
.. _archiving-operations:

9
Doc/whatsnew/3.12.rst
View file

@ -337,6 +337,11 @@ shutil
of the process to *root_dir* to perform archiving.
(Contributed by Serhiy Storchaka in :gh:`74696`.)
* :func:`shutil.rmtree` now accepts a new argument *onexc* which is an
error handler like *onerror* but which expects an exception instance
rather than a *(typ, val, tb)* triplet. *onerror* is deprecated.
(Contributed by Irit Katriel in :gh:`102828`.)
sqlite3
-------
@ -498,6 +503,10 @@ Deprecated
fields are deprecated. Use :data:`sys.last_exc` instead.
(Contributed by Irit Katriel in :gh:`102778`.)
* The *onerror* argument of :func:`shutil.rmtree` is deprecated. Use *onexc*
instead. (Contributed by Irit Katriel in :gh:`102828`.)
Pending Removal in Python 3.13
------------------------------

106
Lib/shutil.py
View file

@ -575,12 +575,12 @@ else:
return os.path.islink(path)
# version vulnerable to race conditions
def _rmtree_unsafe(path, onerror):
def _rmtree_unsafe(path, onexc):
try:
with os.scandir(path) as scandir_it:
entries = list(scandir_it)
except OSError:
onerror(os.scandir, path, sys.exc_info())
except OSError as err:
onexc(os.scandir, path, err)
entries = []
for entry in entries:
fullname = entry.path
@ -596,28 +596,28 @@ def _rmtree_unsafe(path, onerror):
# 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 OSError:
onerror(os.path.islink, fullname, sys.exc_info())
except OSError as err:
onexc(os.path.islink, fullname, err)
continue
_rmtree_unsafe(fullname, onerror)
_rmtree_unsafe(fullname, onexc)
else:
try:
os.unlink(fullname)
except OSError:
onerror(os.unlink, fullname, sys.exc_info())
except OSError as err:
onexc(os.unlink, fullname, err)
try:
os.rmdir(path)
except OSError:
onerror(os.rmdir, path, sys.exc_info())
except OSError as err:
onexc(os.rmdir, path, err)
# Version using fd-based APIs to protect against races
def _rmtree_safe_fd(topfd, path, onerror):
def _rmtree_safe_fd(topfd, path, onexc):
try:
with os.scandir(topfd) as scandir_it:
entries = list(scandir_it)
except OSError as err:
err.filename = path
onerror(os.scandir, path, sys.exc_info())
onexc(os.scandir, path, err)
return
for entry in entries:
fullname = os.path.join(path, entry.name)
@ -630,25 +630,25 @@ def _rmtree_safe_fd(topfd, path, onerror):
try:
orig_st = entry.stat(follow_symlinks=False)
is_dir = stat.S_ISDIR(orig_st.st_mode)
except OSError:
onerror(os.lstat, fullname, sys.exc_info())
except OSError as err:
onexc(os.lstat, fullname, err)
continue
if is_dir:
try:
dirfd = os.open(entry.name, os.O_RDONLY, dir_fd=topfd)
dirfd_closed = False
except OSError:
onerror(os.open, fullname, sys.exc_info())
except OSError as err:
onexc(os.open, fullname, err)
else:
try:
if os.path.samestat(orig_st, os.fstat(dirfd)):
_rmtree_safe_fd(dirfd, fullname, onerror)
_rmtree_safe_fd(dirfd, fullname, onexc)
try:
os.close(dirfd)
dirfd_closed = True
os.rmdir(entry.name, dir_fd=topfd)
except OSError:
onerror(os.rmdir, fullname, sys.exc_info())
except OSError as err:
onexc(os.rmdir, fullname, err)
else:
try:
# This can only happen if someone replaces
@ -656,23 +656,23 @@ def _rmtree_safe_fd(topfd, path, onerror):
# os.scandir or stat.S_ISDIR above.
raise OSError("Cannot call rmtree on a symbolic "
"link")
except OSError:
onerror(os.path.islink, fullname, sys.exc_info())
except OSError as err:
onexc(os.path.islink, fullname, err)
finally:
if not dirfd_closed:
os.close(dirfd)
else:
try:
os.unlink(entry.name, dir_fd=topfd)
except OSError:
onerror(os.unlink, fullname, sys.exc_info())
except OSError as err:
onexc(os.unlink, fullname, err)
_use_fd_functions = ({os.open, os.stat, os.unlink, os.rmdir} <=
os.supports_dir_fd and
os.scandir in os.supports_fd and
os.stat in os.supports_follow_symlinks)
def rmtree(path, ignore_errors=False, onerror=None, *, dir_fd=None):
def rmtree(path, ignore_errors=False, onerror=None, *, onexc=None, dir_fd=None):
"""Recursively delete a directory tree.
If dir_fd is not None, it should be a file descriptor open to a directory;
@ -680,21 +680,39 @@ def rmtree(path, ignore_errors=False, onerror=None, *, dir_fd=None):
dir_fd may not be implemented on your platform.
If it is unavailable, using it will raise a NotImplementedError.
If ignore_errors is set, errors are ignored; otherwise, if onerror
is set, it is called to handle the error with arguments (func,
If ignore_errors is set, errors are ignored; otherwise, if onexc or
onerror is set, it is called to handle the error with arguments (func,
path, exc_info) where func is platform and implementation dependent;
path is the argument to that function that caused it to fail; and
exc_info is a tuple returned by sys.exc_info(). If ignore_errors
is false and onerror is None, an exception is raised.
the value of exc_info describes the exception. For onexc it is the
exception instance, and for onerror it is a tuple as returned by
sys.exc_info(). If ignore_errors is false and both onexc and
onerror are None, the exception is reraised.
onerror is deprecated and only remains for backwards compatibility.
If both onerror and onexc are set, onerror is ignored and onexc is used.
"""
sys.audit("shutil.rmtree", path, dir_fd)
if ignore_errors:
def onerror(*args):
def onexc(*args):
pass
elif onerror is None:
def onerror(*args):
elif onerror is None and onexc is None:
def onexc(*args):
raise
elif onexc is None:
if onerror is None:
def onexc(*args):
raise
else:
# delegate to onerror
def onexc(*args):
func, path, exc = args
if exc is None:
exc_info = None, None, None
else:
exc_info = type(exc), exc, exc.__traceback__
return onerror(func, path, exc_info)
if _use_fd_functions:
# While the unsafe rmtree works fine on bytes, the fd based does not.
if isinstance(path, bytes):
@ -703,30 +721,30 @@ def rmtree(path, ignore_errors=False, onerror=None, *, dir_fd=None):
# lstat()/open()/fstat() trick.
try:
orig_st = os.lstat(path, dir_fd=dir_fd)
except Exception:
onerror(os.lstat, path, sys.exc_info())
except Exception as err:
onexc(os.lstat, path, err)
return
try:
fd = os.open(path, os.O_RDONLY, dir_fd=dir_fd)
fd_closed = False
except Exception:
onerror(os.open, path, sys.exc_info())
except Exception as err:
onexc(os.open, path, err)
return
try:
if os.path.samestat(orig_st, os.fstat(fd)):
_rmtree_safe_fd(fd, path, onerror)
_rmtree_safe_fd(fd, path, onexc)
try:
os.close(fd)
fd_closed = True
os.rmdir(path, dir_fd=dir_fd)
except OSError:
onerror(os.rmdir, path, sys.exc_info())
except OSError as err:
onexc(os.rmdir, path, err)
else:
try:
# symlinks to directories are forbidden, see bug #1669
raise OSError("Cannot call rmtree on a symbolic link")
except OSError:
onerror(os.path.islink, path, sys.exc_info())
except OSError as err:
onexc(os.path.islink, path, err)
finally:
if not fd_closed:
os.close(fd)
@ -737,11 +755,11 @@ def rmtree(path, ignore_errors=False, onerror=None, *, dir_fd=None):
if _rmtree_islink(path):
# symlinks to directories are forbidden, see bug #1669
raise OSError("Cannot call rmtree on a symbolic link")
except OSError:
onerror(os.path.islink, path, sys.exc_info())
# can't continue even if onerror hook returns
except OSError as err:
onexc(os.path.islink, path, err)
# can't continue even if onexc hook returns
return
return _rmtree_unsafe(path, onerror)
return _rmtree_unsafe(path, onexc)
# Allow introspection of whether or not the hardening against symlink
# attacks is supported on the current platform

170
Lib/test/test_shutil.py
View file

@ -195,7 +195,7 @@ class TestRmTree(BaseTest, unittest.TestCase):
shutil.rmtree(victim)
@os_helper.skip_unless_symlink
def test_rmtree_fails_on_symlink(self):
def test_rmtree_fails_on_symlink_onerror(self):
tmp = self.mkdtemp()
dir_ = os.path.join(tmp, 'dir')
os.mkdir(dir_)
@ -213,6 +213,25 @@ class TestRmTree(BaseTest, unittest.TestCase):
self.assertEqual(errors[0][1], link)
self.assertIsInstance(errors[0][2][1], OSError)
@os_helper.skip_unless_symlink
def test_rmtree_fails_on_symlink_onexc(self):
tmp = self.mkdtemp()
dir_ = os.path.join(tmp, 'dir')
os.mkdir(dir_)
link = os.path.join(tmp, 'link')
os.symlink(dir_, link)
self.assertRaises(OSError, shutil.rmtree, link)
self.assertTrue(os.path.exists(dir_))
self.assertTrue(os.path.lexists(link))
errors = []
def onexc(*args):
errors.append(args)
shutil.rmtree(link, onexc=onexc)
self.assertEqual(len(errors), 1)
self.assertIs(errors[0][0], os.path.islink)
self.assertEqual(errors[0][1], link)
self.assertIsInstance(errors[0][2], OSError)
@os_helper.skip_unless_symlink
def test_rmtree_works_on_symlinks(self):
tmp = self.mkdtemp()
@ -236,7 +255,7 @@ class TestRmTree(BaseTest, unittest.TestCase):
self.assertTrue(os.path.exists(file1))
@unittest.skipUnless(_winapi, 'only relevant on Windows')
def test_rmtree_fails_on_junctions(self):
def test_rmtree_fails_on_junctions_onerror(self):
tmp = self.mkdtemp()
dir_ = os.path.join(tmp, 'dir')
os.mkdir(dir_)
@ -255,6 +274,26 @@ class TestRmTree(BaseTest, unittest.TestCase):
self.assertEqual(errors[0][1], link)
self.assertIsInstance(errors[0][2][1], OSError)
@unittest.skipUnless(_winapi, 'only relevant on Windows')
def test_rmtree_fails_on_junctions_onexc(self):
tmp = self.mkdtemp()
dir_ = os.path.join(tmp, 'dir')
os.mkdir(dir_)
link = os.path.join(tmp, 'link')
_winapi.CreateJunction(dir_, link)
self.addCleanup(os_helper.unlink, link)
self.assertRaises(OSError, shutil.rmtree, link)
self.assertTrue(os.path.exists(dir_))
self.assertTrue(os.path.lexists(link))
errors = []
def onexc(*args):
errors.append(args)
shutil.rmtree(link, onexc=onexc)
self.assertEqual(len(errors), 1)
self.assertIs(errors[0][0], os.path.islink)
self.assertEqual(errors[0][1], link)
self.assertIsInstance(errors[0][2], OSError)
@unittest.skipUnless(_winapi, 'only relevant on Windows')
def test_rmtree_works_on_junctions(self):
tmp = self.mkdtemp()
@ -277,7 +316,7 @@ class TestRmTree(BaseTest, unittest.TestCase):
self.assertTrue(os.path.exists(dir3))
self.assertTrue(os.path.exists(file1))
def test_rmtree_errors(self):
def test_rmtree_errors_onerror(self):
# filename is guaranteed not to exist
filename = tempfile.mktemp(dir=self.mkdtemp())
self.assertRaises(FileNotFoundError, shutil.rmtree, filename)
@ -309,6 +348,37 @@ class TestRmTree(BaseTest, unittest.TestCase):
self.assertIsInstance(errors[1][2][1], NotADirectoryError)
self.assertEqual(errors[1][2][1].filename, filename)
def test_rmtree_errors_onexc(self):
# filename is guaranteed not to exist
filename = tempfile.mktemp(dir=self.mkdtemp())
self.assertRaises(FileNotFoundError, shutil.rmtree, filename)
# test that ignore_errors option is honored
shutil.rmtree(filename, ignore_errors=True)
# existing file
tmpdir = self.mkdtemp()
write_file((tmpdir, "tstfile"), "")
filename = os.path.join(tmpdir, "tstfile")
with self.assertRaises(NotADirectoryError) as cm:
shutil.rmtree(filename)
self.assertEqual(cm.exception.filename, filename)
self.assertTrue(os.path.exists(filename))
# test that ignore_errors option is honored
shutil.rmtree(filename, ignore_errors=True)
self.assertTrue(os.path.exists(filename))
errors = []
def onexc(*args):
errors.append(args)
shutil.rmtree(filename, onexc=onexc)
self.assertEqual(len(errors), 2)
self.assertIs(errors[0][0], os.scandir)
self.assertEqual(errors[0][1], filename)
self.assertIsInstance(errors[0][2], NotADirectoryError)
self.assertEqual(errors[0][2].filename, filename)
self.assertIs(errors[1][0], os.rmdir)
self.assertEqual(errors[1][1], filename)
self.assertIsInstance(errors[1][2], NotADirectoryError)
self.assertEqual(errors[1][2].filename, filename)
@unittest.skipIf(sys.platform[:6] == 'cygwin',
"This test can't be run on Cygwin (issue #1071513).")
@ -368,6 +438,100 @@ class TestRmTree(BaseTest, unittest.TestCase):
self.assertTrue(issubclass(exc[0], OSError))
self.errorState = 3
@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_on_exc(self):
self.errorState = 0
os.mkdir(TESTFN)
self.addCleanup(shutil.rmtree, TESTFN)
self.child_file_path = os.path.join(TESTFN, 'a')
self.child_dir_path = os.path.join(TESTFN, 'b')
os_helper.create_empty_file(self.child_file_path)
os.mkdir(self.child_dir_path)
old_dir_mode = os.stat(TESTFN).st_mode
old_child_file_mode = os.stat(self.child_file_path).st_mode
old_child_dir_mode = os.stat(self.child_dir_path).st_mode
# Make unwritable.
new_mode = stat.S_IREAD|stat.S_IEXEC
os.chmod(self.child_file_path, new_mode)
os.chmod(self.child_dir_path, new_mode)
os.chmod(TESTFN, new_mode)
self.addCleanup(os.chmod, TESTFN, old_dir_mode)
self.addCleanup(os.chmod, self.child_file_path, old_child_file_mode)
self.addCleanup(os.chmod, self.child_dir_path, old_child_dir_mode)
shutil.rmtree(TESTFN, onexc=self.check_args_to_onexc)
# Test whether onexc has actually been called.
self.assertEqual(self.errorState, 3,
"Expected call to onexc function did not happen.")
def check_args_to_onexc(self, func, arg, exc):
# test_rmtree_errors deliberately runs rmtree
# on a directory that is chmod 500, which will fail.
# This function is run when shutil.rmtree fails.
# 99.9% of the time it initially fails to remove
# a file in the directory, so the first time through
# func is os.remove.
# However, some Linux machines running ZFS on
# FUSE experienced a failure earlier in the process
# at os.listdir. The first failure may legally
# be either.
if self.errorState < 2:
if func is os.unlink:
self.assertEqual(arg, self.child_file_path)
elif func is os.rmdir:
self.assertEqual(arg, self.child_dir_path)
else:
self.assertIs(func, os.listdir)
self.assertIn(arg, [TESTFN, self.child_dir_path])
self.assertTrue(isinstance(exc, OSError))
self.errorState += 1
else:
self.assertEqual(func, os.rmdir)
self.assertEqual(arg, TESTFN)
self.assertTrue(isinstance(exc, OSError))
self.errorState = 3
def test_both_onerror_and_onexc(self):
onerror_called = False
onexc_called = False
def onerror(*args):
nonlocal onerror_called
onerror_called = True
def onexc(*args):
nonlocal onexc_called
onexc_called = True
os.mkdir(TESTFN)
self.addCleanup(shutil.rmtree, TESTFN)
self.child_file_path = os.path.join(TESTFN, 'a')
self.child_dir_path = os.path.join(TESTFN, 'b')
os_helper.create_empty_file(self.child_file_path)
os.mkdir(self.child_dir_path)
old_dir_mode = os.stat(TESTFN).st_mode
old_child_file_mode = os.stat(self.child_file_path).st_mode
old_child_dir_mode = os.stat(self.child_dir_path).st_mode
# Make unwritable.
new_mode = stat.S_IREAD|stat.S_IEXEC
os.chmod(self.child_file_path, new_mode)
os.chmod(self.child_dir_path, new_mode)
os.chmod(TESTFN, new_mode)
self.addCleanup(os.chmod, TESTFN, old_dir_mode)
self.addCleanup(os.chmod, self.child_file_path, old_child_file_mode)
self.addCleanup(os.chmod, self.child_dir_path, old_child_dir_mode)
shutil.rmtree(TESTFN, onerror=onerror, onexc=onexc)
self.assertTrue(onexc_called)
self.assertFalse(onerror_called)
def test_rmtree_does_not_choke_on_failing_lstat(self):
try:
orig_lstat = os.lstat

3
Misc/NEWS.d/next/Library/2023-03-19-15-30-59.gh-issue-102828.NKClXg.rst Normal file
View file

@ -0,0 +1,3 @@
Add the ``onexc`` arg to :func:`shutil.rmtree`, which is like ``onerror``
but expects an exception instance rather than an exc_info tuple. Deprecate
``onerror``.