This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

Unsupported provider

classification
Title: raise ValueError for empty `path` in os.spawnv[e]
Type: behavior Stage: needs patch
Components: Library (Lib), Windows Versions: Python 3.10, Python 3.9, Python 3.8
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: alfps, amaury.forgeotdarc, brian.curtin, dstanek, eryksun, exarkun, ezio.melotti, ggenellina, loewis, nvetoshkin, paul.moore, schmir, serhiy.storchaka, steve.dower, terry.reedy, tim.golden, zach.ware
Priority: normal Keywords: patch

Created on 2010-03-01 21:12 by alfps, last changed 2022-04-11 14:56 by admin.

Files
File name Uploaded Description Edit
issue8036_0.patch nvetoshkin, 2011-03-11 12:54
issue_8036_1.patch nvetoshkin, 2011-10-26 19:30 args checking + tests review
Messages (20)
msg100272 - (view) Author: Alf P. Steinbach (alfps) Date: 2010-03-01 21:12
In Windows XP, using CPython 3.1.1:

python -c "import os; os.spawnl( os.P_WAIT, 'blah' )"

Crashes the interpreter and Windows pops up the "tell us about it" box.


Python 2.6 is however happy and just reports invalid arg.
msg100281 - (view) Author: Brian Curtin (brian.curtin) * (Python committer) Date: 2010-03-01 23:57
Confirmed on trunk, release31-maint, and py3k on Windows. OS X returns 127 in that case.
msg100699 - (view) Author: Gabriel Genellina (ggenellina) Date: 2010-03-09 05:48
In case it matters, 3.0.1 does NOT crash.
msg100718 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2010-03-09 13:22
2.6 and 3.0.1 used to disable the Microsoft CRT argument error handler: they return EINVAL, but newer versions don't, and should check their arguments before calling _spawnv.

FWIW, the checks are::
    pathname != NULL
    *pathname != '\0'
    argv != NULL
    *argv != NULL
    **argv != '\0'
The first and third checks are guaranteed by the implementation, but the other three should be done in posix_spawnv().
And the other calls to the various nt.spawn* functions probably suffer the same problem.
msg100720 - (view) Author: Jean-Paul Calderone (exarkun) * (Python committer) Date: 2010-03-09 14:13
Why is the Microsoft CRT argument error handler no longer disabled?
msg100721 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2010-03-09 14:24
Because this is a global setting for the whole process. This was discussed with issue4804.
msg127123 - (view) Author: David Stanek (dstanek) Date: 2011-01-26 17:39
Should this just be resolved as a "won't fix"?
msg127124 - (view) Author: Amaury Forgeot d'Arc (amaury.forgeotdarc) * (Python committer) Date: 2011-01-26 17:43
No, the issue can be fixed by better checking the arguments.
msg130563 - (view) Author: Vetoshkin Nikita (nvetoshkin) Date: 2011-03-11 12:54
Attached first attempt to close this issue. Are there enough checks? It passes:
os.spawnl(os.P_WAIT, '')
os.spawnl(os.P_WAIT, 'path')
os.spawnl(os.P_WAIT, 'path', '')
msg146327 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2011-10-24 22:15
This came up on python-list with
os.spawnl (os.P_NOWAIT, r"c:\windows\notepad.exe")
Tim Golden confirmed crash with Win7 on 2.7.?
I did same with 3.2.2 in both interpreter and from IDLE (IDLE hangs and then restarts after closing the 'pythonw has stopped' error message box).
Above apparently works in XP, at least with Py2.2.
msg146329 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2011-10-24 22:21
@nvetoshkin: Can you please also write tests (in Lib/test/test_os.py) for your patch?
msg146369 - (view) Author: Vetoshkin Nikita (nvetoshkin) Date: 2011-10-25 14:23
against py3k branch?
msg146370 - (view) Author: Ezio Melotti (ezio.melotti) * (Python committer) Date: 2011-10-25 14:27
Now we are using Mercurial, and what was called 'py3k' on SVN is now 'default'.  Since we now commit on older branches first and then merge with the most recent ones, the patch should either be against 3.2 or 2.7.
You can check the devguide for more informations.
msg146453 - (view) Author: Vetoshkin Nikita (nvetoshkin) Date: 2011-10-26 19:30
added some tests (not sure if in appropriate place).
msg321356 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-07-10 05:56
All three test cases are raising a VallueError now:

