Skip to content

Commit

Permalink
Add a Windows mode for native commands that allows some commands to u…
Browse files Browse the repository at this point in the history 
…se legacy argument passing (PowerShell#15408)
  • Loading branch information
@JamesWTruher @xtqqczze
JamesWTruher authored and xtqqczze committed Jul 22, 2021
1 parent 5a4a6c5 commit 52f8b0e
Show file tree
Hide file tree
Showing 7 changed files with 306 additions and 54 deletions.
10 changes: 8 additions & 2 deletions src/System.Management.Automation/engine/CommandBase.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -281,8 +281,14 @@ public enum NativeArgumentPassingStyle
/// <summary>Use legacy argument parsing via ProcessStartInfo.Arguments.</summary>
Legacy = 0,

/// <summary>Use new style argument parsing via ProcessStartInfo.ArgumentList.</summary>
Standard = 1
/// <summary>Use new style argument passing via ProcessStartInfo.ArgumentList.</summary>
Standard = 1,

/// <summary>
/// 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.
/// </summary>
Windows = 2
}
#endregion NativeArgumentPassingStyle

Expand Down
9 changes: 7 additions & 2 deletions src/System.Management.Automation/engine/InitialSessionState.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1320,13 +1320,18 @@ private void AddVariables(IEnumerable<SessionStateVariableEntry> variables)

// 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))));
Expand Down
18 changes: 10 additions & 8 deletions src/System.Management.Automation/engine/NativeCommandParameterBinder.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ namespace System.Management.Automation
/// </summary>
internal class NativeCommandParameterBinder : ParameterBinderBase
{
private readonly VariablePath s_nativeArgumentPassingVarPath = new VariablePath(SpecialVariables.NativeArgumentPassing);

#region ctor

/// <summary>
Expand Down Expand Up @@ -187,7 +189,7 @@ internal void AddToArgumentList(CommandParameterInternal parameter, string argum
/// <summary>
/// Gets a value indicating whether to use an ArgumentList or string for arguments when invoking a native executable.
/// </summary>
internal bool UseArgumentList
internal NativeArgumentPassingStyle ArgumentPassingStyle
{
get
{
Expand All @@ -197,17 +199,17 @@ internal bool UseArgumentList
{
// This will default to the new behavior if it is set to anything other than Legacy
var preference = LanguagePrimitives.ConvertTo<NativeArgumentPassingStyle>(
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;
}
}

Expand Down Expand Up @@ -314,7 +316,7 @@ private void AppendOneNativeArgument(ExecutionContext context, CommandParameterI
}
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))
Expand All @@ -331,7 +333,7 @@ private void AppendOneNativeArgument(ExecutionContext context, CommandParameterI
}
}
}
else if (UseArgumentList && currentObj != null)
else if (ArgumentPassingStyle == NativeArgumentPassingStyle.Standard && currentObj != null)
{
// add empty strings to arglist, but not nulls
AddToArgumentList(parameter, arg);
Expand Down
6 changes: 3 additions & 3 deletions src/System.Management.Automation/engine/NativeCommandParameterBinderController.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,13 +50,13 @@ internal string[] ArgumentList
}

/// <summary>
/// Gets a value indicating whether to use the new API for StartInfo.
/// Gets the value indicating what type of native argument binding to use.
/// </summary>
internal bool UseArgumentList
internal NativeArgumentPassingStyle ArgumentPassingStyle
{
get
{
return ((NativeCommandParameterBinder)DefaultParameterBinder).UseArgumentList;
return ((NativeCommandParameterBinder)DefaultParameterBinder).ArgumentPassingStyle;
}
}

Expand Down
153 changes: 117 additions & 36 deletions src/System.Management.Automation/engine/NativeCommandProcessor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,26 @@ internal ProcessOutputObject(object data, MinishellStream stream)
/// </summary>
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<string> s_legacyFileExtensions = new HashSet<string>(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<string> s_legacyCommands = new HashSet<string>(StringComparer.OrdinalIgnoreCase)
{
"cmd",
"cscript",
"wscript",
};

#region ctor/native command properties

/// <summary>
Expand Down Expand Up @@ -474,6 +494,7 @@ private void InitNativeProcess()
// 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);
Expand All @@ -485,7 +506,16 @@ private void InitNativeProcess()

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
{
Expand All @@ -495,7 +525,14 @@ private void InitNativeProcess()
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;
}
}
Expand Down Expand Up @@ -1108,64 +1145,89 @@ private void ProcessOutputRecord(ProcessOutputObject outputValue)
}

