diff --git a/Lib/tempfile.py b/Lib/tempfile.py index 2bc61662459992..92263cb485cc51 100644 --- a/Lib/tempfile.py +++ b/Lib/tempfile.py @@ -57,10 +57,14 @@ if hasattr(_os, 'O_BINARY'): _bin_openflags |= _os.O_BINARY -if hasattr(_os, 'TMP_MAX'): - TMP_MAX = _os.TMP_MAX -else: - TMP_MAX = 10000 +# This is more than enough. +# Each name contains more than 40 random bits. Even with a million of +# temporary files, a chance of conflict is less than 1 per million, +# and with 20 attempts it is less than 1e-120. +# Each name contains over 40 random bits. Even with a million temporary +# files, the chance of a conflict is less than 1 in a million, and with +# 20 attempts, it is less than 1e-120. +TMP_MAX = 20 # This variable _was_ unused for legacy reasons, see issue 10354. # But as of 3.5 we actually use it at runtime so changing it would @@ -196,8 +200,7 @@ def _get_default_tempdir(dirlist=None): for dir in dirlist: if dir != _os.curdir: dir = _os.path.abspath(dir) - # Try only a few names per directory. - for seq in range(100): + for seq in range(TMP_MAX): name = next(namer) filename = _os.path.join(dir, name) try: @@ -213,10 +216,14 @@ def _get_default_tempdir(dirlist=None): except FileExistsError: pass except PermissionError: - # This exception is thrown when a directory with the chosen name - # already exists on windows. - if (_os.name == 'nt' and _os.path.isdir(dir) and - _os.access(dir, _os.W_OK)): + # On Posix, this exception is raised when the user has no + # write access to the parent directory. + # On Windows, it is also raised when a directory with + # the chosen name already exists, or if the parent directory + # is not a directory. + # We cannot distinguish between "directory-exists-error" and + # "access-denied-error". + if _os.name == 'nt' and _os.path.isdir(dir): continue break # no point trying more names in this directory except OSError: @@ -258,10 +265,14 @@ def _mkstemp_inner(dir, pre, suf, flags, output_type): except FileExistsError: continue # try again except PermissionError: - # This exception is thrown when a directory with the chosen name - # already exists on windows. - if (_os.name == 'nt' and _os.path.isdir(dir) and - _os.access(dir, _os.W_OK)): + # On Posix, this exception is raised when the user has no + # write access to the parent directory. + # On Windows, it is also raised when a directory with + # the chosen name already exists, or if the parent directory + # is not a directory. + # We cannot distinguish between "directory-exists-error" and + # "access-denied-error". + if _os.name == 'nt' and _os.path.isdir(dir) and seq < TMP_MAX - 1: continue else: raise @@ -386,10 +397,14 @@ def mkdtemp(suffix=None, prefix=None, dir=None): except FileExistsError: continue # try again except PermissionError: - # This exception is thrown when a directory with the chosen name - # already exists on windows. - if (_os.name == 'nt' and _os.path.isdir(dir) and - _os.access(dir, _os.W_OK)): + # On Posix, this exception is raised when the user has no + # write access to the parent directory. + # On Windows, it is also raised when a directory with + # the chosen name already exists, or if the parent directory + # is not a directory. + # We cannot distinguish between "directory-exists-error" and + # "access-denied-error". + if _os.name == 'nt' and _os.path.isdir(dir) and seq < TMP_MAX - 1: continue else: raise diff --git a/Lib/test/test_tempfile.py b/Lib/test/test_tempfile.py index 7eec34f2f294ad..b2b5390af33b00 100644 --- a/Lib/test/test_tempfile.py +++ b/Lib/test/test_tempfile.py @@ -330,17 +330,36 @@ def _mock_candidate_names(*names): class TestBadTempdir: def test_read_only_directory(self): with _inside_empty_temp_dir(): - oldmode = mode = os.stat(tempfile.tempdir).st_mode - mode &= ~(stat.S_IWUSR | stat.S_IWGRP | stat.S_IWOTH) - os.chmod(tempfile.tempdir, mode) + probe = os.path.join(tempfile.tempdir, 'probe') + if os.name == 'nt': + cmd = ['icacls', tempfile.tempdir, '/deny', 'Everyone:(W)'] + stdout = None if support.verbose > 1 else subprocess.DEVNULL + subprocess.run(cmd, check=True, stdout=stdout) + else: + oldmode = mode = os.stat(tempfile.tempdir).st_mode + mode &= ~(stat.S_IWUSR | stat.S_IWGRP | stat.S_IWOTH) + mode = stat.S_IREAD + os.chmod(tempfile.tempdir, mode) try: - if os.access(tempfile.tempdir, os.W_OK): + # Check that the directory is read-only. + try: + os.mkdir(probe) + except PermissionError: + pass + else: + os.rmdir(probe) self.skipTest("can't set the directory read-only") + # gh-66305: Now it takes a split second, but previously + # it took about 10 days on Windows. with self.assertRaises(PermissionError): self.make_temp() - self.assertEqual(os.listdir(tempfile.tempdir), []) finally: - os.chmod(tempfile.tempdir, oldmode) + if os.name == 'nt': + cmd = ['icacls', tempfile.tempdir, '/grant:r', 'Everyone:(M)'] + subprocess.run(cmd, check=True, stdout=stdout) + else: + os.chmod(tempfile.tempdir, oldmode) + self.assertEqual(os.listdir(tempfile.tempdir), []) def test_nonexisting_directory(self): with _inside_empty_temp_dir(): diff --git a/Misc/NEWS.d/next/Library/2026-02-10-16-56-05.gh-issue-66305.PZ6GN8.rst b/Misc/NEWS.d/next/Library/2026-02-10-16-56-05.gh-issue-66305.PZ6GN8.rst new file mode 100644 index 00000000000000..276711e6ba3900 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-02-10-16-56-05.gh-issue-66305.PZ6GN8.rst @@ -0,0 +1,3 @@ +Fixed a hang on Windows in the :mod:`tempfile` module when +trying to create a temporary file or subdirectory in a non-writable +directory.