Skip to content

Commit e1f3590

Browse files
authored
[3.12] GH-157579: Revert GH-158489 (prematurely merged) (#158528)
Revert "[3.12] gh-157579: Fix race condition in the cleanup of tempfile.TemporaryDirectory (GH-157580) (#158489)" This reverts commit 458e713.
1 parent 458e713 commit e1f3590

8 files changed

Lines changed: 18 additions & 240 deletions

File tree

‎Doc/library/tempfile.rst‎

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -207,15 +207,6 @@ The module defines the following user-callable items:
207207
debugging or when you need your cleanup behavior to be conditional based on
208208
other logic.
209209

210-
.. warning::
211-
212-
Cleanup is not robust against the tree being modified while it is removed.
213-
Files outside of the tree may have their permissions and file flags reset.
214-
215-
On systems where :data:`shutil.rmtree.avoids_symlink_attacks` is
216-
false, manipulating symbolic links during cleanup
217-
may cause files outside of the tree to be removed.
218-
219210
.. audit-event:: tempfile.mkdtemp fullpath tempfile.TemporaryDirectory
220211

221212
.. versionadded:: 3.2

‎Lib/os.py‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,6 @@ def _add(str, fn):
110110
_add("HAVE_FCHMODAT", "chmod")
111111
_add("HAVE_FCHOWNAT", "chown")
112112
_add("HAVE_FSTATAT", "stat")
113-
_add("HAVE_LSTAT", "lstat")
114113
_add("HAVE_FUTIMESAT", "utime")
115114
_add("HAVE_LINKAT", "link")
116115
_add("HAVE_MKDIRAT", "mkdir")

‎Lib/shutil.py‎

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -654,7 +654,6 @@ def _rmtree_safe_fd(stack, onexc):
654654
# save a call to os.lstat() when walking subdirectories.
655655
func, dirfd, path, orig_entry = stack.pop()
656656
name = path if orig_entry is None else orig_entry.name
657-
parent_fd = None if func is os.close else dirfd
658657
try:
659658
if func is os.close:
660659
os.close(dirfd)
@@ -698,18 +697,17 @@ def _rmtree_safe_fd(stack, onexc):
698697
try:
699698
os.unlink(entry.name, dir_fd=topfd)
700699
except OSError as err:
701-
onexc(os.unlink, fullname, err, direntry=entry, dir_fd=topfd)
700+
onexc(os.unlink, fullname, err)
702701
except OSError as err:
703702
err.filename = path
704-
onexc(func, path, err, direntry=orig_entry, dir_fd=parent_fd)
703+
onexc(func, path, err)
705704

706705
_use_fd_functions = ({os.open, os.stat, os.unlink, os.rmdir} <=
707706
os.supports_dir_fd and
708707
os.scandir in os.supports_fd and
709708
os.stat in os.supports_follow_symlinks)
710709

711-
def rmtree(path, ignore_errors=False, onerror=None, *, onexc=None, dir_fd=None,
712-
_onexc_kwargs=False):
710+
def rmtree(path, ignore_errors=False, onerror=None, *, onexc=None, dir_fd=None):
713711
"""Recursively delete a directory tree.
714712
715713
If dir_fd is not None, it should be a file descriptor open to a directory;
@@ -732,29 +730,24 @@ def rmtree(path, ignore_errors=False, onerror=None, *, onexc=None, dir_fd=None,
732730

733731
sys.audit("shutil.rmtree", path, dir_fd)
734732
if ignore_errors:
735-
def onexc(*args, **kwargs):
733+
def onexc(*args):
736734
pass
737735
elif onerror is None and onexc is None:
738-
def onexc(*args, **kwargs):
736+
def onexc(*args):
739737
raise
740738
elif onexc is None:
741739
if onerror is None:
742-
def onexc(*args, **kwargs):
740+
def onexc(*args):
743741
raise
744742
else:
745743
# delegate to onerror
746-
def onexc(*args, **kwargs):
744+
def onexc(*args):
747745
func, path, exc = args
748746
if exc is None:
749747
exc_info = None, None, None
750748
else:
751749
exc_info = type(exc), exc, exc.__traceback__
752750
return onerror(func, path, exc_info)
753-
elif not _onexc_kwargs:
754-
# Only the internal caller in tempfile asks for the extra arguments.
755-
_onexc = onexc
756-
def onexc(func, path, err, **kwargs):
757-
return _onexc(func, path, err)
758751

759752
if _use_fd_functions:
760753
# While the unsafe rmtree works fine on bytes, the fd based does not.

‎Lib/tempfile.py‎

Lines changed: 11 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@
4343
import shutil as _shutil
4444
import errno as _errno
4545
from random import Random as _Random
46-
import stat as _stat
4746
import sys as _sys
4847
import types as _types
4948
import weakref as _weakref
@@ -277,68 +276,15 @@ def _dont_follow_symlinks(func, path, *args):
277276
elif _os.name == 'nt' or not _os.path.islink(path):
278277
func(path, *args)
279278

280-
def _resetflags(path):
279+
def _resetperms(path):
281280
try:
282281
chflags = _os.chflags
283282
except AttributeError:
284283
pass
285284
else:
286285
_dont_follow_symlinks(chflags, path, 0)
287-
288-
def _resetperms(path):
289-
_resetflags(path)
290286
_dont_follow_symlinks(_os.chmod, path, 0o700)
291287

292-
# True if TemporaryDirectory._rmtree() can work relative to open directories
293-
# instead of resolving paths again.
294-
_rmtree_use_dir_fd = (
295-
{_os.chmod, _os.unlink, _os.lstat} <= _os.supports_dir_fd
296-
and _os.chmod in _os.supports_fd
297-
)
298-
299-
def _resetperms_fd(dir_fd, path):
300-
# Same as _resetperms(), but for the directory referred to by dir_fd.
301-
if dir_fd is None:
302-
_resetperms(path)
303-
return
304-
_resetflags(path)
305-
_os.chmod(dir_fd, 0o700)
306-
307-
try:
308-
_nofollow_mode = _os.O_RDONLY | _os.O_NONBLOCK | _os.O_NOFOLLOW
309-
except AttributeError:
310-
_nofollow_mode = None
311-
312-
def _resetperms_at(name, dir_fd, path):
313-
# Same as _resetperms(), but name is resolved relative to the directory
314-
# file descriptor dir_fd. path is only used for os.chflags(), which
315-
# doesn't support dir_fd or file descriptors.
316-
if dir_fd is None:
317-
_resetperms(path)
318-
return
319-
_resetflags(path)
320-
if _os.chmod in _os.supports_follow_symlinks:
321-
_os.chmod(name, 0o700, dir_fd=dir_fd, follow_symlinks=False)
322-
else:
323-
# dir_fd & follow_symlinks is not supported on this platform.
324-
# Try chmod opening the file with O_NOFOLLOW.
325-
if _nofollow_mode is not None:
326-
try:
327-
fd = _os.open(name, _nofollow_mode, dir_fd=dir_fd)
328-
except OSError:
329-
pass
330-
else:
331-
try:
332-
_os.chmod(fd, 0o700)
333-
finally:
334-
_os.close(fd)
335-
return
336-
# If that did not work, we change by name, which is subject to a race
337-
# condition.
338-
stat = _os.lstat(name, dir_fd=dir_fd)
339-
if not _stat.S_ISLNK(stat.st_mode):
340-
_os.chmod(name, 0o700, dir_fd=dir_fd)
341-
342288

343289
# User visible interfaces.
344290

@@ -946,42 +892,23 @@ def __init__(self, suffix=None, prefix=None, dir=None,
946892
ignore_errors=self._ignore_cleanup_errors, delete=self._delete)
947893

948894
@classmethod
949-
def _rmtree(cls, name, ignore_errors=False, repeated=False, dir_fd=None,
950-
fullname=None):
951-
if fullname is None:
952-
fullname = name
953-
954-
def onexc(func, path, exc, direntry=None, dir_fd=None):
895+
def _rmtree(cls, name, ignore_errors=False, repeated=False):
896+
def onexc(func, path, exc):
955897
if isinstance(exc, PermissionError):
956898
if repeated and path == name:
957899
if ignore_errors:
958900
return
959901
raise
960902

961-
# fullpath is path as seen from the working directory
962-
fullpath = fullname + path[len(name):]
963-
# base is path relative to dir_fd, the directory rmtree()
964-
# reached it through, or the whole path when there is none
965-
if dir_fd is None or not _rmtree_use_dir_fd:
966-
base, dir_fd = path, None
967-
elif direntry is None:
968-
base = path
969-
else:
970-
base = direntry.name
971-
972903
try:
973904
if path != name:
974-
# The parent directory of path is the one referred to
975-
# by dir_fd.
976-
_resetperms_fd(dir_fd, _os.path.dirname(fullpath))
977-
_resetperms_at(base, dir_fd, fullpath)
905+
_resetperms(_os.path.dirname(path))
906+
_resetperms(path)
978907

979908
try:
980-
_os.unlink(base, dir_fd=dir_fd)
909+
_os.unlink(path)
981910
except IsADirectoryError:
982-
cls._rmtree(base, ignore_errors=ignore_errors,
983-
repeated=(path == name),
984-
dir_fd=dir_fd, fullname=fullpath)
911+
cls._rmtree(path, ignore_errors=ignore_errors)
985912
except PermissionError:
986913
# The PermissionError handler was originally added for
987914
# FreeBSD in directories, but it seems that it is raised
@@ -990,27 +917,21 @@ def onexc(func, path, exc, direntry=None, dir_fd=None):
990917
# raise NotADirectoryError and mask the PermissionError.
991918
# So we must re-raise the current PermissionError if
992919
# path is not a directory.
993-
if (not _os.path.isdir(fullpath)
994-
or _os.path.isjunction(fullpath)):
920+
if not _os.path.isdir(path) or _os.path.isjunction(path):
995921
if ignore_errors:
996922
return
997923
raise
998-
cls._rmtree(base, ignore_errors=ignore_errors,
999-
repeated=(path == name),
1000-
dir_fd=dir_fd, fullname=fullpath)
924+
cls._rmtree(path, ignore_errors=ignore_errors,
925+
repeated=(path == name))
1001926
except FileNotFoundError:
1002927
pass
1003-
except OSError:
1004-
if ignore_errors:
1005-
return
1006-
raise
1007928
elif isinstance(exc, FileNotFoundError):
1008929
pass
1009930
else:
1010931
if not ignore_errors:
1011932
raise
1012933

1013-
_shutil.rmtree(name, onexc=onexc, dir_fd=dir_fd, _onexc_kwargs=True)
934+
_shutil.rmtree(name, onexc=onexc)
1014935

1015936
@classmethod
1016937
def _cleanup(cls, name, warn_message, ignore_errors=False, delete=True):

‎Lib/test/support/__init__.py‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2598,8 +2598,3 @@ def control_characters_c0() -> list[str]:
25982598
C0 control characters defined as the byte range 0x00-0x1F, and 0x7F.
25992599
"""
26002600
return [chr(c) for c in range(0x00, 0x20)] + ["\x7F"]
2601-
2602-
2603-
_ROOT_IN_POSIX = hasattr(os, 'geteuid') and os.geteuid() == 0
2604-
requires_root_user = unittest.skipUnless(_ROOT_IN_POSIX, "test needs root privilege")
2605-
requires_non_root_user = unittest.skipIf(_ROOT_IN_POSIX, "test needs non-root account")

‎Lib/test/test_shutil.py‎

Lines changed: 0 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -497,37 +497,6 @@ def check_args_to_onexc(self, func, arg, exc):
497497
self.assertTrue(isinstance(exc, OSError))
498498
self.errorState = 3
499499

500-
@os_helper.skip_if_dac_override
501-
@os_helper.skip_unless_working_chmod
502-
@unittest.skipUnless(shutil.rmtree.avoids_symlink_attacks,
503-
'requires the fd based implementation of rmtree()')
504-
def test_on_exc_kwargs(self):
505-
os.mkdir(TESTFN)
506-
self.addCleanup(shutil.rmtree, TESTFN)
507-
508-
child_dir_path = os.path.join(TESTFN, 'b')
509-
child_file_path = os.path.join(child_dir_path, 'a')
510-
os.mkdir(child_dir_path)
511-
os_helper.create_empty_file(child_file_path)
512-
old_child_dir_mode = os.stat(child_dir_path).st_mode
513-
# Make unwritable.
514-
new_mode = stat.S_IREAD|stat.S_IEXEC
515-
os.chmod(child_dir_path, new_mode)
516-
517-
self.addCleanup(os.chmod, child_dir_path, old_child_dir_mode)
518-
519-
calls = []
520-
def onexc(func, path, err, direntry=None, dir_fd=None):
521-
calls.append((func, path, err))
522-
if func is os.unlink:
523-
self.assertEqual(direntry.name, os.path.basename(path))
524-
self.assertTrue(os.path.samestat(
525-
os.stat(path), os.stat(direntry.name, dir_fd=dir_fd)))
526-
527-
shutil.rmtree(TESTFN, onexc=onexc, _onexc_kwargs=True)
528-
self.assertIn((os.unlink, child_file_path),
529-
[(func, path) for func, path, err in calls])
530-
531500
@unittest.skipIf(sys.platform[:6] == 'cygwin',
532501
"This test can't be run on Cygwin (issue #1071513).")
533502
@os_helper.skip_if_dac_override

0 commit comments

Comments
 (0)