From 5acfb8258afa1ecb445f699585ffbaf7529a640d Mon Sep 17 00:00:00 2001 From: "Miss Islington (bot)" <31488909+miss-islington@users.noreply.github.com> Date: Tue, 5 Dec 2023 14:58:36 +0100 Subject: [PATCH] [3.12] bpo-43153: Don't mask `PermissionError` with `NotADirectoryError` during tempdirectory cleanup (GH-29940) (GH-112753) (cherry picked from commit 8cdfee1bb902fd1e38d79170b751ef13a0907262) Co-authored-by: Ken Jin Co-authored-by: andrei kulakov Co-authored-by: Serhiy Storchaka --- Lib/tempfile.py | 28 +++++++++++++++++-- Lib/test/test_tempfile.py | 11 ++++++++ .../2021-12-06-22-10-53.bpo-43153.J7mjSy.rst | 4 +++ 3 files changed, 41 insertions(+), 2 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2021-12-06-22-10-53.bpo-43153.J7mjSy.rst diff --git a/Lib/tempfile.py b/Lib/tempfile.py index 2b4f4313247128..55403ad1faf46d 100644 --- a/Lib/tempfile.py +++ b/Lib/tempfile.py @@ -41,6 +41,7 @@ import io as _io import os as _os import shutil as _shutil +import stat as _stat import errno as _errno from random import Random as _Random import sys as _sys @@ -889,8 +890,31 @@ def resetperms(path): try: _os.unlink(path) - # PermissionError is raised on FreeBSD for directories - except (IsADirectoryError, PermissionError): + except IsADirectoryError: + cls._rmtree(path, ignore_errors=ignore_errors) + except PermissionError: + # The PermissionError handler was originally added for + # FreeBSD in directories, but it seems that it is raised + # on Windows too. + # bpo-43153: Calling _rmtree again may + # raise NotADirectoryError and mask the PermissionError. + # So we must re-raise the current PermissionError if + # path is not a directory. + try: + st = _os.lstat(path) + except OSError: + if ignore_errors: + return + raise + if (_stat.S_ISLNK(st.st_mode) or + not _stat.S_ISDIR(st.st_mode) or + (hasattr(st, 'st_file_attributes') and + st.st_file_attributes & _stat.FILE_ATTRIBUTE_REPARSE_POINT and + st.st_reparse_tag == _stat.IO_REPARSE_TAG_MOUNT_POINT) + ): + if ignore_errors: + return + raise cls._rmtree(path, ignore_errors=ignore_errors) except FileNotFoundError: pass diff --git a/Lib/test/test_tempfile.py b/Lib/test/test_tempfile.py index 1673507e2f7c91..f4aef887799ed4 100644 --- a/Lib/test/test_tempfile.py +++ b/Lib/test/test_tempfile.py @@ -1641,6 +1641,17 @@ def test_explicit_cleanup_ignore_errors(self): temp_path.exists(), f"TemporaryDirectory {temp_path!s} exists after cleanup") + @unittest.skipUnless(os.name == "nt", "Only on Windows.") + def test_explicit_cleanup_correct_error(self): + with tempfile.TemporaryDirectory() as working_dir: + temp_dir = self.do_create(dir=working_dir) + with open(os.path.join(temp_dir.name, "example.txt"), 'wb'): + # Previously raised NotADirectoryError on some OSes + # (e.g. Windows). See bpo-43153. + with self.assertRaises(PermissionError): + temp_dir.cleanup() + + @os_helper.skip_unless_symlink def test_cleanup_with_symlink_to_a_directory(self): # cleanup() should not follow symlinks to directories (issue #12464) diff --git a/Misc/NEWS.d/next/Library/2021-12-06-22-10-53.bpo-43153.J7mjSy.rst b/Misc/NEWS.d/next/Library/2021-12-06-22-10-53.bpo-43153.J7mjSy.rst new file mode 100644 index 00000000000000..7800e0a4869adf --- /dev/null +++ b/Misc/NEWS.d/next/Library/2021-12-06-22-10-53.bpo-43153.J7mjSy.rst @@ -0,0 +1,4 @@ +On Windows, ``tempfile.TemporaryDirectory`` previously masked a +``PermissionError`` with ``NotADirectoryError`` during directory cleanup. It +now correctly raises ``PermissionError`` if errors are not ignored. Patch by +Andrei Kulakov and Ken Jin.