[Python-checkins] bpo-32890, os: Use errno instead of GetLastError() in execve() and truncate() (GH-5784)

[Miss Islington (bot)]

https://github.com/python/cpython/commit/d9a2665fc4573c4d311a89750737ad4cc3310252
commit: d9a2665fc4573c4d311a89750737ad4cc3310252
branch: 3.6
author: Miss Islington (bot) <31488909+miss-islington at users.noreply.github.com>
committer: GitHub <noreply at github.com>
date: 2018-10-19T17:49:00-07:00
summary:

bpo-32890, os: Use errno instead of GetLastError() in execve() and truncate() (GH-5784)


path_error() uses GetLastError() on Windows, but some os functions
are implemented via CRT APIs which report errors via errno.
This may result in raising OSError with invalid error code (such
as zero).

Introduce posix_path_error() function and use it where appropriate.
(cherry picked from commit 834603112e6ca35944dd21105b01fca562dc3241)

Co-authored-by: Alexey Izbyshev <izbyshev at ispras.ru>

files:
A Misc/NEWS.d/next/Windows/2018-03-08-20-02-38.bpo-32890.3jzFzY.rst
M Lib/test/test_os.py
M Modules/posixmodule.c

diff --git a/Lib/test/test_os.py b/Lib/test/test_os.py
index 8339f849e84e..9e35d55b8056 100644
--- a/Lib/test/test_os.py
+++ b/Lib/test/test_os.py
@@ -1582,6 +1582,16 @@ def test_execve_invalid_env(self):
         with self.assertRaises(ValueError):
             os.execve(args[0], args, newenv)
 
+    @unittest.skipUnless(sys.platform == "win32", "Win32-specific test")
+    def test_execve_with_empty_path(self):
+        # bpo-32890: Check GetLastError() misuse
+        try:
+            os.execve('', ['arg'], {})
+        except OSError as e:
+            self.assertTrue(e.winerror is None or e.winerror != 0)
+        else:
+            self.fail('No OSError raised')
+
 
 @unittest.skipUnless(sys.platform == "win32", "Win32 specific tests")
 class Win32ErrorTests(unittest.TestCase):
diff --git a/Misc/NEWS.d/next/Windows/2018-03-08-20-02-38.bpo-32890.3jzFzY.rst b/Misc/NEWS.d/next/Windows/2018-03-08-20-02-38.bpo-32890.3jzFzY.rst
new file mode 100644
index 000000000000..e8a63b334192
--- /dev/null
+++ b/Misc/NEWS.d/next/Windows/2018-03-08-20-02-38.bpo-32890.3jzFzY.rst
@@ -0,0 +1,2 @@
+Fix usage of GetLastError() instead of errno in os.execve() and
+os.truncate().
diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c
index af6b9dc4ef03..7358c393b1a0 100644
--- a/Modules/posixmodule.c
+++ b/Modules/posixmodule.c
@@ -1340,6 +1340,12 @@ win32_error_object(const char* function, PyObject* filename)
 
 #endif /* MS_WINDOWS */
 
+static PyObject *
+posix_path_object_error(PyObject *path)
+{
+    return PyErr_SetFromErrnoWithFilenameObject(PyExc_OSError, path);
+}
+
 static PyObject *
 path_object_error(PyObject *path)
 {
@@ -1347,7 +1353,7 @@ path_object_error(PyObject *path)
     return PyErr_SetExcFromWindowsErrWithFilenameObject(
                 PyExc_OSError, 0, path);
 #else
-    return PyErr_SetFromErrnoWithFilenameObject(PyExc_OSError, path);
+    return posix_path_object_error(path);
 #endif
 }
 
@@ -1368,6 +1374,12 @@ path_error(path_t *path)
     return path_object_error(path->object);
 }
 
+static PyObject *
+posix_path_error(path_t *path)
+{
+    return posix_path_object_error(path->object);
+}
+
 static PyObject *
 path_error2(path_t *path, path_t *path2)
 {
@@ -5041,7 +5053,7 @@ os_execve_impl(PyObject *module, path_t *path, PyObject *argv, PyObject *env)
 
     /* If we get here it's definitely an error */
 
-    path_error(path);
+    posix_path_error(path);
 
     free_string_array(envlist, envc);
   fail:
@@ -8770,7 +8782,7 @@ os_truncate_impl(PyObject *module, path_t *path, Py_off_t length)
     _Py_END_SUPPRESS_IPH
     Py_END_ALLOW_THREADS
     if (result < 0)
-        return path_error(path);
+        return posix_path_error(path);
 
     Py_RETURN_NONE;
 }