win32reactor spawnProcess incorrectly escapes cmd line

[justinj]

Recent changes to win32eventreactor.py in _cmdLineQuote incorrectly replace 
all slashes (\) with double slashes (\\).  The attempt of the code is to deal 
with MS command line goofiness as defined at 
http://msdn.microsoft.com/library/default.asp?url=/library/en-
us/vclang/html/_pluslang_Parsing_C.2b2b_.Command.2d.Line_Arguments.asp.  
Unfortunately the change broke the current code so that the current code 
doesn't work if the command you are spawning can't deal with the double 
slashes.

[itamarst]

James checked in a fix for this.

[osuchw]

Currently every argument gets quoted.  Some programs eg. SAS.EXE get confused by
this.  Attached patch, inspired by subprocess from std lib fixes it for me.

[justinj]

I'm reassigning this to me.  Let me know if you have a problem with that.

[justinj]

Resolved in r15008.

[bwh]

Firstly, some Windows commands really really shouldn't have their arguments
quoted. In particular, if I want to run a command line through cmd, I should
prefix it with "cmd /c" and *not* quote it, even though it contains spaces.
Quoting the command line causes cmd to treat the whole thing as a command name!

Secondly, backslashes at the end of a quoted string also need to be escaped. The
quoting function should be:

_cmdLineEscapeRe = re.compile(r'(\\*)"')
_cmdLineEscapeQuotedRe = re.compile(r'(\\*)("|$)')
def _cmdLineEscapeQuoted(m):
    return m.group(1) + m.group(1) + (m.group(2) and '\\"' or '')
def _cmdLineQuote(s):
    if (" " in s) or ("\t" in s):
        return '"' + _cmdLineEscapeQuoteRe.sub(_cmdLineEscapeQuoted, s) + '"'
    else:
        return _cmdLineEscapeRe.sub(r'\1\1\\"', s)

[bwh]

Following on from my first point, can I suggest that spawnProcess on Windows
should accept command line strings as well as argument sequences?

[exarkun]

Can you supply a unit test for the quoting change?  I recommend changing
test_process.ProcessTestCase.testCommandLine to cover the behavior.

[glyph]

The quoting fix looks correct, however, the reason that spawnProcess does
quoting the way that it does is that the C runtime does dequoting according to
the same rules.  I don't see a reason to add pass-arguments-as-string to
spawnProcess - I think that your example can be easily satisfied by other means.
 For example, if you want to run "cmd /c dir /?", you can pass that as ["cmd",
"/c", "dir", "/?"], which (although certainly not portable) at least doesn't
require a new API or a different data structure.  The behaviour of cmd /c as
opposed to sh -c seems to be simply the difference between a function that takes
a list as an argument vs. a function that takes *args as an argument.

[bwh]

The quoting/escaping function works with programs that use the Microsoft C
run-time library or CommandLineToArgvW or something similar to parse their
command-line, which covers most Windows console programs. However, there are
plenty of programs that parse differently for backward-compatibility with DOS or
because they are primarily GUI programs and don't have a main() function. I want
(and I believe I need) a way to run processes with arbitrary command-lines.
Here's another example:

    cmd /c set foo=^"bar && baz

This will run baz with the variable foo="bar in its environment. Escaping the
double-quote in the usual way does *not* work for the set command. I don't
believe it's possible to call spawnProcess in such a way that it will run this
command line.

How about adding a spawnCommand that runs 'cmd /c ' + command on Windows and is
equivalent to spawnProcess(..., '/bin/sh', ['sh', '-c', command], ...) on Posix
systems?

[bwh]

Sorry, that was an insufficiently weird example. How about:

    cmd /c set foo=  ^"bar  && baz

This will set the value of foo to '  "bar  '.

[exarkun]

Quoting rules should be defined by ProcessProtocol. The default should do what people usually expect, but making it overridable will provide support for ridiculous programs which can't deal with correct quoting rules.

[exarkun]

Also, "" isn't quoted properly.

[exarkun]

#3933 was a duplicate of this.

[khorn]

see also: #3876