C:\py\cpython3.8>python -c "import os; os.spawnl(os.P_WAIT, '')"
Running Debug|x64 interpreter...
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "C:\py\cpython3.8\\lib\os.py", line 931, in spawnl
    return spawnv(mode, file, args)
ValueError: spawnv() arg 2 cannot be empty

C:\py\cpython3.8>python -c "import os; os.spawnl(os.P_WAIT, '__not_existing_path__')"
Running Debug|x64 interpreter...
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "C:\py\cpython3.8\\lib\os.py", line 931, in spawnl
    return spawnv(mode, file, args)
ValueError: spawnv() arg 2 cannot be empty

C:\py\cpython3.8>python -c "import os; os.spawnl(os.P_WAIT, '__not_existing_path__', '')"
Running Debug|x64 interpreter...
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "C:\py\cpython3.8\\lib\os.py", line 931, in spawnl
    return spawnv(mode, file, args)
ValueError: spawnv() arg 2 first element cannot be empty


But the following one still causes a crash:

  python -c "import os; os.spawnl(os.P_WAIT, '', 'non-empty')"
msg321357 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2018-07-10 06:28
Serhiy, an empty file path shouldn't crash the interpreter. The per-thread invalid parameter handler is suppressed before calling _wspawnv, so it should raise OSError (EINVAL) if the file path is empty. That's what I observe in 3.7. Are you suggesting to raise ValueError in this case instead of calling _wspawnv?
msg321358 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-07-10 07:08
Crashing happens only with the debug build. With the release mode I got OSError(EINVAL). I think it is better to raise the same error in all builds. ValueError is not new, it is already raised for paths containing NUL and too long paths.
msg321398 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-07-10 22:18
> Crashing happens only with the debug build.

I don't understand why Python behaves differently in debug mode. For me, if Python is able to trigger an exception on EINVAL, we should also get a regular Python exception in debug mode (and not a crash).
msg321405 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2018-07-11 01:31
> I don't understand why Python behaves differently in debug mode.
> For me, if Python is able to trigger an exception on EINVAL, we 
> should also get a regular Python exception in debug mode (and not 
> a crash)

The debug build's behavior isn't related to the invalid parameter handler, and it's not necessarily a crash. The default report mode for CRT_ERROR and CRT_ASSERT is CRTDBG_MODE_WNDW. This pops up a dialog asking whether to abort the process, ignore the error (fail and set errno), or retry if a debugger is attached (i.e. break into the debugger with a first-chance exception).

The mode can be changed to CRTDBG_MODE_DEBUG, which writes a message to the debugger, if one is attached to the process:

    msvcrt.CrtSetReportMode(msvcrt.CRT_ASSERT, msvcrt.CRTDBG_MODE_DEBUG)

