Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions Lib/test/test_os.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,15 @@ def create_file(filename, content=b'content'):
fp.write(content)


# On platforms without a native spawnv(), os.py provides a Python fallback
# built on fork()+exec*() that reports argument conversion failures from the
# child as exit status 127 instead of raising, so tests of the C
# implementation's error paths cannot run against it.
requires_native_spawnv = unittest.skipUnless(
isinstance(getattr(os, 'spawnv', None), types.BuiltinFunctionType),
'requires the native C os.spawnv')


# bpo-41625: On AIX, splice() only works with a socket, not with a pipe.
requires_splice_pipe = unittest.skipIf(sys.platform.startswith("aix"),
'on AIX, splice() only accepts sockets')
Expand Down Expand Up @@ -3596,6 +3605,25 @@ def test_spawnve_noargs(self):
self.assertRaises(ValueError, os.spawnve, os.P_NOWAIT, program, ('',), {})
self.assertRaises(ValueError, os.spawnve, os.P_NOWAIT, program, [''], {})

@requires_native_spawnv
def test_spawnv_arg_conversion_errors(self):
# A non-path argv item gets a TypeError naming the argument...
with self.assertRaisesRegex(TypeError, 'must contain only strings'):
os.spawnv(os.P_NOWAIT, sys.executable, [sys.executable, 123])
# ...but other conversion errors must not be masked as TypeError
# (gh-151416).
with self.assertRaises(ValueError):
os.spawnv(os.P_NOWAIT, sys.executable,
[sys.executable, 'embedded\0null'])

class RaisingPath:
def __fspath__(self):
raise RuntimeError('gotcha')

with self.assertRaisesRegex(RuntimeError, 'gotcha'):
os.spawnv(os.P_NOWAIT, sys.executable,
[sys.executable, RaisingPath()])

def _test_invalid_env(self, spawn):
program = sys.executable
args = self.quote_args([program, '-c', 'pass'])
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
Fix a crash in :func:`os.spawnv` and :func:`os.spawnve` when an *argv*
item's :meth:`~os.PathLike.__fspath__` method mutates the *argv* list
during argument conversion. :func:`!os.spawnv` argument conversion errors
other than :exc:`TypeError`, such as the :exc:`ValueError` for an embedded
null, are no longer replaced with a generic :exc:`TypeError`.
46 changes: 32 additions & 14 deletions Modules/posixmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -7624,18 +7624,15 @@ os_spawnv_impl(PyObject *module, int mode, path_t *path, PyObject *argv)
int i;
Py_ssize_t argc;
intptr_t spawnval;
PyObject *(*getitem)(PyObject *, Py_ssize_t);

/* spawnv has three arguments: (mode, path, argv), where
argv is a list or tuple of strings. */

if (PyList_Check(argv)) {
argc = PyList_Size(argv);
getitem = PyList_GetItem;
}
else if (PyTuple_Check(argv)) {
argc = PyTuple_Size(argv);
getitem = PyTuple_GetItem;
}
else {
PyErr_SetString(PyExc_TypeError,
Expand All @@ -7653,14 +7650,29 @@ os_spawnv_impl(PyObject *module, int mode, path_t *path, PyObject *argv)
return PyErr_NoMemory();
}
for (i = 0; i < argc; i++) {
if (!fsconvert_strdup((*getitem)(argv, i),
&argvlist[i])) {
// The item must be a strong reference because of possible
// side-effects of PyUnicode_FS{Converter,Decoder}() in
// fsconvert_strdup(): an item's __fspath__() can mutate a list
// *argv*, releasing the list's reference to the item (gh-151416).
PyObject *item = PySequence_ITEM(argv, i);
if (item == NULL) {
free_string_array(argvlist, i);
PyErr_SetString(
PyExc_TypeError,
"spawnv() arg 2 must contain only strings");
return NULL;
}
if (!fsconvert_strdup(item, &argvlist[i])) {
Py_DECREF(item);
free_string_array(argvlist, i);
// Add argument context to the converter's terse TypeError, but
// let MemoryError, codec errors, embedded-null ValueError, etc.
// propagate unmasked.
if (PyErr_ExceptionMatches(PyExc_TypeError)) {
PyErr_SetString(
PyExc_TypeError,
"spawnv() arg 2 must contain only strings");
}
return NULL;
}
Py_DECREF(item);
if (i == 0 && !argvlist[0][0]) {
free_string_array(argvlist, i + 1);
PyErr_SetString(
Expand Down Expand Up @@ -7731,7 +7743,6 @@ os_spawnve_impl(PyObject *module, int mode, path_t *path, PyObject *argv,
PyObject *res = NULL;
Py_ssize_t argc, i, envc;
intptr_t spawnval;
PyObject *(*getitem)(PyObject *, Py_ssize_t);
Py_ssize_t lastarg = 0;

/* spawnve has four arguments: (mode, path, argv, env), where
Expand All @@ -7740,11 +7751,9 @@ os_spawnve_impl(PyObject *module, int mode, path_t *path, PyObject *argv,

if (PyList_Check(argv)) {
argc = PyList_Size(argv);
getitem = PyList_GetItem;
}
else if (PyTuple_Check(argv)) {
argc = PyTuple_Size(argv);
getitem = PyTuple_GetItem;
}
else {
PyErr_SetString(PyExc_TypeError,
Expand All @@ -7768,12 +7777,21 @@ os_spawnve_impl(PyObject *module, int mode, path_t *path, PyObject *argv,
goto fail_0;
}
for (i = 0; i < argc; i++) {
if (!fsconvert_strdup((*getitem)(argv, i),
&argvlist[i]))
{
// The item must be a strong reference because of possible
// side-effects of PyUnicode_FS{Converter,Decoder}() in
// fsconvert_strdup(): an item's __fspath__() can mutate a list
// *argv*, releasing the list's reference to the item (gh-151416).
PyObject *item = PySequence_ITEM(argv, i);
if (item == NULL) {
lastarg = i;
goto fail_1;
}
if (!fsconvert_strdup(item, &argvlist[i])) {
Py_DECREF(item);
lastarg = i;
goto fail_1;
}
Py_DECREF(item);
if (i == 0 && !argvlist[0][0]) {
lastarg = i + 1;
PyErr_SetString(
Expand Down
Loading