Parse Environment.CommandLine directly vs trusting string[] main

[EricZimmerman]

Depending on the shell being used, project does not get strings ending in " right due to broken args parsing (a long-standing issue it seems)

powershell 6.x seems to work as expected, but 5.x does not

see
https://twitter.com/jonsequitur/status/1077619442073231360
for more

here is another example (i think) if bad parsing due to " at the end when constructing FileInfo object. may be related to the malformed path as outlined above as well.

[EricZimmerman]

just did a test of the last scenario re:FileInfo. it too is resolved once i installed PowerShell Core 6.x

[zivkan]

I recently had to investigate argument processing with regards to quoted paths for nuget.exe. From what I learned, I disagree that the reported issue is a bug. As Jon said on twitter, it's all about the shell. My concern is that changing System.CommandLine to work with these cases when the shell is cmd.exe or powershell 5 might break expected behaviour when using other shells, or might not work in all cases.

@EricZimmerman can you try your tests again but this time passing --file-option as the first argument, not the last one? I'm curious what happens when there are other arguments after the file argument. also what happens when there are two or more file arguments.

I haven't yet started playing around with System.CommandLine, but I used this little program to test with:

    static void Main(string[] args)
    {
        Console.WriteLine("args:");
        for (int i = 0; i < args.Length; i++)
        {
            Console.WriteLine("{0}: [{1}]", i, args[i]);
        }

        Console.WriteLine();
        Console.WriteLine("System.Environment.CommandLine:");
        Console.WriteLine(Environment.CommandLine);
    }

