Fix using variable for nested foreach parallel calls (#14548)

This commit is contained in:
Paul Higinbotham
2021-02-02 15:34:08 -08:00
committed by GitHub
parent 8e3c3e04f8
commit 0039807082
4 changed files with 231 additions and 12 deletions
@@ -381,6 +381,17 @@ namespace Microsoft.PowerShell.Commands
private Exception _taskCollectionException;
private string _currentLocationPath;
// List of Foreach-Object command names and aliases.
// TODO: Look into using SessionState.Internal.GetAliasTable() to find all user created aliases.
// But update Alias command logic to maintain reverse table that lists all aliases mapping
// to a single command definition, for performance.
private static string[] forEachNames = new string[]
{
"ForEach-Object",
"foreach",
"%"
};
private void InitParallelParameterSet()
{
// The following common parameters are not (yet) supported in this parameter set.
@@ -407,12 +418,12 @@ namespace Microsoft.PowerShell.Commands
{
}
bool allowUsingExpression = this.Context.SessionState.LanguageMode != PSLanguageMode.NoLanguage;
_usingValuesMap = ScriptBlockToPowerShellConverter.GetUsingValuesAsDictionary(
Parallel,
allowUsingExpression,
this.Context,
null);
var allowUsingExpression = this.Context.SessionState.LanguageMode != PSLanguageMode.NoLanguage;
_usingValuesMap = ScriptBlockToPowerShellConverter.GetUsingValuesForEachParallel(
scriptBlock: Parallel,
isTrustedInput: allowUsingExpression,
context: this.Context,
foreachNames: forEachNames);
// Validate using values map, which is a map of '$using:' variables referenced in the script.
// Script block variables are not allowed since their behavior is undefined outside the runspace
@@ -2440,7 +2440,7 @@ namespace Microsoft.PowerShell.Commands
throw new ArgumentNullException(nameof(localScriptBlock), "Caller needs to make sure the parameter value is not null");
}
var allUsingExprs = UsingExpressionAstSearcher.FindAllUsingExpressionExceptForWorkflow(localScriptBlock.Ast);
var allUsingExprs = UsingExpressionAstSearcher.FindAllUsingExpressions(localScriptBlock.Ast);
return allUsingExprs.Select(usingExpr => UsingExpressionAst.ExtractUsingVariable((UsingExpressionAst)usingExpr)).ToList();
}
@@ -174,11 +174,14 @@ namespace System.Management.Automation
internal class UsingExpressionAstSearcher : AstSearcher
{
internal static IEnumerable<Ast> FindAllUsingExpressionExceptForWorkflow(Ast ast)
internal static IEnumerable<Ast> FindAllUsingExpressions(Ast ast)
{
Diagnostics.Assert(ast != null, "caller to verify arguments");
var searcher = new UsingExpressionAstSearcher(astParam => astParam is UsingExpressionAst, stopOnFirst: false, searchNestedScriptBlocks: true);
var searcher = new UsingExpressionAstSearcher(
callback: astParam => astParam is UsingExpressionAst,
stopOnFirst: false,
searchNestedScriptBlocks: true);
ast.InternalVisit(searcher);
return searcher.Results;
}
@@ -313,6 +316,145 @@ namespace System.Management.Automation
}
}
/// <summary>
/// Get using values as dictionary for the Foreach-Object parallel cmdlet.
/// Ignore any using expressions that are associated with inner nested Foreach-Object parallel calls,
/// since they are only effective in the nested call scope and not the current outer scope.
/// </summary>
/// <param name = "scriptBlock">Scriptblock to search.</param>
/// <param name = "isTrustedInput">True when input is trusted.</param>
/// <param name = "context">Execution context.</param>
/// <param name = "foreachNames">List of foreach command names and aliases.</param>
/// <returns>Dictionary of using variable map.</returns>
internal static Dictionary<string, object> GetUsingValuesForEachParallel(
ScriptBlock scriptBlock,
bool isTrustedInput,
ExecutionContext context,
string[] foreachNames)
{
// Using variables for Foreach-Object -Parallel use are restricted to be within the
// Foreach-Object -Parallel call scope. This will filter the using variable map to variables
// only within the current (outer) Foreach-Object -Parallel call scope.
var usingAsts = UsingExpressionAstSearcher.FindAllUsingExpressions(scriptBlock.Ast).ToList();
UsingExpressionAst usingAst = null;
var usingValueMap = new Dictionary<string, object>(usingAsts.Count);
Version oldStrictVersion = null;
try
{
if (context != null)
{
oldStrictVersion = context.EngineSessionState.CurrentScope.StrictModeVersion;
context.EngineSessionState.CurrentScope.StrictModeVersion = PSVersionInfo.PSVersion;
}
for (int i = 0; i < usingAsts.Count; ++i)
{
usingAst = (UsingExpressionAst)usingAsts[i];
if (IsInForeachParallelCallingScope(usingAst, foreachNames))
{
var value = Compiler.GetExpressionValue(usingAst.SubExpression, isTrustedInput, context);
string usingAstKey = PsUtils.GetUsingExpressionKey(usingAst);
usingValueMap.TryAdd(usingAstKey, value);
}
}
}
catch (RuntimeException rte)
{
if (rte.ErrorRecord.FullyQualifiedErrorId.Equals("VariableIsUndefined", StringComparison.Ordinal))
{
throw InterpreterError.NewInterpreterException(
targetObject: null,
exceptionType: typeof(RuntimeException),
errorPosition: usingAst.Extent,
resourceIdAndErrorId: "UsingVariableIsUndefined",
resourceString: AutomationExceptions.UsingVariableIsUndefined,
args: rte.ErrorRecord.TargetObject);
}
}
finally
{
if (context != null)
{
context.EngineSessionState.CurrentScope.StrictModeVersion = oldStrictVersion;
}
}
return usingValueMap;
}
/// <summary>
/// Walks the using Ast to verify it is used within a foreach-object -parallel command
/// and parameter set scope, and not from within a nested foreach-object -parallel call.
/// </summary>
/// <param name="usingAst">Using Ast to check.</param>
/// <param name-"foreachNames">List of foreach-object command names.</param>
/// <returns>True if using expression is in current call scope.</returns>
private static bool IsInForeachParallelCallingScope(
UsingExpressionAst usingAst,
string[] foreachNames)
{
/*
Example:
$Test1 = "Hello"
1 | ForEach-Object -Parallel {
$using:Test1
$Test2 = "Goodbye"
1 | ForEach-Object -Parallel {
$using:Test1 # Invalid using scope
$using:Test2 # Valid using scope
}
}
*/
Diagnostics.Assert(usingAst != null, "usingAst argument cannot be null.");
// Search up the parent Ast chain for 'Foreach-Object -Parallel' commands.
Ast currentParent = usingAst.Parent;
int foreachNestedCount = 0;
while (currentParent != null)
{
// Look for Foreach-Object outer commands
if (currentParent is CommandAst commandAst)
{
foreach (var commandElement in commandAst.CommandElements)
{
if (commandElement is StringConstantExpressionAst commandName)
{
bool found = false;
foreach (var foreachName in foreachNames)
{
if (commandName.Value.Equals(foreachName, StringComparison.OrdinalIgnoreCase))
{
found = true;
break;
}
}
if (found)
{
// Verify this is foreach-object with parallel parameter set.
var bindingResult = StaticParameterBinder.BindCommand(commandAst);
if (bindingResult.BoundParameters.ContainsKey("Parallel"))
{
foreachNestedCount++;
break;
}
}
}
}
}
if (foreachNestedCount > 1)
{
// This using expression Ast is outside the original calling scope.
return false;
}
currentParent = currentParent.Parent;
}
return foreachNestedCount == 1;
}
/// <summary>
/// Get using values in the dictionary form.
/// </summary>
@@ -343,11 +485,16 @@ namespace System.Management.Automation
/// A tuple of the dictionary-form and the array-form using values.
/// If the array-form using value is null, then there are UsingExpressions used in different scopes.
/// </returns>
private static Tuple<Dictionary<string, object>, object[]> GetUsingValues(Ast body, bool isTrustedInput, ExecutionContext context, Dictionary<string, object> variables, bool filterNonUsingVariables)
private static Tuple<Dictionary<string, object>, object[]> GetUsingValues(
Ast body,
bool isTrustedInput,
ExecutionContext context,
Dictionary<string, object> variables,
bool filterNonUsingVariables)
{
Diagnostics.Assert(context != null || variables != null, "can't retrieve variables with no context and no variables");
var usingAsts = UsingExpressionAstSearcher.FindAllUsingExpressionExceptForWorkflow(body).ToList();
var usingAsts = UsingExpressionAstSearcher.FindAllUsingExpressions(body).ToList();
var usingValueArray = new object[usingAsts.Count];
var usingValueMap = new Dictionary<string, object>(usingAsts.Count);
HashSet<string> usingVarNames = (variables != null && filterNonUsingVariables) ? new HashSet<string>() : null;
@@ -456,7 +603,7 @@ namespace System.Management.Automation
/// Check if the given UsingExpression is in a different scope from the previous UsingExpression that we analyzed.
/// </summary>
/// <remarks>
/// Note that the value of <paramref name="usingExpr"/> is retrieved by calling 'UsingExpressionAstSearcher.FindAllUsingExpressionExceptForWorkflow'.
/// Note that the value of <paramref name="usingExpr"/> is retrieved by calling 'UsingExpressionAstSearcher.FindAllUsingExpressions'.
/// So <paramref name="usingExpr"/> is guaranteed not inside a workflow.
/// </remarks>
/// <param name="usingExpr">The UsingExpression to analyze.</param>
@@ -26,6 +26,67 @@ Describe 'ForEach-Object -Parallel Basic Tests' -Tags 'CI' {
$result[1] | Should -BeExactly $varArray[1]
}
It 'Verifies in scope using variables in nested calls' {
$Test = "Test1"
$results = 1..2 | ForEach-Object -Parallel {
$using:Test
$Test = "Test2"
1..2 | ForEach-Object -Parallel {
$using:Test
$Test = "Test3"
1..2 | ForEach-Object -Parallel {
$using:Test
}
}
}
$results.Count | Should -BeExactly 14
$groups = $results | Group-Object -AsHashTable
$groups['Test1'].Count | Should -BeExactly 2
$groups['Test2'].Count | Should -BeExactly 4
$groups['Test3'].Count | Should -BeExactly 8
}
It 'Verifies in scope using variables with different names in nested calls' {
$Test1 = "TestA"
$results = 1..2 | ForEach-Object -parallel {
$using:Test1
$Test2 = "TestB"
1..2 | ForEach-Object -parallel {
$using:Test2
}
}
$results.Count | Should -BeExactly 6
$groups = $results | Group-Object -AsHashTable
$groups['TestA'].Count | Should -BeExactly 2
$groups['TestB'].Count | Should -BeExactly 4
}
It 'Verifies using variable in nested scriptblock' {
$test = 'testC'
$results = 1..2 | ForEach-Object -parallel {
& { $using:test }
}
$results.Count | Should -BeExactly 2
$groups = $results | Group-Object -AsHashTable
$groups['TestC'].Count | Should -BeExactly 2
}
It 'Verifies expected error for out of scope using variable in nested calls' {
$Test = "TestZ"
1..1 | ForEach-Object -Parallel {
$using:Test
# Variable '$Test' is not defined in this scope.
1..1 | ForEach-Object -Parallel {
$using:Test
}
} -ErrorVariable usingErrors 2>$null
$usingErrors[0].FullyQualifiedErrorId | Should -BeExactly 'UsingVariableIsUndefined,Microsoft.PowerShell.Commands.ForEachObjectCommand'
}
It 'Verifies terminating error streaming' {
$result = 1..1 | ForEach-Object -Parallel { throw 'Terminating Error!'; "Hello" } 2>&1