Skip to content

Commit

Permalink
bpo-43153: Don't mask PermissionError with NotADirectoryError dur…
Browse files Browse the repository at this point in the history
…ing tempdirectory cleanup (pythonGH-29940)

(cherry picked from commit 8cdfee1)

Co-authored-by: Ken Jin <kenjin@python.org>
Co-authored-by: andrei kulakov <andrei.avk@gmail.com>
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>
  • Loading branch information
3 people authored and miss-islington committed Dec 5, 2023
1 parent 80c314c commit 89e3368
Show file tree
Hide file tree
Showing 3 changed files with 41 additions and 2 deletions.
28 changes: 26 additions & 2 deletions Lib/tempfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -876,8 +877,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
Expand Down
11 changes: 11 additions & 0 deletions Lib/test/test_tempfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -1533,6 +1533,17 @@ def test_explict_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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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.

0 comments on commit 89e3368

Please sign in to comment.