properly escape trailing backslash so that it doesn't end up escaping the quotes #4965
Conversation
SteveL-MSFT
requested review from
BrucePay,
daxian-dbw and
vors
as code owners
|
Since not every application use cmd /c 'echo \'will print Also you should not only double one cmd /c 'echo \\\'will print |
| } | ||
|
|
||
| It "Should correctly quote paths with spaces" { | ||
| $lines = testexe -echoargs '.\test 1\' '.\test 2\' |
There was a problem hiding this comment.
Should we check double quotes?
|
@PetSerAl for |
|
How about multiple trailing testexe -echoargs '.\test 1\\' ".\test 2\\" |
|
@PetSerAl I'll work on that and add a test |
|
Apparently these strange trailing backslash and double quotes rules are documented |
| // just the normal double quotes, no other special quotes. Also note that mismatched | ||
| // quotes are supported | ||
| if (NeedQuotes(arg)) | ||
| { |
There was a problem hiding this comment.
How about:
_arguments.Append('"');
_arguments.Append(arg);
for (int i = arg.Length-1; i >= 0 && arg[i] == '\\'; i--)
{
_arguments.Append('\\');
}
_arguments.Append('"'); ?
There was a problem hiding this comment.
That's much better than my attempt. Will change.
| for (int i = arg.Length-1; i >= 0 && arg[i] == '\\'; i--) | ||
| { | ||
| _arguments.Append(arg); | ||
| _arguments.Append('\\'); |
There was a problem hiding this comment.
Shouldn't it be "\\\\"?
There was a problem hiding this comment.
No, unless you want to triplicate trailing \. arg already have one copy of \. This just adding second copy on top of it.
|
@daxian-dbw @lzybkr Could you please review? |
1 similar comment
|
@daxian-dbw @lzybkr Could you please review? |
|
@daxian-dbw is in training today, @anmenaga can you review? |
|
Sorry that I missed the the first "@mention" notification. I will take a look later today. |
5 participants
When we create the arg string, an arg of
.\test 1\ends up being".\test 1\"which is stored in StartInfo.Arguments which then corefx passes as-is to SHELLEXECUTEINFO.The native command receives the arg as
.\test 1"as the last\"is treated as escaping the quotes. Fix is to add an extra backslash to escape the last enclosing quote.Fix #4358