Get rid of remaining mktemp calls

Except for Platform/win32.py, the engine code no longer
called insecure/deprecated tempfile.mktemp. That location
is now also changed to use better methods.  A couple of
vestiges of old ways (like in comments) cleaned out.

A couple of unit tests also converted - these don't have
any impact on the scons engine, of course.

Signed-off-by: Mats Wichmann <mats@linux.com>

7 files changed, 72 insertions, 78 deletions

diff --git a/runtest.py b/runtest.py
index 464b593..554b4f6 100755
--- a/runtest.py
+++ b/runtest.py
@@ -556,7 +556,6 @@ if external:
 os.environ['SCONS_RUNTEST_DIR'] = scons_runtest_dir
 os.environ['SCONS_SCRIPT_DIR'] = scons_script_dir
 os.environ['SCONS_CWD'] = cwd
-
 os.environ['SCONS_VERSION'] = version
 
 old_pythonpath = os.environ.get('PYTHONPATH')
diff --git a/src/CHANGES.txt b/src/CHANGES.txt
index 5d0e3a2..f97b87e 100755
--- a/src/CHANGES.txt
+++ b/src/CHANGES.txt
@@ -24,6 +24,7 @@ RELEASE  VERSION/DATE TO BE FILLED IN LATER
     - Remove deprecated SourceCode
     - str.format syntax errors fixed
     - a bunch of linter/checker syntax fixups
+    - Convert remaining uses of insecure/deprecated mktemp method.
 
 
 RELEASE 3.1.2 - Mon, 17 Dec 2019 02:06:27 +0000
diff --git a/src/engine/SCons/Platform/__init__.py b/src/engine/SCons/Platform/__init__.py
index 7d959b7..058241d 100644
--- a/src/engine/SCons/Platform/__init__.py
+++ b/src/engine/SCons/Platform/__init__.py
@@ -192,11 +192,6 @@ class TempFileMunge(object):
         if cmdlist is not None:
             return cmdlist
 
-        # We do a normpath because mktemp() has what appears to be
-        # a bug in Windows that will use a forward slash as a path
-        # delimiter.  Windows' link mistakes that for a command line
-        # switch and barfs.
-        #
         # Default to the .lnk suffix for the benefit of the Phar Lap
         # linkloc linker, which likes to append an .lnk suffix if
         # none is given.
@@ -206,7 +201,7 @@ class TempFileMunge(object):
             suffix = '.lnk'
 
         fd, tmp = tempfile.mkstemp(suffix, text=True)
-        native_tmp = SCons.Util.get_native_path(os.path.normpath(tmp))
+        native_tmp = SCons.Util.get_native_path(tmp)
 
         if env.get('SHELL', None) == 'sh':
             # The sh shell will try to escape the backslashes in the
diff --git a/src/engine/SCons/Platform/win32.py b/src/engine/SCons/Platform/win32.py
index 77c048e..bb1b46d 100644
--- a/src/engine/SCons/Platform/win32.py
+++ b/src/engine/SCons/Platform/win32.py
@@ -178,61 +178,68 @@ def piped_spawn(sh, escape, cmd, args, env, stdout, stderr):
     #   we redirect it into a temporary file tmpFileStdout
     #   (tmpFileStderr) and copy the contents of this file
     #   to stdout (stderr) given in the argument
+    # Note that because this will paste shell redirection syntax
+    # into the cmdline, we have to call a shell to run the command,
+    # even though that's a bit of a performance hit.
     if not sh:
         sys.stderr.write("scons: Could not find command interpreter, is it in your PATH?\n")
         return 127
-    else:
-        # one temporary file for stdout and stderr
-        tmpFileStdout = os.path.normpath(tempfile.mktemp())
-        tmpFileStderr = os.path.normpath(tempfile.mktemp())
-
-        # check if output is redirected
-        stdoutRedirected = 0
-        stderrRedirected = 0
-        for arg in args:
-            # are there more possibilities to redirect stdout ?
-            if arg.find( ">", 0, 1 ) != -1 or arg.find( "1>", 0, 2 ) != -1:
-                stdoutRedirected = 1
-            # are there more possibilities to redirect stderr ?
-            if arg.find( "2>", 0, 2 ) != -1:
-                stderrRedirected = 1
-
-        # redirect output of non-redirected streams to our tempfiles
-        if stdoutRedirected == 0:
-            args.append(">" + str(tmpFileStdout))
-        if stderrRedirected == 0:
-            args.append("2>" + str(tmpFileStderr))
-
-        # actually do the spawn
+
+    # one temporary file for stdout and stderr
+    tmpFileStdout, tmpFileStdoutName = tempfile.mkstemp(text=True)
+    os.close(tmpFileStdout)  # don't need open until the subproc is done
+    tmpFileStderr, tmpFileStderrName = tempfile.mkstemp(text=True)
+    os.close(tmpFileStderr)
+
+    # check if output is redirected
+    stdoutRedirected = False
+    stderrRedirected = False
+    for arg in args:
+        # are there more possibilities to redirect stdout ?
+        if arg.find(">", 0, 1) != -1 or arg.find("1>", 0, 2) != -1:
+            stdoutRedirected = True
+        # are there more possibilities to redirect stderr ?
+        if arg.find("2>", 0, 2) != -1:
+            stderrRedirected = True
+
+    # redirect output of non-redirected streams to our tempfiles
+    if not stdoutRedirected:
+        args.append(">" + tmpFileStdoutName)
+    if not stderrRedirected:
+        args.append("2>" + tmpFileStderrName)
+
+    # actually do the spawn
+    try:
+        args = [sh, '/C', escape(' '.join(args))]
+        ret = spawnve(os.P_WAIT, sh, args, env)
+    except OSError as e:
+        # catch any error
         try:
