diff --git a/src/System.Management.Automation/engine/CommandBase.cs b/src/System.Management.Automation/engine/CommandBase.cs index 016b22ff59..048eafd56c 100644 --- a/src/System.Management.Automation/engine/CommandBase.cs +++ b/src/System.Management.Automation/engine/CommandBase.cs @@ -281,8 +281,14 @@ namespace System.Management.Automation /// Use legacy argument parsing via ProcessStartInfo.Arguments. Legacy = 0, - /// Use new style argument parsing via ProcessStartInfo.ArgumentList. - Standard = 1 + /// Use new style argument passing via ProcessStartInfo.ArgumentList. + Standard = 1, + + /// + /// Use specific to Windows passing style which is Legacy for selected files on Windows, but + /// Standard for everything else. This is the default behavior for Windows. + /// + Windows = 2 } #endregion NativeArgumentPassingStyle diff --git a/src/System.Management.Automation/engine/InitialSessionState.cs b/src/System.Management.Automation/engine/InitialSessionState.cs index ae25c8d0ed..99aa639d09 100644 --- a/src/System.Management.Automation/engine/InitialSessionState.cs +++ b/src/System.Management.Automation/engine/InitialSessionState.cs @@ -1320,13 +1320,18 @@ namespace System.Management.Automation.Runspaces // If the PSNativeCommandArgumentPassing feature is enabled, create the variable which controls the behavior // Since the BuiltInVariables list is static, and this should be done dynamically - // we need to do this here. + // we need to do this here. Also, since the defaults are different based on platform we need a + // bit more logic. if (ExperimentalFeature.IsEnabled("PSNativeCommandArgumentPassing")) { + NativeArgumentPassingStyle style = NativeArgumentPassingStyle.Standard; + if (Platform.IsWindows) { + style = NativeArgumentPassingStyle.Windows; + } Variables.Add( new SessionStateVariableEntry( SpecialVariables.NativeArgumentPassing, - NativeArgumentPassingStyle.Standard, + style, RunspaceInit.NativeCommandArgumentPassingDescription, ScopedItemOptions.None, new ArgumentTypeConverterAttribute(typeof(NativeArgumentPassingStyle)))); diff --git a/src/System.Management.Automation/engine/NativeCommandParameterBinder.cs b/src/System.Management.Automation/engine/NativeCommandParameterBinder.cs index 35c956e280..511449bc7e 100644 --- a/src/System.Management.Automation/engine/NativeCommandParameterBinder.cs +++ b/src/System.Management.Automation/engine/NativeCommandParameterBinder.cs @@ -20,6 +20,8 @@ namespace System.Management.Automation /// internal class NativeCommandParameterBinder : ParameterBinderBase { + private readonly VariablePath s_nativeArgumentPassingVarPath = new VariablePath(SpecialVariables.NativeArgumentPassing); + #region ctor /// @@ -187,7 +189,7 @@ namespace System.Management.Automation /// /// Gets a value indicating whether to use an ArgumentList or string for arguments when invoking a native executable. /// - internal bool UseArgumentList + internal NativeArgumentPassingStyle ArgumentPassingStyle { get { @@ -197,17 +199,17 @@ namespace System.Management.Automation { // This will default to the new behavior if it is set to anything other than Legacy var preference = LanguagePrimitives.ConvertTo( - Context.GetVariableValue(new VariablePath(SpecialVariables.NativeArgumentPassing), NativeArgumentPassingStyle.Standard)); - return preference != NativeArgumentPassingStyle.Legacy; + Context.GetVariableValue(s_nativeArgumentPassingVarPath, NativeArgumentPassingStyle.Standard)); + return preference; } catch { - // The value is not convertable send back true - return true; + // The value is not convertable send back Legacy + return NativeArgumentPassingStyle.Legacy; } } - return false; + return NativeArgumentPassingStyle.Legacy; } } @@ -314,7 +316,7 @@ namespace System.Management.Automation } else { - if (argArrayAst != null && UseArgumentList) + if (argArrayAst != null && ArgumentPassingStyle == NativeArgumentPassingStyle.Standard) { // We have a literal array, so take the extent, break it on spaces and add them to the argument list. foreach (string element in argArrayAst.Extent.Text.Split(' ', StringSplitOptions.RemoveEmptyEntries)) @@ -331,7 +333,7 @@ namespace System.Management.Automation } } } - else if (UseArgumentList && currentObj != null) + else if (ArgumentPassingStyle == NativeArgumentPassingStyle.Standard && currentObj != null) { // add empty strings to arglist, but not nulls AddToArgumentList(parameter, arg); diff --git a/src/System.Management.Automation/engine/NativeCommandParameterBinderController.cs b/src/System.Management.Automation/engine/NativeCommandParameterBinderController.cs index 933c71f550..9a1dab71be 100644 --- a/src/System.Management.Automation/engine/NativeCommandParameterBinderController.cs +++ b/src/System.Management.Automation/engine/NativeCommandParameterBinderController.cs @@ -50,13 +50,13 @@ namespace System.Management.Automation } /// - /// Gets a value indicating whether to use the new API for StartInfo. + /// Gets the value indicating what type of native argument binding to use. /// - internal bool UseArgumentList + internal NativeArgumentPassingStyle ArgumentPassingStyle { get { - return ((NativeCommandParameterBinder)DefaultParameterBinder).UseArgumentList; + return ((NativeCommandParameterBinder)DefaultParameterBinder).ArgumentPassingStyle; } } diff --git a/src/System.Management.Automation/engine/NativeCommandProcessor.cs b/src/System.Management.Automation/engine/NativeCommandProcessor.cs index 32f2ee1f7e..3407532acd 100644 --- a/src/System.Management.Automation/engine/NativeCommandProcessor.cs +++ b/src/System.Management.Automation/engine/NativeCommandProcessor.cs @@ -135,6 +135,26 @@ namespace System.Management.Automation /// internal class NativeCommandProcessor : CommandProcessorBase { + // This is the list of files which will trigger Legacy behavior if + // PSNativeCommandArgumentPassing is set to "Windows". + private static readonly IReadOnlySet s_legacyFileExtensions = new HashSet(StringComparer.OrdinalIgnoreCase) + { + ".js", + ".wsf", + ".cmd", + ".bat", + ".vbs", + }; + + // The following native commands have non-standard behavior with regard to argument passing, + // so we use Legacy argument parsing for them when PSNativeCommandArgumentPassing is set to Windows. + private static readonly IReadOnlySet s_legacyCommands = new HashSet(StringComparer.OrdinalIgnoreCase) + { + "cmd", + "cscript", + "wscript", + }; + #region ctor/native command properties /// @@ -474,6 +494,7 @@ namespace System.Management.Automation // on Windows desktops, see if there is a file association for this command. If so then we'll use that. string executable = FindExecutable(startInfo.FileName); bool notDone = true; + // check to see what mode we should be in for argument passing if (!string.IsNullOrEmpty(executable)) { isWindowsApplication = IsWindowsApplication(executable); @@ -485,7 +506,16 @@ namespace System.Management.Automation string oldArguments = startInfo.Arguments; string oldFileName = startInfo.FileName; - startInfo.Arguments = "\"" + startInfo.FileName + "\" " + startInfo.Arguments; + // Check to see whether this executable should be using Legacy mode argument parsing + bool useSpecialArgumentPassing = UseSpecialArgumentPassing(oldFileName); + if (useSpecialArgumentPassing) + { + startInfo.Arguments = "\"" + oldFileName + "\" " + startInfo.Arguments; + } + else + { + startInfo.ArgumentList.Insert(0, oldFileName); + } startInfo.FileName = executable; try { @@ -495,7 +525,14 @@ namespace System.Management.Automation catch (Win32Exception) { // Restore the old filename and arguments to try shell execute last... - startInfo.Arguments = oldArguments; + if (useSpecialArgumentPassing) + { + startInfo.Arguments = oldArguments; + } + else + { + startInfo.ArgumentList.RemoveAt(0); + } startInfo.FileName = oldFileName; } } @@ -1108,25 +1145,69 @@ namespace System.Management.Automation } /// - /// Gets the start info for process. + /// Get whether we should treat this executable with special handling and use the legacy passing style. /// - /// - /// - /// - /// - /// - private ProcessStartInfo GetProcessStartInfo(bool redirectOutput, bool redirectError, bool redirectInput, bool soloCommand) - { - ProcessStartInfo startInfo = new ProcessStartInfo(); - startInfo.FileName = this.Path; + /// + private bool UseSpecialArgumentPassing(string filePath) => + NativeParameterBinderController.ArgumentPassingStyle switch + { + NativeArgumentPassingStyle.Legacy => true, + NativeArgumentPassingStyle.Windows => ShouldUseLegacyPassingStyle(filePath), + _ => false + }; - if (IsExecutable(this.Path)) + /// + /// Gets the ProcessStartInfo for process. + /// + /// A boolean that indicates that, when true, output from the process is redirected to a stream, and otherwise is sent to stdout. + /// A boolean that indicates that, when true, error output from the process is redirected to a stream, and otherwise is sent to stderr. + /// A boolean that indicates that, when true, input to the process is taken from a stream, and otherwise is taken from stdin. + /// A boolean that indicates, when true, that the command to be executed is not part of a pipeline, and otherwise indicates that is is. + /// A ProcessStartInfo object which is the base of the native invocation. + private ProcessStartInfo GetProcessStartInfo( + bool redirectOutput, + bool redirectError, + bool redirectInput, + bool soloCommand) + { + var startInfo = new ProcessStartInfo + { + FileName = this.Path + }; + + if (!IsExecutable(this.Path)) + { + if (Platform.IsNanoServer || Platform.IsIoT) + { + // Shell doesn't exist on headless SKUs, so documents cannot be associated with an application. + // Therefore, we cannot run document in this case. + throw InterpreterError.NewInterpreterException( + this.Path, + typeof(RuntimeException), + this.Command.InvocationExtent, + "CantActivateDocumentInPowerShellCore", + ParserStrings.CantActivateDocumentInPowerShellCore, + this.Path); + } + + // We only want to ShellExecute something that is standalone... + if (!soloCommand) + { + throw InterpreterError.NewInterpreterException( + this.Path, + typeof(RuntimeException), + this.Command.InvocationExtent, + "CantActivateDocumentInPipeline", + ParserStrings.CantActivateDocumentInPipeline, + this.Path); + } + + startInfo.UseShellExecute = true; + } + else { startInfo.UseShellExecute = false; - if (redirectInput) - { - startInfo.RedirectStandardInput = true; - } + startInfo.RedirectStandardInput = redirectInput; if (redirectOutput) { @@ -1140,32 +1221,13 @@ namespace System.Management.Automation startInfo.StandardErrorEncoding = Console.OutputEncoding; } } - else - { - if (Platform.IsNanoServer || Platform.IsIoT) - { - // Shell doesn't exist on headless SKUs, so documents cannot be associated with an application. - // Therefore, we cannot run document in this case. - throw InterpreterError.NewInterpreterException(this.Path, typeof(RuntimeException), - this.Command.InvocationExtent, "CantActivateDocumentInPowerShellCore", ParserStrings.CantActivateDocumentInPowerShellCore, this.Path); - } - - // We only want to ShellExecute something that is standalone... - if (!soloCommand) - { - throw InterpreterError.NewInterpreterException(this.Path, typeof(RuntimeException), - this.Command.InvocationExtent, "CantActivateDocumentInPipeline", ParserStrings.CantActivateDocumentInPipeline, this.Path); - } - - startInfo.UseShellExecute = true; - } // For minishell value of -outoutFormat parameter depends on value of redirectOutput. // So we delay the parameter binding. Do parameter binding for minishell now. if (_isMiniShell) { MinishellParameterBinderController mpc = (MinishellParameterBinderController)NativeParameterBinderController; - mpc.BindParameters(arguments, redirectOutput, this.Command.Context.EngineHostInterface.Name); + mpc.BindParameters(arguments, startInfo.RedirectStandardOutput, this.Command.Context.EngineHostInterface.Name); startInfo.CreateNoWindow = mpc.NonInteractive; } @@ -1174,7 +1236,8 @@ namespace System.Management.Automation // We provide the user a way to select the new behavior via a new preference variable using (ParameterBinderBase.bindingTracer.TraceScope("BIND NAMED native application line args [{0}]", this.Path)) { - if (!NativeParameterBinderController.UseArgumentList) + // We need to check if we're using legacy argument passing or it's a special case. + if (UseSpecialArgumentPassing(startInfo.FileName)) { using (ParameterBinderBase.bindingTracer.TraceScope("BIND argument [{0}]", NativeParameterBinderController.Arguments)) { @@ -1203,9 +1266,27 @@ namespace System.Management.Automation context.EngineSessionState.GetNamespaceCurrentLocation( context.ProviderNames.FileSystem).ProviderPath; startInfo.WorkingDirectory = WildcardPattern.Unescape(rawPath); + return startInfo; } + /// + /// Determine if we have a special file which will change the way native argument passing + /// is done on Windows. We use legacy behavior for cmd.exe, .bat, .cmd files. + /// + /// The file to use when checking how to pass arguments. + /// A boolean indicating what passing style should be used. + private static bool ShouldUseLegacyPassingStyle(string filePath) + { + if (string.IsNullOrEmpty(filePath)) + { + return false; + } + + return s_legacyFileExtensions.Contains(IO.Path.GetExtension(filePath)) + || s_legacyCommands.Contains(IO.Path.GetFileNameWithoutExtension(filePath)); + } + private static bool IsDownstreamOutDefault(Pipe downstreamPipe) { Diagnostics.Assert(downstreamPipe != null, "Caller makes sure the passed-in parameter is not null."); diff --git a/test/powershell/Host/ConsoleHost.Tests.ps1 b/test/powershell/Host/ConsoleHost.Tests.ps1 index a601f44e42..28561da23c 100644 --- a/test/powershell/Host/ConsoleHost.Tests.ps1 +++ b/test/powershell/Host/ConsoleHost.Tests.ps1 @@ -240,7 +240,7 @@ Describe "ConsoleHost unit tests" -tags "Feature" { $LASTEXITCODE | Should -Be 64 } - It "Empty space command should succeed" { + It "Empty space command should succeed on non-Windows" -skip:$IsWindows { & $powershell -noprofile -c '' | Should -BeNullOrEmpty $LASTEXITCODE | Should -Be 0 } diff --git a/test/powershell/Language/Scripting/NativeExecution/NativeCommandArguments.Tests.ps1 b/test/powershell/Language/Scripting/NativeExecution/NativeCommandArguments.Tests.ps1 index b279f04c9e..0c7b7f9d51 100644 --- a/test/powershell/Language/Scripting/NativeExecution/NativeCommandArguments.Tests.ps1 +++ b/test/powershell/Language/Scripting/NativeExecution/NativeCommandArguments.Tests.ps1 @@ -2,7 +2,165 @@ # Licensed under the MIT License. [System.Diagnostics.CodeAnalysis.SuppressMessageAttribute("PSAvoidGlobalVars", "")] param() -Describe "Will error correctly if an attempt to set variable to improper value" { + +Describe "Behavior is specific for each platform" -tags "CI" { + BeforeAll { + $skipTests = $EnabledExperimentalFeatures -notcontains 'PSNativeCommandArgumentPassing' + } + It "PSNativeCommandArgumentPassing is set to 'Windows' on Windows systems" -skip:(-not $IsWindows) { + $PSNativeCommandArgumentPassing | Should -Be "Windows" + } + It "PSNativeCommandArgumentPassing is set to 'Standard' on non-Windows systems" -skip:($IsWindows) { + $PSNativeCommandArgumentPassing | Should -Be "Standard" + } + It "Has proper behavior on Windows" -skip:(-not $IsWindows) { + "@echo off`nSET V1=1" > "$TESTDRIVE\script 1.cmd" + "@echo off`nSET V2=a`necho %V1%" > "$TESTDRIVE\script 2.cmd" + "@echo off`necho %V1%`necho %V2%" > "$TESTDRIVE\script 3.cmd" + $result = cmd /c """${TESTDRIVE}\script 1.cmd"" && ""${TESTDRIVE}\script 2.cmd"" && ""${TESTDRIVE}\script 3.cmd""" + $result.Count | Should -Be 3 + $result[0] | Should -Be 1 + $result[1] | Should -Be 1 + $result[2] | Should Be "a" + } + +} + +Describe "tests for multiple languages and extensions" -tags "CI" { + AfterAll { + if (-not $IsWindows -or + $EnabledExperimentalFeatures -notcontains 'PSNativeCommandArgumentPassing') { + return + } + $PSNativeCommandArgumentPassing = $passingStyle + } + BeforeAll { + $testCases = @( + @{ + Command = "cscript.exe" + Filename = "test.wsf" + ExpectedResults = @( + "Argument 0 is: " + "Argument 1 is: " + "Argument 2 is: " + "Argument 3 is: " + ) + Script = @' + + + + +'@ + } + @{ + Command = "cscript.exe" + Filename = "test.vbs" + ExpectedResults = @( + "Argument 0 is: " + "Argument 1 is: " + "Argument 2 is: " + "Argument 3 is: " + ) + Script = @' +for i = 0 to wScript.arguments.count - 1 + wscript.echo "Argument " & i & " is: <" & (wScript.arguments(i)) & ">" +next +'@ + } + @{ + Command = "cscript" + Filename = "test.js" + ExpectedResults = @( + "Argument 0 is: " + "Argument 1 is: " + "Argument 2 is: " + "Argument 3 is: " + ) + Script = @' +for(i = 0; i < WScript.Arguments.Count(); i++) { + WScript.echo("Argument " + i + " is: <" + WScript.Arguments(i) + ">"); +} +'@ + } + @{ + Command = "" + Filename = "test.bat" + ExpectedResults = @( + "Argument 1 is: " + "Argument 2 is: " + "Argument 3 is: <""ab cd"">" + "Argument 4 is: <""a'b c'd"">" + ) + Script = @' +@echo off +echo Argument 1 is: ^<%1^> +echo Argument 2 is: ^<%2^> +echo Argument 3 is: ^<%3^> +echo Argument 4 is: ^<%4^> +'@ + } + @{ + Command = "" + Filename = "test.cmd" + ExpectedResults = @( + "Argument 1 is: " + "Argument 2 is: " + "Argument 3 is: <""ab cd"">" + "Argument 4 is: <""a'b c'd"">" + ) + Script = @' +@echo off +echo Argument 1 is: ^<%1^> +echo Argument 2 is: ^<%2^> +echo Argument 3 is: ^<%3^> +echo Argument 4 is: ^<%4^> +'@ + } + ) + + # determine whether we should skip the tests we just defined + # doing it in this order ensures that the test output will show each skipped test + $skipTests = -not $IsWindows -or $EnabledExperimentalFeatures -notcontains 'PSNativeCommandArgumentPassing' + if ($skipTests) { + return + } + + # save the passing style + $passingStyle = $PSNativeCommandArgumentPassing + # explicitely set the passing style to Windows + $PSNativeCommandArgumentPassing = "Windows" + } + + It "Invoking '' is compatible with PowerShell 5" -TestCases $testCases -Skip:$($skipTests) { + param ( $Command, $Arguments, $Filename, $Script, $ExpectedResults ) + cscript //h:cscript //nologo //s + $a = 'a"b c"d' + $scriptPath = Join-Path $TESTDRIVE $Filename + $Script | out-file -encoding ASCII $scriptPath + if ($Command) { + $results = & $Command $scriptPath $a 'a"b c"d' a"b c"d "a'b c'd" 2> "${TESTDRIVE}/error.txt" + } + else { + $results = & $scriptPath $a 'a"b c"d' a"b c"d "a'b c'd" 2> "${TESTDRIVE}/error.txt" + } + $errorContent = Get-Content "${TESTDRIVE}/error.txt" -ErrorAction Ignore + $errorContent | Should -BeNullOrEmpty + $results.Count | Should -Be 4 + $results[0] | Should -Be $ExpectedResults[0] + $results[1] | Should -Be $ExpectedResults[1] + $results[2] | Should -Be $ExpectedResults[2] + $results[3] | Should -Be $ExpectedResults[3] + } +} + + +Describe "Will error correctly if an attempt to set variable to improper value" -tags "CI" { It "will error when setting variable incorrectly" { if ($EnabledExperimentalFeatures -contains 'PSNativeCommandArgumentPassing') { { $global:PSNativeCommandArgumentPassing = "zzz" } | Should -Throw -ExceptionType System.Management.Automation.ArgumentTransformationMetadataException @@ -107,7 +265,7 @@ foreach ( $argumentListValue in "Standard","Legacy" ) { } } } -Describe 'PSPath to native commands' { +Describe 'PSPath to native commands' -tags "CI" { BeforeAll { $featureEnabled = $EnabledExperimentalFeatures.Contains('PSNativePSPathResolution') $originalDefaultParameterValues = $PSDefaultParameterValues.Clone()