For example, in this case a message is written about the failed assertion in exec\spawv.cpp on line 276 (in this case the debugger is cdb, sharing the same console as Python, so the failed assertion message is inlined with Python's traceback):
    
    >>> os.spawnl(os.P_WAIT, '', 'non-empty')
    minkernel\crts\ucrt\src\desktopcrt\exec\spawnv.cpp(276) : Assertion failed: file_name[0] != '\0'
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "C:\Program Files\Python37\lib\os.py", line 931, in spawnl
        return spawnv(mode, file, args)
    OSError: [Errno 22] Invalid argument

or set the report mode to CRTDBG_MODE_FILE with the file set to CRTDBG_FILE_STDERR:

    msvcrt.CrtSetReportMode(msvcrt.CRT_ASSERT, msvcrt.CRTDBG_MODE_FILE)
    msvcrt.CrtSetReportFile(msvcrt.CRT_ASSERT, msvcrt.CRTDBG_FILE_STDERR)
msg387874 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2021-03-01 15:27
The internal spawn function in ucrt, common_spawnv(), verifies its parameters as follows:

    _VALIDATE_RETURN(file_name       != nullptr, EINVAL, -1);
    _VALIDATE_RETURN(file_name[0]    != '\0',    EINVAL, -1);
    _VALIDATE_RETURN(arguments       != nullptr, EINVAL, -1);
    _VALIDATE_RETURN(arguments[0]    != nullptr, EINVAL, -1);
    _VALIDATE_RETURN(arguments[0][0] != '\0',    EINVAL, -1);

Currently os.spawnv() and os.spawnve() check for and raise ValueError for all of these cases except the second one, in which file_name is an empty string. Microsoft doesn't document this case [1]:

    These functions validate their parameters. If either cmdname or argv 
    is a null pointer, or if argv points to null pointer, or argv[0] is 
    an empty string, the invalid parameter handler is invoked, as 
    described in Parameter Validation.

In a release build, this case fails with EINVAL. In a debug build, the default error-reporting settings display a pop message about the invalid argument. The message box is easy enough to suppress. But for the sake of consistency with the other cases, os_spawnv_impl in Modules/posixmodule.c should raise a ValueError if `path` is an empty string. For example:

    #ifdef HAVE_WSPAWNV
        if (!path->wide[0]) {
            PyErr_SetString(PyExc_ValueError,
                "spawnv() arg 1 cannot be an empty string");
            return NULL;
        }
    #endif

---
[1] https://docs.microsoft.com/en-us/cpp/c-runtime-library/reference/spawnv-wspawnv
History
Date User Action Args
2022-04-11 14:56:58adminsetgithub: 52284
2021-03-08 19:50:50vstinnersetnosy: - vstinner
2021-03-01 15:27:31eryksunsetpriority: high -> normal

type: crash -> behavior

components: + Windows
title: Interpreter crashes on invalid arg to spawnl on Windows -> raise ValueError for empty `path` in os.spawnv[e]
nosy: + tim.golden, zach.ware, paul.moore, steve.dower
versions: + Python 3.8, Python 3.9, Python 3.10, - Python 2.7, Python 3.2, Python 3.3
messages: + msg387874
2018-07-11 01:31:23eryksunsetmessages: + msg321405
2018-07-10 22:18:41vstinnersetmessages: + msg321398
2018-07-10 07:08:13serhiy.storchakasetmessages: + msg321358
2018-07-10 06:28:17eryksunsetnosy: + eryksun
messages: + msg321357
2018-07-10 05:56:20serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg321356
2012-02-01 14:04:27pitroulinkissue13917 superseder
2011-10-26 19:30:34nvetoshkinsetfiles: + issue_8036_1.patch

messages: + msg146453
2011-10-25 14:27:15ezio.melottisetnosy: + ezio.melotti
messages: + msg146370
2011-10-25 14:23:47nvetoshkinsetmessages: + msg146369
2011-10-24 22:21:12vstinnersetmessages: + msg146329
2011-10-24 22:20:04vstinnersetnosy: + vstinner
2011-10-24 22:15:05terry.reedysetnosy: + terry.reedy, loewis
messages: + msg146327
2011-06-12 22:21:10terry.reedysetversions: + Python 3.3, - Python 3.1
2011-03-11 12:54:01nvetoshkinsetfiles: + issue8036_0.patch

nosy: + nvetoshkin
messages: + msg130563

keywords: + patch
2011-02-24 15:58:11schmirsetnosy: + schmir
2011-01-26 17:43:41amaury.forgeotdarcsetnosy: exarkun, amaury.forgeotdarc, ggenellina, dstanek, brian.curtin, alfps
messages: + msg127124
2011-01-26 17:39:30dstaneksetnosy: + dstanek
messages: + msg127123
2010-03-09 14:24:58amaury.forgeotdarcsetmessages: + msg100721
2010-03-09 14:13:46exarkunsetnosy: + exarkun
messages: + msg100720
2010-03-09 13:22:52amaury.forgeotdarcsetnosy: + amaury.forgeotdarc
messages: + msg100718
2010-03-09 05:48:07ggenellinasetnosy: + ggenellina
messages: + msg100699
2010-03-01 23:58:00brian.curtinsetpriority: high


title: Interpreter crashes on invalid arg to spawnl (Windows XP) -> Interpreter crashes on invalid arg to spawnl on Windows
nosy: + brian.curtin
versions: + Python 2.7, Python 3.2
messages: + msg100281
stage: needs patch
2010-03-01 21:12:27alfpscreate