-            args = [sh, '/C', escape(' '.join(args))]
-            ret = spawnve(os.P_WAIT, sh, args, env)
-        except OSError as e:
-            # catch any error
-            try:
-                ret = exitvalmap[e.errno]
-            except KeyError:
-                sys.stderr.write("scons: unknown OSError exception code %d - %s: %s\n" % (e.errno, cmd, e.strerror))
-            if stderr is not None:
-                stderr.write("scons: %s: %s\n" % (cmd, e.strerror))
-        # copy child output from tempfiles to our streams
-        # and do clean up stuff
-        if stdout is not None and stdoutRedirected == 0:
-            try:
-                with open(tmpFileStdout, "r" ) as tmp:
-                    stdout.write(tmp.read())
-                os.remove(tmpFileStdout)
-            except (IOError, OSError):
-                pass
+            ret = exitvalmap[e.errno]
+        except KeyError:
+            sys.stderr.write("scons: unknown OSError exception code %d - %s: %s\n" % (e.errno, cmd, e.strerror))
+        if stderr is not None:
+            stderr.write("scons: %s: %s\n" % (cmd, e.strerror))
 
-        if stderr is not None and stderrRedirected == 0:
-            try:
-                with open(tmpFileStderr, "r" ) as tmp:
-                    stderr.write(tmp.read())
-                os.remove(tmpFileStderr)
-            except (IOError, OSError):
-                pass
-        return ret
+    # copy child output from tempfiles to our streams
+    # and do clean up stuff
+    if stdout is not None and not stdoutRedirected:
+        try:
+            with open(tmpFileStdoutName, "r") as tmpFileStdout:
+                stdout.write(tmpFileStdout.read())
+            os.remove(tmpFileStdoutName)
+        except (IOError, OSError):
+            pass
+
+    if stderr is not None and not stderrRedirected:
+        try:
+            with open(tmpFileStderrName, "r") as tmpFileStderr:
+                stderr.write(tmpFileStderr.read())
+            os.remove(tmpFileStderrName)
+        except (IOError, OSError):
+            pass
+
+    return ret
 
 
 def exec_spawn(l, env):
diff --git a/src/engine/SCons/UtilTests.py b/src/engine/SCons/UtilTests.py
index 7f6508d..e2dd57f 100644
--- a/src/engine/SCons/UtilTests.py
+++ b/src/engine/SCons/UtilTests.py
@@ -506,13 +506,14 @@ class UtilTestCase(unittest.TestCase):
     def test_get_native_path(self):
         """Test the get_native_path() function."""
         import tempfile
-        filename = tempfile.mktemp()
-        str = '1234567890 ' + filename
+        f, filename = tempfile.mkstemp(text=True)
+        os.close(f)
+        data = '1234567890 ' + filename
         try:
             with open(filename, 'w') as f:
-                f.write(str)
-            with open(get_native_path(filename)) as f:
-                assert f.read() == str
+                f.write(data)
+            with open(get_native_path(filename), 'r') as f:
+                assert f.read() == data
         finally:
             try:
                 os.unlink(filename)
diff --git a/src/engine/SCons/cppTests.py b/src/engine/SCons/cppTests.py
index 0b346e8..eff7715 100644
--- a/src/engine/SCons/cppTests.py
+++ b/src/engine/SCons/cppTests.py
@@ -770,12 +770,6 @@ import re
 import shutil
 import tempfile
 
-tempfile.template = 'cppTests.'
-if os.name in ('posix', 'nt'):
-    tempfile.template = 'cppTests.' + str(os.getpid()) + '.'
-else:
-    tempfile.template = 'cppTests.'
-
 _Cleanup = []
 
 def _clean():
@@ -785,19 +779,17 @@ def _clean():
 
 atexit.register(_clean)
 
+if os.name in ('posix', 'nt'):
+    tmpprefix = 'cppTests.' + str(os.getpid()) + '.'
+else:
+    tmpprefix = 'cppTests.'
+
 class fileTestCase(unittest.TestCase):
     cpp_class = cpp.DumbPreProcessor
 
     def setUp(self):
-        try:
-            path = tempfile.mktemp(prefix=tempfile.template)
-        except TypeError:
-            # The tempfile.mktemp() function in earlier versions of Python
-            # has no prefix argument, but uses the tempfile.template
-            # value that we set above.
-            path = tempfile.mktemp()
+        path = tempfile.mkdtemp(prefix=tmpprefix)
         _Cleanup.append(path)
-        os.mkdir(path)
         self.tempdir = path
         self.orig_cwd = os.getcwd()
         os.chdir(path)
diff --git a/testing/framework/TestCmdTests.py b/testing/framework/TestCmdTests.py
index 0c7b455..48eba1e 100644
--- a/testing/framework/TestCmdTests.py
+++ b/testing/framework/TestCmdTests.py
@@ -2814,8 +2814,7 @@ class symlink_TestCase(TestCmdTestCase):
 class tempdir_TestCase(TestCmdTestCase):
     def setUp(self):
         TestCmdTestCase.setUp(self)
-        self._tempdir = tempfile.mktemp()
-        os.mkdir(self._tempdir)
+        self._tempdir = tempfile.mkdtemp()
         os.chdir(self._tempdir)
 
     def tearDown(self):