Fix ForEach-Object -Parallel when passing in script block variable (#16564) (#17860)

This commit is contained in:
Aditya Patwardhan 2022-08-08 13:56:34 -07:00
parent c446ba0e8b
commit f153a64400
3 changed files with 235 additions and 57 deletions

View File

@ -381,17 +381,6 @@ 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 readonly string[] forEachNames = new string[]
{
"ForEach-Object",
"foreach",
"%"
};
private void InitParallelParameterSet()
{
// The following common parameters are not (yet) supported in this parameter set.
@ -422,8 +411,7 @@ namespace Microsoft.PowerShell.Commands
_usingValuesMap = ScriptBlockToPowerShellConverter.GetUsingValuesForEachParallel(
scriptBlock: Parallel,
isTrustedInput: allowUsingExpression,
context: this.Context,
foreachNames: forEachNames);
context: this.Context);
// 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

View File

@ -324,13 +324,11 @@ namespace System.Management.Automation
/// <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)
ExecutionContext context)
{
// 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
@ -350,7 +348,7 @@ namespace System.Management.Automation
for (int i = 0; i < usingAsts.Count; ++i)
{
usingAst = (UsingExpressionAst)usingAsts[i];
if (IsInForeachParallelCallingScope(usingAst, foreachNames))
if (IsInForeachParallelCallingScope(scriptBlock.Ast, usingAst))
{
var value = Compiler.GetExpressionValue(usingAst.SubExpression, isTrustedInput, context);
string usingAstKey = PsUtils.GetUsingExpressionKey(usingAst);
@ -382,17 +380,61 @@ namespace System.Management.Automation
return usingValueMap;
}
// 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 readonly string[] forEachNames = new string[]
{
"ForEach-Object",
"foreach",
"%"
};
private static bool FindForEachInCommand(CommandAst commandAst)
{
// Command name is always the first element in the CommandAst.
// e.g., 'foreach -parallel {}'
var commandNameElement = (commandAst.CommandElements.Count > 0) ? commandAst.CommandElements[0] : null;
if (commandNameElement 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"))
{
return true;
}
}
}
return false;
}
/// <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="scriptblockAst">Scriptblock Ast containing this using Ast</param>
/// <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)
Ast scriptblockAst,
UsingExpressionAst usingAst)
{
Diagnostics.Assert(usingAst != null, "usingAst argument cannot be null.");
/*
Example:
$Test1 = "Hello"
@ -405,54 +447,23 @@ namespace System.Management.Automation
}
}
*/
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)
while (currentParent != scriptblockAst)
{
// Look for Foreach-Object outer commands
if (currentParent is CommandAst commandAst)
if (currentParent is CommandAst commandAst &&
FindForEachInCommand(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.
// Using Ast is outside the invoking foreach scope.
return false;
}
currentParent = currentParent.Parent;
}
return foreachNestedCount == 1;
return true;
}
/// <summary>

View File

