From 8bca1f50c58709583fce981f7127d2610cf3579a Mon Sep 17 00:00:00 2001 From: Douglas Thrift Date: Sat, 3 Sep 2016 21:13:22 -0700 Subject: [PATCH 1/6] Fix passing escaped double quoted spaces to native executables --- CHANGELOG.md | 2 ++ .../engine/NativeCommandParameterBinder.cs | 6 ++++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d17e37647e4..81bb2128e56 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,8 @@ Changelog Unreleased ---------- +- Fix passing escaped double quoted spaces to native executables + v6.0.0-alpha.9 - 2016-08-15 --------------------------- diff --git a/src/System.Management.Automation/engine/NativeCommandParameterBinder.cs b/src/System.Management.Automation/engine/NativeCommandParameterBinder.cs index 14083cb80ac..62e282ca377 100644 --- a/src/System.Management.Automation/engine/NativeCommandParameterBinder.cs +++ b/src/System.Management.Automation/engine/NativeCommandParameterBinder.cs @@ -194,11 +194,11 @@ private void appendOneNativeArgument(ExecutionContext context, object obj, char // just the normal double quotes, no other special quotes. Also note that mismatched // quotes are supported. - bool needQuotes = false; + bool needQuotes = false, followingBackslash = false; int quoteCount = 0; for (int i = 0; i < arg.Length; i++) { - if (arg[i] == '"') + if (arg[i] == '"' && !followingBackslash) { quoteCount += 1; } @@ -206,6 +206,8 @@ private void appendOneNativeArgument(ExecutionContext context, object obj, char { needQuotes = true; } + + followingBackslash = arg[i] == '\\'; } if (needQuotes) From 77a4e96795f9440b414ead2abf4352f697c7aa79 Mon Sep 17 00:00:00 2001 From: Douglas Thrift Date: Sun, 11 Sep 2016 17:13:28 -0700 Subject: [PATCH 2/6] Add test/csharp/csharp.nuget.props to .gitignore --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 1d151847f6d..c34b1850462 100644 --- a/.gitignore +++ b/.gitignore @@ -5,6 +5,7 @@ project.lock.json /debug/ /staging/ /Packages/ +test/csharp/csharp.nuget.props # dotnet cli install/uninstall scripts dotnet-install.ps1 From 7816516c244627da2054140dc569e2dce08336cb Mon Sep 17 00:00:00 2001 From: Douglas Thrift Date: Sun, 11 Sep 2016 21:13:14 -0700 Subject: [PATCH 3/6] Add tests for native command arguments --- test/EchoArgs/EchoArgs.cs | 28 +++++++++++++++++ test/EchoArgs/project.json | 28 +++++++++++++++++ .../NativeCommandArguments.Tests.ps1 | 31 +++++++++++++++++++ 3 files changed, 87 insertions(+) create mode 100644 test/EchoArgs/EchoArgs.cs create mode 100644 test/EchoArgs/project.json create mode 100644 test/powershell/Scripting/NativeExecution/NativeCommandArguments.Tests.ps1 diff --git a/test/EchoArgs/EchoArgs.cs b/test/EchoArgs/EchoArgs.cs new file mode 100644 index 00000000000..88d0f5e409e --- /dev/null +++ b/test/EchoArgs/EchoArgs.cs @@ -0,0 +1,28 @@ +//--------------------------------------------------------------------- +// Author: Keith Hill +// +// Description: Very simple little console class that you can use to see +// how PowerShell is passing parameters to legacy console +// apps. +// +// Creation Date: March 06, 2006 +//--------------------------------------------------------------------- +using System; + +namespace Pscx.Applications +{ + class EchoArgs + { + static void Main(string[] args) + { + for (int i = 0; i < args.Length; i++) + { + Console.WriteLine("Arg {0} is <{1}>", i, args[i]); + } + + /*Console.WriteLine("\nCommand line:"); + Console.WriteLine(Environment.CommandLine);*/ + Console.WriteLine(); + } + } +} diff --git a/test/EchoArgs/project.json b/test/EchoArgs/project.json new file mode 100644 index 00000000000..106dcc46a71 --- /dev/null +++ b/test/EchoArgs/project.json @@ -0,0 +1,28 @@ +{ + "name": "echoargs", + "version": "1.0.0-*", + "description": "Very simple little console class that you can use to see how PowerShell is passing parameters to legacy console apps.", + + "buildOptions": { + "emitEntryPoint": true + }, + + "frameworks": { + "netcoreapp1.0": { + "dependencies": { + "Microsoft.NETCore.App": "1.0.0" + } + } + }, + + "runtimes": { + "ubuntu.16.04-x64": { }, + "ubuntu.14.04-x64": { }, + "debian.8-x64": { }, + "centos.7-x64": { }, + "win7-x64": { }, + "win81-x64": { }, + "win10-x64": { }, + "osx.10.11-x64": { } + } +} diff --git a/test/powershell/Scripting/NativeExecution/NativeCommandArguments.Tests.ps1 b/test/powershell/Scripting/NativeExecution/NativeCommandArguments.Tests.ps1 new file mode 100644 index 00000000000..bf26060cd24 --- /dev/null +++ b/test/powershell/Scripting/NativeExecution/NativeCommandArguments.Tests.ps1 @@ -0,0 +1,31 @@ +Describe "Native Command Arguments" -tags "CI" { + $testPath = 'TestDrive:\test.txt' + $powershellTestDir = $PSScriptRoot + while ($powershellTestDir -notmatch 'test[\\/]powershell$') { + $powershellTestDir = Split-Path $powershellTestDir + } + $echoArgsDir = Join-Path (Split-Path $powershellTestDir) EchoArgs + + function Start-EchoArgs { + Push-Location $echoArgsDir + try { + dotnet run $args + } finally { + Pop-Location + } + } + + It "Should handle quoted spaces correctly" { + $a = 'a"b c"d' + Start-EchoArgs $a 'a"b c"d' a"b c"d >$testPath + $testPath | Should Contain 'Arg 0 is ' + $testPath | Should Contain 'Arg 1 is ' + $testPath | Should Contain 'Arg 2 is ' + } + + It "Should handle spaces between escaped quotes" { + Start-EchoArgs 'a\"b c\"d' "a\`"b c\`"d" >$testPath + $testPath | Should Contain 'Arg 0 is ' + $testPath | Should Contain 'Arg 1 is ' + } +} From 7e8070276f30712ec39d1d69f7e4eb7ed80d6a9e Mon Sep 17 00:00:00 2001 From: Douglas Thrift Date: Sun, 11 Sep 2016 23:14:30 -0700 Subject: [PATCH 4/6] Move and "publish" EchoArgs for the tests * Move EchoArgs from test/EchoArgs to test/tools/EchoArgs * Use "dotnet publish" for building EchoArgs in build.psm1 so the test can call it directly --- .gitignore | 1 + build.psm1 | 16 ++++++++++++++++ .../NativeCommandArguments.Tests.ps1 | 15 +++------------ test/{ => tools}/EchoArgs/EchoArgs.cs | 4 ---- test/{ => tools}/EchoArgs/project.json | 0 5 files changed, 20 insertions(+), 16 deletions(-) rename test/{ => tools}/EchoArgs/EchoArgs.cs (82%) rename test/{ => tools}/EchoArgs/project.json (100%) diff --git a/.gitignore b/.gitignore index c34b1850462..99763674d59 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,6 @@ bin/ obj/ +run/ project.lock.json *-tests.xml /debug/ diff --git a/build.psm1 b/build.psm1 index ad3891a66dc..b668b3aec70 100644 --- a/build.psm1 +++ b/build.psm1 @@ -547,6 +547,20 @@ function Get-PesterTag { $o } +function Publish-EchoArgs { + [CmdletBinding()] + param() + + Find-Dotnet + + Push-Location "$PSScriptRoot/test/tools/EchoArgs" + try { + dotnet publish --output run + } finally { + Pop-Location + } +} + function Start-PSPester { [CmdletBinding()] param( @@ -563,6 +577,8 @@ function Start-PSPester { ) Write-Verbose "Running pester tests at '$path' with tag '$($Tag -join ''', ''')' and ExcludeTag '$($ExcludeTag -join ''', ''')'" -Verbose + # Publish EchoArgs so it can be run by tests + Publish-EchoArgs # All concatenated commands/arguments are suffixed with the delimiter (space) $Command = "" diff --git a/test/powershell/Scripting/NativeExecution/NativeCommandArguments.Tests.ps1 b/test/powershell/Scripting/NativeExecution/NativeCommandArguments.Tests.ps1 index bf26060cd24..6d2541ea44f 100644 --- a/test/powershell/Scripting/NativeExecution/NativeCommandArguments.Tests.ps1 +++ b/test/powershell/Scripting/NativeExecution/NativeCommandArguments.Tests.ps1 @@ -4,27 +4,18 @@ Describe "Native Command Arguments" -tags "CI" { while ($powershellTestDir -notmatch 'test[\\/]powershell$') { $powershellTestDir = Split-Path $powershellTestDir } - $echoArgsDir = Join-Path (Split-Path $powershellTestDir) EchoArgs - - function Start-EchoArgs { - Push-Location $echoArgsDir - try { - dotnet run $args - } finally { - Pop-Location - } - } + $echoArgs = Join-Path (Split-Path $powershellTestDir) tools/EchoArgs/run/echoargs It "Should handle quoted spaces correctly" { $a = 'a"b c"d' - Start-EchoArgs $a 'a"b c"d' a"b c"d >$testPath + & $echoArgs $a 'a"b c"d' a"b c"d >$testPath $testPath | Should Contain 'Arg 0 is ' $testPath | Should Contain 'Arg 1 is ' $testPath | Should Contain 'Arg 2 is ' } It "Should handle spaces between escaped quotes" { - Start-EchoArgs 'a\"b c\"d' "a\`"b c\`"d" >$testPath + & $echoArgs 'a\"b c\"d' "a\`"b c\`"d" >$testPath $testPath | Should Contain 'Arg 0 is ' $testPath | Should Contain 'Arg 1 is ' } diff --git a/test/EchoArgs/EchoArgs.cs b/test/tools/EchoArgs/EchoArgs.cs similarity index 82% rename from test/EchoArgs/EchoArgs.cs rename to test/tools/EchoArgs/EchoArgs.cs index 88d0f5e409e..ca2958fdfa0 100644 --- a/test/EchoArgs/EchoArgs.cs +++ b/test/tools/EchoArgs/EchoArgs.cs @@ -19,10 +19,6 @@ static void Main(string[] args) { Console.WriteLine("Arg {0} is <{1}>", i, args[i]); } - - /*Console.WriteLine("\nCommand line:"); - Console.WriteLine(Environment.CommandLine);*/ - Console.WriteLine(); } } } diff --git a/test/EchoArgs/project.json b/test/tools/EchoArgs/project.json similarity index 100% rename from test/EchoArgs/project.json rename to test/tools/EchoArgs/project.json From 4d5d0a0034dcc112c8b3fe3f8eef4b0e7c5acdad Mon Sep 17 00:00:00 2001 From: Douglas Thrift Date: Mon, 12 Sep 2016 10:39:42 -0700 Subject: [PATCH 5/6] Rename Publish-EchoArgs to Publish-PSTestTools * Rename Publish-EchoArgs to Publish-PSTestTools so it can be used for other tools as well in the future * Publish EchoArgs to the bin directory instead of run to match convention * Add source URL to EchoArgs header comment * Use wildcard of "*.nuget.props" to match "test/csharp/csharp.nuget.props" in .gitignore --- .gitignore | 3 +-- build.psm1 | 8 ++++---- .../NativeExecution/NativeCommandArguments.Tests.ps1 | 2 +- test/tools/EchoArgs/EchoArgs.cs | 1 + 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/.gitignore b/.gitignore index 99763674d59..61dd6cb62f7 100644 --- a/.gitignore +++ b/.gitignore @@ -1,12 +1,11 @@ bin/ obj/ -run/ project.lock.json *-tests.xml /debug/ /staging/ /Packages/ -test/csharp/csharp.nuget.props +*.nuget.props # dotnet cli install/uninstall scripts dotnet-install.ps1 diff --git a/build.psm1 b/build.psm1 index b668b3aec70..0bee2ed8a4d 100644 --- a/build.psm1 +++ b/build.psm1 @@ -547,15 +547,16 @@ function Get-PesterTag { $o } -function Publish-EchoArgs { +function Publish-PSTestTools { [CmdletBinding()] param() Find-Dotnet + # Publish EchoArgs so it can be run by tests Push-Location "$PSScriptRoot/test/tools/EchoArgs" try { - dotnet publish --output run + dotnet publish --output bin } finally { Pop-Location } @@ -577,8 +578,7 @@ function Start-PSPester { ) Write-Verbose "Running pester tests at '$path' with tag '$($Tag -join ''', ''')' and ExcludeTag '$($ExcludeTag -join ''', ''')'" -Verbose - # Publish EchoArgs so it can be run by tests - Publish-EchoArgs + Publish-PSTestTools # All concatenated commands/arguments are suffixed with the delimiter (space) $Command = "" diff --git a/test/powershell/Scripting/NativeExecution/NativeCommandArguments.Tests.ps1 b/test/powershell/Scripting/NativeExecution/NativeCommandArguments.Tests.ps1 index 6d2541ea44f..716aea86a81 100644 --- a/test/powershell/Scripting/NativeExecution/NativeCommandArguments.Tests.ps1 +++ b/test/powershell/Scripting/NativeExecution/NativeCommandArguments.Tests.ps1 @@ -4,7 +4,7 @@ Describe "Native Command Arguments" -tags "CI" { while ($powershellTestDir -notmatch 'test[\\/]powershell$') { $powershellTestDir = Split-Path $powershellTestDir } - $echoArgs = Join-Path (Split-Path $powershellTestDir) tools/EchoArgs/run/echoargs + $echoArgs = Join-Path (Split-Path $powershellTestDir) tools/EchoArgs/bin/echoargs It "Should handle quoted spaces correctly" { $a = 'a"b c"d' diff --git a/test/tools/EchoArgs/EchoArgs.cs b/test/tools/EchoArgs/EchoArgs.cs index ca2958fdfa0..209d5d86155 100644 --- a/test/tools/EchoArgs/EchoArgs.cs +++ b/test/tools/EchoArgs/EchoArgs.cs @@ -1,5 +1,6 @@ //--------------------------------------------------------------------- // Author: Keith Hill +// Source: https://github.com/Pscx/Pscx/blob/master/Src/EchoArgs/EchoArgs.cs // // Description: Very simple little console class that you can use to see // how PowerShell is passing parameters to legacy console From ddf30c4008cf6a12a60e6f79ca4fbef2619c955e Mon Sep 17 00:00:00 2001 From: Douglas Thrift Date: Tue, 13 Sep 2016 13:34:44 -0700 Subject: [PATCH 6/6] Use a variable instead of a file for testing args * Use a variable (which can be indexed by line) instead of a file to test the output of arguments passed to echoargs * Add comments explaining the tests --- .../NativeCommandArguments.Tests.ps1 | 35 ++++++++++++++----- 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/test/powershell/Scripting/NativeExecution/NativeCommandArguments.Tests.ps1 b/test/powershell/Scripting/NativeExecution/NativeCommandArguments.Tests.ps1 index 716aea86a81..a55d56ed44d 100644 --- a/test/powershell/Scripting/NativeExecution/NativeCommandArguments.Tests.ps1 +++ b/test/powershell/Scripting/NativeExecution/NativeCommandArguments.Tests.ps1 @@ -1,22 +1,41 @@ Describe "Native Command Arguments" -tags "CI" { - $testPath = 'TestDrive:\test.txt' + # Find where test/powershell is so we can find the echoargs command relative to it $powershellTestDir = $PSScriptRoot while ($powershellTestDir -notmatch 'test[\\/]powershell$') { $powershellTestDir = Split-Path $powershellTestDir } $echoArgs = Join-Path (Split-Path $powershellTestDir) tools/EchoArgs/bin/echoargs + # When passing arguments to native commands, quoted segments that contain + # spaces need to be quoted with '"' characters when they are passed to the + # native command (or to bash or sh on Linux). + # + # This test checks that the proper quoting is occuring by passing arguments + # to the echoargs native command and looking at how it got the arguments. It "Should handle quoted spaces correctly" { $a = 'a"b c"d' - & $echoArgs $a 'a"b c"d' a"b c"d >$testPath - $testPath | Should Contain 'Arg 0 is ' - $testPath | Should Contain 'Arg 1 is ' - $testPath | Should Contain 'Arg 2 is ' + $lines = & $echoArgs $a 'a"b c"d' a"b c"d + ($lines | measure).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 ' } + # In order to pass '"' characters so they are actually part of command line + # arguments for native commands, they need to be escaped with a '\' (this + # is in addition to the '`' escaping needed inside '"' quoted strings in + # PowerShell). + # + # This functionality was broken in PowerShell 5.0 and 5.1, so this test + # will fail on those versions unless the fix is backported to them. + # + # This test checks that the proper quoting and escaping is occurring by + # passing arguments with escaped quotes to the echoargs native command and + # looking at how it got the arguments. It "Should handle spaces between escaped quotes" { - & $echoArgs 'a\"b c\"d' "a\`"b c\`"d" >$testPath - $testPath | Should Contain 'Arg 0 is ' - $testPath | Should Contain 'Arg 1 is ' + $lines = & $echoArgs 'a\"b c\"d' "a\`"b c\`"d" + ($lines | measure).Count | Should Be 2 + $lines[0] | Should Be 'Arg 0 is ' + $lines[1] | Should Be 'Arg 1 is ' } }