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@lavar.de>
Co-authored-by: Gregory P. Smith <greg@krypto.org>

4 files changed, 98 insertions, 10 deletions

diff --git a/Doc/library/tempfile.rst b/Doc/library/tempfile.rst
index f9421da..2b8a35e 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 c3fe61a..dc088d9 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 77d710e..5822c75 100644
--- a/Lib/test/test_tempfile.py
+++ b/Lib/test/test_tempfile.py
@@ -667,6 +667,37 @@ class TestMkstemp(BaseTestCase):
         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 @@ class TestMkdtemp(TestBadTempdir, BaseTestCase):
             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 0000000..a7a4a1c
--- /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.