Both cmd.exe and bash (I tested git bash from git for windows. I didn't test that bash on linux or mac behave the same) give me the following results:

>dotnet CommandLineTest.dll --one 1 --two "c:\two\" --three 3 --four "c:\four\"
args:
0: [--one]
1: [1]
2: [--two]
3: [c:\two" --three 3 --four c:\four"]

System.Environment.CommandLine:
C:\Users\andyz\source\repos\CommandLineTest\bin\Debug\netcoreapp2.2\publish\CommandLineTest.dll --one 1 --two "c:\two\" --three 3 --four c:\four\""

while powershell (both 5 and 6) give me:

> dotnet CommandLineTest.dll --one 1 --two "c:\two\" --three 3 --four "c:\four\"
args:
0: [--one]
1: [1]
2: [--two]
3: [c:\two\]
4: [--three]
5: [3]
6: [--four]
7: [c:\four\]

System.Environment.CommandLine:
C:\Users\andyz\source\repos\CommandLineTest\bin\Debug\netcoreapp2.2\publish\CommandLineTest.dll --one 1 --two c:\two\ --three 3 --four c:\four\

Bash and cmd.exe let you put the quotes in ridiculous places, but they both appear to normalise the strings before executing my program.

>dotnet CommandLineTest.dll "c:\path with space\\"
args:
0: [c:\path with space\]

System.Environment.CommandLine:
C:\Users\andyz\source\repos\CommandLineTest\bin\Debug\netcoreapp2.2\publish\CommandLineTest.dll "c:\path with space\\"

>dotnet CommandLineTest.dll c:\\"path with space"\
args:
0: [c:\path with space\]

System.Environment.CommandLine:
C:\Users\andyz\source\repos\CommandLineTest\bin\Debug\netcoreapp2.2\publish\CommandLineTest.dll "c:\path with space\\"

>dotnet CommandLineTest.dll c:\pat"h w"it"h s"pace\
args:
0: [c:\path with space\]

System.Environment.CommandLine:
C:\Users\andyz\source\repos\CommandLineTest\bin\Debug\netcoreapp2.2\publish\CommandLineTest.dll "c:\path with space\\"

What's interesting in Bash is that \" is clearly an escaped quote, so that the quote character gets put in the argument, while a " is used to start and end string literals. If a string literal is not terminated, then Bash asks for the next line of the command line, and keeps doing so until the string literal is terminated:

$ dotnet CommandLineTest.dll "one two
> three
> four"
args:
0: [one two
three
four]

System.Environment.CommandLine:
C:\Users\andyz\source\repos\CommandLineTest\bin\Debug\netcoreapp2.2\publish\CommandLineTest.dll "one two
three
four"

I haven't found a way to put new line characters in arguments in cmd.exe or powershell. They both implicitly end the string literal at the end of the line.

What I find most annoying is the infuriating way that cmd.exe treats backslashes. It's only an escaping character when the last backslash in a sequence of backslashes is followed by a double quote character

>dotnet CommandLineTest.dll "c:\path\"
args:
0: [c:\path"]

System.Environment.CommandLine:
C:\Users\andyz\source\repos\CommandLineTest\bin\Debug\netcoreapp2.2\publish\CommandLineTest.dll "c:\path\""

>dotnet CommandLineTest.dll "c:\\path\\"
args:
0: [c:\\path\]

System.Environment.CommandLine:
C:\Users\andyz\source\repos\CommandLineTest\bin\Debug\netcoreapp2.2\publish\CommandLineTest.dll c:\\path\

>dotnet CommandLineTest.dll "c:\\\path\\\"
args:
0: [c:\\\path\"]

System.Environment.CommandLine:
C:\Users\andyz\source\repos\CommandLineTest\bin\Debug\netcoreapp2.2\publish\CommandLineTest.dll "c:\\\path\\\""

>dotnet CommandLineTest.dll "c:\\\\path\\\\"
args:
0: [c:\\\\path\\]

System.Environment.CommandLine:
C:\Users\andyz\source\repos\CommandLineTest\bin\Debug\netcoreapp2.2\publish\CommandLineTest.dll c:\\\\path\\

bash always treats a \ as an escape character, so if you want to put that character in a command line argument, you must always use \\, for example "\\\\server\\share\\folder\\", unlike cmd.exe which would be "\\server\share\folder\\". Powershell doesn't treat \ as an escape character at all, but I also can't figure out how to put a quote in the argument list passed to my program.

Personally, my recommendation is to try to convince the world to use / as the path separator, even on Windows. Then these quoted directory names will not be a problem at all. .NET's System.IO APIs support both forward and back slashes, although I don't know if that's .NET specific, or if Windows itself does. But I know there's no chance of achieving this, or Windows ever switching to using a forward slash as the default path separator, so my recommendation is very much idealistic. A more reasonable recommendation is to keep using Main string[] args, and educate anyone who encounters this issue.

In conclusion, if System.CommandLine switches from using Main's string[] args to using Environment.CommandLine to avoid the quoted directory problem in cmd.exe when using Windows path separators, I'm concerned that it might introduce real bugs if customers try to use it in different ways. For example, putting quote characters in an argument, or a quote character in a string literal, or arguments with new line characters. I only tested cmd.exe, powershell and bash, I didn't test other unix shells like tcsh, ksh, zsh and countless others. If parsing does change to use Environment.CommandLine, I think you should implement a whole lot of tests so that the most common patterns on the most common shells are tested.

All of this only because Windows uses the same character for path separation as the escape character :(

[KathleenDollard]

Going back to the original issue...

This may be a docs issue. We still need to resolve guidance on this, but the original code passes "args" explicitly. It could pass something else explicitly.

OTOH, this conversation makes me wonder if there is a feature here to provide a way to retrieve args that works correctly across shells and platforms.

[EricZimmerman]

that's the entire point I would say. I do not want my end users to have to do 3 different things depending on what shell they have, if its a Tuesday, etc.

the way the parsing is now in .net with args seems wrong/broken. every single command line parsing tool out there i have looked at as this exact problem because they are handed args in an incorrect way, because of how the closing quote is escaped.

IMO, end users should be able to type a valid path with spaces, ending with or without the slash, and have it work as expected.

why not roll the parser you guys built for this into .net proper,replacing whatever is generating args right now? fix the problem upstream, and the issue in this project goes away too (then again, who wants to wait for .net updates when this can handle things? =] )

[jonsequitur]

I suspect that changing this behavior upstream would break all kinds of things. But since System.CommandLine is a new API maybe we have an opportunity to make it behave more intuitively from the outset.

[zivkan]

After some testing, I found that:

• Windows allows either \ or / to be used as a path separator and does not allow " (or various other characters) in the file system entry name
• Linux only uses / as a path separator, and allows both \ and " as part of a file or directory name (from what I can tell, it allows everything other than / and the null terminator)

I don't have a Mac to test on, but I'm pretty confident it will work the same as Linux.

Since I value cross-platform support (meaning I prefer a library to support whatever the platform supports. I do not want to use a library that supports only the lowest common denominator across all platforms), if this feature is to be implemented, I propose that \" be treated as terminating a string literal when at the end of the command line, or when followed immediately by whitespace, but in both cases only on Windows, and only when parsing the value of a file or directory argument (maybe only directory arguments?). Do not do this when parsing string arguments. Consider for example the following command line git commit -m "add \"something\" to whitelist". Blindly treating all \" as terminating string literals would prevent strings with double quote characters in them and therefore break this use case.

I believe this should work because on Windows we can relatively safely assume that "path\" represents path\ because it's a valid path ending in the path separator. We can assume that it does not represent path" because Windows doesn't allow " in the file system entry name.

However, on Linux \ is not accepted as a path separator, and both \ and " are allowed in file and directory names. This means if \" was changed to \, then on Linux if a user wanted to use a file or directory name that has a double quote character, System.CommandLine would need some non-standard syntax to support it, or it wouldn't be possible at all. Therefore on Linux "path\" should continue to be parsed as path" (and the string literal is still not terminated) and not path\ (correctly terminated string literal), like on Windows.

Having written all that, I'm still against this because

• Having the library behave differently on Windows and Linux doesn't feel like a good thing to do
• While I agree with apps improving their UI to help users be successful is important, if this is implemented, then apps using System.CommandLine will behave differently to apps not using System.CommandLine and therefore a user running multiple apps (either interactively or though a script) might be more confused about the different behaviours (why does app x work by app y doesn't?). Is it more important to help users be successful with only your app, or educate users to be successful with all apps?
• I didn't even think about the other differences between shells that need to be handled if the goal is to allow a user to use the same command line across all different shells and have it work the same way, even on a single platform. This comment deals only with directories with trailing slash. It doesn't consider what work System.CommandLine would need to do to handle multiple slashes in the middle or beginning on paths, or what to do when the user correctly escapes paths in their shell ("\\server\share\path\" vs "\\\\server\\share\\path\\" in bash while it's "\\server\share\path\\" in cmd.exe). If this is implemented, there probably needs to be quite a few unit tests to handle different command lines within a single shell, and the same command line used in different shells.

[jnm2]

All Environment.CommandLine does is escape Environment.GetCommandLineArgs() back into a single string. It doesn't give you anything that Main's string[] args doesn't.

https://github.com/dotnet/corefx/blob/v2.2.2/src/System.Runtime.Extensions/src/System/Environment.cs#L62-L68

        public static string CommandLine
        {
            get
            {
                return PasteArguments.Paste(GetCommandLineArgs(), pasteFirstArgumentUsingArgV0Rules: true);
            }
        }

P/invoking GetCommandLine if on Windows is an option though.

[jnm2]

Regardless of shell, Windows' native OS representation of arguments to a process is a single string. Regardless of shell, Linux's native OS representation is an actual string array. Since System.CommandLine does want to end up with a string array, it should just use string[] args. Otherwise we're needlessly roundtripping to and from a Windowsism on Linux.

Arguments to a process are shell-agnostic. They are an interface to the process and it's completely up to the process to decide how to interpret the single string (in the case of Windows) or the string array (Linux). It's the job of the shell script to make sure it's constructing the appropriate single string (Windows) or string array (Linux) that it's sending to the process it's starting, just as though you're calling a C# method that takes a single string or one that takes a string array. The process, like this analogous method, gets to make all the rules.

Properly escaping and unescaping from one to the other requires some sort of standard. The de facto standard on Windows is C's argv which is tricky (almost everyone gets it wrong). Please see https://blogs.msdn.microsoft.com/twistylittlepassagesallalike/2011/04/23/everyone-quotes-command-line-arguments-the-wrong-way/.

Also:

• https://docs.microsoft.com/cpp/cpp/parsing-cpp-command-line-arguments

• Proper implementation tests: https://github.com/nunit/nunit3-vs-adapter/blob/a7513e0bfa473a659a80942afd863e8779418164/src/NUnit.TestAdapter.Tests.Acceptance/WorkspaceTools/ProcessUtilsTests.cs#L7-L89

• Proper implementation: https://github.com/nunit/nunit3-vs-adapter/blob/a7513e0bfa473a659a80942afd863e8779418164/src/NUnit.TestAdapter.Tests.Acceptance/WorkspaceTools/ProcessUtils.cs#L76-L160

• "Value of Environment.CommandLine is different with the actual input:" https://github.com/dotnet/corefx/issues/29014

• New ProcessStartInfo.ArgumentList API: https://github.com/dotnet/corefx/issues/23592

[brandonschmidt]

@jnm2 -- So in a Windows shell, what's the best way currently to get a string array of args?

ProcessStartInfo.ArgumentList isn't available to the current executing process. For example, this will throw an exception:

Console.WriteLine($"Throws exception: {System.Diagnostics.Process.GetCurrentProcess().StartInfo.ArgumentList}");

This code does ultimately call a native OS library, but is that implementation correct?

Environment.GetCommandLineArgs()

[jnm2]

This code does ultimately call a native OS library, but is that implementation correct?

Yes, both the string[] passed to Main and Environment.GetCommandLineArgs() do parse the command line the using the standard argv rules.

The downside of Environment.GetCommandLineArgs() is that it doesn't exclude the first command line argument myapp.exe or the first two arguments in the case of the first being dotnet and the second being myapp.dll.

string[] passed to Main should be trusted.

[brandonschmidt]

OK, so to summarize:

The reported issue is caused by the shell tool being used on Windows. The issue is not caused by string[] args on Main, System.Environment.CommandLine, or System.Environment.GetCommandLineArgs().

HOWEVER

Given what we learned, this issue COULD theoretically exist in System.CommandLine.Parsing.CommandLineStringSplitter.Split method because it is a duplication of the logic in argv.

In such a case, user-code that calls the methods below (not an exhaustive list):

System.CommandLine.ParserExtensions.Parse(this Parser parser, string commandLine)

System.CommandLine.Invocation.InvocationExtensions.InvokeAsync(this Command command, string commandLine, IConsole console = null)

would have an issue.

HOWEVER

No one has actually demonstrated such a replicable scenario yet in this ticket.

Is my understanding correct?