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

Invalid handling of parameter values starting with double quotes ("), e.g. --order-by='"quoted" value' #891

Closed
sandreas opened this issue Jul 5, 2022 · 2 comments
Labels
bug Something isn't working

Comments

@sandreas
Copy link

sandreas commented Jul 5, 2022

Information

  • OS: Linux, Ubuntu 18.04 LTS
  • Version: 0.44
  • Terminal: xterm-256color, zsh

Describe the bug
Parameter values cannot start with double quotes ("), they are either replaced or handled completely wrong. Additionally, parameter values are TRIMMED (remove leading spaces), so adding a space to workaround this is not possible.

The following example even cuts off the parameter value and treats everything after the space as EXTRA argument, in the following case value is handled as positional argument and not part of --order-by:

cli-tester test --order-by '"quoted" value' input.mp3

This may be a security flaw under specific circumstances.

More examples of invalid handling:

# `quoted` instead of `"quoted" value` and cut off handling as positional argument
cli-tester test --order-by '"quoted" value' input.mp3
cli-tester test --order-by='"quoted" value' input.mp3

# `quoted` instead of `"quoted"` 
cli-tester test --order-by '"quoted"' input.mp3

# `quoted` instead of ` "quoted"` 
cli-tester test --order-by ' "quoted"' input.mp3

The following examples work like expected, if not starting with double quotes or single quotes are used

# `value "quoted" like expected
cli-tester test --order-by 'value "quoted"' input.mp3

# `'quoted' value` like expected
cli-tester test --order-by "'quoted' value" input.mp3

# `\"quoted" value' value` like expected
cli-tester test --order-by='\"quoted" value' input.mp3

To Reproduce

Expected behavior
I would expect double quotes (") and spaces ( ) CAN be a valid part of a parameter value and should not be replaced or parsed out in any way.

@sandreas sandreas added the bug Something isn't working label Jul 5, 2022
@sandreas sandreas changed the title Invalid handling of parameter values starting with double quotes ("), e.g. --meta-description='"quoted" value' Invalid handling of parameter values starting with double quotes ("), e.g. --order-by='"quoted" value' Jul 5, 2022
@phil-scott-78
Copy link
Contributor

Unfortunately, this is out of our hands. We are just parsing the args that .NET gets from its command line handler. In fact, on Windows I get different results from you with different examples.

David Deley has a nice resource keeping track of all the nuisances.

I'm gonna go ahead and close this because as you can tell from David's doc there are just so many possible combinations of behaviors there really isn't anyway we'd be able to rework this in a logical way so we'll continue relying on the OS and framework's behaviors.

@sandreas
Copy link
Author

sandreas commented Jul 9, 2022

Unfortunately, this is out of our hands. We are just parsing the args that .NET gets from its command line handler. In fact, on Windows I get different results from you with different examples.

@phil-scott-78 Thank you for your response, but I still think, that it IS a problem in spectre.console. On Windows, single quotes (') CANNOT be used to enclose arguments, so on Windows this problem might not be reproducible, because only double quotes (") are supported for enclosure.

On POSIX you can either use double quotes or single quotes (" or ') to enclose an argument - the difference is, that in double quotes shell variables are evaluated (e.g. echo --path="$HOME" => --path=/home/sandreas, while echo --path='$HOME' => --path=$HOME)

It would be a HUGE problem in dotnet to not parse command line args correctly on macOS and Linux, so I tried the code example below with the according shell command to verify it is NOT a dotnet problem (maybe I'm still wrong, but my example works correctly without using spectre.console and FAILS when I try to integrate it):

// Program.cs, dotnet 6
    Console.WriteLine("== Environment.CommandLine ==");
    Console.WriteLine(Environment.CommandLine);
    Console.WriteLine("");

    Console.WriteLine("==  Environment.GetCommandLineArgs() ==");
    foreach(var a in Environment.GetCommandLineArgs()){
        Console.WriteLine(a);
    }
    Console.WriteLine("");

    Console.WriteLine("== args ==");
    foreach(var b in args){
        Console.WriteLine(b);
    }
    return 0;

Output:

./dist/cli-tester test --order-by '"quoted" value' input.mp3
== Environment.CommandLine ==
/home/sandreas/projects/tone/cli-tester/dist/cli-tester.dll test --order-by "\"quoted\" value" input.mp3

==  Environment.GetCommandLineArgs() ==
/home/sandreas/projects/tone/cli-tester/dist/cli-tester.dll
test
--order-by
"quoted" value
input.mp3

== args ==
test
--order-by
"quoted" value
input.mp3

Side note: If you don't really compile and run the binary, but try with the debugger / IDE, it might reorganize the command arguments you are running and break the args string WITHIN the IDE - this happened to me in JetBrains Rider and resulted in:

== Environment.CommandLine ==
/home/sandreas/projects/tone/cli-tester/bin/Debug/net6.0/cli-tester.dll test --order-by 'quoted value' input.mp3

==  Environment.GetCommandLineArgs() ==
/home/sandreas/projects/tone/cli-tester/bin/Debug/net6.0/cli-tester.dll
test
--order-by
'quoted
value'
input.mp3

== args ==
test
--order-by
'quoted
value'
input.mp3

So looking at the args dump for a real compiled program, which I use in my example, the "quoted" value is definitely preserved as I would expect. That makes me think something happens with args in CommandApp.RunAsync(args)

I'm gonna go ahead and close this

Could you please verify my results from above with a real compiled program on a POSIX system? That would be awesome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants