From 15c7249c6ca0f0e844e0bf5cde23556fc402e16d Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Wed, 15 Dec 2021 13:44:40 -0800 Subject: [PATCH 01/12] Report error when PowerShell built-in modules are missing --- .../engine/CommandDiscovery.cs | 25 ++++--- .../engine/InitialSessionState.cs | 55 +++----------- .../engine/Modules/ImportModuleCommand.cs | 2 +- .../engine/Modules/ModuleCmdletBase.cs | 73 +++++++++++++------ .../engine/Modules/ModuleIntrinsics.cs | 14 +--- .../engine/Modules/ModuleUtils.cs | 2 +- .../engine/remoting/client/remoterunspace.cs | 8 -- .../resources/DiscoveryExceptions.resx | 4 + .../resources/Modules.resx | 3 + 9 files changed, 86 insertions(+), 100 deletions(-) diff --git a/src/System.Management.Automation/engine/CommandDiscovery.cs b/src/System.Management.Automation/engine/CommandDiscovery.cs index 6d7fb77eba8..2ec887afdfb 100644 --- a/src/System.Management.Automation/engine/CommandDiscovery.cs +++ b/src/System.Management.Automation/engine/CommandDiscovery.cs @@ -1091,24 +1091,27 @@ private static CommandInfo TryModuleAutoDiscovery(string commandName, if (exportedCommands == null) { continue; } - CommandTypes exportedCommandTypes; // Skip if module only has class or other types and no commands. - if (exportedCommands.TryGetValue(commandName, out exportedCommandTypes)) + if (exportedCommands.TryGetValue(commandName, out CommandTypes exportedCommandTypes)) { - Exception exception; discoveryTracer.WriteLine("Found in module: {0}", expandedModulePath); - Collection matchingModule = AutoloadSpecifiedModule(expandedModulePath, context, + Collection matchingModule = AutoloadSpecifiedModule( + expandedModulePath, + context, cmdletInfo != null ? cmdletInfo.Visibility : SessionStateEntryVisibility.Private, - out exception); - lastError = exception; - if ((matchingModule == null) || (matchingModule.Count == 0)) + out lastError); + + if (matchingModule == null || matchingModule.Count == 0) { - string error = StringUtil.Format(DiscoveryExceptions.CouldNotAutoImportMatchingModule, commandName, moduleShortName); - CommandNotFoundException commandNotFound = new CommandNotFoundException( + string errorMessage = lastError is null + ? StringUtil.Format(DiscoveryExceptions.CouldNotAutoImportMatchingModule, commandName, moduleShortName) + : StringUtil.Format(DiscoveryExceptions.CouldNotAutoImportMatchingModuleWithErrorMessage, commandName, moduleShortName, lastError.Message); + + throw new CommandNotFoundException( originalCommandName, lastError, - "CouldNotAutoloadMatchingModule", error); - throw commandNotFound; + nameof(DiscoveryExceptions.CouldNotAutoImportMatchingModule), + errorMessage); } result = LookupCommandInfo(commandName, commandTypes, searchResolutionOptions, commandOrigin, context); diff --git a/src/System.Management.Automation/engine/InitialSessionState.cs b/src/System.Management.Automation/engine/InitialSessionState.cs index 05eb871053d..9ee683f83d0 100644 --- a/src/System.Management.Automation/engine/InitialSessionState.cs +++ b/src/System.Management.Automation/engine/InitialSessionState.cs @@ -1663,11 +1663,6 @@ public InitialSessionState Clone() ss.DisableFormatUpdates = this.DisableFormatUpdates; - foreach (var s in this.defaultSnapins) - { - ss.defaultSnapins.Add(s); - } - foreach (var s in ImportedSnapins) { ss.ImportedSnapins.Add(s.Key, s.Value); @@ -3801,34 +3796,26 @@ public PSSnapInInfo ImportPSSnapIn(string name, out PSSnapInException warning) // Now actually load the snapin... PSSnapInInfo snapin = ImportPSSnapIn(newPSSnapIn, out warning); - if (snapin != null) - { - ImportedSnapins.Add(snapin.Name, snapin); - } return snapin; } internal PSSnapInInfo ImportCorePSSnapIn() { - // Load Microsoft.PowerShell.Core as a snapin + // Load Microsoft.PowerShell.Core as a snapin. PSSnapInInfo coreSnapin = PSSnapInReader.ReadCoreEngineSnapIn(); - this.defaultSnapins.Add(coreSnapin); - try - { - PSSnapInException warning; - this.ImportPSSnapIn(coreSnapin, out warning); - } - catch (PSSnapInException) - { - throw; - } - + ImportPSSnapIn(coreSnapin, out _); return coreSnapin; } internal PSSnapInInfo ImportPSSnapIn(PSSnapInInfo psSnapInInfo, out PSSnapInException warning) { + if (psSnapInInfo == null) + { + ArgumentNullException e = new ArgumentNullException(nameof(psSnapInInfo)); + throw e; + } + // See if the snapin is already loaded. If has been then there will be an entry in the // Assemblies list for it already... bool reload = true; @@ -3861,12 +3848,6 @@ internal PSSnapInInfo ImportPSSnapIn(PSSnapInInfo psSnapInInfo, out PSSnapInExce Dictionary> aliases = null; Dictionary providers = null; - if (psSnapInInfo == null) - { - ArgumentNullException e = new ArgumentNullException(nameof(psSnapInInfo)); - throw e; - } - Assembly assembly = null; string helpFile = null; @@ -3985,29 +3966,16 @@ internal PSSnapInInfo ImportPSSnapIn(PSSnapInInfo psSnapInInfo, out PSSnapInExce } } + ImportedSnapins.Add(psSnapInInfo.Name, psSnapInInfo); return psSnapInInfo; } internal List GetPSSnapIn(string psSnapinName) { List loadedSnapins = null; - foreach (var defaultSnapin in defaultSnapins) - { - if (defaultSnapin.Name.Equals(psSnapinName, StringComparison.OrdinalIgnoreCase)) - { - if (loadedSnapins == null) - { - loadedSnapins = new List(); - } - - loadedSnapins.Add(defaultSnapin); - } - } - - PSSnapInInfo importedSnapin = null; - if (ImportedSnapins.TryGetValue(psSnapinName, out importedSnapin)) + if (ImportedSnapins.TryGetValue(psSnapinName, out PSSnapInInfo importedSnapin)) { - if (loadedSnapins == null) + if (loadedSnapins is null) { loadedSnapins = new List(); } @@ -4886,7 +4854,6 @@ internal static void RemoveAllDrivesForProvider(ProviderInfo pi, SessionStateInt internal static readonly string CoreSnapin = "Microsoft.PowerShell.Core"; internal static readonly string CoreModule = "Microsoft.PowerShell.Core"; - internal Collection defaultSnapins = new Collection(); // The list of engine modules to create warnings when you try to remove them internal static readonly HashSet EngineModules = new HashSet(StringComparer.OrdinalIgnoreCase) diff --git a/src/System.Management.Automation/engine/Modules/ImportModuleCommand.cs b/src/System.Management.Automation/engine/Modules/ImportModuleCommand.cs index 2cbb30bcb00..b36fec172de 100644 --- a/src/System.Management.Automation/engine/Modules/ImportModuleCommand.cs +++ b/src/System.Management.Automation/engine/Modules/ImportModuleCommand.cs @@ -762,7 +762,7 @@ private PSModuleInfo ImportModule_LocallyViaName(ImportModuleOptions importModul // Check if module could be a snapin. This was the case for PowerShell version 2 engine modules. if (InitialSessionState.IsEngineModule(name)) { - PSSnapInInfo snapin = ModuleCmdletBase.GetEngineSnapIn(Context, name); + PSSnapInInfo snapin = GetEngineSnapIn(Context, name); // Return the command if we found a module if (snapin != null) diff --git a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs index 90019e3bfed..73a6eed1b10 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs @@ -280,6 +280,18 @@ internal List MatchAll "ModuleVersion" }; + private static readonly HashSet s_builtInModules = new(StringComparer.OrdinalIgnoreCase) + { + "CimCmdlets", + "Microsoft.PowerShell.Diagnostics", + "Microsoft.PowerShell.Host", + "Microsoft.PowerShell.Management", + "Microsoft.PowerShell.Security", + "Microsoft.PowerShell.Utility", + "Microsoft.WSMan.Management", + "PSDiagnostics", + }; + /// /// When module manifests lack a CompatiblePSEditions field, /// they will be treated as if they have this value. @@ -2375,17 +2387,15 @@ internal PSModuleInfo LoadModuleManifest( { if (importingModule) { + // In case the module to be loaded in WinCompat mode is a PowerShell built-in module, such as the Utility module, + // we need to check whether the built-in module is actually available in the $PSHOME module path. + // This is because a user may not correctly deploy the built-in modules, and that would result in some confusing + // errors when module auto-loading silently attempts to load those modules from the 'System32' module path. + CheckAvailabilityOfBuiltInModules(ModuleIntrinsics.GetModuleName(moduleManifestPath)); IList moduleProxies = ImportModulesUsingWinCompat(new string[] { moduleManifestPath }, null, options); - // we are loading by a single ManifestPath so expect max of 1 - if (moduleProxies.Count > 0) - { - return moduleProxies[0]; - } - else - { - return null; - } + // We are loading by a single ManifestPath so expect max of 1 + return moduleProxies.Count > 0 ? moduleProxies[0] : null; } } else @@ -3706,7 +3716,7 @@ internal static object IsModuleLoaded(ExecutionContext context, ModuleSpecificat // If the RequiredModule is one of the Engine modules, then they could have been loaded as snapins (using InitialSessionState.CreateDefault()) if (result == null && InitialSessionState.IsEngineModule(requiredModule.Name)) { - result = ModuleCmdletBase.GetEngineSnapIn(context, requiredModule.Name); + result = GetEngineSnapIn(context, requiredModule.Name); if (result != null) { loaded = true; @@ -4883,10 +4893,37 @@ internal static void SyncCurrentLocationHandler(object sender, LocationChangedEv } } - internal static System.EventHandler SyncCurrentLocationDelegate; + internal static EventHandler SyncCurrentLocationDelegate; internal virtual IList ImportModulesUsingWinCompat(IEnumerable moduleNames, IEnumerable moduleFullyQualifiedNames, ImportModuleOptions importModuleOptions) { throw new System.NotImplementedException(); } + private void CheckAvailabilityOfBuiltInModules(string moduleName) + { + // Check if the module is a PowerShell built-in module. If so, normalize the module name; if not, skip the module. + if (!s_builtInModules.TryGetValue(moduleName, out moduleName)) + { + return; + } + + // When it was imported as a snapin, we skip the check because it's OK to not have the module available in such case. + if (GetEngineSnapIn(Context, moduleName) is not null) + { + return; + } + + // Check if the module exists in the $PSHOME module path. If not, throws exception. + string psHomeModulePath = ModuleIntrinsics.GetPSHomeModulePath(); + string moduleFolder = Path.Join(psHomeModulePath, moduleName); + if (!Directory.Exists(moduleFolder)) + { + throw new InvalidOperationException( + StringUtil.Format( + Modules.CannotFindBuiltInModules, + moduleName, + psHomeModulePath)); + } + } + private void RemoveTypesAndFormatting( IList formatFilesToRemove, IList typeFilesToRemove) @@ -7364,19 +7401,9 @@ private static void ValidateCommandName(ModuleCmdletBase cmdlet, /// internal static PSSnapInInfo GetEngineSnapIn(ExecutionContext context, string name) { - HashSet snapinSet = new HashSet(); - List cmdlets = context.SessionState.InvokeCommand.GetCmdlets(); - foreach (CmdletInfo cmdlet in cmdlets) - { - PSSnapInInfo snapin = cmdlet.PSSnapIn; - if (snapin != null && !snapinSet.Contains(snapin)) - snapinSet.Add(snapin); - } - - foreach (PSSnapInInfo snapin in snapinSet) + if (context.CurrentRunspace.InitialSessionState.ImportedSnapins.TryGetValue(name, out PSSnapInInfo snapin)) { - if (string.Equals(snapin.Name, name, StringComparison.OrdinalIgnoreCase)) - return snapin; + return snapin; } return null; diff --git a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs index 1d8fe6bc8b5..c83b055315a 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs @@ -976,20 +976,10 @@ internal static string GetPSHomeModulePath() try { string psHome = Utils.DefaultPowerShellAppBase; - if (!string.IsNullOrEmpty(psHome)) - { - // Win8: 584267 Powershell Modules are listed twice in x86, and cannot be removed - // This happens because ModuleTable uses Path as the key and CBS installer - // expands the path to include "SysWOW64" (for - // HKEY_LOCAL_MACHINE\SOFTWARE\Wow6432Node\Microsoft\PowerShell\3\PowerShellEngine ApplicationBase). - // Because of this, the module that is getting loaded during startup (through LocalRunspace) - // is using "SysWow64" in the key. Later, when Import-Module is called, it loads the - // module using ""System32" in the key. #if !UNIX - psHome = psHome.ToLowerInvariant().Replace("\\syswow64\\", "\\system32\\"); + psHome = psHome.ToLowerInvariant().Replace(@"\syswow64\", @"\system32\"); #endif - Interlocked.CompareExchange(ref s_psHomeModulePath, Path.Combine(psHome, "Modules"), null); - } + Interlocked.CompareExchange(ref s_psHomeModulePath, Path.Combine(psHome, "Modules"), null); } catch (System.Security.SecurityException) { diff --git a/src/System.Management.Automation/engine/Modules/ModuleUtils.cs b/src/System.Management.Automation/engine/Modules/ModuleUtils.cs index 26d3174ca3b..8dab2283fc5 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleUtils.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleUtils.cs @@ -121,7 +121,7 @@ internal static bool IsPSEditionCompatible( #if UNIX return true; #else - if (!ModuleUtils.IsOnSystem32ModulePath(moduleManifestPath)) + if (!IsOnSystem32ModulePath(moduleManifestPath)) { return true; } diff --git a/src/System.Management.Automation/engine/remoting/client/remoterunspace.cs b/src/System.Management.Automation/engine/remoting/client/remoterunspace.cs index 7ddd657872e..f2e9bd9093e 100644 --- a/src/System.Management.Automation/engine/remoting/client/remoterunspace.cs +++ b/src/System.Management.Automation/engine/remoting/client/remoterunspace.cs @@ -221,11 +221,7 @@ public override InitialSessionState InitialSessionState { get { -#pragma warning disable 56503 - throw PSTraceSource.NewNotImplementedException(); - -#pragma warning restore 56503 } } @@ -236,11 +232,7 @@ public override JobManager JobManager { get { -#pragma warning disable 56503 - throw PSTraceSource.NewNotImplementedException(); - -#pragma warning restore 56503 } } diff --git a/src/System.Management.Automation/resources/DiscoveryExceptions.resx b/src/System.Management.Automation/resources/DiscoveryExceptions.resx index d3923969e8d..2d8445deb4f 100644 --- a/src/System.Management.Automation/resources/DiscoveryExceptions.resx +++ b/src/System.Management.Automation/resources/DiscoveryExceptions.resx @@ -206,6 +206,10 @@ The #requires statement must be in one of the following formats: The '{0}' command was found in the module '{1}', but the module could not be loaded. For more information, run 'Import-Module {1}'. + + The '{0}' command was found in the module '{1}', but the module could not be loaded due to the following error: [{2}] +For more information, run 'Import-Module {1}'. + The module '{0}' could not be loaded. For more information, run 'Import-Module {0}'. diff --git a/src/System.Management.Automation/resources/Modules.resx b/src/System.Management.Automation/resources/Modules.resx index 345811502b6..ff532b505a3 100644 --- a/src/System.Management.Automation/resources/Modules.resx +++ b/src/System.Management.Automation/resources/Modules.resx @@ -624,4 +624,7 @@ Cannot create new module while the session is in ConstrainedLanguage mode. + + The built-in module '{0}' cannot be found under the $PSHOME module path '{1}'. Please make sure the PowerShell built-in modules are available because they are required for PowerShell to function properly. + From 55293692e651dc8868a7538a8de6c81c527711bb Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Wed, 15 Dec 2021 14:45:21 -0800 Subject: [PATCH 02/12] Fix a test failure --- src/System.Management.Automation/engine/CommandDiscovery.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/System.Management.Automation/engine/CommandDiscovery.cs b/src/System.Management.Automation/engine/CommandDiscovery.cs index 2ec887afdfb..5e404c41072 100644 --- a/src/System.Management.Automation/engine/CommandDiscovery.cs +++ b/src/System.Management.Automation/engine/CommandDiscovery.cs @@ -1110,7 +1110,7 @@ private static CommandInfo TryModuleAutoDiscovery(string commandName, throw new CommandNotFoundException( originalCommandName, lastError, - nameof(DiscoveryExceptions.CouldNotAutoImportMatchingModule), + "CouldNotAutoloadMatchingModule", errorMessage); } From c6af3c90f46a0201bc6b16ca4fd64c206cfb229b Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Wed, 15 Dec 2021 14:55:28 -0800 Subject: [PATCH 03/12] A little cleanup changes --- .../engine/CommandDiscovery.cs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/System.Management.Automation/engine/CommandDiscovery.cs b/src/System.Management.Automation/engine/CommandDiscovery.cs index 5e404c41072..0ae57e0ebad 100644 --- a/src/System.Management.Automation/engine/CommandDiscovery.cs +++ b/src/System.Management.Automation/engine/CommandDiscovery.cs @@ -1063,14 +1063,12 @@ private static CommandInfo TryModuleAutoDiscovery(string commandName, return null; CmdletInfo cmdletInfo = context.SessionState.InvokeCommand.GetCmdlet("Microsoft.PowerShell.Core\\Get-Module"); - if ((commandOrigin == CommandOrigin.Internal) || - ((cmdletInfo != null) && (cmdletInfo.Visibility == SessionStateEntryVisibility.Public))) + if (commandOrigin == CommandOrigin.Internal || cmdletInfo?.Visibility == SessionStateEntryVisibility.Public) { // Search for a module with a matching command, as long as the user would have the ability to // import the module. cmdletInfo = context.SessionState.InvokeCommand.GetCmdlet("Microsoft.PowerShell.Core\\Import-Module"); - if (((commandOrigin == CommandOrigin.Internal) || - ((cmdletInfo != null) && (cmdletInfo.Visibility == SessionStateEntryVisibility.Public)))) + if (commandOrigin == CommandOrigin.Internal || cmdletInfo?.Visibility == SessionStateEntryVisibility.Public) { discoveryTracer.WriteLine("Executing non module-qualified search: {0}", commandName); context.CommandDiscovery.RegisterLookupCommandInfoAction("ActiveModuleSearch", commandName); @@ -1085,8 +1083,8 @@ private static CommandInfo TryModuleAutoDiscovery(string commandName, { // WinBlue:69141 - We need to get the full path here because the module path might be C:\Users\User1\DOCUME~1 // While the exportedCommands are cached, they are cached with the full path - string expandedModulePath = IO.Path.GetFullPath(modulePath); - string moduleShortName = System.IO.Path.GetFileNameWithoutExtension(expandedModulePath); + string expandedModulePath = Path.GetFullPath(modulePath); + string moduleShortName = Path.GetFileNameWithoutExtension(expandedModulePath); var exportedCommands = AnalysisCache.GetExportedCommands(expandedModulePath, false, context); if (exportedCommands == null) { continue; } From d7db8b3629f8a4670a76dce0eccbb04ebe94d0fe Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Thu, 16 Dec 2021 17:06:27 -0800 Subject: [PATCH 04/12] Try not to assume the built-in moduels are only available under PSHOME module path --- .../engine/CommandDiscovery.cs | 8 +- .../engine/InitialSessionState.cs | 12 +- .../engine/Modules/ImportModuleCommand.cs | 105 +++++++++++------- .../engine/Modules/ModuleCmdletBase.cs | 72 ++++-------- .../resources/Modules.resx | 4 +- 5 files changed, 93 insertions(+), 108 deletions(-) diff --git a/src/System.Management.Automation/engine/CommandDiscovery.cs b/src/System.Management.Automation/engine/CommandDiscovery.cs index 0ae57e0ebad..f19715f4307 100644 --- a/src/System.Management.Automation/engine/CommandDiscovery.cs +++ b/src/System.Management.Automation/engine/CommandDiscovery.cs @@ -376,9 +376,8 @@ private static void VerifyRequiredSnapins(IEnumerable req foreach (var requiresPSSnapIn in requiresPSSnapIns) { - IEnumerable loadedPSSnapIns = null; - loadedPSSnapIns = context.InitialSessionState.GetPSSnapIn(requiresPSSnapIn.Name); - if (loadedPSSnapIns == null || !loadedPSSnapIns.Any()) + var loadedPSSnapIn = context.InitialSessionState.GetPSSnapIn(requiresPSSnapIn.Name); + if (loadedPSSnapIn is null) { if (requiresMissingPSSnapIns == null) { @@ -390,8 +389,7 @@ private static void VerifyRequiredSnapins(IEnumerable req else { // the requires PSSnapin is loaded. now check the PSSnapin version - PSSnapInInfo loadedPSSnapIn = loadedPSSnapIns.First(); - Diagnostics.Assert(loadedPSSnapIn.Version != null, + Dbg.Assert(loadedPSSnapIn.Version != null, string.Format( CultureInfo.InvariantCulture, "Version is null for loaded PSSnapin {0}.", loadedPSSnapIn)); diff --git a/src/System.Management.Automation/engine/InitialSessionState.cs b/src/System.Management.Automation/engine/InitialSessionState.cs index 9ee683f83d0..1aec5a99af4 100644 --- a/src/System.Management.Automation/engine/InitialSessionState.cs +++ b/src/System.Management.Automation/engine/InitialSessionState.cs @@ -3970,20 +3970,14 @@ internal PSSnapInInfo ImportPSSnapIn(PSSnapInInfo psSnapInInfo, out PSSnapInExce return psSnapInInfo; } - internal List GetPSSnapIn(string psSnapinName) + internal PSSnapInInfo GetPSSnapIn(string psSnapinName) { - List loadedSnapins = null; if (ImportedSnapins.TryGetValue(psSnapinName, out PSSnapInInfo importedSnapin)) { - if (loadedSnapins is null) - { - loadedSnapins = new List(); - } - - loadedSnapins.Add(importedSnapin); + return importedSnapin; } - return loadedSnapins; + return null; } [SuppressMessage("Microsoft.Reliability", "CA2001:AvoidCallingProblematicMethods", MessageId = "System.Reflection.Assembly.LoadFrom")] diff --git a/src/System.Management.Automation/engine/Modules/ImportModuleCommand.cs b/src/System.Management.Automation/engine/Modules/ImportModuleCommand.cs index b36fec172de..a4ec3634ee2 100644 --- a/src/System.Management.Automation/engine/Modules/ImportModuleCommand.cs +++ b/src/System.Management.Automation/engine/Modules/ImportModuleCommand.cs @@ -762,7 +762,7 @@ private PSModuleInfo ImportModule_LocallyViaName(ImportModuleOptions importModul // Check if module could be a snapin. This was the case for PowerShell version 2 engine modules. if (InitialSessionState.IsEngineModule(name)) { - PSSnapInInfo snapin = GetEngineSnapIn(Context, name); + PSSnapInInfo snapin = Context.CurrentRunspace.InitialSessionState.GetPSSnapIn(name); // Return the command if we found a module if (snapin != null) @@ -1945,27 +1945,26 @@ private bool IsModuleInDenyList(string[] moduleDenyList, string moduleName, Modu return match; } - private List FilterModuleCollection(IEnumerable moduleCollection) + private IEnumerable FilterModuleCollection(IEnumerable moduleCollection) { - List filteredModuleCollection = null; - if (moduleCollection != null) + if (moduleCollection is null) { - // the ModuleDeny list is cached in PowerShellConfig object - string[] moduleDenyList = PowerShellConfig.Instance.GetWindowsPowerShellCompatibilityModuleDenyList(); - if (moduleDenyList?.Any() != true) - { - filteredModuleCollection = new List(moduleCollection); - } - else + return null; + } + + // The ModuleDeny list is cached in PowerShellConfig object + string[] moduleDenyList = PowerShellConfig.Instance.GetWindowsPowerShellCompatibilityModuleDenyList(); + if (moduleDenyList is null || moduleDenyList.Length == 0) + { + return moduleCollection; + } + + var filteredModuleCollection = new List(); + foreach (var module in moduleCollection) + { + if (!IsModuleInDenyList(moduleDenyList, module as string, module as ModuleSpecification)) { - filteredModuleCollection = new List(); - foreach (var module in moduleCollection) - { - if (!IsModuleInDenyList(moduleDenyList, module as string, module as ModuleSpecification)) - { - filteredModuleCollection.Add(module); - } - } + filteredModuleCollection.Add(module); } } @@ -1977,38 +1976,62 @@ private void PrepareNoClobberWinCompatModuleImport(string moduleName, ModuleSpec Debug.Assert(string.IsNullOrEmpty(moduleName) ^ (moduleSpec == null), "Either moduleName or moduleSpec must be specified"); // moduleName can be just a module name and it also can be a full path to psd1 from which we need to extract the module name - string coreModuleToLoad = ModuleIntrinsics.GetModuleName(moduleSpec == null ? moduleName : moduleSpec.Name); + string moduleToLoad = ModuleIntrinsics.GetModuleName(moduleSpec is null ? moduleName : moduleSpec.Name); + + var isBuiltInModule = BuiltInModules.TryGetValue(moduleToLoad, out string normalizedName); + if (isBuiltInModule) + { + moduleToLoad = normalizedName; + } - var isModuleToLoadEngineModule = InitialSessionState.IsEngineModule(coreModuleToLoad); string[] noClobberModuleList = PowerShellConfig.Instance.GetWindowsPowerShellCompatibilityNoClobberModuleList(); - if (isModuleToLoadEngineModule || ((noClobberModuleList != null) && noClobberModuleList.Contains(coreModuleToLoad, StringComparer.OrdinalIgnoreCase))) + if (isBuiltInModule || noClobberModuleList?.Contains(moduleToLoad, StringComparer.OrdinalIgnoreCase) == true) { // if it is one of engine modules - first try to load it from $PSHOME\Modules // otherwise rely on $env:PSModulePath (in which WinPS module location has to go after CorePS module location) - if (isModuleToLoadEngineModule) + bool tryLoadingModuleLocally = true; + if (isBuiltInModule) { - string expectedCoreModulePath = Path.Combine(ModuleIntrinsics.GetPSHomeModulePath(), coreModuleToLoad); - if (Directory.Exists(expectedCoreModulePath)) + PSSnapInInfo loadedSnapin = Context.CurrentRunspace.InitialSessionState.GetPSSnapIn(moduleToLoad); + tryLoadingModuleLocally = loadedSnapin is null; + + if (tryLoadingModuleLocally) { - coreModuleToLoad = expectedCoreModulePath; + string expectedCoreModulePath = Path.Combine(ModuleIntrinsics.GetPSHomeModulePath(), moduleToLoad); + if (Directory.Exists(expectedCoreModulePath)) + { + moduleToLoad = expectedCoreModulePath; + } } } - if (moduleSpec == null) - { - ImportModule_LocallyViaName_WithTelemetry(importModuleOptions, coreModuleToLoad); - } - else + if (tryLoadingModuleLocally) { - ModuleSpecification tmpModuleSpec = new ModuleSpecification() + bool savedValue = importModuleOptions.CoreEditionCompatibleOnly; + importModuleOptions.CoreEditionCompatibleOnly = true; + + PSModuleInfo moduleInfo = moduleSpec is null + ? ImportModule_LocallyViaName_WithTelemetry(importModuleOptions, moduleToLoad) + : ImportModule_LocallyViaFQName( + importModuleOptions, + new ModuleSpecification() + { + Guid = moduleSpec.Guid, + MaximumVersion = moduleSpec.MaximumVersion, + Version = moduleSpec.Version, + RequiredVersion = moduleSpec.RequiredVersion, + Name = moduleToLoad + }); + + if (moduleInfo is null && isBuiltInModule) { - Guid = moduleSpec.Guid, - MaximumVersion = moduleSpec.MaximumVersion, - Version = moduleSpec.Version, - RequiredVersion = moduleSpec.RequiredVersion, - Name = coreModuleToLoad - }; - ImportModule_LocallyViaFQName(importModuleOptions, tmpModuleSpec); + throw new InvalidOperationException( + StringUtil.Format( + Modules.CannotFindBuiltInModules, + moduleName)); + } + + importModuleOptions.CoreEditionCompatibleOnly = savedValue; } importModuleOptions.NoClobberExportPSSession = true; @@ -2020,8 +2043,8 @@ internal override IList ImportModulesUsingWinCompat(IEnumerable moduleProxyList = new List(); #if !UNIX // one of the two parameters can be passed: either ModuleNames (most of the time) or ModuleSpecifications (they are used in different parameter sets) - List filteredModuleNames = FilterModuleCollection(moduleNames); - List filteredModuleFullyQualifiedNames = FilterModuleCollection(moduleFullyQualifiedNames); + IEnumerable filteredModuleNames = FilterModuleCollection(moduleNames); + IEnumerable filteredModuleFullyQualifiedNames = FilterModuleCollection(moduleFullyQualifiedNames); // do not setup WinCompat resources if we have no modules to import if ((filteredModuleNames?.Any() != true) && (filteredModuleFullyQualifiedNames?.Any() != true)) diff --git a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs index 73a6eed1b10..e7e2397f265 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs @@ -105,6 +105,12 @@ protected internal struct ImportModuleOptions /// Historically -AllowClobber in these scenarios was set as True. /// internal bool NoClobberExportPSSession; + + /// + /// When this flag is true, we only attempt to load a core-edition compatible module, and never try implicitly loading + /// a incompatible module via 'WinCompat' mechanism. + /// + internal bool CoreEditionCompatibleOnly; } /// @@ -280,7 +286,10 @@ internal List MatchAll "ModuleVersion" }; - private static readonly HashSet s_builtInModules = new(StringComparer.OrdinalIgnoreCase) + /// + /// List of PowerShell built-in modules that are shipped with PowerShell only, not on PS Gallery. + /// + protected static readonly HashSet BuiltInModules = new(StringComparer.OrdinalIgnoreCase) { "CimCmdlets", "Microsoft.PowerShell.Diagnostics", @@ -2387,12 +2396,15 @@ internal PSModuleInfo LoadModuleManifest( { if (importingModule) { - // In case the module to be loaded in WinCompat mode is a PowerShell built-in module, such as the Utility module, - // we need to check whether the built-in module is actually available in the $PSHOME module path. - // This is because a user may not correctly deploy the built-in modules, and that would result in some confusing - // errors when module auto-loading silently attempts to load those modules from the 'System32' module path. - CheckAvailabilityOfBuiltInModules(ModuleIntrinsics.GetModuleName(moduleManifestPath)); - IList moduleProxies = ImportModulesUsingWinCompat(new string[] { moduleManifestPath }, null, options); + if (options.CoreEditionCompatibleOnly) + { + return null; + } + + IList moduleProxies = ImportModulesUsingWinCompat( + moduleNames: new string[] { moduleManifestPath }, + moduleFullyQualifiedNames: null, + importModuleOptions: options); // We are loading by a single ManifestPath so expect max of 1 return moduleProxies.Count > 0 ? moduleProxies[0] : null; @@ -3716,7 +3728,7 @@ internal static object IsModuleLoaded(ExecutionContext context, ModuleSpecificat // If the RequiredModule is one of the Engine modules, then they could have been loaded as snapins (using InitialSessionState.CreateDefault()) if (result == null && InitialSessionState.IsEngineModule(requiredModule.Name)) { - result = GetEngineSnapIn(context, requiredModule.Name); + result = context.CurrentRunspace.InitialSessionState.GetPSSnapIn(requiredModule.Name); if (result != null) { loaded = true; @@ -4897,33 +4909,6 @@ internal static void SyncCurrentLocationHandler(object sender, LocationChangedEv internal virtual IList ImportModulesUsingWinCompat(IEnumerable moduleNames, IEnumerable moduleFullyQualifiedNames, ImportModuleOptions importModuleOptions) { throw new System.NotImplementedException(); } - private void CheckAvailabilityOfBuiltInModules(string moduleName) - { - // Check if the module is a PowerShell built-in module. If so, normalize the module name; if not, skip the module. - if (!s_builtInModules.TryGetValue(moduleName, out moduleName)) - { - return; - } - - // When it was imported as a snapin, we skip the check because it's OK to not have the module available in such case. - if (GetEngineSnapIn(Context, moduleName) is not null) - { - return; - } - - // Check if the module exists in the $PSHOME module path. If not, throws exception. - string psHomeModulePath = ModuleIntrinsics.GetPSHomeModulePath(); - string moduleFolder = Path.Join(psHomeModulePath, moduleName); - if (!Directory.Exists(moduleFolder)) - { - throw new InvalidOperationException( - StringUtil.Format( - Modules.CannotFindBuiltInModules, - moduleName, - psHomeModulePath)); - } - } - private void RemoveTypesAndFormatting( IList formatFilesToRemove, IList typeFilesToRemove) @@ -5346,9 +5331,8 @@ internal PSModuleInfo LoadUsingExtensions(PSModuleInfo parentModule, string moduleName, string fileBaseName, string extension, string moduleBase, string prefix, SessionState ss, ImportModuleOptions options, ManifestProcessingFlags manifestProcessingFlags, out bool found) { - bool throwAwayModuleFileFound = false; return LoadUsingExtensions(parentModule, moduleName, fileBaseName, extension, moduleBase, prefix, ss, - options, manifestProcessingFlags, out found, out throwAwayModuleFileFound); + options, manifestProcessingFlags, out found, out _); } /// @@ -5449,7 +5433,6 @@ internal PSModuleInfo LoadUsingExtensions(PSModuleInfo parentModule, } else if (File.Exists(fileName)) { - moduleFileFound = true; // Win8: 325243 - Added the version check so that we do not unload modules with the same name but different version if (BaseForce && DoesAlreadyLoadedModuleSatisfyConstraints(module)) { @@ -7396,19 +7379,6 @@ private static void ValidateCommandName(ModuleCmdletBase cmdlet, } } - /// - /// Search a PSSnapin with the specified name. - /// - internal static PSSnapInInfo GetEngineSnapIn(ExecutionContext context, string name) - { - if (context.CurrentRunspace.InitialSessionState.ImportedSnapins.TryGetValue(name, out PSSnapInInfo snapin)) - { - return snapin; - } - - return null; - } - /// /// Returns the context cached ModuleTable module for import only if found and has safe language boundaries while /// exporting all functions by default. diff --git a/src/System.Management.Automation/resources/Modules.resx b/src/System.Management.Automation/resources/Modules.resx index ff532b505a3..b87cfaa2011 100644 --- a/src/System.Management.Automation/resources/Modules.resx +++ b/src/System.Management.Automation/resources/Modules.resx @@ -624,7 +624,7 @@ Cannot create new module while the session is in ConstrainedLanguage mode. - - The built-in module '{0}' cannot be found under the $PSHOME module path '{1}'. Please make sure the PowerShell built-in modules are available because they are required for PowerShell to function properly. + + Cannot find the built-in module '{0}' that is compatible with the 'Core' edition. Please make sure the PowerShell built-in modules are available. They usually come along with the PowerShell package under the $PSHOME module path, and are required for PowerShell to function properly. From 1eb96805faa1235f4e0c5cb0a7853349fdeb1c42 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Tue, 21 Dec 2021 23:50:07 -0800 Subject: [PATCH 05/12] More changes and tests --- .../engine/Modules/ImportModuleCommand.cs | 154 +++++++++++------- .../engine/Modules/ModuleCmdletBase.cs | 48 ++++-- .../resources/Modules.resx | 2 +- .../CompatiblePSEditions.Module.Tests.ps1 | 138 +++++++++++++++- 4 files changed, 268 insertions(+), 74 deletions(-) diff --git a/src/System.Management.Automation/engine/Modules/ImportModuleCommand.cs b/src/System.Management.Automation/engine/Modules/ImportModuleCommand.cs index a4ec3634ee2..45011edb908 100644 --- a/src/System.Management.Automation/engine/Modules/ImportModuleCommand.cs +++ b/src/System.Management.Automation/engine/Modules/ImportModuleCommand.cs @@ -636,6 +636,8 @@ private PSModuleInfo ImportModule_LocallyViaName_WithTelemetry(ImportModuleOptio private PSModuleInfo ImportModule_LocallyViaName(ImportModuleOptions importModuleOptions, string name) { + bool shallWriteError = !importModuleOptions.SkipSystem32ModulesAndSuppressError; + try { bool found = false; @@ -671,14 +673,14 @@ private PSModuleInfo ImportModule_LocallyViaName(ImportModuleOptions importModul } bool alreadyLoaded = false; - if (!string.IsNullOrEmpty(rootedPath)) + var manifestProcessingFlags = ManifestProcessingFlags.LoadElements | ManifestProcessingFlags.NullOnFirstError; + if (shallWriteError) { - // TODO/FIXME: use IsModuleAlreadyLoaded to get consistent behavior - // TODO/FIXME: (for example checking ModuleType != Manifest below seems incorrect - cdxml modules also declare their own version) - // PSModuleInfo alreadyLoadedModule = null; - // TryGetFromModuleTable(rootedPath, out alreadyLoadedModule); - // if (!BaseForce && IsModuleAlreadyLoaded(alreadyLoadedModule)) + manifestProcessingFlags |= ManifestProcessingFlags.WriteErrors; + } + if (!string.IsNullOrEmpty(rootedPath)) + { // If the module has already been loaded, just emit it and continue... if (!BaseForce && TryGetFromModuleTable(rootedPath, out PSModuleInfo module)) { @@ -725,9 +727,14 @@ private PSModuleInfo ImportModule_LocallyViaName(ImportModuleOptions importModul RemoveModule(moduleToRemove); } - foundModule = LoadModule(rootedPath, null, this.BasePrefix, null, ref importModuleOptions, - ManifestProcessingFlags.LoadElements | ManifestProcessingFlags.WriteErrors | ManifestProcessingFlags.NullOnFirstError, - out found); + foundModule = LoadModule( + fileName: rootedPath, + moduleBase: null, + prefix: BasePrefix, + ss: null, /*SessionState*/ + ref importModuleOptions, + manifestProcessingFlags, + out found); } else if (Directory.Exists(rootedPath)) { @@ -738,21 +745,24 @@ private PSModuleInfo ImportModule_LocallyViaName(ImportModuleOptions importModul } // Load the latest valid version if it is a multi-version module directory - foundModule = LoadUsingMultiVersionModuleBase(rootedPath, - ManifestProcessingFlags.LoadElements | - ManifestProcessingFlags.WriteErrors | - ManifestProcessingFlags.NullOnFirstError, - importModuleOptions, out found); + foundModule = LoadUsingMultiVersionModuleBase(rootedPath, manifestProcessingFlags, importModuleOptions, out found); if (!found) { // If the path is a directory, double up the end of the string // then try to load that using extensions... rootedPath = Path.Combine(rootedPath, Path.GetFileName(rootedPath)); - foundModule = LoadUsingExtensions(null, rootedPath, rootedPath, null, null, this.BasePrefix, /*SessionState*/ null, - importModuleOptions, - ManifestProcessingFlags.LoadElements | ManifestProcessingFlags.WriteErrors | ManifestProcessingFlags.NullOnFirstError, - out found); + foundModule = LoadUsingExtensions( + parentModule: null, + moduleName: rootedPath, + fileBaseName: rootedPath, + extension: null, + moduleBase: null, + prefix: BasePrefix, + ss: null, /*SessionState*/ + importModuleOptions, + manifestProcessingFlags, + out found); } } } @@ -786,16 +796,28 @@ private PSModuleInfo ImportModule_LocallyViaName(ImportModuleOptions importModul // If there is no extension, we'll have to search using the extensions if (!string.IsNullOrEmpty(Path.GetExtension(name))) { - foundModule = LoadModule(name, null, this.BasePrefix, null, ref importModuleOptions, - ManifestProcessingFlags.LoadElements | ManifestProcessingFlags.WriteErrors | ManifestProcessingFlags.NullOnFirstError, - out found); + foundModule = LoadModule( + fileName: name, + moduleBase: null, + prefix: BasePrefix, + ss: null, /*SessionState*/ + ref importModuleOptions, + manifestProcessingFlags, + out found); } else { - foundModule = LoadUsingExtensions(null, name, name, null, null, this.BasePrefix, /*SessionState*/ null, - importModuleOptions, - ManifestProcessingFlags.LoadElements | ManifestProcessingFlags.WriteErrors | ManifestProcessingFlags.NullOnFirstError, - out found); + foundModule = LoadUsingExtensions( + parentModule: null, + moduleName: name, + fileBaseName: name, + extension: null, + moduleBase: null, + prefix: BasePrefix, + ss: null, /*SessionState*/ + importModuleOptions, + manifestProcessingFlags, + out found); } } else @@ -807,14 +829,17 @@ private PSModuleInfo ImportModule_LocallyViaName(ImportModuleOptions importModul this.AddToAppDomainLevelCache = true; } - found = LoadUsingModulePath(found, modulePath, name, /* SessionState*/ null, - importModuleOptions, - ManifestProcessingFlags.LoadElements | ManifestProcessingFlags.WriteErrors | ManifestProcessingFlags.NullOnFirstError, - out foundModule); + found = LoadUsingModulePath( + modulePath, + name, + ss: null, /* SessionState*/ + importModuleOptions, + manifestProcessingFlags, + out foundModule); } } - if (!found) + if (!found && shallWriteError) { ErrorRecord er = null; string message = null; @@ -856,8 +881,10 @@ private PSModuleInfo ImportModule_LocallyViaName(ImportModuleOptions importModul } catch (PSInvalidOperationException e) { - ErrorRecord er = new ErrorRecord(e.ErrorRecord, e); - WriteError(er); + if (shallWriteError) + { + WriteError(new ErrorRecord(e.ErrorRecord, e)); + } } return null; @@ -1987,16 +2014,15 @@ private void PrepareNoClobberWinCompatModuleImport(string moduleName, ModuleSpec string[] noClobberModuleList = PowerShellConfig.Instance.GetWindowsPowerShellCompatibilityNoClobberModuleList(); if (isBuiltInModule || noClobberModuleList?.Contains(moduleToLoad, StringComparer.OrdinalIgnoreCase) == true) { - // if it is one of engine modules - first try to load it from $PSHOME\Modules - // otherwise rely on $env:PSModulePath (in which WinPS module location has to go after CorePS module location) - bool tryLoadingModuleLocally = true; + bool shouldLoadModuleLocally = true; if (isBuiltInModule) { PSSnapInInfo loadedSnapin = Context.CurrentRunspace.InitialSessionState.GetPSSnapIn(moduleToLoad); - tryLoadingModuleLocally = loadedSnapin is null; + shouldLoadModuleLocally = loadedSnapin is null; - if (tryLoadingModuleLocally) + if (shouldLoadModuleLocally) { + // If it is one of built-in modules, first try loading it from $PSHOME\Modules, otherwise rely on $env:PSModulePath. string expectedCoreModulePath = Path.Combine(ModuleIntrinsics.GetPSHomeModulePath(), moduleToLoad); if (Directory.Exists(expectedCoreModulePath)) { @@ -2005,10 +2031,15 @@ private void PrepareNoClobberWinCompatModuleImport(string moduleName, ModuleSpec } } - if (tryLoadingModuleLocally) + if (shouldLoadModuleLocally) { - bool savedValue = importModuleOptions.CoreEditionCompatibleOnly; - importModuleOptions.CoreEditionCompatibleOnly = true; + // Here we want to load a core-edition compatible version of the module, so the loading procedure will skip + // the 'System32' module path when searching. Also, we want to suppress writing out errors in case that a + // core-compatible version of the module cannot be found, because: + // 1. that's OK as long as it's not a PowerShell built-in module such as the 'Utility' moudle; + // 2. the error message will be confusing to the user. + bool savedValue = importModuleOptions.SkipSystem32ModulesAndSuppressError; + importModuleOptions.SkipSystem32ModulesAndSuppressError = true; PSModuleInfo moduleInfo = moduleSpec is null ? ImportModule_LocallyViaName_WithTelemetry(importModuleOptions, moduleToLoad) @@ -2023,15 +2054,22 @@ private void PrepareNoClobberWinCompatModuleImport(string moduleName, ModuleSpec Name = moduleToLoad }); + // If we failed to load a core-compatible version of a built-in module, we should stop trying to load the + // module in 'WinCompat' mode and report an error. This could happen when a user didn't correctly deploy + // the built-in modules, which would result in very confusing errors when the module auto-loading silently + // attempts to load those built-in modules in 'WinCompat' mode from the 'System32' module path. + // + // If the loading failed but it's NOT a built-in module, then it's fine to ignore this failure and continue + // to load the module in 'WinCompat' mode. if (moduleInfo is null && isBuiltInModule) { throw new InvalidOperationException( StringUtil.Format( - Modules.CannotFindBuiltInModules, - moduleName)); + Modules.CannotFindCoreCompatibleBuiltInModule, + moduleToLoad)); } - importModuleOptions.CoreEditionCompatibleOnly = savedValue; + importModuleOptions.SkipSystem32ModulesAndSuppressError = savedValue; } importModuleOptions.NoClobberExportPSSession = true; @@ -2047,24 +2085,11 @@ internal override IList ImportModulesUsingWinCompat(IEnumerable filteredModuleFullyQualifiedNames = FilterModuleCollection(moduleFullyQualifiedNames); // do not setup WinCompat resources if we have no modules to import - if ((filteredModuleNames?.Any() != true) && (filteredModuleFullyQualifiedNames?.Any() != true)) + if (filteredModuleNames?.Any() != true && filteredModuleFullyQualifiedNames?.Any() != true) { return moduleProxyList; } - var winPSVersionString = Utils.GetWindowsPowerShellVersionFromRegistry(); - if (!winPSVersionString.StartsWith("5.1", StringComparison.OrdinalIgnoreCase)) - { - string errorMessage = string.Format(CultureInfo.InvariantCulture, Modules.WinCompatRequredVersionError, winPSVersionString); - throw new InvalidOperationException(errorMessage); - } - - PSSession WindowsPowerShellCompatRemotingSession = CreateWindowsPowerShellCompatResources(); - if (WindowsPowerShellCompatRemotingSession == null) - { - return new List(); - } - // perform necessary preparations if module has to be imported with NoClobber mode if (filteredModuleNames != null) { @@ -2082,13 +2107,26 @@ internal override IList ImportModulesUsingWinCompat(IEnumerable(); + } + // perform the module import / proxy generation moduleProxyList = ImportModule_RemotelyViaPsrpSession(importModuleOptions, filteredModuleNames, filteredModuleFullyQualifiedNames, WindowsPowerShellCompatRemotingSession, usingWinCompat: true); foreach (PSModuleInfo moduleProxy in moduleProxyList) { moduleProxy.IsWindowsPowerShellCompatModule = true; - System.Threading.Interlocked.Increment(ref s_WindowsPowerShellCompatUsageCounter); + Interlocked.Increment(ref s_WindowsPowerShellCompatUsageCounter); string message = StringUtil.Format(Modules.WinCompatModuleWarning, moduleProxy.Name, WindowsPowerShellCompatRemotingSession.Name); WriteWarning(message); diff --git a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs index e7e2397f265..533c90cb0d5 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs @@ -107,10 +107,10 @@ protected internal struct ImportModuleOptions internal bool NoClobberExportPSSession; /// - /// When this flag is true, we only attempt to load a core-edition compatible module, and never try implicitly loading - /// a incompatible module via 'WinCompat' mechanism. + /// Flag that controls skipping the System32 module path when searching a module in module paths. It also suppresses + /// writing out errors when specified. /// - internal bool CoreEditionCompatibleOnly; + internal bool SkipSystem32ModulesAndSuppressError; } /// @@ -328,14 +328,25 @@ internal List MatchAll private readonly Dictionary _currentlyProcessingModules = new Dictionary(); - internal bool LoadUsingModulePath(bool found, IEnumerable modulePath, string name, SessionState ss, - ImportModuleOptions options, ManifestProcessingFlags manifestProcessingFlags, out PSModuleInfo module) + internal bool LoadUsingModulePath( + IEnumerable modulePath, + string name, + SessionState ss, + ImportModuleOptions options, + ManifestProcessingFlags manifestProcessingFlags, + out PSModuleInfo module) { - return LoadUsingModulePath(null, found, modulePath, name, ss, options, manifestProcessingFlags, out module); + return LoadUsingModulePath(parentModule: null, modulePath, name, ss, options, manifestProcessingFlags, out module); } - internal bool LoadUsingModulePath(PSModuleInfo parentModule, bool found, IEnumerable modulePath, string name, SessionState ss, - ImportModuleOptions options, ManifestProcessingFlags manifestProcessingFlags, out PSModuleInfo module) + internal bool LoadUsingModulePath( + PSModuleInfo parentModule, + IEnumerable modulePath, + string name, + SessionState ss, + ImportModuleOptions options, + ManifestProcessingFlags manifestProcessingFlags, + out PSModuleInfo module) { string extension = Path.GetExtension(name); string fileBaseName; @@ -346,11 +357,18 @@ internal bool LoadUsingModulePath(PSModuleInfo parentModule, bool found, IEnumer extension = null; } else + { fileBaseName = name.Substring(0, name.Length - extension.Length); + } // Now search using the module path... + bool found = false; foreach (string path in modulePath) { + if (options.SkipSystem32ModulesAndSuppressError && ModuleUtils.IsOnSystem32ModulePath(path)) + { + continue; + } #if UNIX foreach (string folder in Directory.EnumerateDirectories(path)) { @@ -844,7 +862,7 @@ private PSModuleInfo LoadModuleNamedInManifest(PSModuleInfo parentModule, Module } // Otherwise try the module path - found = LoadUsingModulePath(parentModule, found, modulePath, + found = LoadUsingModulePath(parentModule, modulePath, moduleSpecification.Name, ss, options, manifestProcessingFlags, out module); } @@ -2396,11 +2414,6 @@ internal PSModuleInfo LoadModuleManifest( { if (importingModule) { - if (options.CoreEditionCompatibleOnly) - { - return null; - } - IList moduleProxies = ImportModulesUsingWinCompat( moduleNames: new string[] { moduleManifestPath }, moduleFullyQualifiedNames: null, @@ -5384,6 +5397,13 @@ internal PSModuleInfo LoadUsingExtensions(PSModuleInfo parentModule, if (!string.IsNullOrEmpty(Context.ModuleBeingProcessed) && Context.ModuleBeingProcessed.Equals(fileName, StringComparison.OrdinalIgnoreCase)) { + // If the module being processed points to the same module manifest file, then we should stop trying other + // module extensions, because we are actually attempting to load the same module that is being processed. + if (StringLiterals.PowerShellDataFileExtension.Equals(ext, StringComparison.OrdinalIgnoreCase)) + { + break; + } + continue; } diff --git a/src/System.Management.Automation/resources/Modules.resx b/src/System.Management.Automation/resources/Modules.resx index b87cfaa2011..2e50a3c5317 100644 --- a/src/System.Management.Automation/resources/Modules.resx +++ b/src/System.Management.Automation/resources/Modules.resx @@ -625,6 +625,6 @@ Cannot create new module while the session is in ConstrainedLanguage mode. - Cannot find the built-in module '{0}' that is compatible with the 'Core' edition. Please make sure the PowerShell built-in modules are available. They usually come along with the PowerShell package under the $PSHOME module path, and are required for PowerShell to function properly. + Cannot find the built-in module '{0}' that is compatible with the 'Core' edition. Please make sure the PowerShell built-in modules are available. They usually come with the PowerShell package under the $PSHOME module path, and are required for PowerShell to function properly. diff --git a/test/powershell/Modules/Microsoft.PowerShell.Core/CompatiblePSEditions.Module.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Core/CompatiblePSEditions.Module.Tests.ps1 index 72a7c9eddb1..b689decee55 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Core/CompatiblePSEditions.Module.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Core/CompatiblePSEditions.Module.Tests.ps1 @@ -11,7 +11,7 @@ function Add-ModulePath if ($Prepend) { - $env:PSModulePAth = $Path + [System.IO.Path]::PathSeparator + $env:PSModulePath + $env:PSModulePath = $Path + [System.IO.Path]::PathSeparator + $env:PSModulePath } else { @@ -643,6 +643,29 @@ Describe "Additional tests for Import-Module with WinCompat" -Tag "Feature" { '{"Microsoft.PowerShell:ExecutionPolicy": "RemoteSigned", "WindowsPowerShellCompatibilityNoClobberModuleList": ["' + $ModuleName2 + '"]}' | Out-File -Force $ConfigPath & $pwsh -NoProfile -NonInteractive -settingsFile $ConfigPath -c "[System.Management.Automation.Internal.InternalTestHooks]::SetTestHook('TestWindowsPowerShellPSHomeLocation', `'$basePath`');Test-${ModuleName2}PSEdition;Test-$ModuleName2" | Should -Be @('Desktop','Core') } + + It "NoClobber WinCompat list in powershell.config is a Desktop-edition module" { + ## The 'PersistentMemory' module should not be imported twice. + $ConfigPath = Join-Path $TestDrive 'powershell.config.json' + '{"Microsoft.PowerShell:ExecutionPolicy": "RemoteSigned", "WindowsPowerShellCompatibilityNoClobberModuleList": ["PersistentMemory"]}' | Out-File -Force $ConfigPath + $env:PSModulePath = '' + + ## 'PersistentMemory' is listed in the no-clobber list, so we will first try loading a core-edition compatible + ## version of the module before loading the remote one. And the 'system32' module path will be skipped in this + ## attempt, which is expected. + ## If we don't skip the 'system32' module path in this loading attempt, the 'PersistentMemory' module will be + ## imported twice as a remote module, and thus 'Remove-Module PersistentMemory' won't close the WinCompat session. + $results = & $pwsh -NoProfile -NonInteractive -settingsFile $ConfigPath -c { + Import-Module PersistentMemory -UseWindowsPowerShell -WarningAction Ignore + Get-Module PersistentMemory | ForEach-Object { $_.ModuleType.ToString() } + (Get-PSSession | Measure-Object).Count + Remove-Module PersistentMemory + (Get-PSSession | Measure-Object).Count + } + $results[0] | Should -BeExactly 'Script' + $results[1] | Should -BeExactly 1 + $results[2] | Should -BeExactly 0 + } } Context "Tests around PSModulePath in WinCompat process" { @@ -1343,3 +1366,116 @@ Describe "Import-Module nested module behaviour with Edition checking" -Tag "Fea } } } + +Describe "WinCompat importing should check availablity of built-in modules" -Tag "CI" { + BeforeAll { + if (-not $IsWindows ) { + $originalDefaultParameterValues = $PSDefaultParameterValues.Clone() + $PSDefaultParameterValues["it:skip"] = $true + return + } + + ## Copy the current PowerShell instance to a temp location + $tempDir = Join-Path ([System.IO.Path]::GetTempPath()) "WinCompat" + $pwshDir = Join-Path $tempDir "pwsh" + $moduleDir = Join-Path $tempDir "Modules" + $savedModulePath = $env:PSModulePath + + if (Test-Path $tempDir) { + Remove-Item $tempDir -Recurse -Force + } + + Write-Host "Making a copy of the running PowerShell instance ..." -ForegroundColor Yellow + Copy-Item $PSHOME $pwshDir -Recurse -Force + Move-Item $pwshDir\Modules $moduleDir -Force + Write-Host "-- Done copying!" -ForegroundColor Yellow + } + + AfterAll { + if (-not $IsWindows) { + $global:PSDefaultParameterValues = $originalDefaultParameterValues + return + } + + $env:PSModulePath = $savedModulePath + Remove-Item $tempDir -Recurse -Force + } + + It "Missing built-in modules will trigger error instead of loading the non-compatible ones in System32 directory" { + $env:PSModulePath = '' + $result = & "$pwshDir\pwsh.exe" -NoProfile -NonInteractive -c { + try { + Start-Transcript + } catch { + $_.FullyQualifiedErrorId + $_.Exception.Message + } + } + + $result[0] | Should -BeExactly "CouldNotAutoloadMatchingModule" + $result[1] | Should -BeLike "*'Start-Transcript'*'Microsoft.PowerShell.Host'*'Microsoft.PowerShell.Host'*'Core'*`$PSHOME*'Import-Module Microsoft.PowerShell.Host'*" + + $result = & "$pwshDir\pwsh.exe" -NoProfile -NonInteractive -c { + try { + Import-Module Microsoft.PowerShell.Host + } catch { + $_.FullyQualifiedErrorId + $_.Exception.Message + } + } + $result[0] | Should -BeExactly "System.InvalidOperationException,Microsoft.PowerShell.Commands.ImportModuleCommand" + $result[1] | Should -BeLike "*'Microsoft.PowerShell.Host'*'Core'*`$PSHOME*" + + $result = & "$pwshDir\pwsh.exe" -NoProfile -NonInteractive -c { + try { + Import-Module CimCmdlets + } catch { + $_.FullyQualifiedErrorId + $_.Exception.Message + } + } + $result[0] | Should -BeExactly "System.InvalidOperationException,Microsoft.PowerShell.Commands.ImportModuleCommand" + $result[1] | Should -BeLike "*'CimCmdlets'*'Core'*`$PSHOME*" + + $result = & "$pwshDir\pwsh.exe" -NoProfile -NonInteractive -c { + try { + Import-Module Microsoft.PowerShell.Utility + } catch { + $_.FullyQualifiedErrorId + $_.Exception.Message + } + } + $result[0] | Should -BeExactly "System.InvalidOperationException,Microsoft.PowerShell.Commands.ImportModuleCommand" + $result[1] | Should -BeLike "*'Microsoft.PowerShell.Utility'*'Core'*`$PSHOME*" + + ## Attempt to load a 'Desktop' edition module should fail because 'Export-PSSession' cannot be found. + $result = & "$pwshDir\pwsh.exe" -NoProfile -NonInteractive -c { + try { + Import-Module PersistentMemory -ErrorAction Stop + } catch { + $_.FullyQualifiedErrorId + $_.Exception.Message + } + } + $result[0] | Should -BeExactly "CommandNotFoundException,Microsoft.PowerShell.Commands.ImportModuleCommand" + $result[1] | Should -BeLike "*'PersistentMemory'*'Export-PSSession'*'Microsoft.PowerShell.Utility'*" + } + + It "When built-in modules are available but not in `$PSHOME module path, things should work" { + $env:PSModulePath = '' + $result = & "$pwshDir\pwsh.exe" -NoProfile -NonInteractive -c @" + `$env:PSModulePath += ';$moduleDir' + Import-Module Microsoft.PowerShell.Utility -UseWindowsPowerShell -WarningAction Ignore + Get-Module Microsoft.PowerShell.Utility | ForEach-Object ModuleType + Get-Module Microsoft.PowerShell.Utility | Where-Object ModuleType -eq 'Manifest' | ForEach-Object Path + Get-Module Microsoft.PowerShell.Utility | Where-Object ModuleType -eq 'Script' | ForEach-Object { `$_.ExportedCommands.Keys } +"@ + $result | Should -HaveCount 6 + $result[0] | Should -BeExactly 'Manifest' + $result[1] | Should -BeExactly 'Script' + $result[2] | Should -BeExactly "$moduleDir\Microsoft.PowerShell.Utility\Microsoft.PowerShell.Utility.psd1" + $result[3] | Should -BeExactly 'Convert-String' + $result[4] | Should -BeExactly 'ConvertFrom-String' + $result[5] | Should -BeExactly 'CFS' + } +} From c32f182868980272edfdc31bfe5d155d1c3c1132 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Wed, 22 Dec 2021 00:14:17 -0800 Subject: [PATCH 06/12] Minor fix --- .../engine/Modules/ModuleCmdletBase.cs | 7 ------- 1 file changed, 7 deletions(-) diff --git a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs index 533c90cb0d5..a4d1a7057f1 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs @@ -5397,13 +5397,6 @@ internal PSModuleInfo LoadUsingExtensions(PSModuleInfo parentModule, if (!string.IsNullOrEmpty(Context.ModuleBeingProcessed) && Context.ModuleBeingProcessed.Equals(fileName, StringComparison.OrdinalIgnoreCase)) { - // If the module being processed points to the same module manifest file, then we should stop trying other - // module extensions, because we are actually attempting to load the same module that is being processed. - if (StringLiterals.PowerShellDataFileExtension.Equals(ext, StringComparison.OrdinalIgnoreCase)) - { - break; - } - continue; } From d22f34fd937871cde894cd62d2bc2187d802fd51 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Mon, 3 Jan 2022 12:16:03 -0800 Subject: [PATCH 07/12] Fix the test. 'PersistentMemory' is not available on Windows Server, so use 'RemoteDesktop' instead --- .../CompatiblePSEditions.Module.Tests.ps1 | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/test/powershell/Modules/Microsoft.PowerShell.Core/CompatiblePSEditions.Module.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Core/CompatiblePSEditions.Module.Tests.ps1 index b689decee55..fceb1348d38 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Core/CompatiblePSEditions.Module.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Core/CompatiblePSEditions.Module.Tests.ps1 @@ -645,21 +645,21 @@ Describe "Additional tests for Import-Module with WinCompat" -Tag "Feature" { } It "NoClobber WinCompat list in powershell.config is a Desktop-edition module" { - ## The 'PersistentMemory' module should not be imported twice. + ## The 'RemoteDesktop' module (available on Windows Server) should not be imported twice. $ConfigPath = Join-Path $TestDrive 'powershell.config.json' - '{"Microsoft.PowerShell:ExecutionPolicy": "RemoteSigned", "WindowsPowerShellCompatibilityNoClobberModuleList": ["PersistentMemory"]}' | Out-File -Force $ConfigPath + '{"Microsoft.PowerShell:ExecutionPolicy": "RemoteSigned", "WindowsPowerShellCompatibilityNoClobberModuleList": ["RemoteDesktop"]}' | Out-File -Force $ConfigPath $env:PSModulePath = '' - ## 'PersistentMemory' is listed in the no-clobber list, so we will first try loading a core-edition compatible - ## version of the module before loading the remote one. And the 'system32' module path will be skipped in this + ## 'RemoteDesktop' is listed in the no-clobber list, so we will first try loading a core-edition compatible + ## version of the module before loading the remote one. The 'system32' module path will be skipped in this ## attempt, which is expected. - ## If we don't skip the 'system32' module path in this loading attempt, the 'PersistentMemory' module will be - ## imported twice as a remote module, and thus 'Remove-Module PersistentMemory' won't close the WinCompat session. + ## If we don't skip the 'system32' module path in this loading attempt, the 'RemoteDesktop' module will be + ## imported twice as a remote module, and then 'Remove-Module RemoteDesktop' won't close the WinCompat session. $results = & $pwsh -NoProfile -NonInteractive -settingsFile $ConfigPath -c { - Import-Module PersistentMemory -UseWindowsPowerShell -WarningAction Ignore - Get-Module PersistentMemory | ForEach-Object { $_.ModuleType.ToString() } + Import-Module RemoteDesktop -UseWindowsPowerShell -WarningAction Ignore + Get-Module RemoteDesktop | ForEach-Object { $_.ModuleType.ToString() } (Get-PSSession | Measure-Object).Count - Remove-Module PersistentMemory + Remove-Module RemoteDesktop (Get-PSSession | Measure-Object).Count } $results[0] | Should -BeExactly 'Script' @@ -1451,14 +1451,14 @@ Describe "WinCompat importing should check availablity of built-in modules" -Tag ## Attempt to load a 'Desktop' edition module should fail because 'Export-PSSession' cannot be found. $result = & "$pwshDir\pwsh.exe" -NoProfile -NonInteractive -c { try { - Import-Module PersistentMemory -ErrorAction Stop + Import-Module RemoteDesktop -ErrorAction Stop } catch { $_.FullyQualifiedErrorId $_.Exception.Message } } $result[0] | Should -BeExactly "CommandNotFoundException,Microsoft.PowerShell.Commands.ImportModuleCommand" - $result[1] | Should -BeLike "*'PersistentMemory'*'Export-PSSession'*'Microsoft.PowerShell.Utility'*" + $result[1] | Should -BeLike "*'RemoteDesktop'*'Export-PSSession'*'Microsoft.PowerShell.Utility'*" } It "When built-in modules are available but not in `$PSHOME module path, things should work" { From 831c6c0dda5c5047ef12fcce653ecd2dd604c895 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Mon, 3 Jan 2022 16:37:47 -0800 Subject: [PATCH 08/12] Address Paul's feedback --- .../engine/CommandDiscovery.cs | 14 +++----------- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/src/System.Management.Automation/engine/CommandDiscovery.cs b/src/System.Management.Automation/engine/CommandDiscovery.cs index f19715f4307..5de18cdc952 100644 --- a/src/System.Management.Automation/engine/CommandDiscovery.cs +++ b/src/System.Management.Automation/engine/CommandDiscovery.cs @@ -379,11 +379,7 @@ private static void VerifyRequiredSnapins(IEnumerable req var loadedPSSnapIn = context.InitialSessionState.GetPSSnapIn(requiresPSSnapIn.Name); if (loadedPSSnapIn is null) { - if (requiresMissingPSSnapIns == null) - { - requiresMissingPSSnapIns = new Collection(); - } - + requiresMissingPSSnapIns ??= new Collection(); requiresMissingPSSnapIns.Add(BuildPSSnapInDisplayName(requiresPSSnapIn)); } else @@ -398,11 +394,7 @@ private static void VerifyRequiredSnapins(IEnumerable req if (!AreInstalledRequiresVersionsCompatible( requiresPSSnapIn.Version, loadedPSSnapIn.Version)) { - if (requiresMissingPSSnapIns == null) - { - requiresMissingPSSnapIns = new Collection(); - } - + requiresMissingPSSnapIns ??= new Collection(); requiresMissingPSSnapIns.Add(BuildPSSnapInDisplayName(requiresPSSnapIn)); } } @@ -1097,7 +1089,7 @@ private static CommandInfo TryModuleAutoDiscovery(string commandName, cmdletInfo != null ? cmdletInfo.Visibility : SessionStateEntryVisibility.Private, out lastError); - if (matchingModule == null || matchingModule.Count == 0) + if (matchingModule is null || matchingModule.Count == 0) { string errorMessage = lastError is null ? StringUtil.Format(DiscoveryExceptions.CouldNotAutoImportMatchingModule, commandName, moduleShortName) From d23d611cc7b6e5ab09c596d6b72a0266fdbd8fa9 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Wed, 5 Jan 2022 12:26:27 -0800 Subject: [PATCH 09/12] Address Ilya's feedback --- .../CompatiblePSEditions.Module.Tests.ps1 | 103 ++++++++---------- 1 file changed, 47 insertions(+), 56 deletions(-) diff --git a/test/powershell/Modules/Microsoft.PowerShell.Core/CompatiblePSEditions.Module.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Core/CompatiblePSEditions.Module.Tests.ps1 index fceb1348d38..057890d1cc5 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Core/CompatiblePSEditions.Module.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Core/CompatiblePSEditions.Module.Tests.ps1 @@ -1401,64 +1401,55 @@ Describe "WinCompat importing should check availablity of built-in modules" -Tag Remove-Item $tempDir -Recurse -Force } - It "Missing built-in modules will trigger error instead of loading the non-compatible ones in System32 directory" { - $env:PSModulePath = '' - $result = & "$pwshDir\pwsh.exe" -NoProfile -NonInteractive -c { - try { - Start-Transcript - } catch { - $_.FullyQualifiedErrorId - $_.Exception.Message - } - } - - $result[0] | Should -BeExactly "CouldNotAutoloadMatchingModule" - $result[1] | Should -BeLike "*'Start-Transcript'*'Microsoft.PowerShell.Host'*'Microsoft.PowerShell.Host'*'Core'*`$PSHOME*'Import-Module Microsoft.PowerShell.Host'*" - - $result = & "$pwshDir\pwsh.exe" -NoProfile -NonInteractive -c { - try { - Import-Module Microsoft.PowerShell.Host - } catch { - $_.FullyQualifiedErrorId - $_.Exception.Message - } - } - $result[0] | Should -BeExactly "System.InvalidOperationException,Microsoft.PowerShell.Commands.ImportModuleCommand" - $result[1] | Should -BeLike "*'Microsoft.PowerShell.Host'*'Core'*`$PSHOME*" - - $result = & "$pwshDir\pwsh.exe" -NoProfile -NonInteractive -c { - try { - Import-Module CimCmdlets - } catch { - $_.FullyQualifiedErrorId - $_.Exception.Message - } - } - $result[0] | Should -BeExactly "System.InvalidOperationException,Microsoft.PowerShell.Commands.ImportModuleCommand" - $result[1] | Should -BeLike "*'CimCmdlets'*'Core'*`$PSHOME*" + It "Missing built-in modules will trigger error instead of loading the non-compatible ones in System32 directory. Running ''" -TestCases @( + @{ + Command = 'Start-Transcript'; + FullyQualifiedErrorId = "CouldNotAutoloadMatchingModule"; + ExceptionMessage = "*'Start-Transcript'*'Microsoft.PowerShell.Host'*'Microsoft.PowerShell.Host'*'Core'*`$PSHOME*'Import-Module Microsoft.PowerShell.Host'*" + } + @{ + Command = 'Import-Module Microsoft.PowerShell.Host'; + FullyQualifiedErrorId = "System.InvalidOperationException,Microsoft.PowerShell.Commands.ImportModuleCommand" + ExceptionMessage = "*'Microsoft.PowerShell.Host'*'Core'*`$PSHOME*" + } + @{ + Command = 'Import-Module CimCmdlets' + FullyQualifiedErrorId = "System.InvalidOperationException,Microsoft.PowerShell.Commands.ImportModuleCommand" + ExceptionMessage = "*'CimCmdlets'*'Core'*`$PSHOME*" + } + @{ + Command = 'Import-Module Microsoft.PowerShell.Utility' + FullyQualifiedErrorId = "System.InvalidOperationException,Microsoft.PowerShell.Commands.ImportModuleCommand" + ExceptionMessage = "*'Microsoft.PowerShell.Utility'*'Core'*`$PSHOME*" + } + @{ + ## Attempt to load a 'Desktop' edition module should fail because 'Export-PSSession' cannot be found. + Command = 'Import-Module RemoteDesktop -ErrorAction Stop' + FullyQualifiedErrorId = "CommandNotFoundException,Microsoft.PowerShell.Commands.ImportModuleCommand" + ExceptionMessage = "*'RemoteDesktop'*'Export-PSSession'*'Microsoft.PowerShell.Utility'*" + } + ) { + param( + $Command, + $FullyQualifiedErrorId, + $ExceptionMessage + ) - $result = & "$pwshDir\pwsh.exe" -NoProfile -NonInteractive -c { - try { - Import-Module Microsoft.PowerShell.Utility - } catch { - $_.FullyQualifiedErrorId - $_.Exception.Message - } - } - $result[0] | Should -BeExactly "System.InvalidOperationException,Microsoft.PowerShell.Commands.ImportModuleCommand" - $result[1] | Should -BeLike "*'Microsoft.PowerShell.Utility'*'Core'*`$PSHOME*" + $template = @' + try {{ + {0} + }} catch {{ + $_.FullyQualifiedErrorId + $_.Exception.Message + }} +'@ + $env:PSModulePath = '' + $script = $template -f $Command + $scriptBlock = [scriptblock]::Create($script) - ## Attempt to load a 'Desktop' edition module should fail because 'Export-PSSession' cannot be found. - $result = & "$pwshDir\pwsh.exe" -NoProfile -NonInteractive -c { - try { - Import-Module RemoteDesktop -ErrorAction Stop - } catch { - $_.FullyQualifiedErrorId - $_.Exception.Message - } - } - $result[0] | Should -BeExactly "CommandNotFoundException,Microsoft.PowerShell.Commands.ImportModuleCommand" - $result[1] | Should -BeLike "*'RemoteDesktop'*'Export-PSSession'*'Microsoft.PowerShell.Utility'*" + $result = & "$pwshDir\pwsh.exe" -NoProfile -NonInteractive -c $scriptBlock + $result[0] | Should -BeExactly $FullyQualifiedErrorId + $result[1] | Should -BeLike $ExceptionMessage } It "When built-in modules are available but not in `$PSHOME module path, things should work" { From a01a88d30b03243fbb054ff71df5ccc99755af77 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Thu, 6 Jan 2022 13:59:10 -0800 Subject: [PATCH 10/12] Use 'PersistentMemory' for Win Client and 'RemoteDesktop' for Win Server --- .../engine/Modules/ModuleIntrinsics.cs | 7 +++ .../CompatiblePSEditions.Module.Tests.ps1 | 62 ++++++++++++++----- 2 files changed, 53 insertions(+), 16 deletions(-) diff --git a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs index c83b055315a..1ef82b54179 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs @@ -977,6 +977,13 @@ internal static string GetPSHomeModulePath() { string psHome = Utils.DefaultPowerShellAppBase; #if !UNIX + // Win8: 584267 Powershell Modules are listed twice in x86, and cannot be removed. + // This happens because 'ModuleTable' uses Path as the key and x86 WinPS has "SysWOW64" in its $PSHOME. + // Because of this, the module that is getting loaded during startup (through LocalRunspace) is using + // "SysWow64" in the key. Later, when 'Import-Module' is called, it loads the module using ""System32" + // in the key. + // For the cross-platform PowerShell, a user can choose to install it under "C:\Windows\SysWOW64", and + // thus it may have the same problem as described above. psHome = psHome.ToLowerInvariant().Replace(@"\syswow64\", @"\system32\"); #endif Interlocked.CompareExchange(ref s_psHomeModulePath, Path.Combine(psHome, "Modules"), null); diff --git a/test/powershell/Modules/Microsoft.PowerShell.Core/CompatiblePSEditions.Module.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Core/CompatiblePSEditions.Module.Tests.ps1 index 057890d1cc5..6d2c183cd63 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Core/CompatiblePSEditions.Module.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Core/CompatiblePSEditions.Module.Tests.ps1 @@ -131,6 +131,22 @@ function New-TestNestedModule [scriptblock]::Create($newManifestCmd).Invoke() } +function Get-DesktopModuleToUse { + $system32Path = "$env:windir\system32\WindowsPowerShell\v1.0\Modules" + $persistentMemoryModule = "PersistentMemory" + $remoteDesktopModule = "RemoteDesktop" + + if (Test-Path -PathType Container "$system32Path\$persistentMemoryModule") { + return $persistentMemoryModule + } elseif (Test-Path -PathType Container "$system32Path\$remoteDesktopModule") { + return $remoteDesktopModule + } else { + return $null + } +} + +$desktopModuleToUse = Get-DesktopModuleToUse + Describe "Get-Module with CompatiblePSEditions-checked paths" -Tag "CI" { BeforeAll { @@ -645,23 +661,32 @@ Describe "Additional tests for Import-Module with WinCompat" -Tag "Feature" { } It "NoClobber WinCompat list in powershell.config is a Desktop-edition module" { - ## The 'RemoteDesktop' module (available on Windows Server) should not be imported twice. + if (-not $desktopModuleToUse) { + Write-Host 'Skip the test case "NoClobber WinCompat list in powershell.config is a Desktop-edition module" because neither the "PersistentMemory" module nor the "RemoteDesktop" module is available under the System32 module path' -ForegroundColor Yellow + return + } + + ## The 'Desktop' edition module 'PersistentMemory' (available on Windows Client) or 'RemoteDesktop' (available on Windows Server) should not be imported twice. $ConfigPath = Join-Path $TestDrive 'powershell.config.json' - '{"Microsoft.PowerShell:ExecutionPolicy": "RemoteSigned", "WindowsPowerShellCompatibilityNoClobberModuleList": ["RemoteDesktop"]}' | Out-File -Force $ConfigPath +@" +{"Microsoft.PowerShell:ExecutionPolicy": "RemoteSigned", "WindowsPowerShellCompatibilityNoClobberModuleList": ["$desktopModuleToUse"]} +"@ | Out-File -Force $ConfigPath $env:PSModulePath = '' - ## 'RemoteDesktop' is listed in the no-clobber list, so we will first try loading a core-edition compatible - ## version of the module before loading the remote one. The 'system32' module path will be skipped in this - ## attempt, which is expected. - ## If we don't skip the 'system32' module path in this loading attempt, the 'RemoteDesktop' module will be - ## imported twice as a remote module, and then 'Remove-Module RemoteDesktop' won't close the WinCompat session. - $results = & $pwsh -NoProfile -NonInteractive -settingsFile $ConfigPath -c { - Import-Module RemoteDesktop -UseWindowsPowerShell -WarningAction Ignore - Get-Module RemoteDesktop | ForEach-Object { $_.ModuleType.ToString() } - (Get-PSSession | Measure-Object).Count - Remove-Module RemoteDesktop - (Get-PSSession | Measure-Object).Count - } + ## The desktop-edition module is listed in the no-clobber list, so we will first try loading a core-edition + ## compatible version of the module before loading the remote one. The 'system32' module path will be skipped + ## in this attempt, which is by-design. + ## If we don't skip the 'system32' module path in this loading attempt, the desktop-edition module will be + ## imported twice as a remote module, and then 'Remove-Module' won't close the WinCompat session. + $script = @" +Import-Module $desktopModuleToUse -UseWindowsPowerShell -WarningAction Ignore +Get-Module $desktopModuleToUse | ForEach-Object { `$_.ModuleType.ToString() } +(Get-PSSession | Measure-Object).Count +Remove-Module $desktopModuleToUse +(Get-PSSession | Measure-Object).Count +"@ + $scriptBlock = [scriptblock]::Create($script) + $results = & $pwsh -NoProfile -NonInteractive -settingsFile $ConfigPath -c $scriptBlock $results[0] | Should -BeExactly 'Script' $results[1] | Should -BeExactly 1 $results[2] | Should -BeExactly 0 @@ -1424,9 +1449,9 @@ Describe "WinCompat importing should check availablity of built-in modules" -Tag } @{ ## Attempt to load a 'Desktop' edition module should fail because 'Export-PSSession' cannot be found. - Command = 'Import-Module RemoteDesktop -ErrorAction Stop' + Command = $desktopModuleToUse ? "Import-Module $desktopModuleToUse -ErrorAction Stop" : 'Skip the test case that loads a desktop-edition module because neither the "PersistentMemory" module nor the "RemoteDesktop" module is available under the System32 module path' FullyQualifiedErrorId = "CommandNotFoundException,Microsoft.PowerShell.Commands.ImportModuleCommand" - ExceptionMessage = "*'RemoteDesktop'*'Export-PSSession'*'Microsoft.PowerShell.Utility'*" + ExceptionMessage = "*'$desktopModuleToUse'*'Export-PSSession'*'Microsoft.PowerShell.Utility'*" } ) { param( @@ -1435,6 +1460,11 @@ Describe "WinCompat importing should check availablity of built-in modules" -Tag $ExceptionMessage ) + if ($Command.StartsWith("Skip")) { + Write-Host $Command -ForegroundColor Yellow + return + } + $template = @' try {{ {0} From 862852152e3a68de54c6b19d63da26cf076c0f30 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Thu, 6 Jan 2022 14:15:14 -0800 Subject: [PATCH 11/12] Update comment --- .../engine/Modules/ModuleIntrinsics.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs index 1ef82b54179..ada03fb4914 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs @@ -983,7 +983,7 @@ internal static string GetPSHomeModulePath() // "SysWow64" in the key. Later, when 'Import-Module' is called, it loads the module using ""System32" // in the key. // For the cross-platform PowerShell, a user can choose to install it under "C:\Windows\SysWOW64", and - // thus it may have the same problem as described above. + // thus it may have the same problem as described above. So we keep this line of code. psHome = psHome.ToLowerInvariant().Replace(@"\syswow64\", @"\system32\"); #endif Interlocked.CompareExchange(ref s_psHomeModulePath, Path.Combine(psHome, "Modules"), null); From 0809a613931f6f35def5bca41717b5af7dedd604 Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Thu, 6 Jan 2022 16:10:14 -0800 Subject: [PATCH 12/12] Update the test a bit --- .../CompatiblePSEditions.Module.Tests.ps1 | 36 ++++++++++++------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/test/powershell/Modules/Microsoft.PowerShell.Core/CompatiblePSEditions.Module.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Core/CompatiblePSEditions.Module.Tests.ps1 index 6d2c183cd63..f9094254ec3 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Core/CompatiblePSEditions.Module.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Core/CompatiblePSEditions.Module.Tests.ps1 @@ -662,8 +662,7 @@ Describe "Additional tests for Import-Module with WinCompat" -Tag "Feature" { It "NoClobber WinCompat list in powershell.config is a Desktop-edition module" { if (-not $desktopModuleToUse) { - Write-Host 'Skip the test case "NoClobber WinCompat list in powershell.config is a Desktop-edition module" because neither the "PersistentMemory" module nor the "RemoteDesktop" module is available under the System32 module path' -ForegroundColor Yellow - return + throw 'Neither the "PersistentMemory" module nor the "RemoteDesktop" module is available. Please check and use a desktop-edition module that is under the System32 module path.' } ## The 'Desktop' edition module 'PersistentMemory' (available on Windows Client) or 'RemoteDesktop' (available on Windows Server) should not be imported twice. @@ -1447,12 +1446,6 @@ Describe "WinCompat importing should check availablity of built-in modules" -Tag FullyQualifiedErrorId = "System.InvalidOperationException,Microsoft.PowerShell.Commands.ImportModuleCommand" ExceptionMessage = "*'Microsoft.PowerShell.Utility'*'Core'*`$PSHOME*" } - @{ - ## Attempt to load a 'Desktop' edition module should fail because 'Export-PSSession' cannot be found. - Command = $desktopModuleToUse ? "Import-Module $desktopModuleToUse -ErrorAction Stop" : 'Skip the test case that loads a desktop-edition module because neither the "PersistentMemory" module nor the "RemoteDesktop" module is available under the System32 module path' - FullyQualifiedErrorId = "CommandNotFoundException,Microsoft.PowerShell.Commands.ImportModuleCommand" - ExceptionMessage = "*'$desktopModuleToUse'*'Export-PSSession'*'Microsoft.PowerShell.Utility'*" - } ) { param( $Command, @@ -1460,11 +1453,6 @@ Describe "WinCompat importing should check availablity of built-in modules" -Tag $ExceptionMessage ) - if ($Command.StartsWith("Skip")) { - Write-Host $Command -ForegroundColor Yellow - return - } - $template = @' try {{ {0} @@ -1478,10 +1466,32 @@ Describe "WinCompat importing should check availablity of built-in modules" -Tag $scriptBlock = [scriptblock]::Create($script) $result = & "$pwshDir\pwsh.exe" -NoProfile -NonInteractive -c $scriptBlock + $result | Should -HaveCount 2 $result[0] | Should -BeExactly $FullyQualifiedErrorId $result[1] | Should -BeLike $ExceptionMessage } + It "Attempt to load a 'Desktop' edition module should fail because 'Export-PSSession' cannot be found" { + if (-not $desktopModuleToUse) { + throw 'Neither the "PersistentMemory" module nor the "RemoteDesktop" module is available. Please check and use a desktop-edition module that is under the System32 module path.' + } + + $script = @" + try { + Import-Module $desktopModuleToUse -ErrorAction Stop + } catch { + `$_.FullyQualifiedErrorId + `$_.Exception.Message + } +"@ + $env:PSModulePath = '' + $scriptBlock = [scriptblock]::Create($script) + $result = & "$pwshDir\pwsh.exe" -NoProfile -NonInteractive -c $scriptBlock + $result | Should -HaveCount 2 + $result[0] | Should -BeExactly "CommandNotFoundException,Microsoft.PowerShell.Commands.ImportModuleCommand" + $result[1] | Should -BeLike "*'$desktopModuleToUse'*'Export-PSSession'*'Microsoft.PowerShell.Utility'*" + } + It "When built-in modules are available but not in `$PSHOME module path, things should work" { $env:PSModulePath = '' $result = & "$pwshDir\pwsh.exe" -NoProfile -NonInteractive -c @"