process icon indicating copy to clipboard operation
process copied to clipboard

Quotation in System.Process.system for Windows

Open thomie opened this issue 10 years ago • 6 comments

Moved from https://ghc.haskell.org/trac/ghc/ticket/5376:

Wagnerdm writes: I had a bit of fun recently tracking down quoting issues with the "system" command in Windows. For the examples below, I'll consistently use "Windows> " as the beginning of some text sent to the Windows command prompt cmd.exe, and use "GHC> " as the beginning of some text sent to a ghci session running in cmd.exe with System.Cmd imported.

The situation is this: I want to hand off a command line which has both a possibly-quoted command name and a (definitely) quoted argument. For concreteness, let's use "more" as the command and "foo.txt" as the argument, so that you can follow along at home on your favorite Windows system.

Windows> echo foo > foo.txt
Windows> "more" "foo.txt"
foo

All good so far. But:

GHC> system "\"more\" \"foo.txt\""
'more" "foo.txt' is not recognized as an internal or external command, operable program or batch file.
ExitFailure 1

After some digging, I discovered that system is shipping out to cmd /c, and indeed:

Windows> cmd /c "more" "foo.txt"
'more" "foo.txt' is not recognized as an internal or external command, operable program or batch file.

I don't know what the right fix is. However, after a bit of playing around, I discovered the following:

Windows> cmd /c ""more" "foo.txt""
foo
GHC> system "\"\"more\" \"foo.txt\"\""
foo
ExitSuccess

Wrapping commands with an extra pair of double-quotes this way seemed to give behavior matching the bare cmd.exe for all the examples I could think of, even ones I thought it would break. For example:

GHC> system "\"more foo.txt\""
foo
ExitSuccess

If this turns out to be the right thing to do, it's pretty easy to implement. In the commandToProcess function, at libraries/process/System/Process/Internals.hs:455, the change is just

-   return (cmd, translate cmd ++ "/c " ++ string)
+   return (cmd, translate cmd ++ "/c \"" ++ string ++ "\"")

(And in any case, the examples above should answer this amusing comment, immediately following those lines:

    -- We don't want to put the cmd into a single
    -- argument, because cmd.exe will not try to split it up.  Instead,
    -- we just tack the command on the end of the cmd.exe command line,
    -- which partly works.  There seem to be some quoting issues, but
    -- I don't have the energy to find+fix them right now (ToDo). --SDM
    -- (later) Now I don't know what the above comment means.  sigh.

Later, Brandon comments: What that comment means is that how CMD.EXE handles spaces is Windows-version-dependent. On the other hand, I think it's mostly consistent between XP and Windows 7 --- and I feel sorry for anyone forced to use an older version.

thomie avatar Jan 06 '16 09:01 thomie

simonmar writes in https://ghc.haskell.org/trac/ghc/ticket/5376#comment:3:

The comment worries me a bit, because it implies that when I tried this before it didn't work. Perhaps, as Brandon says, that is because the behaviour changed in Windows at some point (though I find it slightly hard to believe that this was on a Windows version earlier than XP).

Note that the behaviour should be much more reliable if you don't go via cmd, and run the command directly. I believe we do get the quoting right in that case.

If you want to take this forward, I suggest writing a few tests as well as the patch itself.

thomie avatar Jan 06 '16 09:01 thomie

Note that GHC 8 will drop support for Windows XP, according to https://github.com/ghc/ghc/commit/6e6438e15f33cb94ad6338e950e693f59d046385. So I guess the suggested fix might be ok.

-   return (cmd, translate cmd ++ "/c " ++ string)
+   return (cmd, translate cmd ++ "/c \"" ++ string ++ "\"")

thomie avatar Jan 06 '16 09:01 thomie

I'm definitely open to a PR for a behavior change + test cases.

snoyberg avatar Jan 07 '16 07:01 snoyberg

I've run into this problem, just trying to run a process with command line arguments - it wraps every argument in quotes. And this is using proc and withCreateProcess.

See: https://github.com/spacekitteh/sc2hs/blob/master/src/Network/SC2/Process.hs#L85

spacekitteh avatar Jan 28 '19 01:01 spacekitteh

Ran into this issue with purescript/spago#635. To summarize what I wrote there, cmd.exe is called incorrectly here. It should be called with the /d and /s flags, and lastly /c and then the whole command-line you want to run, wrapped in another set of quotes.

You can see how (for example) Node.js does it here.

The /d flag is like --noprofile/--norc for Bash, and the /s opts out of special quote processing.

I can submit a PR for this, but if tests are also required then I'd appreciate some pointers.

stkb avatar May 28 '20 17:05 stkb