Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Quotation in System.Process.system for Windows #51

Open
thomie opened this issue Jan 6, 2016 · 5 comments
Open

Quotation in System.Process.system for Windows #51

thomie opened this issue Jan 6, 2016 · 5 comments

Comments

@thomie
Copy link
Contributor

thomie commented Jan 6, 2016

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
Copy link
Contributor Author

thomie commented Jan 6, 2016

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
Copy link
Contributor Author

thomie commented Jan 6, 2016

Note that GHC 8 will drop support for Windows XP, according to ghc/ghc@6e6438e. So I guess the suggested fix might be ok.

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

@snoyberg
Copy link
Collaborator

snoyberg commented Jan 7, 2016

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

@spacekitteh
Copy link

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

@stkb
Copy link

stkb commented May 28, 2020

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.

hasufell added a commit to hasufell/process that referenced this issue Jul 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants