Skip to content

Stop globbing of quoted arguments to native commands #5188

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
lzybkr merged 2 commits into from
Nov 1, 2017
Merged

Stop globbing of quoted arguments to native commands #5188

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7aa52e0
Use Ast for context in parameter binding
lzybkr Oct 21, 2017
2bcea1d
Glob native command args only when not quoted
lzybkr Oct 22, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/System.Management.Automation/commands/utility/FormatAndOutput/common/BaseCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ internal void AddNamedParameter(string parameterName, object parameterValue)
{
_commandParameterList.Add(
CommandParameterInternal.CreateParameterWithArgument(
PositionUtilities.EmptyExtent, parameterName, null,
PositionUtilities.EmptyExtent, parameterValue,
/*parameterAst*/null, parameterName, null,
/*argumentAst*/null, parameterValue,
false));
}

Expand Down
34 changes: 16 additions & 18 deletions src/System.Management.Automation/engine/CmdletParameterBinderController.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -450,8 +450,8 @@ private bool BindDefaultParameters(uint validParameterSetFlag, Dictionary<Merged

CommandParameterInternal bindableArgument =
CommandParameterInternal.CreateParameterWithArgument(
PositionUtilities.EmptyExtent, parameterName, "-" + parameterName + ":",
PositionUtilities.EmptyExtent, argumentValue, false);
/*parameterAst*/null, parameterName, "-" + parameterName + ":",
/*argumentAst*/null, argumentValue, false);

bool bindResult =
BindParameter(
Expand Down Expand Up @@ -1678,12 +1678,10 @@ private void HandleRemainingArguments()

// If there are multiple arguments, it's not clear how best to represent the extent as the extent
// may be disjoint, as in 'echo a -verbose b', we have 'a' and 'b' in UnboundArguments.
IScriptExtent argumentExtent = UnboundArguments.Count == 1
? UnboundArguments[0].ArgumentExtent
: PositionUtilities.EmptyExtent;
var argumentAst = UnboundArguments.Count == 1 ? UnboundArguments[0].ArgumentAst : null;
var cpi = CommandParameterInternal.CreateParameterWithArgument(
PositionUtilities.EmptyExtent, varargsParameter.Parameter.Name, "-" + varargsParameter.Parameter.Name + ":",
argumentExtent, valueFromRemainingArguments, false);
/*parameterAst*/null, varargsParameter.Parameter.Name, "-" + varargsParameter.Parameter.Name + ":",
argumentAst, valueFromRemainingArguments, false);

// To make all of the following work similarly (the first is handled elsewhere, but second and third are
// handled here):
Expand All @@ -1693,7 +1691,7 @@ private void HandleRemainingArguments()
// we unwrap our List, but only if there is a single argument which is a collection.
if (valueFromRemainingArguments.Count == 1 && LanguagePrimitives.IsObjectEnumerable(valueFromRemainingArguments[0]))
{
cpi.SetArgumentValue(UnboundArguments[0].ArgumentExtent, valueFromRemainingArguments[0]);
cpi.SetArgumentValue(UnboundArguments[0].ArgumentAst, valueFromRemainingArguments[0]);
}