@ -87,6 +87,185 @@ Describe 'ForEach-Object -Parallel Basic Tests' -Tags 'CI' {
$usingErrors[0].FullyQualifiedErrorId | Should -BeExactly 'UsingVariableIsUndefined,Microsoft.PowerShell.Commands.ForEachObjectCommand'
}
It 'Verifies using variables with passed-in script block object' {
$var = "Hello"
$varArray = "Hello","There"
$sBlock = [scriptblock]::Create('$using:var; $using:varArray[1]')
$result = 1..1 | ForEach-Object -Parallel $sBlock
$result[0] | Should -BeExactly $var
$result[1] | Should -BeExactly $varArray[1]
}
It 'Verifies using variables with passed-in {} script block object' {
$var = "Hello"
$varArray = "Hello","There"
$sBlock = { $using:var; $using:varArray[1] }
$result = 1..1 | ForEach-Object -Parallel $sBlock
$result[0] | Should -BeExactly $var
$result[1] | Should -BeExactly $varArray[1]
}
It 'Verifies in scope using same-name variables in nested calls for passed-in script block objects' {
$Test1 = "Test1"
$sBlock = [scriptblock]::Create(@'
$using:Test1
$Test2 = "Test2"
$sBlock2 = [scriptblock]::Create('$using:Test2')
1..2 | ForEach-Object -Parallel $sBlock2
'@)
$results = 1..2 | ForEach-Object -Parallel $sBlock
$results.Count | Should -BeExactly 6
$groups = $results | Group-Object -AsHashTable
$groups['Test1'].Count | Should -BeExactly 2
$groups['Test2'].Count | Should -BeExactly 4
}
It 'Verifies in scope using same-name variables in nested calls for passed-in {} script block objects' {
$Test1 = "Test1"
$sBlock = {
$using:Test1
$Test2 = "Test2"
$sBlock2 = [scriptblock]::Create('$using:Test2')
1..2 | ForEach-Object -Parallel $sBlock2
}
$results = 1..2 | ForEach-Object -Parallel $sBlock
$results.Count | Should -BeExactly 6
$groups = $results | Group-Object -AsHashTable
$groups['Test1'].Count | Should -BeExactly 2
$groups['Test2'].Count | Should -BeExactly 4
}
It 'Verifies in scope using same-name variables in nested calls for mixed script block objects' {
$Test1 = "Test1"
$sBlock = [scriptblock]::Create(@'
$using:Test1
$Test2 = "Test2"
1..2 | ForEach-Object -Parallel { $using:Test2 }
'@)
$results = 1..2 | ForEach-Object -Parallel $sBlock
$results.Count | Should -BeExactly 6
$groups = $results | Group-Object -AsHashTable
$groups['Test1'].Count | Should -BeExactly 2
$groups['Test2'].Count | Should -BeExactly 4
}
It 'Verifies in scope using same-name variables in nested calls for mixed script block {} objects' {
$Test1 = "Test1"
$sBlock = {
$using:Test1
$Test2 = "Test2"
1..2 | ForEach-Object -Parallel { $using:Test2 }
}
$results = 1..2 | ForEach-Object -Parallel $sBlock
$results.Count | Should -BeExactly 6
$groups = $results | Group-Object -AsHashTable
$groups['Test1'].Count | Should -BeExactly 2
$groups['Test2'].Count | Should -BeExactly 4
}
It 'Verifies in scope using different-name variables in nested calls for passed-in script block objects' {
$Test1 = "Test1"
$sBlock = [scriptblock]::Create(@'
$using:Test1
$Test2 = "Test2"
$sBlock2 = [scriptblock]::Create('$using:Test2')
1..2 | ForEach-Object -Parallel $sBlock2
'@)
$results = 1..2 | ForEach-Object -Parallel $sBlock
$results.Count | Should -BeExactly 6
$groups = $results | Group-Object -AsHashTable
$groups['Test1'].Count | Should -BeExactly 2
$groups['Test2'].Count | Should -BeExactly 4
}
It 'Verifies in scope using different-name variables in nested calls for passed-in script {} block objects' {
$Test1 = "Test1"
$sBlock = {
$using:Test1
$Test2 = "Test2"
$sBlock2 = [scriptblock]::Create('$using:Test2')
1..2 | ForEach-Object -Parallel $sBlock2
}
$results = 1..2 | ForEach-Object -Parallel $sBlock
$results.Count | Should -BeExactly 6
$groups = $results | Group-Object -AsHashTable
$groups['Test1'].Count | Should -BeExactly 2
$groups['Test2'].Count | Should -BeExactly 4
}
It 'Verifies in scope using different-name variables in nested calls for mixed script block objects' {
$Test1 = "Test1"
$sBlock = [scriptblock]::Create(@'
$using:Test1
$Test2 = "Test2"
1..2 | ForEach-Object -Parallel { $using:Test2 }
'@)
$results = 1..2 | ForEach-Object -Parallel $sBlock
$results.Count | Should -BeExactly 6
$groups = $results | Group-Object -AsHashTable
$groups['Test1'].Count | Should -BeExactly 2
$groups['Test2'].Count | Should -BeExactly 4
}
It 'Verifies in scope using different-name variables in nested calls for mixed script block {} objects' {
$Test1 = "Test1"
$sBlock = {
$using:Test1
$Test2 = "Test2"
1..2 | ForEach-Object -Parallel { $using:Test2 }
}
$results = 1..2 | ForEach-Object -Parallel $sBlock
$results.Count | Should -BeExactly 6
$groups = $results | Group-Object -AsHashTable
$groups['Test1'].Count | Should -BeExactly 2
$groups['Test2'].Count | Should -BeExactly 4
}
It 'Verifies expected error for out of scope using variable in nested calls for passed-in script block objects' {
$Test = "TestZ"
$sBlock = [scriptblock]::Create(@'
$using:Test
$sBlock2 = [scriptblock]::Create('$using:Test')
1..1 | ForEach-Object -Parallel $sBlock2
'@)
1..1 | ForEach-Object -Parallel $SBlock -ErrorVariable usingErrors 2>$null
$usingErrors[0].FullyQualifiedErrorId | Should -BeExactly 'UsingVariableIsUndefined,Microsoft.PowerShell.Commands.ForEachObjectCommand'
}
It 'Verifies expected error for out of scope using variable in nested calls for passed-in {} script block objects' {
$Test = "TestZ"
$sBlock = [scriptblock]::Create(@'
$using:Test
$sBlock2 = { $using:Test }
1..1 | ForEach-Object -Parallel $sBlock2
'@)
1..1 | ForEach-Object -Parallel $SBlock -ErrorVariable usingErrors 2>$null
$usingErrors[0].FullyQualifiedErrorId | Should -BeExactly 'UsingVariableIsUndefined,Microsoft.PowerShell.Commands.ForEachObjectCommand'
}
It 'Verifies expected error for out of scope using variable in nested calls for mixed script block objects' {
$Test = "TestZ"
$sBlock = [scriptblock]::Create(@'
$using:Test
1..1 | ForEach-Object -Parallel { $using:Test }
'@)
1..1 | ForEach-Object -Parallel $SBlock -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
@ -150,7 +329,7 @@ Describe 'ForEach-Object -Parallel Basic Tests' -Tags 'CI' {
$pr[0].StatusDescription | Should -Be Running
$ps.Dispose()
}
It 'Verifies information data streaming' {
$actualInformation = 1..1 | ForEach-Object -Parallel { Write-Information "Information!" } 6>&1