From c2dfae8ccb03354e6bb58e244d6936385e3e3fdf Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Tue, 15 Jan 2019 16:22:25 -0800 Subject: [PATCH] Fix 'FixupFileName' to not load resolved assembly during module discovery (#8634) --- .../engine/CommandDiscovery.cs | 26 --- .../engine/Modules/ImportModuleCommand.cs | 2 +- .../engine/Modules/ModuleCmdletBase.cs | 195 +++++++----------- .../engine/hostifaces/PowerShell.cs | 2 +- .../resources/Modules.resx | 6 - .../Get-Module.Tests.ps1 | 47 +++++ .../Import-Module.Tests.ps1 | 2 +- 7 files changed, 130 insertions(+), 150 deletions(-) diff --git a/src/System.Management.Automation/engine/CommandDiscovery.cs b/src/System.Management.Automation/engine/CommandDiscovery.cs index 54b0ac968d..62488f606c 100644 --- a/src/System.Management.Automation/engine/CommandDiscovery.cs +++ b/src/System.Management.Automation/engine/CommandDiscovery.cs @@ -486,32 +486,6 @@ namespace System.Management.Automation } } - #region comment out RequiresNetFrameworkVersion feature 8/10/2010 - /* - * The "#requires -NetFrameworkVersion" feature is CUT OFF. - * This method will be reenabled will be CUT OFF too - /* - internal static void VerifyNetFrameworkVersion(ExternalScriptInfo scriptInfo) - { - Version requiresNetFrameworkVersion = scriptInfo.RequiresNetFrameworkVersion; - - if (requiresNetFrameworkVersion != null) - { - if (!Utils.IsNetFrameworkVersionSupported(requiresNetFrameworkVersion)) - { - ScriptRequiresException scriptRequiresException = - new ScriptRequiresException( - scriptInfo.Name, - scriptInfo.NetFrameworkVersionLineNumber, - requiresNetFrameworkVersion, - "ScriptRequiresUnmatchedNetFrameworkVersion"); - throw scriptRequiresException; - } - } - } - */ - #endregion - /// /// Used to determine compatibility between the versions in the requires statement and /// the installed version. The version can be PSSnapin or msh. diff --git a/src/System.Management.Automation/engine/Modules/ImportModuleCommand.cs b/src/System.Management.Automation/engine/Modules/ImportModuleCommand.cs index 52be13b9a1..c1f5547df4 100644 --- a/src/System.Management.Automation/engine/Modules/ImportModuleCommand.cs +++ b/src/System.Management.Automation/engine/Modules/ImportModuleCommand.cs @@ -1259,7 +1259,7 @@ namespace Microsoft.PowerShell.Commands return true; } - if (manifestEntries.Any(s => FixupFileName(string.Empty, s, ".ps1xml").EndsWith(cimModuleFile.FileName, StringComparison.OrdinalIgnoreCase))) + if (manifestEntries.Any(s => FixupFileName(string.Empty, s, ".ps1xml", isImportingModule: true).EndsWith(cimModuleFile.FileName, StringComparison.OrdinalIgnoreCase))) { return true; } diff --git a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs index 5325dc0787..46ba23bbc0 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs @@ -634,13 +634,13 @@ namespace Microsoft.PowerShell.Commands Version savedBaseRequiredVersion = BaseRequiredVersion; Guid? savedBaseGuid = BaseGuid; - var importingModule = 0 != (manifestProcessingFlags & ManifestProcessingFlags.LoadElements); + var importingModule = manifestProcessingFlags.HasFlag(ManifestProcessingFlags.LoadElements); string extension = Path.GetExtension(moduleSpecification.Name); // First check for fully-qualified paths - either absolute or relative string rootedPath = ResolveRootedFilePath(moduleSpecification.Name, this.Context); if (string.IsNullOrEmpty(rootedPath)) { - rootedPath = FixupFileName(moduleBase, moduleSpecification.Name, extension); + rootedPath = FixupFileName(moduleBase, moduleSpecification.Name, extension, importingModule); } else { @@ -1461,9 +1461,9 @@ namespace Microsoft.PowerShell.Commands { string message; - var bailOnFirstError = 0 != (manifestProcessingFlags & ManifestProcessingFlags.NullOnFirstError); - var importingModule = 0 != (manifestProcessingFlags & ManifestProcessingFlags.LoadElements); - var writingErrors = 0 != (manifestProcessingFlags & ManifestProcessingFlags.WriteErrors); + var bailOnFirstError = manifestProcessingFlags.HasFlag(ManifestProcessingFlags.NullOnFirstError); + var importingModule = manifestProcessingFlags.HasFlag(ManifestProcessingFlags.LoadElements); + var writingErrors = manifestProcessingFlags.HasFlag(ManifestProcessingFlags.WriteErrors); Dbg.Assert(moduleManifestPath != null, "moduleManifestPath for module (.psd1) can't be null"); string moduleBase = Path.GetDirectoryName(moduleManifestPath); @@ -1610,7 +1610,7 @@ namespace Microsoft.PowerShell.Commands // have an extension and the module table is indexed by full names, we // may have search through all the extensions. PSModuleInfo loadedModule = null; - string rootedPath = this.FixupFileName(moduleBase, actualRootModule, null); + string rootedPath = this.FixupFileName(moduleBase, actualRootModule, extension: null, importingModule); string mtpExtension = Path.GetExtension(rootedPath); if (!string.IsNullOrEmpty(mtpExtension) && ModuleIntrinsics.IsPowerShellModuleExtension(mtpExtension)) { @@ -1620,7 +1620,7 @@ namespace Microsoft.PowerShell.Commands { foreach (string extensionToTry in ModuleIntrinsics.PSModuleExtensions) { - rootedPath = this.FixupFileName(moduleBase, actualRootModule, extensionToTry); + rootedPath = this.FixupFileName(moduleBase, actualRootModule, extensionToTry, importingModule); TryGetFromModuleTable(rootedPath, out loadedModule); if (loadedModule != null) break; @@ -1909,27 +1909,6 @@ namespace Microsoft.PowerShell.Commands containedErrors = true; if (bailOnFirstError) return null; } -#if !CORECLR // CLR version is not applicable to CoreCLR - else if (requestedClrVersion != null) - { - Version currentClrVersion = Environment.Version; - if (currentClrVersion < requestedClrVersion) - { - containedErrors = true; - if (writingErrors) - { - message = StringUtil.Format(Modules.ModuleManifestInsufficientCLRVersion, currentClrVersion, - moduleManifestPath, requestedClrVersion); - InvalidOperationException ioe = new InvalidOperationException(message); - ErrorRecord er = new ErrorRecord(ioe, "Modules_InsufficientCLRVersion", - ErrorCategory.ResourceUnavailable, moduleManifestPath); - WriteError(er); - } - - if (bailOnFirstError) return null; - } - } -#endif // Test the required .NET Framework version Version requestedDotNetFrameworkVersion; @@ -1940,35 +1919,6 @@ namespace Microsoft.PowerShell.Commands containedErrors = true; if (bailOnFirstError) return null; } -#if !CORECLR // .NET Framework Version is not applicable to CoreCLR - else if (requestedDotNetFrameworkVersion != null) - { - bool higherThanKnownHighestVersion = false; - if ( - !Utils.IsNetFrameworkVersionSupported(requestedDotNetFrameworkVersion, - out higherThanKnownHighestVersion)) - { - containedErrors = true; - if (writingErrors) - { - message = StringUtil.Format(Modules.InvalidDotNetFrameworkVersion, - moduleManifestPath, requestedDotNetFrameworkVersion); - InvalidOperationException ioe = new InvalidOperationException(message); - ErrorRecord er = new ErrorRecord(ioe, "Modules_InsufficientDotNetFrameworkVersion", - ErrorCategory.ResourceUnavailable, moduleManifestPath); - WriteError(er); - } - - if (bailOnFirstError) return null; - } - else if (higherThanKnownHighestVersion) - { - string cannotDetectNetFrameworkVersionMessage = - StringUtil.Format(Modules.CannotDetectNetFrameworkVersion, requestedDotNetFrameworkVersion); - WriteVerbose(cannotDetectNetFrameworkVersionMessage); - } - } -#endif // HelpInfo URI string helpInfoUri = null; @@ -2258,18 +2208,16 @@ namespace Microsoft.PowerShell.Commands } else { - string fileName = FixupFileName(moduleBase, assembly, StringLiterals.PowerShellNgenAssemblyExtension); + string fileName = FixupFileName(moduleBase, assembly, StringLiterals.PowerShellNgenAssemblyExtension, importingModule, out bool pathIsResolved); + if (!pathIsResolved) + { + fileName = FixupFileName(moduleBase, assembly, StringLiterals.PowerShellILAssemblyExtension, importingModule); + } + string loadMessage = StringUtil.Format(Modules.LoadingFile, "Assembly", fileName); WriteVerbose(loadMessage); iss.Assemblies.Add(new SessionStateAssemblyEntry(assembly, fileName)); fixedUpAssemblyPathList.Add(fileName); - - fileName = FixupFileName(moduleBase, assembly, StringLiterals.PowerShellILAssemblyExtension); - - loadMessage = StringUtil.Format(Modules.LoadingFile, "Assembly", fileName); - WriteVerbose(loadMessage); - iss.Assemblies.Add(new SessionStateAssemblyEntry(assembly, fileName)); - fixedUpAssemblyPathList.Add(fileName); doBind = true; } } @@ -4430,8 +4378,8 @@ namespace Microsoft.PowerShell.Commands { list = null; - List listOfStrings; - if (!GetListOfStringsFromData(data, moduleManifestPath, key, manifestProcessingFlags, out listOfStrings)) + bool importingModule = manifestProcessingFlags.HasFlag(ManifestProcessingFlags.LoadElements); + if (!GetListOfStringsFromData(data, moduleManifestPath, key, manifestProcessingFlags, out List listOfStrings)) { return false; } @@ -4452,7 +4400,7 @@ namespace Microsoft.PowerShell.Commands { try { - string fixedFileName = FixupFileName(moduleBase, s, extension); + string fixedFileName = FixupFileName(moduleBase, s, extension, importingModule); var dir = Path.GetDirectoryName(fixedFileName); if (string.Equals(psHome, dir, StringComparison.OrdinalIgnoreCase) || @@ -4472,7 +4420,7 @@ namespace Microsoft.PowerShell.Commands } catch (Exception e) { - if (0 != (manifestProcessingFlags & ManifestProcessingFlags.WriteErrors)) + if (manifestProcessingFlags.HasFlag(ManifestProcessingFlags.WriteErrors)) { this.ThrowTerminatingError(GenerateInvalidModuleMemberErrorRecord(key, moduleManifestPath, e)); } @@ -4612,51 +4560,81 @@ namespace Microsoft.PowerShell.Commands /// /// A utility routine to fix up a file name so it's rooted and has an extension. /// + internal string FixupFileName(string moduleBase, string name, string extension, bool isImportingModule) + { + return FixupFileName(moduleBase, name, extension, isImportingModule, pathIsResolved: out _); + } + + /// + /// A utility routine to fix up a file name so it's rooted and has an extension. + /// + /// + /// When fixing up an assembly file, this method loads the resovled assembly if it's in the process of actually loading a module. + /// Read the comments in the method for the detailed information. + /// /// The base path to use if the file is not rooted. /// The file name to resolve. - /// The extension to use. - /// - internal string FixupFileName(string moduleBase, string name, string extension) + /// The extension to use in case the given name has no extension. + /// Indicate if we are loading a module. + /// Indicate if the returned path is fully resolved. + /// + /// The resolved file path. Or, the combined path of and when the file path cannot be resolved. + /// + internal string FixupFileName(string moduleBase, string name, string extension, bool isImportingModule, out bool pathIsResolved) { - // First check for full-qualified paths - either absolute or relative - string resolvedName; - if (!IsRooted(name)) + pathIsResolved = false; + string originalName = name; + string originalExt = Path.GetExtension(name); + + if (string.IsNullOrEmpty(originalExt)) { - // The manifest file should only be related to moduleBase path. - resolvedName = ResolveRootedFilePath(Path.Combine(moduleBase, name), this.Context); - } - else - { - resolvedName = ResolveRootedFilePath(name, this.Context); + name += extension; } - if (string.IsNullOrEmpty(resolvedName)) + // Try to get the resolved fully qualified path to the file. + // Note that, the 'IsRooted' method also returns true for relative paths, in which case we need to check for 'combinedPath' as well. + // * For example, the 'Microsoft.WSMan.Management.psd1' in Windows PowerShell defines 'FormatsToProcess="..\..\WSMan.format.ps1xml"'. + // * For such a module, we will have the following input when reaching this method: + // - moduleBase = 'C:\Windows\System32\WindowsPowerShell\v1.0\Modules\Microsoft.WSMan.Management' + // - name = '..\..\WSMan.format.ps1xml' + // Check for combinedPath in this case will get us the normalized rooted path 'C:\Windows\System32\WindowsPowerShell\v1.0\WSMan.format.ps1xml'. + // The 'Microsoft.WSMan.Management' module in PowerShell Core was updated to not use the relative path for 'FormatsToProcess' entry, + // but it's safer to keep the original behavior to avoid unexpected breaking changes. + string combinedPath = Path.Combine(moduleBase, name); + string resolvedPath = IsRooted(name) + ? ResolveRootedFilePath(name, Context) ?? ResolveRootedFilePath(combinedPath, Context) + : ResolveRootedFilePath(combinedPath, Context); + + // Return the path if successfully resolved. + if (resolvedPath != null) { - resolvedName = Path.Combine(moduleBase, name); - } - // Again resolve the file path - // This is so that any relative path references are expanded to give the absolute path - // C:\Windows\System32\WindowsPowerShell\V1.0\Modules\Microsoft.PowerShell.WSMAN\..\..\WSMan.format.ps1xml is expanded to - // C:\Windows\System32\WindowsPowerShell\V1.0\WSMan.format.ps1xml - string resolvedName2 = ResolveRootedFilePath(resolvedName, this.Context); - string ext = Path.GetExtension(name); - string result = !string.IsNullOrEmpty(resolvedName2) ? resolvedName2 : resolvedName; - if (string.IsNullOrEmpty(ext)) - { - result += extension; + if (isImportingModule && resolvedPath.EndsWith(".dll", StringComparison.OrdinalIgnoreCase)) + { + // If we are fixing up an assembly file path and we are actually loading the module, then we load the resolved assembly file here. + // This is because we process type/format ps1xml files before 'RootModule' and 'NestedModules' entries during the module loading. + // A types.ps1xml file could refer to a type defined in the assembly that is specified in the 'RootModule' or 'NestedModule', and + // in that case, processing the types.ps1xml file would fail because it happens before processing the 'RootModule', which loads + // the assembly. We cannot move the processing of types.ps1xml file after processing 'RootModule' either, because the 'RootModule' + // might refer to members defined in the types.ps1xml file. In order to make it work for this paradox, we have to load the resolved + // assembly when we are actually loading the module. However, when it's module analysis, there is no need to load the assembly. + ExecutionContext.LoadAssembly(name: null, filename: resolvedPath, error: out _); + } + + pathIsResolved = true; + return resolvedPath; } - // For dlls, we cannot get the path from the provider. - // We need to load the assembly and then get the path. - // If the module is already loaded, this is not expensive since the assembly is already loaded in the AppDomain - // If the dll is not loaded, we load it from the resolved path. - // We attempt to load it from the resolved path before we try to look up in GAC on Windows. - if (!string.IsNullOrEmpty(ext) && ext.Equals(".dll", StringComparison.OrdinalIgnoreCase)) + // Path resolution failed, use the combined path as default. + string result = combinedPath; + + // For the given assembly name, the intention could be to use the assembly from TPAs or GAC (on Windows). + // So try loading the assembly using the passed-in name only, and use the assembly location if that succeeds. + if (!string.IsNullOrEmpty(originalExt) && originalExt.Equals(".dll", StringComparison.OrdinalIgnoreCase)) { - Exception ignored = null; - Assembly assembly = ExecutionContext.LoadAssembly(name, result, out ignored); + Assembly assembly = ExecutionContext.LoadAssembly(name: originalName, filename: null, error: out _); if (assembly != null) { + pathIsResolved = true; result = assembly.Location; } } @@ -5460,19 +5438,6 @@ namespace Microsoft.PowerShell.Commands if (!scriptName.EndsWith(".cdxml", StringComparison.OrdinalIgnoreCase)) { CommandDiscovery.VerifyScriptRequirements(scriptInfo, Context); - - // Verify that the NetFrameWorkVersion is correct... - - #region comment out RequiresNetFrameworkVersion feature 8/10/2010 - - /* - * The "#requires -NetFrameworkVersion" feature is CUT OFF. - * The call of "VerifyNetFrameworkVersion" will be CUT OFF too. - /* - CommandDiscovery.VerifyNetFrameworkVersion(scriptInfo); - */ - - #endregion } // If we got this far, the check succeeded and we don't need to check again. diff --git a/src/System.Management.Automation/engine/hostifaces/PowerShell.cs b/src/System.Management.Automation/engine/hostifaces/PowerShell.cs index aa557c17ae..e7e4893a20 100644 --- a/src/System.Management.Automation/engine/hostifaces/PowerShell.cs +++ b/src/System.Management.Automation/engine/hostifaces/PowerShell.cs @@ -5,7 +5,7 @@ using System.Collections; using System.Collections.Generic; using System.Collections.ObjectModel; using System.Diagnostics; -using System.Diagnostics.CodeAnalysis; // for fxcop. +using System.Diagnostics.CodeAnalysis; using System.Management.Automation; using System.Management.Automation.Host; using System.Management.Automation.Internal; diff --git a/src/System.Management.Automation/resources/Modules.resx b/src/System.Management.Automation/resources/Modules.resx index 6c1f5b1a9a..477600724f 100644 --- a/src/System.Management.Automation/resources/Modules.resx +++ b/src/System.Management.Automation/resources/Modules.resx @@ -177,9 +177,6 @@ The version '{0}' of module '{1}' does not meet the required minimum version '{2}'. Verify that the version number is supported, and then try loading the module again. - - The version of the Common Language Runtime (CLR) on this computer is '{0}'. The module '{1}' requires a minimum CLR version of '{2}' to run. Verify that you are running the minimum required version of CLR, and then try again. - The version of PowerShell on this computer is '{0}'. The module '{1}' requires a minimum PowerShell version of '{2}' to run. Verify that you have the minimum required version of PowerShell installed, and then try again. @@ -342,9 +339,6 @@ The current processor architecture is: {0}. The module '{1}' requires the following architecture: {2}. - - The module '{0}' requires the following version of the .NET Framework: {1}. The required version is not installed. - The name of the current PowerShell host is: '{0}'. The module '{1}' requires the following PowerShell host: '{2}'. diff --git a/test/powershell/Modules/Microsoft.PowerShell.Core/Get-Module.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Core/Get-Module.Tests.ps1 index 17178faf6d..b6ea7d8dca 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Core/Get-Module.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Core/Get-Module.Tests.ps1 @@ -170,4 +170,51 @@ Describe "Get-Module -ListAvailable" -Tags "CI" { $modules.Name | Sort-Object | Should -BeExactly $ExpectedModule } } + + Context "Module analysis shouldn't load assembly" { + BeforeAll { + $tempModulePath = Join-Path $TestDrive "TempModules" + $testModuleDir = Join-Path $tempModulePath "MyModuelTest" + $moduleManifest = Join-Path $testModuleDir "MyModuelTest.psd1" + $assemblyPath = Join-Path $testModuleDir "MyModuelTestCommandAssembly.dll" + + $null = New-Item $testModuleDir -ItemType Directory -ErrorAction SilentlyContinue + if (-not (Test-Path $moduleManifest)) + { + Set-Content $moduleManifest -Value @' + @{ + RootModule = 'MyModuelTestCommandAssembly.dll' + ModuleVersion = '0.0.1' + GUID = '5776ed43-1607-4e64-be76-acacdf8e9c8c' + FunctionsToExport = @() + CmdletsToExport = @("Get-Test") + AliasesToExport = @() + } +'@ + } + + $code = @' + using System.Management.Automation; + + [Cmdlet("Get", "Test")] + public class MyModuelTestCommand : PSCmdlet + { + protected override void ProcessRecord() + { + WriteObject("BLAH"); + } + } +'@ + if (-not (Test-Path $assemblyPath)) + { + Add-Type -TypeDefinition $code -OutputAssembly $assemblyPath + } + } + + It "'Get-Module -ListAvailable' should not load the module assembly" { + ## $fullName should be null and thus the result should just be the module's name. + $result = pwsh -c "`$env:PSModulePath = '$tempModulePath'; `$module = Get-Module -ListAvailable; `$fullName = [System.AppDomain]::CurrentDomain.GetAssemblies() | Where-Object Location -eq $assemblyPath | Foreach-Object FullName; `$module.Name + `$fullName" + $result | Should -BeExactly "MyModuelTest" + } + } } diff --git a/test/powershell/Modules/Microsoft.PowerShell.Core/Import-Module.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Core/Import-Module.Tests.ps1 index 2aa59cd840..ed72e3ff3d 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Core/Import-Module.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Core/Import-Module.Tests.ps1 @@ -196,7 +196,7 @@ namespace ModuleCmdlets $loadedAssemblyLocation = pwsh -noprofile -c "Import-Module $destPath -Force; [Microsoft.PowerShell.ScheduledJob.AddJobTriggerCommand].Assembly.Location" $loadedAssemblyLocation | Should -BeLike "$TestDrive*\Microsoft.PowerShell.ScheduledJob.dll" } - } +} Describe "Import-Module should be case insensitive" -Tags 'CI' { BeforeAll {