try
Expand Down Expand Up @@ -3036,8 +3034,8 @@ internal bool HandleUnboundMandatoryParameters(
{
var argument =
CommandParameterInternal.CreateParameterWithArgument(
PositionUtilities.EmptyExtent, entry.Key, "-" + entry.Key + ":",
PositionUtilities.EmptyExtent, entry.Value,
/*parameterAst*/null, entry.Key, "-" + entry.Key + ":",
/*argumentAst*/null, entry.Value,
false);

// Ignore the result since any failure should cause an exception
Expand Down Expand Up @@ -3920,8 +3918,8 @@ private bool InvokeAndBindDelayBindScriptBlock(PSObject inputToOperateOn, out bo

// Create a new CommandParameterInternal for the output of the script block.
var newArgument = CommandParameterInternal.CreateParameterWithArgument(
argument.ParameterExtent, argument.ParameterName, "-" + argument.ParameterName + ":",
argument.ArgumentExtent, newValue,
argument.ParameterAst, argument.ParameterName, "-" + argument.ParameterName + ":",
argument.ArgumentAst, newValue,
false);

if (!BindParameter(newArgument, parameter, ParameterBindingFlags.ShouldCoerceType))
Expand Down Expand Up @@ -4284,8 +4282,8 @@ private bool BindPipelineParameter(

// Now bind the new value
CommandParameterInternal param = CommandParameterInternal.CreateParameterWithArgument(
PositionUtilities.EmptyExtent, parameter.Parameter.Name, "-" + parameter.Parameter.Name + ":",
PositionUtilities.EmptyExtent, parameterValue,
/*parameterAst*/null, parameter.Parameter.Name, "-" + parameter.Parameter.Name + ":",
/*argumentAst*/null, parameterValue,
false);

flags = flags & ~ParameterBindingFlags.DelayBindScriptBlock;
Expand All @@ -4306,8 +4304,8 @@ protected override void SaveDefaultScriptParameterValue(string name, object valu
{
_defaultParameterValues.Add(name,
CommandParameterInternal.CreateParameterWithArgument(
PositionUtilities.EmptyExtent, name, "-" + name + ":",
PositionUtilities.EmptyExtent, value,
/*parameterAst*/null, name, "-" + name + ":",
/*argumentAst*/null, value,
false));
}

Expand All @@ -4331,8 +4329,8 @@ private void BackupDefaultParameter(MergedCompiledCommandParameter parameter)
_defaultParameterValues.Add(
parameter.Parameter.Name,
CommandParameterInternal.CreateParameterWithArgument(
PositionUtilities.EmptyExtent, parameter.Parameter.Name, "-" + parameter.Parameter.Name + ":",
PositionUtilities.EmptyExtent, defaultParameterValue,
/*parameterAst*/null, parameter.Parameter.Name, "-" + parameter.Parameter.Name + ":",
/*argumentAst*/null, defaultParameterValue,
false));
}
}
Expand Down
67 changes: 38 additions & 29 deletions src/System.Management.Automation/engine/CommandParameter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,13 @@ internal sealed class CommandParameterInternal
{
private class Parameter
{
internal IScriptExtent extent;
internal Ast ast;
internal string parameterName;
internal string parameterText;
}
private class Argument
{
internal IScriptExtent extent;
internal Ast ast;
internal object value;
internal bool splatted;
internal bool arrayIsSingleArgumentForNativeCommand;
Expand Down Expand Up @@ -65,20 +65,36 @@ internal string ParameterText
}
}

/// <summary>
/// The ast of the parameter, if one was specified.
/// </summary>
internal Ast ParameterAst
{
get => _parameter?.ast;
}

/// <summary>
/// The extent of the parameter, if one was specified.
/// </summary>
internal IScriptExtent ParameterExtent
{
get { return _parameter != null ? _parameter.extent : PositionUtilities.EmptyExtent; }
get => ParameterAst?.Extent ?? PositionUtilities.EmptyExtent;
}

/// <summary>
/// The ast of the optional argument, if one was specified.
/// </summary>
internal Ast ArgumentAst
{
get => _argument?.ast;
}

/// <summary>
/// The extent of the optional argument, if one was specified.
/// </summary>
internal IScriptExtent ArgumentExtent
{
get { return _argument != null ? _argument.extent : PositionUtilities.EmptyExtent; }
get => ArgumentAst?.Extent ?? PositionUtilities.EmptyExtent;
}

/// <summary>
Expand Down Expand Up @@ -108,18 +124,16 @@ internal bool ArrayIsSingleArgumentForNativeCommand
}

/// <summary>
/// Set the argument value and extent.
/// Set the argument value and ast.
/// </summary>
internal void SetArgumentValue(IScriptExtent extent, object value)
internal void SetArgumentValue(Ast ast, object value)
{
Diagnostics.Assert(extent != null, "Caller to verify extent argument");

if (_argument == null)
{
_argument = new Argument();
}
_argument.value = value;
_argument.extent = extent;
_argument.ast = ast;
}

/// <summary>
Expand All @@ -130,9 +144,8 @@ internal IScriptExtent ErrorExtent
{
get
{
return _argument != null && _argument.extent != PositionUtilities.EmptyExtent
? _argument.extent
: _parameter != null ? _parameter.extent : PositionUtilities.EmptyExtent;
var argExtent = ArgumentExtent;
return argExtent != PositionUtilities.EmptyExtent ? argExtent : ParameterExtent;
}
}

Expand All @@ -141,42 +154,40 @@ internal IScriptExtent ErrorExtent
/// <summary>
/// Create a parameter when no argument has been specified.
/// </summary>
/// <param name="extent">The extent in script of the parameter.</param>
/// <param name="ast">The ast in script of the parameter.</param>
/// <param name="parameterName">The parameter name (with no leading dash).</param>
/// <param name="parameterText">The text of the parameter, as it did, or would, appear in script.</param>
internal static CommandParameterInternal CreateParameter(
IScriptExtent extent,
string parameterName,
string parameterText)
string parameterText,
Ast ast = null)
{
Diagnostics.Assert(extent != null, "Caller to verify extent argument");
return new CommandParameterInternal
{
_parameter =
new Parameter { extent = extent, parameterName = parameterName, parameterText = parameterText }
new Parameter { ast = ast, parameterName = parameterName, parameterText = parameterText }
};
}

/// <summary>
/// Create a positional argument to a command.
/// </summary>
/// <param name="extent">The extent of the argument value in the script.</param>
/// <param name="value">The argument value.</param>
/// <param name="ast">The ast of the argument value in the script.</param>
/// <param name="splatted">True if the argument value is to be splatted, false otherwise.</param>
/// <param name="arrayIsSingleArgumentForNativeCommand">If the command is native, pass the string with commas instead of multiple arguments</param>
internal static CommandParameterInternal CreateArgument(
IScriptExtent extent,
object value,
Ast ast = null,
bool splatted = false,
bool arrayIsSingleArgumentForNativeCommand = false)
{
Diagnostics.Assert(extent != null, "Caller to verify extent argument");
return new CommandParameterInternal
{
_argument = new Argument
{
extent = extent,
value = value,
ast = ast,
splatted = splatted,
arrayIsSingleArgumentForNativeCommand = arrayIsSingleArgumentForNativeCommand
}
Expand All @@ -193,28 +204,26 @@ internal static CommandParameterInternal CreateArgument(
/// * In the parameter binder when it resolves a positional argument
/// * Other random places that manually construct command processors and know their arguments.
/// </summary>
/// <param name="parameterExtent">The extent in script of the parameter.</param>
/// <param name="parameterAst">The ast in script of the parameter.</param>
/// <param name="parameterName">The parameter name (with no leading dash).</param>
/// <param name="parameterText">The text of the parameter, as it did, or would, appear in script.</param>
/// <param name="argumentExtent">The extent of the argument value in the script.</param>
/// <param name="argumentAst">The ast of the argument value in the script.</param>
/// <param name="value">The argument value.</param>
/// <param name="spaceAfterParameter">Used in native commands to correctly handle -foo:bar vs. -foo: bar</param>
/// <param name="arrayIsSingleArgumentForNativeCommand">If the command is native, pass the string with commas instead of multiple arguments</param>
internal static CommandParameterInternal CreateParameterWithArgument(
IScriptExtent parameterExtent,
Ast parameterAst,
string parameterName,
string parameterText,
IScriptExtent argumentExtent,
Ast argumentAst,
object value,
bool spaceAfterParameter,
bool arrayIsSingleArgumentForNativeCommand = false)
{
Diagnostics.Assert(parameterExtent != null, "Caller to verify parameterExtent argument");
Diagnostics.Assert(argumentExtent != null, "Caller to verify argumentExtent argument");
return new CommandParameterInternal
{
_parameter = new Parameter { extent = parameterExtent, parameterName = parameterName, parameterText = parameterText },
_argument = new Argument { extent = argumentExtent, value = value, arrayIsSingleArgumentForNativeCommand = arrayIsSingleArgumentForNativeCommand },
_parameter = new Parameter { ast = parameterAst, parameterName = parameterName, parameterText = parameterText },
_argument = new Argument { ast = argumentAst, value = value, arrayIsSingleArgumentForNativeCommand = arrayIsSingleArgumentForNativeCommand },
_spaceAfterParameter = spaceAfterParameter
};
}
Expand Down
8 changes: 4 additions & 4 deletions src/System.Management.Automation/engine/CommandProcessorBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -265,13 +265,13 @@ internal static CommandProcessorBase CreateGetHelpCommandProcessor(

CommandProcessorBase helpCommandProcessor = context.CreateCommand("get-help", false);
var cpi = CommandParameterInternal.CreateParameterWithArgument(
PositionUtilities.EmptyExtent, "Name", "-Name:",
PositionUtilities.EmptyExtent, helpTarget,
/*parameterAst*/null, "Name", "-Name:",
/*argumentAst*/null, helpTarget,
false);
helpCommandProcessor.AddParameter(cpi);
cpi = CommandParameterInternal.CreateParameterWithArgument(
PositionUtilities.EmptyExtent, "Category", "-Category:",
PositionUtilities.EmptyExtent, helpCategory.ToString(),
/*parameterAst*/null, "Category", "-Category:",
/*argumentAst*/null, helpCategory.ToString(),
false);
helpCommandProcessor.AddParameter(cpi);
return helpCommandProcessor;
Expand Down
Loading