From 5d6b7b1cb7943255b8682ea3663ce2c0da500e96 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Wed, 20 May 2015 00:11:48 +0300 Subject: [PATCH] Issue #22107: tempfile.gettempdir() and tempfile.mkdtemp() now try again when a directory with the chosen name already exists on Windows as well as on Unix. tempfile.mkstemp() now fails early if parent directory is not valid (not exists or is a file) on Windows. --- Lib/tempfile.py | 18 ++++++++++++++- Lib/test/test_tempfile.py | 47 ++++++++++++++++++++++++++++++++++----- Misc/NEWS | 5 +++++ 3 files changed, 63 insertions(+), 7 deletions(-) diff --git a/Lib/tempfile.py b/Lib/tempfile.py index a2764d3e314..0537228ba51 100644 --- a/Lib/tempfile.py +++ b/Lib/tempfile.py @@ -166,6 +166,13 @@ def _get_default_tempdir(): return dir 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)): + continue + break # no point trying more names in this directory except OSError: break # no point trying more names in this directory raise FileNotFoundError(_errno.ENOENT, @@ -204,7 +211,8 @@ def _mkstemp_inner(dir, pre, suf, flags): except PermissionError: # This exception is thrown when a directory with the chosen name # already exists on windows. - if _os.name == 'nt': + if (_os.name == 'nt' and _os.path.isdir(dir) and + _os.access(dir, _os.W_OK)): continue else: raise @@ -296,6 +304,14 @@ def mkdtemp(suffix="", prefix=template, dir=None): return file 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)): + continue + else: + raise raise FileExistsError(_errno.EEXIST, "No usable temporary directory name found") diff --git a/Lib/test/test_tempfile.py b/Lib/test/test_tempfile.py index 576cf4d4508..66412980f6f 100644 --- a/Lib/test/test_tempfile.py +++ b/Lib/test/test_tempfile.py @@ -274,7 +274,39 @@ def _mock_candidate_names(*names): lambda: iter(names)) -class TestMkstempInner(BaseTestCase): +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) + try: + if os.access(tempfile.tempdir, os.W_OK): + self.skipTest("can't set the directory read-only") + with self.assertRaises(PermissionError): + self.make_temp() + self.assertEqual(os.listdir(tempfile.tempdir), []) + finally: + os.chmod(tempfile.tempdir, oldmode) + + def test_nonexisting_directory(self): + with _inside_empty_temp_dir(): + tempdir = os.path.join(tempfile.tempdir, 'nonexistent') + with support.swap_attr(tempfile, 'tempdir', tempdir): + with self.assertRaises(FileNotFoundError): + self.make_temp() + + def test_non_directory(self): + with _inside_empty_temp_dir(): + tempdir = os.path.join(tempfile.tempdir, 'file') + open(tempdir, 'wb').close() + with support.swap_attr(tempfile, 'tempdir', tempdir): + with self.assertRaises((NotADirectoryError, FileNotFoundError)): + self.make_temp() + + +class TestMkstempInner(TestBadTempdir, BaseTestCase): """Test the internal function _mkstemp_inner.""" class mkstemped: @@ -389,7 +421,7 @@ class TestMkstempInner(BaseTestCase): os.lseek(f.fd, 0, os.SEEK_SET) self.assertEqual(os.read(f.fd, 20), b"blat") - def default_mkstemp_inner(self): + def make_temp(self): return tempfile._mkstemp_inner(tempfile.gettempdir(), tempfile.template, '', @@ -400,11 +432,11 @@ class TestMkstempInner(BaseTestCase): # the chosen name already exists with _inside_empty_temp_dir(), \ _mock_candidate_names('aaa', 'aaa', 'bbb'): - (fd1, name1) = self.default_mkstemp_inner() + (fd1, name1) = self.make_temp() os.close(fd1) self.assertTrue(name1.endswith('aaa')) - (fd2, name2) = self.default_mkstemp_inner() + (fd2, name2) = self.make_temp() os.close(fd2) self.assertTrue(name2.endswith('bbb')) @@ -416,7 +448,7 @@ class TestMkstempInner(BaseTestCase): dir = tempfile.mkdtemp() self.assertTrue(dir.endswith('aaa')) - (fd, name) = self.default_mkstemp_inner() + (fd, name) = self.make_temp() os.close(fd) self.assertTrue(name.endswith('bbb')) @@ -528,9 +560,12 @@ class TestMkstemp(BaseTestCase): os.rmdir(dir) -class TestMkdtemp(BaseTestCase): +class TestMkdtemp(TestBadTempdir, BaseTestCase): """Test mkdtemp().""" + def make_temp(self): + return tempfile.mkdtemp() + def do_create(self, dir=None, pre="", suf=""): if dir is None: dir = tempfile.gettempdir() diff --git a/Misc/NEWS b/Misc/NEWS index 45a06397943..94ea12bff2b 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -53,6 +53,11 @@ Core and Builtins Library ------- +- Issue #22107: tempfile.gettempdir() and tempfile.mkdtemp() now try again + when a directory with the chosen name already exists on Windows as well as + on Unix. tempfile.mkstemp() now fails early if parent directory is not + valid (not exists or is a file) on Windows. + - Issue #6598: Increased time precision and random number range in email.utils.make_msgid() to strengthen the uniqueness of the message ID.