Patchwork [2,of,2] procutil: avoid use of deprecated tempfile.mktemp()

login
register
mail settings
Submitter Manuel Jacob
Date July 15, 2020, 1:18 p.m.
Message ID <7e2a3c0ca7c53f732282.1594819126@tmp>
Download mbox | patch
Permalink /patch/46751/
State Accepted
Headers show

Comments

Manuel Jacob - July 15, 2020, 1:18 p.m.
# HG changeset patch
# User Manuel Jacob <me@manueljacob.de>
# Date 1594291924 -7200
#      Thu Jul 09 12:52:04 2020 +0200
# Node ID 7e2a3c0ca7c53f73228207ac41311b0336607a7b
# Parent  8718b4d5e08f6f84df73d1b846b340bbeeedb19c
# EXP-Topic stdio-nits
procutil: avoid use of deprecated tempfile.mktemp()

In the previous version, I used tempfile.mktemp() because it seemed to be the
only way to open a file from two processes (the Python documentation says the
file backing NamedTemporaryFile can’t be opened a second time on Windows).
However, it’s possible when passing the O_TEMPORARY flag to the second open.

Source: https://stackoverflow.com/a/15235559/6366251
Augie Fackler - July 15, 2020, 2:46 p.m.
On Wed, Jul 15, 2020 at 03:18:46PM +0200, Manuel Jacob wrote:
> # HG changeset patch
> # User Manuel Jacob <me@manueljacob.de>
> # Date 1594291924 -7200
> #      Thu Jul 09 12:52:04 2020 +0200
> # Node ID 7e2a3c0ca7c53f73228207ac41311b0336607a7b
> # Parent  8718b4d5e08f6f84df73d1b846b340bbeeedb19c
> # EXP-Topic stdio-nits
> procutil: avoid use of deprecated tempfile.mktemp()

queued, thanks

Patch

diff --git a/tests/test-stdio.py b/tests/test-stdio.py
--- a/tests/test-stdio.py
+++ b/tests/test-stdio.py
@@ -34,6 +34,7 @@ 
 
 
 TEST_LARGE_WRITE_CHILD_SCRIPT = r'''
+import os
 import signal
 import sys
 
@@ -43,7 +44,10 @@ 
 signal.signal(signal.SIGINT, lambda *x: None)
 dispatch.initstdio()
 write_result = procutil.{stream}.write(b'x' * 1048576)
-with open({write_result_fn}, 'w') as write_result_f:
+with os.fdopen(
+    os.open({write_result_fn!r}, os.O_WRONLY | getattr(os, 'O_TEMPORARY', 0)),
+    'w',
+) as write_result_f:
     write_result_f.write(str(write_result))
 '''
 
@@ -201,8 +205,7 @@ 
             )
 
         def post_child_check():
-            with open(write_result_fn, 'r') as write_result_f:
-                write_result_str = write_result_f.read()
+            write_result_str = write_result_f.read()
             if pycompat.ispy3:
                 # On Python 3, we test that the correct number of bytes is
                 # claimed to have been written.
@@ -213,14 +216,10 @@ 
                 expected_write_result_str = 'None'
             self.assertEqual(write_result_str, expected_write_result_str)
 
-        try:
-            # tempfile.mktemp() is unsafe in general, as a malicious process
-            # could create the file before we do. But in tests, we're running
-            # in a controlled environment.
-            write_result_fn = tempfile.mktemp()
+        with tempfile.NamedTemporaryFile('r') as write_result_f:
             self._test(
                 TEST_LARGE_WRITE_CHILD_SCRIPT.format(
-                    stream=stream, write_result_fn=repr(write_result_fn)
+                    stream=stream, write_result_fn=write_result_f.name
                 ),
                 stream,
                 rwpair_generator,
@@ -228,11 +227,6 @@ 
                 python_args,
                 post_child_check=post_child_check,
             )
-        finally:
-            try:
-                os.unlink(write_result_fn)
-            except OSError:
-                pass
 
     def test_large_write_stdout_devnull(self):
         self._test_large_write('stdout', _devnull)