classification
Title: subprocess functions with shell=1 pass wrong command to win32 shell
Type: behavior Stage: resolved
Components: Library (Lib), Windows Versions: Python 3.8, Python 3.7, Python 3.6, Python 2.7
process
Status: closed Resolution: third party
Dependencies: Superseder:
Assigned To: Nosy List: eryksun, mandel59, paul.moore, steve.dower, tim.golden, zach.ware
Priority: normal Keywords: patch

Created on 2018-07-07 10:30 by mandel59, last changed 2021-03-01 14:37 by eryksun. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 8152 closed mandel59, 2018-07-07 10:36
Messages (2)
msg321213 - (view) Author: Ryusei Yamaguchi (mandel59) * Date: 2018-07-07 10:30
In some cases, functions defined in subprocess modules pass wrong command to win32 shell.

Python 3.8.0a0 (heads/master:4629c0d531, Jul  7 2018, 16:37:33) [MSC v.1914 32 bit (Intel)] on win32
Type "help", "copyright", "credits" or "license" for more information.
>>> import subprocess
>>> subprocess.check_output('cmd /c echo hello', shell=1)
b'hello"\r\n'

The result is as same as the following command run with cmd.exe:
C:\> %COMSPEC% /c "cmd /c echo hello"
hello"

Specifying /s switch fixes the issue:
C:\> %COMSPEC% /s /c "cmd /c echo hello"
hello
msg321271 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2018-07-08 11:11
This PR works around a bug in CMD, so I'm inclined to close it as a third-party issue. 

CMD is supposed to implement the following behavior:

    1.  If all of the following conditions are met, then quote 
        characters on the command line are preserved:

        - no /S switch
        - exactly two quote characters
        - no special characters between the two quote characters,
          where special is one of: &<>()@^|
        - there are one or more whitespace characters between the
          two quote characters
        - the string between the two quote characters is the name
          of an executable file.

    2.  Otherwise, old behavior is to see if the first character is
        a quote character and if so, strip the leading character and
        remove the last quote character on the command line, preserving
        any text after the last quote character.

In particular quotes should only be preserved if the quoted string is "the name of an executable file". To determine this, CMD calls an internal function named SearchForExecutable. This function special cases any command that begins with "cmd " (case insensitive) by substituting the current value of the %ComSpec% environment variable for the *entire* string. Of course this search succeeds, so it ends up handling the string "cmd /c echo hello" as if it's an executable. Probably no has noticed this bug since running `cmd /c "cmd /c ..."` is kind of a weird thing to do.

To demonstrate this bug, let's compare running "cmd /c ..." with "cmd.exe /c ...":

    >>> subprocess.check_output('cmd /c "cmd /c echo hello"')
    b'hello"\r\n'


    >>> subprocess.check_output('cmd /c "cmd.exe /c echo hello"')
    b'hello\r\n'

Adding /S makes CMD always use the old behavior that strips the quotes. However, this will change the existing behavior when the command line is an unquoted executable path with spaces in it. For example:

    >>> sys.executable
    'C:\\Program Files\\Python36\\python.exe'

    >>> subprocess.call(sys.executable, shell=True)
    Python 3.6.4 (v3.6.4:d48eceb, Dec 19 2017, 06:54:40) [MSC v.1900 64 bit (AMD64)] on win32
    Type "help", "copyright", "credits" or "license" for more information.
    >>> exit()
    0

With /S, CMD strips the quotes from the command. Without the quotes it parses "C:\Program" as the executable to run:

    >>> subprocess.call('cmd /s /c "{}"'.format(sys.executable))
    'C:\Program' is not recognized as an internal or external command,
    operable program or batch file.
    1
History
Date User Action Args
2021-03-01 14:37:29eryksunsetstatus: open -> closed
resolution: third party
stage: patch review -> resolved
2018-07-08 11:11:19eryksunsetnosy: + eryksun

messages: + msg321271
versions: - Python 3.4, Python 3.5
2018-07-07 10:36:50mandel59setkeywords: + patch
stage: patch review
pull_requests: + pull_request7717
2018-07-07 10:30:07mandel59create