diff --git a/src/System.Management.Automation/engine/InternalCommands.cs b/src/System.Management.Automation/engine/InternalCommands.cs index f02ce3de805..b7b1d87cc82 100644 --- a/src/System.Management.Automation/engine/InternalCommands.cs +++ b/src/System.Management.Automation/engine/InternalCommands.cs @@ -381,6 +381,17 @@ public void Dispose() 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 @@ private void InitParallelParameterSet() { } - 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 diff --git a/src/System.Management.Automation/engine/remoting/commands/PSRemotingCmdlet.cs b/src/System.Management.Automation/engine/remoting/commands/PSRemotingCmdlet.cs index bbb2056189c..3752e4dcfd3 100644 --- a/src/System.Management.Automation/engine/remoting/commands/PSRemotingCmdlet.cs +++ b/src/System.Management.Automation/engine/remoting/commands/PSRemotingCmdlet.cs @@ -2434,7 +2434,7 @@ private static List GetUsingVariables(ScriptBlock localSc 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(); } diff --git a/src/System.Management.Automation/engine/runtime/ScriptBlockToPowerShell.cs b/src/System.Management.Automation/engine/runtime/ScriptBlockToPowerShell.cs index f5d2f51f698..93394be6808 100644 --- a/src/System.Management.Automation/engine/runtime/ScriptBlockToPowerShell.cs +++ b/src/System.Management.Automation/engine/runtime/ScriptBlockToPowerShell.cs @@ -174,11 +174,14 @@ internal static void ThrowError(ScriptBlockToPowerShellNotSupportedException ex, internal class UsingExpressionAstSearcher : AstSearcher { - internal static IEnumerable FindAllUsingExpressionExceptForWorkflow(Ast ast) + internal static IEnumerable 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 @@ internal static PowerShell Convert(ScriptBlockAst body, } } + /// + /// 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. + /// + /// Scriptblock to search. + /// True when input is trusted. + /// Execution context. + /// List of foreach command names and aliases. + /// Dictionary of using variable map. + internal static Dictionary 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(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; + } + + /// + /// 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. + /// + /// Using Ast to check. + /// List of foreach-object command names. + /// True if using expression is in current call scope. + 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; + } + /// /// Get using values in the dictionary form. /// @@ -343,11 +485,16 @@ internal static object[] GetUsingValuesAsArray(ScriptBlock scriptBlock, bool isT /// 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. /// - private static Tuple, object[]> GetUsingValues(Ast body, bool isTrustedInput, ExecutionContext context, Dictionary variables, bool filterNonUsingVariables) + private static Tuple, object[]> GetUsingValues( + Ast body, + bool isTrustedInput, + ExecutionContext context, + Dictionary 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(usingAsts.Count); HashSet usingVarNames = (variables != null && filterNonUsingVariables) ? new HashSet() : null; @@ -456,7 +603,7 @@ private static Tuple, object[]> GetUsingValues(Ast bo /// Check if the given UsingExpression is in a different scope from the previous UsingExpression that we analyzed. /// /// - /// Note that the value of is retrieved by calling 'UsingExpressionAstSearcher.FindAllUsingExpressionExceptForWorkflow'. + /// Note that the value of is retrieved by calling 'UsingExpressionAstSearcher.FindAllUsingExpressions'. /// So is guaranteed not inside a workflow. /// /// The UsingExpression to analyze. diff --git a/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 index 484454e3a85..46f85b80bd4 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Utility/Foreach-Object-Parallel.Tests.ps1 @@ -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