Visitar URL original
gh-102828: add onexc arg to shutil.rmtree. Deprecate onerror. by iritkatriel · Pull Request #102829 · python/cpython · GitHub
Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
iritkatriel merged 4 commits into from
Mar 19, 2023
Merged

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

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
86a1cea
gh-102828: add onexc arg to shutil.rmtree. Deprecate onerror.
iritkatriel Mar 19, 2023
03fa88f
fix typo
iritkatriel Mar 19, 2023
7960670
fix typo
iritkatriel Mar 19, 2023
f5a3aa7
e --> err
iritkatriel Mar 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 15 additions & 9 deletions Doc/library/shutil.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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>`.
Expand All @@ -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
paramter it receives is the tuple returned from :func:`sys.exc_info`.
Comment thread
iritkatriel marked this conversation as resolved.
Outdated

.. audit-event:: shutil.rmtree path,dir_fd shutil.rmtree

Expand All @@ -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
Expand Down Expand Up @@ -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. ::

Expand All @@ -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:

Expand Down
9 changes: 9 additions & 0 deletions Doc/whatsnew/3.12.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
-------
Expand Down Expand Up @@ -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
------------------------------

Expand Down
106 changes: 62 additions & 44 deletions Lib/shutil.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -575,12 +575,12 @@ def _rmtree_islink(path):
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 e:
Comment thread
iritkatriel marked this conversation as resolved.
Outdated
onexc(os.scandir, path, e)
entries = []
for entry in entries:
fullname = entry.path
Expand All @@ -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 e:
onexc(os.path.islink, fullname, e)
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 e:
onexc(os.unlink, fullname, e)
try:
os.rmdir(path)
except OSError:
onerror(os.rmdir, path, sys.exc_info())
except OSError as e:
onexc(os.rmdir, path, e)

# 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)
Expand All @@ -630,71 +630,89 @@ 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 e:
onexc(os.lstat, fullname, e)
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 e:
onexc(os.open, fullname, e)
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 e:
onexc(os.rmdir, fullname, e)
else:
try:
# This can only happen if someone replaces
# a directory with a symlink after the call to
# 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 e:
onexc(os.path.islink, fullname, e)
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 e:
onexc(os.unlink, fullname, e)

_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;
path will then be relative to that directory.
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:
Comment thread
iritkatriel marked this conversation as resolved.
# 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):
Expand All @@ -703,30 +721,30 @@ def onerror(*args):
# 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 e:
onexc(os.lstat, path, e)
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 e:
onexc(os.open, path, e)
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 e:
onexc(os.rmdir, path, e)
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 e:
onexc(os.path.islink, path, e)
finally:
if not fd_closed:
os.close(fd)
Expand All @@ -737,11 +755,11 @@ def onerror(*args):
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 e:
onexc(os.path.islink, path, e)
# 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
Expand Down
Loading