/// <summary>
/// Gets the start info for process.
/// Get whether we should treat this executable with special handling and use the legacy passing style.
/// </summary>
/// <param name="redirectOutput"></param>
/// <param name="redirectError"></param>
/// <param name="redirectInput"></param>
/// <param name="soloCommand"></param>
/// <returns></returns>
private ProcessStartInfo GetProcessStartInfo(bool redirectOutput, bool redirectError, bool redirectInput, bool soloCommand)
/// <param name="filePath"></param>
private bool UseSpecialArgumentPassing(string filePath) =>
NativeParameterBinderController.ArgumentPassingStyle switch
{
NativeArgumentPassingStyle.Legacy => true,
NativeArgumentPassingStyle.Windows => ShouldUseLegacyPassingStyle(filePath),
_ => false
};

/// <summary>
/// Gets the ProcessStartInfo for process.
/// </summary>
/// <param name="redirectOutput">A boolean that indicates that, when true, output from the process is redirected to a stream, and otherwise is sent to stdout.</param>
/// <param name="redirectError">A boolean that indicates that, when true, error output from the process is redirected to a stream, and otherwise is sent to stderr.</param>
/// <param name="redirectInput">A boolean that indicates that, when true, input to the process is taken from a stream, and otherwise is taken from stdin.</param>
/// <param name="soloCommand">A boolean that indicates, when true, that the command to be executed is not part of a pipeline, and otherwise indicates that is is.</param>
/// <returns>A ProcessStartInfo object which is the base of the native invocation.</returns>
private ProcessStartInfo GetProcessStartInfo(
bool redirectOutput,
bool redirectError,
bool redirectInput,
bool soloCommand)
{
ProcessStartInfo startInfo = new ProcessStartInfo();
startInfo.FileName = this.Path;
var startInfo = new ProcessStartInfo
{
FileName = this.Path
};

if (IsExecutable(this.Path))
if (!IsExecutable(this.Path))
{
startInfo.UseShellExecute = false;
if (redirectInput)
if (Platform.IsNanoServer || Platform.IsIoT)
{
startInfo.RedirectStandardInput = true;
// 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);
}

if (redirectOutput)
// We only want to ShellExecute something that is standalone...
if (!soloCommand)
{
startInfo.RedirectStandardOutput = true;
startInfo.StandardOutputEncoding = Console.OutputEncoding;
throw InterpreterError.NewInterpreterException(
this.Path,
typeof(RuntimeException),
this.Command.InvocationExtent,
"CantActivateDocumentInPipeline",
ParserStrings.CantActivateDocumentInPipeline,
this.Path);
}

if (redirectError)
{
startInfo.RedirectStandardError = true;
startInfo.StandardErrorEncoding = Console.OutputEncoding;
}
startInfo.UseShellExecute = true;
}
else
{
if (Platform.IsNanoServer || Platform.IsIoT)
startInfo.UseShellExecute = false;
startInfo.RedirectStandardInput = redirectInput;

if (redirectOutput)
{
// 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);
startInfo.RedirectStandardOutput = true;
startInfo.StandardOutputEncoding = Console.OutputEncoding;
}

// We only want to ShellExecute something that is standalone...
if (!soloCommand)
if (redirectError)
{
throw InterpreterError.NewInterpreterException(this.Path, typeof(RuntimeException),
this.Command.InvocationExtent, "CantActivateDocumentInPipeline", ParserStrings.CantActivateDocumentInPipeline, this.Path);
startInfo.RedirectStandardError = true;
startInfo.StandardErrorEncoding = Console.OutputEncoding;
}

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;
}

Expand All @@ -1174,7 +1236,8 @@ private ProcessStartInfo GetProcessStartInfo(bool redirectOutput, bool redirectE
// 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))
{
Expand Down Expand Up @@ -1203,9 +1266,27 @@ private ProcessStartInfo GetProcessStartInfo(bool redirectOutput, bool redirectE
context.EngineSessionState.GetNamespaceCurrentLocation(
context.ProviderNames.FileSystem).ProviderPath;
startInfo.WorkingDirectory = WildcardPattern.Unescape(rawPath);

return startInfo;
}

/// <summary>
/// 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.
/// </summary>
/// <param name="filePath">The file to use when checking how to pass arguments.</param>
/// <returns>A boolean indicating what passing style should be used.</returns>
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.");
Expand Down
2 changes: 1 addition & 1 deletion test/powershell/Host/ConsoleHost.Tests.ps1
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down

0 comments on commit 52f8b0e

Please sign in to comment.