[Python-checkins] bpo-40701: tempfile mixes str and bytes in an inconsistent manner (GH-20442)
gpshead
webhook-mailer at python.org
Wed Mar 3 15:36:31 EST 2021
https://github.com/python/cpython/commit/9c7927400cd8f1d283bf7915b6b33fea81b8655d
commit: 9c7927400cd8f1d283bf7915b6b33fea81b8655d
branch: master
author: Eric L <ericzolf at users.noreply.github.com>
committer: gpshead <greg at krypto.org>
date: 2021-03-03T12:36:22-08:00
summary:
bpo-40701: tempfile mixes str and bytes in an inconsistent manner (GH-20442)
The case of tempfile.tempdir variable being bytes is now handled consistently.
The getters return the right type and no more error of mixing str and bytes unless explicitly caused by the user.
Adds a regression test.
Expands the documentation to clarify the behavior.
Co-authored-by: Eric L <ewl+git at lavar.de>
Co-authored-by: Gregory P. Smith <greg at krypto.org>
files:
A Misc/NEWS.d/next/Library/2020-05-27-05-42-39.bpo-40701.PBIgW1.rst
M Doc/library/tempfile.rst
M Lib/tempfile.py
M Lib/test/test_tempfile.py
diff --git a/Doc/library/tempfile.rst b/Doc/library/tempfile.rst
index f9421da5fe7df..2b8a35e2651e7 100644
--- a/Doc/library/tempfile.rst
+++ b/Doc/library/tempfile.rst
@@ -248,6 +248,11 @@ The module defines the following user-callable items:
The result of this search is cached, see the description of
:data:`tempdir` below.
+ .. versionchanged:: 3.10
+
+ Always returns a str. Previously it would return any :data:`tempdir`
+ value regardless of type so long as it was not ``None``.
+
.. function:: gettempdirb()
Same as :func:`gettempdir` but the return value is in bytes.
@@ -269,18 +274,30 @@ The module uses a global variable to store the name of the directory
used for temporary files returned by :func:`gettempdir`. It can be
set directly to override the selection process, but this is discouraged.
All functions in this module take a *dir* argument which can be used
-to specify the directory and this is the recommended approach.
+to specify the directory. This is the recommended approach that does
+not surprise other unsuspecting code by changing global API behavior.
.. data:: tempdir
When set to a value other than ``None``, this variable defines the
default value for the *dir* argument to the functions defined in this
- module.
+ module, including its type, bytes or str. It cannot be a
+ :term:`path-like object`.
If ``tempdir`` is ``None`` (the default) at any call to any of the above
functions except :func:`gettempprefix` it is initialized following the
algorithm described in :func:`gettempdir`.
+ .. note::
+
+ Beware that if you set ``tempdir`` to a bytes value, there is a
+ nasty side effect: The global default return type of
+ :func:`mkstemp` and :func:`mkdtemp` changes to bytes when no
+ explicit ``prefix``, ``suffix``, or ``dir`` arguments of type
+ str are supplied. Please do not write code expecting or
+ depending on this. This awkward behavior is maintained for
+ compatibility with the historcal implementation.
+
.. _tempfile-examples:
Examples
diff --git a/Lib/tempfile.py b/Lib/tempfile.py
index c3fe61aa0af4f..dc088d9d7e448 100644
--- a/Lib/tempfile.py
+++ b/Lib/tempfile.py
@@ -99,7 +99,11 @@ def _infer_return_type(*args):
"path components.")
return_type = str
if return_type is None:
- return str # tempfile APIs return a str by default.
+ if tempdir is None or isinstance(tempdir, str):
+ return str # tempfile APIs return a str by default.
+ else:
+ # we could check for bytes but it'll fail later on anyway
+ return bytes
return return_type
@@ -265,17 +269,17 @@ def _mkstemp_inner(dir, pre, suf, flags, output_type):
# User visible interfaces.
def gettempprefix():
- """The default prefix for temporary directories."""
- return template
+ """The default prefix for temporary directories as string."""
+ return _os.fsdecode(template)
def gettempprefixb():
"""The default prefix for temporary directories as bytes."""
- return _os.fsencode(gettempprefix())
+ return _os.fsencode(template)
tempdir = None
-def gettempdir():
- """Accessor for tempfile.tempdir."""
+def _gettempdir():
+ """Private accessor for tempfile.tempdir."""
global tempdir
if tempdir is None:
_once_lock.acquire()
@@ -286,9 +290,13 @@ def gettempdir():
_once_lock.release()
return tempdir
+def gettempdir():
+ """Returns tempfile.tempdir as str."""
+ return _os.fsdecode(_gettempdir())
+
def gettempdirb():
- """A bytes version of tempfile.gettempdir()."""
- return _os.fsencode(gettempdir())
+ """Returns tempfile.tempdir as bytes."""
+ return _os.fsencode(_gettempdir())
def mkstemp(suffix=None, prefix=None, dir=None, text=False):
"""User-callable function to create and return a unique temporary
diff --git a/Lib/test/test_tempfile.py b/Lib/test/test_tempfile.py
index 77d710efaf107..5822c7535e378 100644
--- a/Lib/test/test_tempfile.py
+++ b/Lib/test/test_tempfile.py
@@ -667,6 +667,37 @@ def test_choose_directory(self):
finally:
os.rmdir(dir)
+ def test_for_tempdir_is_bytes_issue40701_api_warts(self):
+ orig_tempdir = tempfile.tempdir
+ self.assertIsInstance(tempfile.tempdir, (str, type(None)))
+ try:
+ fd, path = tempfile.mkstemp()
+ os.close(fd)
+ os.unlink(path)
+ self.assertIsInstance(path, str)
+ tempfile.tempdir = tempfile.gettempdirb()
+ self.assertIsInstance(tempfile.tempdir, bytes)
+ self.assertIsInstance(tempfile.gettempdir(), str)
+ self.assertIsInstance(tempfile.gettempdirb(), bytes)
+ fd, path = tempfile.mkstemp()
+ os.close(fd)
+ os.unlink(path)
+ self.assertIsInstance(path, bytes)
+ fd, path = tempfile.mkstemp(suffix='.txt')
+ os.close(fd)
+ os.unlink(path)
+ self.assertIsInstance(path, str)
+ fd, path = tempfile.mkstemp(prefix='test-temp-')
+ os.close(fd)
+ os.unlink(path)
+ self.assertIsInstance(path, str)
+ fd, path = tempfile.mkstemp(dir=tempfile.gettempdir())
+ os.close(fd)
+ os.unlink(path)
+ self.assertIsInstance(path, str)
+ finally:
+ tempfile.tempdir = orig_tempdir
+
class TestMkdtemp(TestBadTempdir, BaseTestCase):
"""Test mkdtemp()."""
@@ -775,6 +806,32 @@ def test_collision_with_existing_directory(self):
dir2 = tempfile.mkdtemp()
self.assertTrue(dir2.endswith('bbb'))
+ def test_for_tempdir_is_bytes_issue40701_api_warts(self):
+ orig_tempdir = tempfile.tempdir
+ self.assertIsInstance(tempfile.tempdir, (str, type(None)))
+ try:
+ path = tempfile.mkdtemp()
+ os.rmdir(path)
+ self.assertIsInstance(path, str)
+ tempfile.tempdir = tempfile.gettempdirb()
+ self.assertIsInstance(tempfile.tempdir, bytes)
+ self.assertIsInstance(tempfile.gettempdir(), str)
+ self.assertIsInstance(tempfile.gettempdirb(), bytes)
+ path = tempfile.mkdtemp()
+ os.rmdir(path)
+ self.assertIsInstance(path, bytes)
+ path = tempfile.mkdtemp(suffix='-dir')
+ os.rmdir(path)
+ self.assertIsInstance(path, str)
+ path = tempfile.mkdtemp(prefix='test-mkdtemp-')
+ os.rmdir(path)
+ self.assertIsInstance(path, str)
+ path = tempfile.mkdtemp(dir=tempfile.gettempdir())
+ os.rmdir(path)
+ self.assertIsInstance(path, str)
+ finally:
+ tempfile.tempdir = orig_tempdir
+
class TestMktemp(BaseTestCase):
"""Test mktemp()."""
diff --git a/Misc/NEWS.d/next/Library/2020-05-27-05-42-39.bpo-40701.PBIgW1.rst b/Misc/NEWS.d/next/Library/2020-05-27-05-42-39.bpo-40701.PBIgW1.rst
new file mode 100644
index 0000000000000..a7a4a1ce999af
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2020-05-27-05-42-39.bpo-40701.PBIgW1.rst
@@ -0,0 +1,6 @@
+When the :data:`tempfile.tempdir` global variable is set to a value of
+type bytes, it is now handled consistently. Previously exceptions
+could be raised from some tempfile APIs when the directory did not
+already exist in this situation. Also ensures that the
+:func:`tempfile.gettempdir()` and :func:`tempfile.gettempdirb()`
+functions *always* return ``str`` and ``bytes`` respectively.
More information about the Python-checkins
mailing list