Make command line escaping on Windows not add superfluous double quotes

[thomasjm]

This PR is an attempt to fix #51.

As explained there, System.Process currently adds superfluous double quotes when assembling command lines on Windows. For example, if I run readCreateProcess (proc "notepad.exe" ["foo.txt"]) "", then Windows will see a command line like "notepad.exe" "foo.txt".

This quoting is extra-conservative, but it also causes a number of problems which people have reported in #51. In my case, it makes it impossible to run something like readCreateProcess (proc "wsl.exe" ["--help"]) ", because the quotes confuse wsl.exe and it tries to go into shell mode and execute commands on a VM. The quoting is also contrary to what other common process libraries do, like Python's subprocess.run.

Correctness

To show that this alternative implementation is correct, I wrote some tests in this repo: https://github.com/thomasjm/process-escape-windows. I test in the following ways:

• Using CommandLineToArgvW FFI. Using FFI, I assemble random executable names and argument strings and test that the Windows CommandLineToArgvW function parses them correctly (with QuickCheck).
• Running cmd.exe /c child.exe <args>, where child.exe is a specially crafted C program that echoes back its arguments. We have to apply an extra level of escaping for cmd.exe for this to work. This is also tested using QuickCheck.
• I also tested the original translateInternal from this repo using FFI.

Conclusions:

• The current implementation in this package is correct for normal programs! (But not for .bat files; see below)
• The new version in this PR is also correct for normal programs.

About .bat and .cmd files

This package also has a function called translateCmdExeArg, which is applied when you use RawCommand and an executable name that ends with .bat or .cmd.

I tried for a while to make a version of translateCmdExeArg that will work for arbitrary arguments, tested with QuickCheck--but ultimately succumbed to despair. Look at how long and crazy a typical StackOverflow answer is on this subject: https://stackoverflow.com/a/31413730.

For this reason, I haven't touched translateCmdExeArg in this PR. I immediately guessed that it would fail for arguments that contain double quotes. For example, I tested running readCreateProcess (proc childBat ["a\"b"]) "" using a recent Stackage resolver, using child.bat in my test repo, which echoes back the commands it receives. It left a random caret in the result: a"b^.

It's good that translateCmdExeArg has a BatBadBut remediation, but I would say users shouldn't expect it to pass their arguments faithfully through to a batch script if the arguments have unusual characters etc.

Conclusion

Hopefully this is enough validation to support merging this PR! We could also potentially bring my tests over to this package, but that would involve adding a QuickCheck dependency.

[tomjaguarpaw]

Thanks for this. I don't personally have any knowledge or understanding of the Windows process API or access to a Windows system. Maybe one of my process maintainer collaborators can help with this one.