Skip to content

Commit cadd576

Browse files
PaulHiginPaulHigin
authored
Fix ForEach-Object -Parallel when passing in script block variable (#16564)
ForeEach-Object -Parallel using variable processing walks the script block AST to determine if the using variable was defined in the current scope, and does this by tracking the number of 'ForEach-Object -Parallel' commands in the call chain. However, the provided script block is different, depending on whether it is passed in as a variable or via parameter binding. $myScript = '"Hello!"' 1 | ForEach-Object -Parallel ([scriptblock]::Create($myScript)) # Script block Ast does not contain the 'ForEach-Object' command in the call chain 1 | ForEach-Object -Parallel { "Hello!" } # Script block Ast does contain the 'ForEach-Object' command in the call chain This change takes into account the two forms of script blocks
1 parent a54b9a2 commit cadd576

3 files changed

Lines changed: 235 additions & 57 deletions

File tree

‎src/System.Management.Automation/engine/InternalCommands.cs‎

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -382,17 +382,6 @@ public void Dispose()
382382
private Exception _taskCollectionException;
383383
private string _currentLocationPath;
384384

385-
// List of Foreach-Object command names and aliases.
386-
// TODO: Look into using SessionState.Internal.GetAliasTable() to find all user created aliases.
387-
// But update Alias command logic to maintain reverse table that lists all aliases mapping
388-
// to a single command definition, for performance.
389-
private static readonly string[] forEachNames = new string[]
390-
{
391-
"ForEach-Object",
392-
"foreach",
393-
"%"
394-
};
395-
396385
private void InitParallelParameterSet()
397386
{
398387
// The following common parameters are not (yet) supported in this parameter set.
@@ -423,8 +412,7 @@ private void InitParallelParameterSet()
423412
_usingValuesMap = ScriptBlockToPowerShellConverter.GetUsingValuesForEachParallel(
424413
scriptBlock: Parallel,
425414
isTrustedInput: allowUsingExpression,
426-
context: this.Context,
427-
foreachNames: forEachNames);
415+
context: this.Context);
428416

429417
// Validate using values map, which is a map of '$using:' variables referenced in the script.
430418
// Script block variables are not allowed since their behavior is undefined outside the runspace

‎src/System.Management.Automation/engine/runtime/ScriptBlockToPowerShell.cs‎

Lines changed: 54 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -324,13 +324,11 @@ internal static PowerShell Convert(ScriptBlockAst body,
324324
/// <param name = "scriptBlock">Scriptblock to search.</param>
325325
/// <param name = "isTrustedInput">True when input is trusted.</param>
326326
/// <param name = "context">Execution context.</param>
327-
/// <param name = "foreachNames">List of foreach command names and aliases.</param>
328327
/// <returns>Dictionary of using variable map.</returns>
329328
internal static Dictionary<string, object> GetUsingValuesForEachParallel(
330329
ScriptBlock scriptBlock,
331330
bool isTrustedInput,
332-
ExecutionContext context,
333-
string[] foreachNames)
331+
ExecutionContext context)
334332
{
335333
// Using variables for Foreach-Object -Parallel use are restricted to be within the
336334
// Foreach-Object -Parallel call scope. This will filter the using variable map to variables
@@ -350,7 +348,7 @@ internal static Dictionary<string, object> GetUsingValuesForEachParallel(
350348
for (int i = 0; i < usingAsts.Count; ++i)
351349
{
352350
usingAst = (UsingExpressionAst)usingAsts[i];
353-
if (IsInForeachParallelCallingScope(usingAst, foreachNames))
351+
if (IsInForeachParallelCallingScope(scriptBlock.Ast, usingAst))
354352
{
355353
var value = Compiler.GetExpressionValue(usingAst.SubExpression, isTrustedInput, context);
356354
string usingAstKey = PsUtils.GetUsingExpressionKey(usingAst);
@@ -382,17 +380,61 @@ internal static Dictionary<string, object> GetUsingValuesForEachParallel(
382380
return usingValueMap;
383381
}
384382

383+
// List of Foreach-Object command names and aliases.
384+
// TODO: Look into using SessionState.Internal.GetAliasTable() to find all user created aliases.
385+
// But update Alias command logic to maintain reverse table that lists all aliases mapping
386+
// to a single command definition, for performance.
387+
private static readonly string[] forEachNames = new string[]
388+
{
389+
"ForEach-Object",
390+
"foreach",
391+
"%"
392+
};
393+
394+
private static bool FindForEachInCommand(CommandAst commandAst)
395+
{
396+
// Command name is always the first element in the CommandAst.
397+
// e.g., 'foreach -parallel {}'
398+
var commandNameElement = (commandAst.CommandElements.Count > 0) ? commandAst.CommandElements[0] : null;
399+
if (commandNameElement is StringConstantExpressionAst commandName)
400+
{
401+
bool found = false;
402+
foreach (var foreachName in forEachNames)
403+
{
404+
if (commandName.Value.Equals(foreachName, StringComparison.OrdinalIgnoreCase))
405+
{
406+
found = true;
407+
break;
408+
}
409+
}
410+
411+
if (found)
412+
{
413+
// Verify this is foreach-object with parallel parameter set.
414+
var bindingResult = StaticParameterBinder.BindCommand(commandAst);
415+
if (bindingResult.BoundParameters.ContainsKey("Parallel"))
416+
{
417+
return true;
418+
}
419+
}
420+
}
421+
422+
return false;
423+
}
424+
385425
/// <summary>
386426
/// Walks the using Ast to verify it is used within a foreach-object -parallel command
387427
/// and parameter set scope, and not from within a nested foreach-object -parallel call.
388428
/// </summary>
429+
/// <param name="scriptblockAst">Scriptblock Ast containing this using Ast</param>
389430
/// <param name="usingAst">Using Ast to check.</param>
390-
/// <param name-"foreachNames">List of foreach-object command names.</param>
391431
/// <returns>True if using expression is in current call scope.</returns>
392432
private static bool IsInForeachParallelCallingScope(
393-
UsingExpressionAst usingAst,
394-
string[] foreachNames)
433+
Ast scriptblockAst,
434+
UsingExpressionAst usingAst)
395435
{
436+
Diagnostics.Assert(usingAst != null, "usingAst argument cannot be null.");
437+
396438
/*
397439
Example:
398440
$Test1 = "Hello"
@@ -405,54 +447,23 @@ private static bool IsInForeachParallelCallingScope(
405447
}
406448
}
407449
*/
408-
Diagnostics.Assert(usingAst != null, "usingAst argument cannot be null.");
409450

410451
// Search up the parent Ast chain for 'Foreach-Object -Parallel' commands.
411452
Ast currentParent = usingAst.Parent;
412-
int foreachNestedCount = 0;
413-
while (currentParent != null)
453+
while (currentParent != scriptblockAst)
414454
{
415455
// Look for Foreach-Object outer commands
416-
if (currentParent is CommandAst commandAst)
456+
if (currentParent is CommandAst commandAst &&
457+
FindForEachInCommand(commandAst))
417458
{
418-
foreach (var commandElement in commandAst.CommandElements)
419-
{
420-
if (commandElement is StringConstantExpressionAst commandName)
421-
{
422-
bool found = false;
423-
foreach (var foreachName in foreachNames)
424-
{
425-
if (commandName.Value.Equals(foreachName, StringComparison.OrdinalIgnoreCase))
426-
{
427-
found = true;
428-
break;
429-
}
430-
}
431-
432-
if (found)
433-
{
434-
// Verify this is foreach-object with parallel parameter set.
435-
var bindingResult = StaticParameterBinder.BindCommand(commandAst);
436-
if (bindingResult.BoundParameters.ContainsKey("Parallel"))
437-
{
438-
foreachNestedCount++;
439-
break;
440-
}
441-
}
442-
}
443-
}
444-
}
445-
446-
if (foreachNestedCount > 1)
447-
{
448-
// This using expression Ast is outside the original calling scope.
459+
// Using Ast is outside the invoking foreach scope.
449460
return false;
450461
}
451462

452463
currentParent = currentParent.Parent;
453464
}
454465

455-
return foreachNestedCount == 1;
466+
return true;
456467
}
457468

458469
/// <summary>

‎test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1‎

Lines changed: 180 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,185 @@ Describe 'ForEach-Object -Parallel Basic Tests' -Tags 'CI' {
8787
$usingErrors[0].FullyQualifiedErrorId | Should -BeExactly 'UsingVariableIsUndefined,Microsoft.PowerShell.Commands.ForEachObjectCommand'
8888
}
8989

90+
It 'Verifies using variables with passed-in script block object' {
91+
92+
$var = "Hello"
93+
$varArray = "Hello","There"
94+
$sBlock = [scriptblock]::Create('$using:var; $using:varArray[1]')
95+
$result = 1..1 | ForEach-Object -Parallel $sBlock
96+
$result[0] | Should -BeExactly $var
97+
$result[1] | Should -BeExactly $varArray[1]
98+
}
99+
100+
It 'Verifies using variables with passed-in {} script block object' {
101+
102+
$var = "Hello"
103+
$varArray = "Hello","There"
104+
$sBlock = { $using:var; $using:varArray[1] }
105+
$result = 1..1 | ForEach-Object -Parallel $sBlock
106+
$result[0] | Should -BeExactly $var
107+
$result[1] | Should -BeExactly $varArray[1]
108+
}
109+
110+
It 'Verifies in scope using same-name variables in nested calls for passed-in script block objects' {
111+
112+
$Test1 = "Test1"
113+
$sBlock = [scriptblock]::Create(@'
114+
$using:Test1
115+
$Test2 = "Test2"
116+
$sBlock2 = [scriptblock]::Create('$using:Test2')
117+
1..2 | ForEach-Object -Parallel $sBlock2
118+
'@)
119+
$results = 1..2 | ForEach-Object -Parallel $sBlock
120+
$results.Count | Should -BeExactly 6
121+
$groups = $results | Group-Object -AsHashTable
122+
$groups['Test1'].Count | Should -BeExactly 2
123+
$groups['Test2'].Count | Should -BeExactly 4
124+
}
125+
126+
It 'Verifies in scope using same-name variables in nested calls for passed-in {} script block objects' {
127+
128+
$Test1 = "Test1"
129+
$sBlock = {
130+
$using:Test1
131+
$Test2 = "Test2"
132+
$sBlock2 = [scriptblock]::Create('$using:Test2')
133+
1..2 | ForEach-Object -Parallel $sBlock2
134+
}
135+
$results = 1..2 | ForEach-Object -Parallel $sBlock
136+
$results.Count | Should -BeExactly 6
137+
$groups = $results | Group-Object -AsHashTable
138+
$groups['Test1'].Count | Should -BeExactly 2
139+
$groups['Test2'].Count | Should -BeExactly 4
140+
}
141+
142+
It 'Verifies in scope using same-name variables in nested calls for mixed script block objects' {
143+
144+
$Test1 = "Test1"
145+
$sBlock = [scriptblock]::Create(@'
146+
$using:Test1
147+
$Test2 = "Test2"
148+
1..2 | ForEach-Object -Parallel { $using:Test2 }
149+
'@)
150+
$results = 1..2 | ForEach-Object -Parallel $sBlock
151+
$results.Count | Should -BeExactly 6
152+
$groups = $results | Group-Object -AsHashTable
153+
$groups['Test1'].Count | Should -BeExactly 2
154+
$groups['Test2'].Count | Should -BeExactly 4
155+
}
156+
157+
It 'Verifies in scope using same-name variables in nested calls for mixed script block {} objects' {
158+
159+
$Test1 = "Test1"
160+
$sBlock = {
161+
$using:Test1
162+
$Test2 = "Test2"
163+
1..2 | ForEach-Object -Parallel { $using:Test2 }
164+
}
165+
$results = 1..2 | ForEach-Object -Parallel $sBlock
166+
$results.Count | Should -BeExactly 6
167+
$groups = $results | Group-Object -AsHashTable
168+
$groups['Test1'].Count | Should -BeExactly 2
169+
$groups['Test2'].Count | Should -BeExactly 4
170+
}
171+
172+
It 'Verifies in scope using different-name variables in nested calls for passed-in script block objects' {
173+
174+
$Test1 = "Test1"
175+
$sBlock = [scriptblock]::Create(@'
176+
$using:Test1
177+
$Test2 = "Test2"
178+
$sBlock2 = [scriptblock]::Create('$using:Test2')
179+
1..2 | ForEach-Object -Parallel $sBlock2
180+
'@)
181+
$results = 1..2 | ForEach-Object -Parallel $sBlock
182+
$results.Count | Should -BeExactly 6
183+
$groups = $results | Group-Object -AsHashTable
184+
$groups['Test1'].Count | Should -BeExactly 2
185+
$groups['Test2'].Count | Should -BeExactly 4
186+
}
187+
188+
It 'Verifies in scope using different-name variables in nested calls for passed-in script {} block objects' {
189+
190+
$Test1 = "Test1"
191+
$sBlock = {
192+
$using:Test1
193+
$Test2 = "Test2"
194+
$sBlock2 = [scriptblock]::Create('$using:Test2')
195+
1..2 | ForEach-Object -Parallel $sBlock2
196+
}
197+
$results = 1..2 | ForEach-Object -Parallel $sBlock
198+
$results.Count | Should -BeExactly 6
199+
$groups = $results | Group-Object -AsHashTable
200+
$groups['Test1'].Count | Should -BeExactly 2
201+
$groups['Test2'].Count | Should -BeExactly 4
202+
}
203+
204+
It 'Verifies in scope using different-name variables in nested calls for mixed script block objects' {
205+
206+
$Test1 = "Test1"
207+
$sBlock = [scriptblock]::Create(@'
208+
$using:Test1
209+
$Test2 = "Test2"
210+
1..2 | ForEach-Object -Parallel { $using:Test2 }
211+
'@)
212+
$results = 1..2 | ForEach-Object -Parallel $sBlock
213+
$results.Count | Should -BeExactly 6
214+
$groups = $results | Group-Object -AsHashTable
215+
$groups['Test1'].Count | Should -BeExactly 2
216+
$groups['Test2'].Count | Should -BeExactly 4
217+
}
218+
219+
It 'Verifies in scope using different-name variables in nested calls for mixed script block {} objects' {
220+
221+
$Test1 = "Test1"
222+
$sBlock = {
223+
$using:Test1
224+
$Test2 = "Test2"
225+
1..2 | ForEach-Object -Parallel { $using:Test2 }
226+
}
227+
$results = 1..2 | ForEach-Object -Parallel $sBlock
228+
$results.Count | Should -BeExactly 6
229+
$groups = $results | Group-Object -AsHashTable
230+
$groups['Test1'].Count | Should -BeExactly 2
231+
$groups['Test2'].Count | Should -BeExactly 4
232+
}
233+
234+
It 'Verifies expected error for out of scope using variable in nested calls for passed-in script block objects' {
235+
236+
$Test = "TestZ"
237+
$sBlock = [scriptblock]::Create(@'
238+
$using:Test
239+
$sBlock2 = [scriptblock]::Create('$using:Test')
240+
1..1 | ForEach-Object -Parallel $sBlock2
241+
'@)
242+
1..1 | ForEach-Object -Parallel $SBlock -ErrorVariable usingErrors 2>$null
243+
$usingErrors[0].FullyQualifiedErrorId | Should -BeExactly 'UsingVariableIsUndefined,Microsoft.PowerShell.Commands.ForEachObjectCommand'
244+
}
245+
246+
It 'Verifies expected error for out of scope using variable in nested calls for passed-in {} script block objects' {
247+
248+
$Test = "TestZ"
249+
$sBlock = [scriptblock]::Create(@'
250+
$using:Test
251+
$sBlock2 = { $using:Test }
252+
1..1 | ForEach-Object -Parallel $sBlock2
253+
'@)
254+
1..1 | ForEach-Object -Parallel $SBlock -ErrorVariable usingErrors 2>$null
255+
$usingErrors[0].FullyQualifiedErrorId | Should -BeExactly 'UsingVariableIsUndefined,Microsoft.PowerShell.Commands.ForEachObjectCommand'
256+
}
257+
258+
It 'Verifies expected error for out of scope using variable in nested calls for mixed script block objects' {
259+
260+
$Test = "TestZ"
261+
$sBlock = [scriptblock]::Create(@'
262+
$using:Test
263+
1..1 | ForEach-Object -Parallel { $using:Test }
264+
'@)
265+
1..1 | ForEach-Object -Parallel $SBlock -ErrorVariable usingErrors 2>$null
266+
$usingErrors[0].FullyQualifiedErrorId | Should -BeExactly 'UsingVariableIsUndefined,Microsoft.PowerShell.Commands.ForEachObjectCommand'
267+
}
268+
90269
It 'Verifies terminating error streaming' {
91270

92271
$result = 1..1 | ForEach-Object -Parallel { throw 'Terminating Error!'; "Hello" } 2>&1
@@ -150,7 +329,7 @@ Describe 'ForEach-Object -Parallel Basic Tests' -Tags 'CI' {
150329
$pr[0].StatusDescription | Should -Be Running
151330
$ps.Dispose()
152331
}
153-
332+
154333
It 'Verifies information data streaming' {
155334

156335
$actualInformation = 1..1 | ForEach-Object -Parallel { Write-Information "Information!" } 6>&1

0 commit comments

Comments
 (0)