From 4cad7fac78ac0a637fcfa7a60ed8b1ac306b0499 Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Sat, 30 Sep 2017 20:59:19 -0700 Subject: [PATCH 1/4] [feature] escape last backslash before adding quote --- .../engine/NativeCommandParameterBinder.cs | 10 +++++++++- .../NativeExecution/NativeCommandArguments.Tests.ps1 | 7 +++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/src/System.Management.Automation/engine/NativeCommandParameterBinder.cs b/src/System.Management.Automation/engine/NativeCommandParameterBinder.cs index c00f62823e2..9fcd2c5ee47 100644 --- a/src/System.Management.Automation/engine/NativeCommandParameterBinder.cs +++ b/src/System.Management.Automation/engine/NativeCommandParameterBinder.cs @@ -197,7 +197,15 @@ private void appendOneNativeArgument(ExecutionContext context, object obj, char { _arguments.Append('"'); _arguments.Append(arg); - _arguments.Append('"'); + // if trailing backslash, we need to escape it so that it doesn't escape the quotes + if (arg.EndsWith('\\')) + { + _arguments.Append("\\\""); + } + else + { + _arguments.Append('"'); + } } else { diff --git a/test/powershell/Language/Scripting/NativeExecution/NativeCommandArguments.Tests.ps1 b/test/powershell/Language/Scripting/NativeExecution/NativeCommandArguments.Tests.ps1 index aba3637c034..48efebd612a 100644 --- a/test/powershell/Language/Scripting/NativeExecution/NativeCommandArguments.Tests.ps1 +++ b/test/powershell/Language/Scripting/NativeExecution/NativeCommandArguments.Tests.ps1 @@ -31,4 +31,11 @@ Describe "Native Command Arguments" -tags "CI" { $lines[0] | Should Be 'Arg 0 is ' $lines[1] | Should Be 'Arg 1 is ' } + + It "Should correctly quote paths with spaces" { + $lines = testexe -echoargs '.\test 1\' '.\test 2\' + ($lines | measure).Count | Should Be 2 + $lines[0] | Should Be 'Arg 0 is <.\test 1\>' + $lines[1] | Should Be 'Arg 1 is <.\test 2\>' + } } From d3095218381b6e1201226830cfd901cc1b987298 Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Sun, 1 Oct 2017 08:02:36 -0700 Subject: [PATCH 2/4] [feature] modify test to also use double quotes --- .../Scripting/NativeExecution/NativeCommandArguments.Tests.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/powershell/Language/Scripting/NativeExecution/NativeCommandArguments.Tests.ps1 b/test/powershell/Language/Scripting/NativeExecution/NativeCommandArguments.Tests.ps1 index 48efebd612a..d2a65bbc5c1 100644 --- a/test/powershell/Language/Scripting/NativeExecution/NativeCommandArguments.Tests.ps1 +++ b/test/powershell/Language/Scripting/NativeExecution/NativeCommandArguments.Tests.ps1 @@ -33,7 +33,7 @@ Describe "Native Command Arguments" -tags "CI" { } It "Should correctly quote paths with spaces" { - $lines = testexe -echoargs '.\test 1\' '.\test 2\' + $lines = testexe -echoargs '.\test 1\' ".\test 2\" ($lines | measure).Count | Should Be 2 $lines[0] | Should Be 'Arg 0 is <.\test 1\>' $lines[1] | Should Be 'Arg 1 is <.\test 2\>' From bc01df64cd4124dfb6af61e28ad76e391210bef7 Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Sun, 1 Oct 2017 11:25:14 -0700 Subject: [PATCH 3/4] [feature] support multiple trailing backslashes --- .../engine/NativeCommandParameterBinder.cs | 20 +++++++++++++++---- .../NativeCommandArguments.Tests.ps1 | 19 +++++++++++------- 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/src/System.Management.Automation/engine/NativeCommandParameterBinder.cs b/src/System.Management.Automation/engine/NativeCommandParameterBinder.cs index 9fcd2c5ee47..350ce82ef65 100644 --- a/src/System.Management.Automation/engine/NativeCommandParameterBinder.cs +++ b/src/System.Management.Automation/engine/NativeCommandParameterBinder.cs @@ -196,16 +196,28 @@ private void appendOneNativeArgument(ExecutionContext context, object obj, char if (NeedQuotes(arg)) { _arguments.Append('"'); - _arguments.Append(arg); - // if trailing backslash, we need to escape it so that it doesn't escape the quotes + // need to escape all trailing backslashes so the native command receives it correctly if (arg.EndsWith('\\')) { - _arguments.Append("\\\""); + // find index of last character that isn't a backslash + var chars = arg.ToCharArray(); + int index = 0; + for (int i = chars.Length-1; i > 0; i--) + { + if (chars[i] != '\\') + { + index = i; + break; + } + } + _arguments.Append(arg.Substring(0, index)); + _arguments.Append(arg.Substring(index).Replace("\\","\\\\")); } else { - _arguments.Append('"'); + _arguments.Append(arg); } + _arguments.Append('"'); } else { diff --git a/test/powershell/Language/Scripting/NativeExecution/NativeCommandArguments.Tests.ps1 b/test/powershell/Language/Scripting/NativeExecution/NativeCommandArguments.Tests.ps1 index d2a65bbc5c1..aaa82eb40c3 100644 --- a/test/powershell/Language/Scripting/NativeExecution/NativeCommandArguments.Tests.ps1 +++ b/test/powershell/Language/Scripting/NativeExecution/NativeCommandArguments.Tests.ps1 @@ -8,7 +8,7 @@ Describe "Native Command Arguments" -tags "CI" { It "Should handle quoted spaces correctly" { $a = 'a"b c"d' $lines = testexe -echoargs $a 'a"b c"d' a"b c"d - ($lines | measure).Count | Should Be 3 + $lines.Count | Should Be 3 $lines[0] | Should Be 'Arg 0 is ' $lines[1] | Should Be 'Arg 1 is ' $lines[2] | Should Be 'Arg 2 is ' @@ -27,15 +27,20 @@ Describe "Native Command Arguments" -tags "CI" { # looking at how it got the arguments. It "Should handle spaces between escaped quotes" { $lines = testexe -echoargs 'a\"b c\"d' "a\`"b c\`"d" - ($lines | measure).Count | Should Be 2 + $lines.Count | Should Be 2 $lines[0] | Should Be 'Arg 0 is ' $lines[1] | Should Be 'Arg 1 is ' } - It "Should correctly quote paths with spaces" { - $lines = testexe -echoargs '.\test 1\' ".\test 2\" - ($lines | measure).Count | Should Be 2 - $lines[0] | Should Be 'Arg 0 is <.\test 1\>' - $lines[1] | Should Be 'Arg 1 is <.\test 2\>' + It "Should correctly quote paths with spaces: " -TestCases @( + @{arguments = "'.\test 1\' `".\test 2\`"" ; expected = @(".\test 1\",".\test 2\")}, + @{arguments = "'.\test 1\\\' `".\test 2\\`""; expected = @(".\test 1\\\",".\test 2\\")} + ) { + param($arguments, $expected) + $lines = Invoke-Expression "testexe -echoargs $arguments" + $lines.Count | Should Be $expected.Count + for ($i = 0; $i -lt $lines.Count; $i++) { + $lines[$i] | Should Be "Arg $i is <$($expected[$i])>" + } } } From 0deab6a603e4744c7627748bedd76dcfb7148039 Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Mon, 2 Oct 2017 15:08:04 -0700 Subject: [PATCH 4/4] [feature] address PR feedback --- .../engine/NativeCommandParameterBinder.cs | 22 ++++--------------- 1 file changed, 4 insertions(+), 18 deletions(-) diff --git a/src/System.Management.Automation/engine/NativeCommandParameterBinder.cs b/src/System.Management.Automation/engine/NativeCommandParameterBinder.cs index 350ce82ef65..44c298c776e 100644 --- a/src/System.Management.Automation/engine/NativeCommandParameterBinder.cs +++ b/src/System.Management.Automation/engine/NativeCommandParameterBinder.cs @@ -197,25 +197,11 @@ private void appendOneNativeArgument(ExecutionContext context, object obj, char { _arguments.Append('"'); // need to escape all trailing backslashes so the native command receives it correctly - if (arg.EndsWith('\\')) - { - // find index of last character that isn't a backslash - var chars = arg.ToCharArray(); - int index = 0; - for (int i = chars.Length-1; i > 0; i--) - { - if (chars[i] != '\\') - { - index = i; - break; - } - } - _arguments.Append(arg.Substring(0, index)); - _arguments.Append(arg.Substring(index).Replace("\\","\\\\")); - } - else + // according to http://www.daviddeley.com/autohotkey/parameters/parameters.htm#WINCRULESDOC + _arguments.Append(arg); + for (int i = arg.Length-1; i >= 0 && arg[i] == '\\'; i--) { - _arguments.Append(arg); + _arguments.Append('\\'); } _arguments.Append('"'); }