From c6dcb43e924d5490052f3fc4a79e07ba84e20c19 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Thu, 8 Nov 2018 22:36:44 -0800 Subject: [PATCH 01/41] Skip name check when loading required modules --- .../engine/Modules/ModuleCmdletBase.cs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs index 340b95c49c..3b6228d234 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs @@ -4059,10 +4059,17 @@ internal static Collection GetModuleIfAvailable(ModuleSpecificatio tempResult = powerShell.Invoke(); } - // Check if the available module is of the correct version and GUID + // Check if the available module is of the correct version and GUID. The name is already checked. + // GH #8204: The required name here may be the full path, while the module name may be just the module, + // so comparing them may fail incorrectly. foreach (var module in tempResult) { - if (ModuleIntrinsics.IsModuleMatchingModuleSpec(module, requiredModule)) + if (ModuleIntrinsics.IsModuleMatchingConstraints( + module, + guid: requiredModule.Guid, + requiredVersion: requiredModule.RequiredVersion, + minimumVersion: requiredModule.Version, + maximumVersion: GetMaximumVersion(requiredModule.MaximumVersion))) { result.Add(module); } From 4e693d2ac2209b9554025e967ae80c4e6fa58533 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Thu, 8 Nov 2018 23:47:48 -0800 Subject: [PATCH 02/41] [Feature] Fix null ref with max version --- .../engine/Modules/ModuleCmdletBase.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs index 3b6228d234..ceacb97619 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs @@ -4069,7 +4069,7 @@ internal static Collection GetModuleIfAvailable(ModuleSpecificatio guid: requiredModule.Guid, requiredVersion: requiredModule.RequiredVersion, minimumVersion: requiredModule.Version, - maximumVersion: GetMaximumVersion(requiredModule.MaximumVersion))) + maximumVersion: requiredModule.MaximumVersion == null ? null : GetMaximumVersion(requiredModule.MaximumVersion))) { result.Add(module); } From ae5c56b5a07f5a63981853f0a733c518c11e0ba1 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Fri, 9 Nov 2018 01:04:59 -0800 Subject: [PATCH 03/41] [Feature] Fix path comparisons with required modules --- .../engine/Modules/ModuleCmdletBase.cs | 33 ++++++++++++++--- .../engine/Modules/ModuleIntrinsics.cs | 36 ++++++++++++++++++- 2 files changed, 63 insertions(+), 6 deletions(-) diff --git a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs index ceacb97619..9ad85d49ea 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs @@ -1958,6 +1958,15 @@ internal PSModuleInfo LoadModuleManifest( foreach (ModuleSpecification requiredModule in requiredModules) { + string requiredName = requiredModule.Name; + if (requiredName != null + && (requiredName.Contains('/') || requiredName.Contains('\\')) + && !IsRooted(requiredName, relativeRooted: false)) + { + string fullRequiredPath = Path.Combine(moduleBase, requiredName); + requiredModule.Name = ResolveRootedFilePath(fullRequiredPath, Context); + } + ErrorRecord error = null; PSModuleInfo module = LoadRequiredModule(fakeManifestInfo, requiredModule, moduleManifestPath, manifestProcessingFlags, containedErrors, out error); @@ -4548,16 +4557,30 @@ internal string GetAbsolutePath(string moduleBase, string path) } } - internal static bool IsRooted(string filePath) + /// + /// Check if a path is rooted or "relative rooted". + /// + /// The file path to check. + /// If true, paths like "./here" and "..\there" count as rooted ("relative rooted"). + /// True if the path is rooted, false otherwise. + internal static bool IsRooted(string filePath, bool relativeRooted = true) { - return (Path.IsPathRooted(filePath) || - filePath.StartsWith(@".\", StringComparison.Ordinal) || + if (Path.IsPathRooted(filePath) || filePath.IndexOf(":", StringComparison.Ordinal) >= 0) + { + return true; + } + + if (!relativeRooted) + { + return false; + } + + return filePath.StartsWith(@".\", StringComparison.Ordinal) || filePath.StartsWith(@"./", StringComparison.Ordinal) || filePath.StartsWith(@"..\", StringComparison.Ordinal) || filePath.StartsWith(@"../", StringComparison.Ordinal) || filePath.StartsWith(@"~/", StringComparison.Ordinal) || - filePath.StartsWith(@"~\", StringComparison.Ordinal) || - filePath.IndexOf(":", StringComparison.Ordinal) >= 0); + filePath.StartsWith(@"~\", StringComparison.Ordinal); } /// diff --git a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs index 732abffb63..08ee5bb30c 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs @@ -507,6 +507,7 @@ internal static bool IsModuleMatchingConstraints( return AreModuleFieldsMatchingConstraints( out matchFailureReason, moduleInfo.Name, + moduleInfo.Path, moduleInfo.Guid, moduleInfo.Version, name, @@ -521,6 +522,7 @@ internal static bool IsModuleMatchingConstraints( /// Check that given module fields meet any given constraints. /// /// The name of the module to check. + /// The path of the module to check. /// The GUID of the module to check. /// The version of the module to check. /// The name the module must have, if any. @@ -531,6 +533,7 @@ internal static bool IsModuleMatchingConstraints( /// True if the module parameters match all given constraints, false otherwise. internal static bool AreModuleFieldsMatchingConstraints( string moduleName = null, + string modulePath = null, Guid? moduleGuid = null, Version moduleVersion = null, string requiredName = null, @@ -542,6 +545,7 @@ internal static bool AreModuleFieldsMatchingConstraints( return AreModuleFieldsMatchingConstraints( out ModuleMatchFailure matchFailureReason, moduleName, + modulePath, moduleGuid, moduleVersion, requiredName, @@ -556,6 +560,7 @@ internal static bool AreModuleFieldsMatchingConstraints( /// /// The reason the match failed, if any. /// The name of the module to check. + /// The path of the module to check. /// The GUID of the module to check. /// The version of the module to check. /// The name the module must have, if any. @@ -567,6 +572,7 @@ internal static bool AreModuleFieldsMatchingConstraints( internal static bool AreModuleFieldsMatchingConstraints( out ModuleMatchFailure matchFailureReason, string moduleName, + string modulePath, Guid? moduleGuid, Version moduleVersion, string requiredName, @@ -576,7 +582,10 @@ internal static bool AreModuleFieldsMatchingConstraints( Version maximumRequiredVersion) { // If a name is required, check it matches - if (requiredName != null && !requiredName.Equals(moduleName, StringComparison.OrdinalIgnoreCase)) + // A required module name may also be an absolute path, + // so check the module path as well. + if ((requiredName != null && !requiredName.Equals(moduleName, StringComparison.OrdinalIgnoreCase)) + && !MatchesModulePath(modulePath, requiredName)) { matchFailureReason = ModuleMatchFailure.Name; return false; @@ -659,6 +668,31 @@ internal static bool IsVersionMatchingConstraints( return true; } + /// + /// Checks whether a given module path is the same as + /// a required path. + /// + /// The path of the module whose path to check. + /// The path of the required module. + /// + internal static bool MatchesModulePath(string modulePath, string requiredPath) + { + Dbg.Assert(requiredPath != null, $"Caller to verify that {nameof(requiredPath)} is not null"); + + // If the module has no path, then it cannot match + if (modulePath == null) { return false; } + + // We have to trust that paths have been properly normalized and made absolute at this point, + // since we lack to context to resolve any relative paths. + // So either the module is a simple name, or it's an absolute path. + Dbg.Assert(Path.IsPathRooted(requiredPath) || !(requiredPath.Contains('/') || requiredPath.Contains('\\')), "Relative paths must be resolved by the calling context"); +#if UNIX + return String.Equals(modulePath, requiredPath); +#else + return String.Equals(modulePath, requiredPath, StringComparison.OrdinalIgnoreCase); +#endif + } + internal static Version GetManifestModuleVersion(string manifestPath) { try From b3c9a8753766d92714a78876964f9cffd400f492 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Fri, 9 Nov 2018 01:55:56 -0800 Subject: [PATCH 04/41] [Feature] Match module when required path is base dir --- .../engine/Modules/ModuleIntrinsics.cs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs index 08ee5bb30c..b289a11735 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs @@ -686,10 +686,20 @@ internal static bool MatchesModulePath(string modulePath, string requiredPath) // since we lack to context to resolve any relative paths. // So either the module is a simple name, or it's an absolute path. Dbg.Assert(Path.IsPathRooted(requiredPath) || !(requiredPath.Contains('/') || requiredPath.Contains('\\')), "Relative paths must be resolved by the calling context"); + + // The required module may point to the module base directory + string moduleDirPath = Path.GetDirectoryName(modulePath); + + // The module itself may be in a versioned directory + if (Version.TryParse(Path.GetFileName(moduleDirPath), out Version unused)) + { + moduleDirPath = Path.GetDirectoryName(moduleDirPath); + } #if UNIX - return String.Equals(modulePath, requiredPath); + return modulePath.Equals(requiredPath) || moduleDirPath.Equals(requiredPath); #else - return String.Equals(modulePath, requiredPath, StringComparison.OrdinalIgnoreCase); + return modulePath.Equals(requiredPath, StringComparison.OrdinalIgnoreCase) + || moduleDirPath.Equals(requiredPath, StringComparison.OrdinalIgnoreCase); #endif } From 5c04829a289c3351104fe21f3aab7ad13cc25d16 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Fri, 9 Nov 2018 02:49:06 -0800 Subject: [PATCH 05/41] [Feature] Trim end of required module path --- .../engine/Modules/ModuleCmdletBase.cs | 18 ++++++++--- .../engine/Modules/ModuleIntrinsics.cs | 32 +++++++++++++++---- 2 files changed, 39 insertions(+), 11 deletions(-) diff --git a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs index 9ad85d49ea..a23f55f082 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs @@ -1958,13 +1958,23 @@ internal PSModuleInfo LoadModuleManifest( foreach (ModuleSpecification requiredModule in requiredModules) { + // The required module name is essentially raw user input. + // We need to deal with absolute and relative paths here. string requiredName = requiredModule.Name; if (requiredName != null - && (requiredName.Contains('/') || requiredName.Contains('\\')) - && !IsRooted(requiredName, relativeRooted: false)) + && (requiredName.Contains('/') || requiredName.Contains('\\'))) { - string fullRequiredPath = Path.Combine(moduleBase, requiredName); - requiredModule.Name = ResolveRootedFilePath(fullRequiredPath, Context); + // Normalize the path to the platform standard + requiredName = requiredName.Replace(Path.AltDirectorySeparatorChar, Path.DirectorySeparatorChar); + + // Root the path if needed + if (!IsRooted(requiredName, relativeRooted: false)) + { + requiredName = Path.Combine(moduleBase, requiredName); + } + + // Use the filesystem provider to resolve the path + requiredModule.Name = ResolveRootedFilePath(requiredName, Context); } ErrorRecord error = null; diff --git a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs index b289a11735..771da51182 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs @@ -687,7 +687,26 @@ internal static bool MatchesModulePath(string modulePath, string requiredPath) // So either the module is a simple name, or it's an absolute path. Dbg.Assert(Path.IsPathRooted(requiredPath) || !(requiredPath.Contains('/') || requiredPath.Contains('\\')), "Relative paths must be resolved by the calling context"); - // The required module may point to the module base directory +#if UNIX + StringComparison strcmp = StringComparison.Ordinal; +#else + StringComparison strcmp = StringComparison.OrdinalIgnoreCase; +#endif + + // If the required module just matches the module path, we are done + if (modulePath.Equals(requiredPath, strcmp)) + { + return true; + } + + // Save some allocations here if module path doesn't sit under the required path + // (the required path may still refer to some nested module though) + if (!modulePath.StartsWith(requiredPath, strcmp)) + { + return false; + } + + // Otherwise, the required module may point to the module base directory string moduleDirPath = Path.GetDirectoryName(modulePath); // The module itself may be in a versioned directory @@ -695,12 +714,11 @@ internal static bool MatchesModulePath(string modulePath, string requiredPath) { moduleDirPath = Path.GetDirectoryName(moduleDirPath); } -#if UNIX - return modulePath.Equals(requiredPath) || moduleDirPath.Equals(requiredPath); -#else - return modulePath.Equals(requiredPath, StringComparison.OrdinalIgnoreCase) - || moduleDirPath.Equals(requiredPath, StringComparison.OrdinalIgnoreCase); -#endif + + // Make sure there are no trailing separator chars on the required path + requiredPath = requiredPath.TrimEnd(Path.DirectorySeparatorChar); + + return moduleDirPath.Equals(requiredPath); } internal static Version GetManifestModuleVersion(string manifestPath) From 28981dfadabbbb999faba23cdf8153cce999bf3a Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Fri, 9 Nov 2018 02:49:35 -0800 Subject: [PATCH 06/41] [Feature] Add testing for required module autoloading failure --- .../ModuleManifest.Tests.ps1 | 234 ++++++++++++++++++ 1 file changed, 234 insertions(+) create mode 100644 test/powershell/Modules/Microsoft.PowerShell.Core/ModuleManifest.Tests.ps1 diff --git a/test/powershell/Modules/Microsoft.PowerShell.Core/ModuleManifest.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Core/ModuleManifest.Tests.ps1 new file mode 100644 index 0000000000..d9ef82d1a8 --- /dev/null +++ b/test/powershell/Modules/Microsoft.PowerShell.Core/ModuleManifest.Tests.ps1 @@ -0,0 +1,234 @@ +function New-ModuleFromLayout +{ + param( + [Parameter(Mandatory=$true)] + [hashtable] + $Layout, + + [Parameter(Mandatory=$true)] + [string] + $BaseDir + ) + + if (-not (Test-Path $BaseDir)) + { + $null = New-Item -Path $BaseDir -ItemType Directory + } + + foreach ($item in $Layout.get_Keys()) + { + $itemPath = Join-Path $BaseDir $item + $ext = [System.IO.Path]::GetExtension($item) + + switch ($ext) + { + '.psd1' + { + $moduleParameters = $Layout[$item] + New-ModuleManifest -Path $itemPath @moduleParameters + break + } + + { '.psm1','.ps1' -contains $_ } + { + $null = New-Item -Path $itemPath -Value $Layout[$item] + break + } + + default + { + if ($Layout[$item] -is [hashtable]) + { + New-ModuleFromLayout -BaseDir $itemPath -Layout $Layout[$item] + } + break + } + } + } +} + +Describe "Manifest required module autoloading from module path with simple names" -Tags "CI" { + BeforeAll { + $prevModulePath = $env:PSModulePath + $env:PSModulePath = ($TestDrive -as [string]) + [System.IO.Path]::PathSeparator + $env:PSModulePath + + $mainModule = 'mainmod' + $requiredModule = 'reqmod' + + New-ModuleFromLayout -BaseDir $TestDrive -Layout @{ + $mainModule = @{ + "$mainModule.psd1" = @{ + RequiredModules = $requiredModule + } + } + $requiredModule = @{ + "$requiredModule.psd1" = @{} + } + } + } + + AfterAll { + $env:PSModulePath = $prevModulePath + Get-Module $mainModule,$requiredModule | Remove-Module + } + + It "Importing main module loads required modules successfully" { + $mainMod = Import-Module $mainModule -PassThru + $reqMod = Get-Module -Name $requiredModule + + $mainMod.Name | Should -BeExactly $mainModule + $mainMod.RequiredModules[0].Name | Should -Be $requiredModule + $reqMod.Name | Should -BeExactly $requiredModule + } +} + +Describe "Manifest required module autoloading with relative path to dir" -Tags "CI" { + BeforeAll { + $mainModule = 'mainmod' + $requiredModule = 'reqmod' + + $mainModPath = Join-Path $TestDrive $mainModule + $reqModPath = Join-Path $TestDrive $requiredModule + + # Test to ensure that we treat backslashes as path separators on UNIX and vice-versa + $altSep = [System.IO.Path]::AltDirectorySeparatorChar + + New-ModuleFromLayout -BaseDir $TestDrive -Layout @{ + $mainModule = @{ + "$mainModule.psd1" = @{ + RequiredModules = "..${altSep}$requiredModule" + } + } + $requiredModule = @{ + "$requiredModule.psd1" = @{} + } + } + } + + AfterAll { + Get-Module $mainModule,$requiredModule | Remove-Module + } + + It "Importing main module loads required modules successfully" { + $mainMod = Import-Module $mainModPath -PassThru + $reqMod = Get-Module -Name $requiredModule + + $mainMod.Name | Should -BeExactly $mainModule + $mainMod.RequiredModules[0].Name | Should -Be $requiredModule + $reqMod.Name | Should -BeExactly $requiredModule + } +} + +Describe "Manifest required module autoloading with relative path to manifest" -Tags "CI" { + BeforeAll { + $mainModule = 'mainmod' + $requiredModule = 'reqmod' + + $mainModPath = Join-Path $TestDrive $mainModule "$mainModule.psd1" + $reqModPath = Join-Path $TestDrive $requiredModule "$requiredModule.psd1" + + # Test to ensure that we treat backslashes as path separators on UNIX and vice-versa + $altSep = [System.IO.Path]::AltDirectorySeparatorChar + $sep = [System.IO.Path]::DirectorySeparatorChar + + New-ModuleFromLayout -BaseDir $TestDrive -Layout @{ + $mainModule = @{ + "$mainModule.psd1" = @{ + RequiredModules = "..${altSep}$requiredModule${sep}$requiredModule.psd1" + } + } + $requiredModule = @{ + "$requiredModule.psd1" = @{} + } + } + } + + AfterAll { + Get-Module $mainModule,$requiredModule | Remove-Module + } + + It "Importing main module loads required modules successfully" { + $mainMod = Import-Module $mainModPath -PassThru + $reqMod = Get-Module -Name $requiredModule + + $mainMod.Name | Should -BeExactly $mainModule + $mainMod.RequiredModules[0].Name | Should -Be $requiredModule + $reqMod.Name | Should -BeExactly $requiredModule + } +} + +Describe "Manifest required module autoloading with absolute path to dir" -Tags "CI" { + BeforeAll { + $mainModule = 'mainmod' + $requiredModule = 'reqmod' + + $mainModPath = Join-Path $TestDrive $mainModule "$mainModule.psd1" + $reqModPath = Join-Path $TestDrive $requiredModule "$requiredModule.psd1" + + # Test to ensure that we treat backslashes as path separators on UNIX and vice-versa + $altSep = [System.IO.Path]::AltDirectorySeparatorChar + $sep = [System.IO.Path]::DirectorySeparatorChar + + New-ModuleFromLayout -BaseDir $TestDrive -Layout @{ + $mainModule = @{ + "$mainModule.psd1" = @{ + RequiredModules = "$TestDrive${altSep}$requiredModule${sep}" + } + } + $requiredModule = @{ + "$requiredModule.psd1" = @{} + } + } + } + + AfterAll { + Get-Module $mainModule,$requiredModule | Remove-Module + } + + It "Importing main module loads required modules successfully" { + $mainMod = Import-Module $mainModPath -PassThru + $reqMod = Get-Module -Name $requiredModule + + $mainMod.Name | Should -BeExactly $mainModule + $mainMod.RequiredModules[0].Name | Should -Be $requiredModule + $reqMod.Name | Should -BeExactly $requiredModule + } +} + +Describe "Manifest required module autoloading with absolute path to manifest" -Tags "CI" { + BeforeAll { + $mainModule = 'mainmod' + $requiredModule = 'reqmod' + + $mainModPath = Join-Path $TestDrive $mainModule "$mainModule.psd1" + $reqModPath = Join-Path $TestDrive $requiredModule "$requiredModule.psd1" + + # Test to ensure that we treat backslashes as path separators on UNIX and vice-versa + $altSep = [System.IO.Path]::AltDirectorySeparatorChar + $sep = [System.IO.Path]::DirectorySeparatorChar + + New-ModuleFromLayout -BaseDir $TestDrive -Layout @{ + $mainModule = @{ + "$mainModule.psd1" = @{ + RequiredModules = "$TestDrive${altSep}$requiredModule${sep}$requiredModule.psd1" + } + } + $requiredModule = @{ + "$requiredModule.psd1" = @{} + } + } + } + + AfterAll { + Get-Module $mainModule,$requiredModule | Remove-Module + } + + It "Importing main module loads required modules successfully" { + $mainMod = Import-Module $mainModPath -PassThru + $reqMod = Get-Module -Name $requiredModule + + $mainMod.Name | Should -BeExactly $mainModule + $mainMod.RequiredModules[0].Name | Should -Be $requiredModule + $reqMod.Name | Should -BeExactly $requiredModule + } +} From cea4dd56a8e8319c843e734deb147789daa61a22 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Fri, 9 Nov 2018 02:53:33 -0800 Subject: [PATCH 07/41] [Feature] Address CodeFactor issues --- .../engine/Modules/ModuleIntrinsics.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs index 771da51182..ae79b807ed 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs @@ -674,13 +674,16 @@ internal static bool IsVersionMatchingConstraints( /// /// The path of the module whose path to check. /// The path of the required module. - /// + /// True if the module path matches the required path, false otherwise. internal static bool MatchesModulePath(string modulePath, string requiredPath) { Dbg.Assert(requiredPath != null, $"Caller to verify that {nameof(requiredPath)} is not null"); // If the module has no path, then it cannot match - if (modulePath == null) { return false; } + if (modulePath == null) + { + return false; + } // We have to trust that paths have been properly normalized and made absolute at this point, // since we lack to context to resolve any relative paths. From b8e513aedce3cece109bdd781b5c28dc02f9f8f4 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Fri, 9 Nov 2018 12:16:35 -0800 Subject: [PATCH 08/41] [Feature] Make relative paths work in module specification checks --- .../engine/GetCommandCommand.cs | 6 +- .../engine/Modules/GetModuleCommand.cs | 4 +- .../engine/Modules/ModuleCmdletBase.cs | 26 +++- .../engine/Modules/ModuleIntrinsics.cs | 129 +++++++++++++++--- .../engine/Modules/RemoveModuleCommand.cs | 2 +- .../engine/Utils.cs | 5 +- .../help/UpdatableHelpCommandBase.cs | 2 +- 7 files changed, 141 insertions(+), 33 deletions(-) diff --git a/src/System.Management.Automation/engine/GetCommandCommand.cs b/src/System.Management.Automation/engine/GetCommandCommand.cs index 536fb15d3e..1e618650ad 100644 --- a/src/System.Management.Automation/engine/GetCommandCommand.cs +++ b/src/System.Management.Automation/engine/GetCommandCommand.cs @@ -589,7 +589,7 @@ private bool IsNounVerbMatch(CommandInfo command) { if (!_moduleSpecifications.Any( moduleSpecification => - ModuleIntrinsics.IsModuleMatchingModuleSpec(command.Module, moduleSpecification))) + ModuleIntrinsics.IsModuleMatchingModuleSpec(SessionState.Path.CurrentLocation.Path, Context, command.Module, moduleSpecification))) { break; } @@ -1113,7 +1113,7 @@ private bool IsCommandMatch(ref CommandInfo current, out bool isDuplicate) bool foundModuleMatch = false; foreach (var moduleSpecification in _moduleSpecifications) { - if (ModuleIntrinsics.IsModuleMatchingModuleSpec(current.Module, moduleSpecification)) + if (ModuleIntrinsics.IsModuleMatchingModuleSpec(SessionState.Path.CurrentLocation.Path, Context, current.Module, moduleSpecification)) { foundModuleMatch = true; break; @@ -1261,7 +1261,7 @@ private IEnumerable GetMatchingCommandsFromModules(string commandNa { isModuleMatch = SessionStateUtilities.MatchesAnyWildcardPattern(module.Name, _modulePatterns, true); } - else if (_moduleSpecifications.Any(moduleSpecification => ModuleIntrinsics.IsModuleMatchingModuleSpec(module, moduleSpecification))) + else if (_moduleSpecifications.Any(moduleSpecification => ModuleIntrinsics.IsModuleMatchingModuleSpec(SessionState.Path.CurrentLocation.Path, Context, module, moduleSpecification))) { isModuleMatch = true; } diff --git a/src/System.Management.Automation/engine/Modules/GetModuleCommand.cs b/src/System.Management.Automation/engine/Modules/GetModuleCommand.cs index c6745a4a44..c3cbadecbb 100644 --- a/src/System.Management.Automation/engine/Modules/GetModuleCommand.cs +++ b/src/System.Management.Automation/engine/Modules/GetModuleCommand.cs @@ -531,7 +531,7 @@ private IEnumerable FilterModulesForEditionAndSpecification( /// The modules to filter by specification match. /// The specification lookup table to filter the modules on. /// The modules that match their corresponding table entry, or which have no table entry. - private static IEnumerable FilterModulesForSpecificationMatch( + private IEnumerable FilterModulesForSpecificationMatch( IEnumerable modules, IDictionary moduleSpecificationTable) { @@ -548,7 +548,7 @@ private static IEnumerable FilterModulesForSpecificationMatch( } // Modules with table entries only get returned if they match them - if (ModuleIntrinsics.IsModuleMatchingModuleSpec(module, moduleSpecification)) + if (ModuleIntrinsics.IsModuleMatchingModuleSpec(SessionState.Path.CurrentLocation.Path, Context, module, moduleSpecification)) { yield return module; } diff --git a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs index a23f55f082..a3f12d6565 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs @@ -1656,6 +1656,8 @@ internal PSModuleInfo LoadModuleManifest( if (bailOnFirstError) return null; } else if (!ModuleIntrinsics.AreModuleFieldsMatchingConstraints( + moduleBase, + Context, moduleGuid: manifestGuid, moduleVersion: moduleVersion, requiredGuid: requiredModuleGuid, @@ -3641,12 +3643,13 @@ private static ErrorRecord GenerateInvalidModuleMemberErrorRecord(string manifes /// If loadElements is false, we check requireModule for correctness but do /// not check if the modules are loaded. /// + /// Path to resolve relative module paths in the specification against. /// Execution Context. /// Either a string or a hash of ModuleName, optional Guid, and ModuleVersion. /// The reason the module failed to load, or null on success. /// Sets if the module/snapin is already present. /// null if the module is not loaded or loadElements is false, the loaded module otherwise. - internal static object IsModuleLoaded(ExecutionContext context, ModuleSpecification requiredModule, out ModuleMatchFailure matchFailureReason, out bool loaded) + internal static object IsModuleLoaded(string basePath, ExecutionContext context, ModuleSpecification requiredModule, out ModuleMatchFailure matchFailureReason, out bool loaded) { loaded = false; Dbg.Assert(requiredModule != null, "Caller should verify requiredModuleSpecification != null"); @@ -3659,7 +3662,7 @@ internal static object IsModuleLoaded(ExecutionContext context, ModuleSpecificat foreach (PSModuleInfo module in context.Modules.GetModules(new string[] { "*" }, false)) { // Check that the module meets the module constraints give - if (ModuleIntrinsics.IsModuleMatchingModuleSpec(out matchFailure, module, requiredModule)) + if (ModuleIntrinsics.IsModuleMatchingModuleSpec(out matchFailure, basePath, context, module, requiredModule)) { result = module; loaded = true; @@ -3733,7 +3736,7 @@ internal static PSModuleInfo LoadRequiredModule(ExecutionContext context, ModuleMatchFailure loadFailureReason = ModuleMatchFailure.None; bool loaded = false; - object loadedModule = IsModuleLoaded(context, requiredModuleSpecification, out loadFailureReason, out loaded); + object loadedModule = IsModuleLoaded(currentModule.ModuleBase, context, requiredModuleSpecification, out loadFailureReason, out loaded); if (loadedModule == null) { @@ -3765,7 +3768,7 @@ internal static PSModuleInfo LoadRequiredModule(ExecutionContext context, out error); if (!hasRequiredModulesCyclicReference) { - result = ImportRequiredModule(context, requiredModuleSpecification, out error); + result = ImportRequiredModule(currentModule.ModuleBase, context, requiredModuleSpecification, out error); } else { @@ -3889,7 +3892,11 @@ internal static PSModuleInfo LoadRequiredModule(ExecutionContext context, return result; } - private static PSModuleInfo ImportRequiredModule(ExecutionContext context, ModuleSpecification requiredModule, out ErrorRecord error) + private static PSModuleInfo ImportRequiredModule( + string basePath, + ExecutionContext context, + ModuleSpecification requiredModule, + out ErrorRecord error) { error = null; PSModuleInfo result = null; @@ -3947,7 +3954,7 @@ private static PSModuleInfo ImportRequiredModule(ExecutionContext context, Modul ModuleMatchFailure loadFailureReason; bool loaded = false; - object r = IsModuleLoaded(context, ms, out loadFailureReason, out loaded); + object r = IsModuleLoaded(basePath, context, ms, out loadFailureReason, out loaded); Dbg.Assert(r is PSModuleInfo, "The returned value should be PSModuleInfo"); @@ -4045,7 +4052,8 @@ internal bool VerifyIfNestedModuleIsAvailable(ModuleSpecification nestedModuleSp // Checks if module is available to be loaded // ModuleName ---> checks if module can be loaded using Module loading rules // ModuleManifest --> checks if manifest is valid - internal static Collection GetModuleIfAvailable(ModuleSpecification requiredModule, + internal static Collection GetModuleIfAvailable( + ModuleSpecification requiredModule, Runspace rsToUse = null) { Collection result = new Collection(); @@ -4084,6 +4092,8 @@ internal static Collection GetModuleIfAvailable(ModuleSpecificatio foreach (var module in tempResult) { if (ModuleIntrinsics.IsModuleMatchingConstraints( + basePath: null, // basePath is not needed, since we don't need path resolution + context: null, // context is also only required for path resolution module, guid: requiredModule.Guid, requiredVersion: requiredModule.RequiredVersion, @@ -5053,6 +5063,8 @@ private bool ShouldModuleBeRemoved(PSModuleInfo module, string moduleNameInRemov internal bool DoesAlreadyLoadedModuleSatisfyConstraints(PSModuleInfo alreadyLoadedModule) { return ModuleIntrinsics.IsModuleMatchingConstraints( + basePath: null, // basePath is not needed here, since we are only checking the version + context: null, // context is not needed, since we don't need path resolution alreadyLoadedModule, guid: BaseGuid, requiredVersion: BaseRequiredVersion, diff --git a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs index ae79b807ed..96b9ab90cc 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs @@ -351,7 +351,11 @@ private List GetModuleCore(string[] patterns, bool all, bool exact return modulesMatched.OrderBy(m => m.Name).ToList(); } - internal List GetModules(ModuleSpecification[] fullyQualifiedName, bool all) + internal List GetModules( + string basePath, + ExecutionContext context, + ModuleSpecification[] fullyQualifiedName, + bool all) { List modulesMatched = new List(); @@ -362,7 +366,7 @@ internal List GetModules(ModuleSpecification[] fullyQualifiedName, foreach (PSModuleInfo module in ModuleTable.Values) { // See if this is the requested module... - if (IsModuleMatchingModuleSpec(module, moduleSpec)) + if (IsModuleMatchingModuleSpec(basePath, context, module, moduleSpec)) { modulesMatched.Add(module); } @@ -381,7 +385,7 @@ internal List GetModules(ModuleSpecification[] fullyQualifiedName, string path = pair.Key; PSModuleInfo module = pair.Value; // See if this is the requested module... - if (IsModuleMatchingModuleSpec(module, moduleSpec)) + if (IsModuleMatchingModuleSpec(basePath, context, module, moduleSpec)) { modulesMatched.Add(module); found[path] = true; @@ -397,7 +401,7 @@ internal List GetModules(ModuleSpecification[] fullyQualifiedName, { PSModuleInfo module = pair.Value; // See if this is the requested module... - if (IsModuleMatchingModuleSpec(module, moduleSpec)) + if (IsModuleMatchingModuleSpec(basePath, context, module, moduleSpec)) { modulesMatched.Add(module); } @@ -414,22 +418,40 @@ internal List GetModules(ModuleSpecification[] fullyQualifiedName, /// /// Check if a given module info object matches a given module specification. /// + /// The base path to resolve relative required paths from. + /// The current execution context. /// The module info object to check. /// The module specification to match the module info object against. /// True if the module info object meets all the constraints on the module specification, false otherwise. - internal static bool IsModuleMatchingModuleSpec(PSModuleInfo moduleInfo, ModuleSpecification moduleSpec) + internal static bool IsModuleMatchingModuleSpec( + string basePath, + ExecutionContext context, + PSModuleInfo moduleInfo, + ModuleSpecification moduleSpec) { - return IsModuleMatchingModuleSpec(out ModuleMatchFailure matchFailureReason, moduleInfo, moduleSpec); + return IsModuleMatchingModuleSpec( + out ModuleMatchFailure matchFailureReason, + basePath, + context, + moduleInfo, + moduleSpec); } /// /// Check if a given module info object matches a given module specification. /// /// The constraint that caused the match failure, if any. + /// The base path to resolve relative required paths against. + /// The current execution context. /// The module info object to check. /// The module specification to match the module info object against. /// True if the module info object meets all the constraints on the module specification, false otherwise. - internal static bool IsModuleMatchingModuleSpec(out ModuleMatchFailure matchFailureReason, PSModuleInfo moduleInfo, ModuleSpecification moduleSpec) + internal static bool IsModuleMatchingModuleSpec( + out ModuleMatchFailure matchFailureReason, + string basePath, + ExecutionContext context, + PSModuleInfo moduleInfo, + ModuleSpecification moduleSpec) { if (moduleSpec == null) { @@ -439,6 +461,8 @@ internal static bool IsModuleMatchingModuleSpec(out ModuleMatchFailure matchFail return IsModuleMatchingConstraints( out matchFailureReason, + basePath, + context, moduleInfo, moduleSpec.Name, moduleSpec.Guid, @@ -451,6 +475,8 @@ internal static bool IsModuleMatchingModuleSpec(out ModuleMatchFailure matchFail /// Check if a given module info object matches the given constraints. /// Constraints given as null are ignored. /// + /// The base path to resolve required paths against. + /// The current execution context. /// The module info object to check. /// The name of the expected module. /// The guid of the expected module. @@ -459,6 +485,8 @@ internal static bool IsModuleMatchingModuleSpec(out ModuleMatchFailure matchFail /// The maximum required version of the expected module. /// True if the module info object matches all given constraints, false otherwise. internal static bool IsModuleMatchingConstraints( + string basePath, + ExecutionContext context, PSModuleInfo moduleInfo, string name = null, Guid? guid = null, @@ -468,6 +496,8 @@ internal static bool IsModuleMatchingConstraints( { return IsModuleMatchingConstraints( out ModuleMatchFailure matchFailureReason, + basePath, + context, moduleInfo, name, guid, @@ -481,6 +511,8 @@ internal static bool IsModuleMatchingConstraints( /// Constraints given as null are ignored. /// /// The reason for the module constraint match failing. + /// The base path to resolve relative required paths against. + /// The current execution context. /// The module info object to check. /// The name of the expected module. /// The guid of the expected module. @@ -490,6 +522,8 @@ internal static bool IsModuleMatchingConstraints( /// True if the module info object matches all given constraints, false otherwise. internal static bool IsModuleMatchingConstraints( out ModuleMatchFailure matchFailureReason, + string basePath, + ExecutionContext context, PSModuleInfo moduleInfo, string name, Guid? guid, @@ -506,6 +540,8 @@ internal static bool IsModuleMatchingConstraints( return AreModuleFieldsMatchingConstraints( out matchFailureReason, + basePath, + context, moduleInfo.Name, moduleInfo.Path, moduleInfo.Guid, @@ -521,6 +557,8 @@ internal static bool IsModuleMatchingConstraints( /// /// Check that given module fields meet any given constraints. /// + /// The base path to resolve relative required paths against. + /// The current execution context. /// The name of the module to check. /// The path of the module to check. /// The GUID of the module to check. @@ -532,6 +570,8 @@ internal static bool IsModuleMatchingConstraints( /// The maximum version the module may have, if any. /// True if the module parameters match all given constraints, false otherwise. internal static bool AreModuleFieldsMatchingConstraints( + string basePath, + ExecutionContext context, string moduleName = null, string modulePath = null, Guid? moduleGuid = null, @@ -544,6 +584,8 @@ internal static bool AreModuleFieldsMatchingConstraints( { return AreModuleFieldsMatchingConstraints( out ModuleMatchFailure matchFailureReason, + basePath, + context, moduleName, modulePath, moduleGuid, @@ -559,6 +601,8 @@ internal static bool AreModuleFieldsMatchingConstraints( /// Check that given module fields meet any given constraints. /// /// The reason the match failed, if any. + /// The base path to resolve relative required paths against. + /// The current execution context. /// The name of the module to check. /// The path of the module to check. /// The GUID of the module to check. @@ -571,6 +615,8 @@ internal static bool AreModuleFieldsMatchingConstraints( /// True if the module parameters match all given constraints, false otherwise. internal static bool AreModuleFieldsMatchingConstraints( out ModuleMatchFailure matchFailureReason, + string basePath, + ExecutionContext context, string moduleName, string modulePath, Guid? moduleGuid, @@ -585,7 +631,7 @@ internal static bool AreModuleFieldsMatchingConstraints( // A required module name may also be an absolute path, // so check the module path as well. if ((requiredName != null && !requiredName.Equals(moduleName, StringComparison.OrdinalIgnoreCase)) - && !MatchesModulePath(modulePath, requiredName)) + && !MatchesModulePath(modulePath, requiredName, basePath, context)) { matchFailureReason = ModuleMatchFailure.Name; return false; @@ -674,8 +720,16 @@ internal static bool IsVersionMatchingConstraints( /// /// The path of the module whose path to check. /// The path of the required module. + /// The base path to resolve relative required module paths against. + /// The current execution context. /// True if the module path matches the required path, false otherwise. - internal static bool MatchesModulePath(string modulePath, string requiredPath) + /// + /// Because a module specification can contain a relative path, we are + /// forced to pass a base path and context down to this point. + /// A better solution would be to consolidate the module specification + /// codepath to force path normalization in the caller. + /// + internal static bool MatchesModulePath(string modulePath, string requiredPath, string basePath, ExecutionContext context) { Dbg.Assert(requiredPath != null, $"Caller to verify that {nameof(requiredPath)} is not null"); @@ -685,11 +739,7 @@ internal static bool MatchesModulePath(string modulePath, string requiredPath) return false; } - // We have to trust that paths have been properly normalized and made absolute at this point, - // since we lack to context to resolve any relative paths. - // So either the module is a simple name, or it's an absolute path. - Dbg.Assert(Path.IsPathRooted(requiredPath) || !(requiredPath.Contains('/') || requiredPath.Contains('\\')), "Relative paths must be resolved by the calling context"); - + requiredPath = NormalizeModuleName(requiredPath, basePath, context); #if UNIX StringComparison strcmp = StringComparison.Ordinal; #else @@ -718,12 +768,57 @@ internal static bool MatchesModulePath(string modulePath, string requiredPath) moduleDirPath = Path.GetDirectoryName(moduleDirPath); } - // Make sure there are no trailing separator chars on the required path - requiredPath = requiredPath.TrimEnd(Path.DirectorySeparatorChar); - return moduleDirPath.Equals(requiredPath); } + /// + /// Takes the name of a module as used in a module specification + /// and either returns it as a simple name (if it was a simple name) + /// or a fully qualified, PowerShell-resolved path. + /// + /// The name of the module from the specification. + /// The path to base relative paths off. + /// The current execution context. + /// + /// The simple module name if the given one was simple, + /// otherwise a fully resolved, absolute path to the module. + /// + /// + /// 2018-11-09 rjmholt: + /// There are several, possibly inconsistent, path handling mechanisms + /// in the module cmdlets. After looking through all of them and seeing + /// they all make some assumptions about their caller I wrote this method. + /// Hopefully we can find a standard path resolution API to settle on. + /// + internal static string NormalizeModuleName( + string moduleName, + string basePath, + ExecutionContext executionContext) + { + if (moduleName == null) + { + return null; + } + + // The module name may be a simple name rather than a path + if (!moduleName.Contains(Path.DirectorySeparatorChar) && !moduleName.Contains(Path.AltDirectorySeparatorChar)) + { + return moduleName; + } + + // Standardize directory separators + moduleName = moduleName.Replace(Path.AltDirectorySeparatorChar, Path.DirectorySeparatorChar); + + // Path.IsRooted() is deprecated -- see https://github.com/dotnet/corefx/issues/22345 + if (!Path.IsPathFullyQualified(moduleName)) + { + moduleName = Path.Join(basePath, moduleName); + } + + // Use the PowerShell filesystem provider to fully resolve the path + return ModuleCmdletBase.GetResolvedPath(moduleName, executionContext).TrimEnd(Path.DirectorySeparatorChar); + } + internal static Version GetManifestModuleVersion(string manifestPath) { try diff --git a/src/System.Management.Automation/engine/Modules/RemoveModuleCommand.cs b/src/System.Management.Automation/engine/Modules/RemoveModuleCommand.cs index bde9976d2e..2437b47064 100644 --- a/src/System.Management.Automation/engine/Modules/RemoveModuleCommand.cs +++ b/src/System.Management.Automation/engine/Modules/RemoveModuleCommand.cs @@ -84,7 +84,7 @@ protected override void ProcessRecord() if (FullyQualifiedName != null) { - foreach (var m in Context.Modules.GetModules(FullyQualifiedName, false)) + foreach (var m in Context.Modules.GetModules(SessionState.Path.CurrentLocation.Path, Context, FullyQualifiedName, false)) { modulesToRemove.Add(m, new List { m }); } diff --git a/src/System.Management.Automation/engine/Utils.cs b/src/System.Management.Automation/engine/Utils.cs index a940f3aaac..585b6527ad 100644 --- a/src/System.Management.Automation/engine/Utils.cs +++ b/src/System.Management.Automation/engine/Utils.cs @@ -828,16 +828,17 @@ internal static List GetModules(string module, ExecutionContext co /// Uses Get-Module -ListAvailable cmdlet. /// /// + /// /// /// /// List of PSModuleInfo's or Null. /// - internal static List GetModules(ModuleSpecification fullyQualifiedName, ExecutionContext context) + internal static List GetModules(ModuleSpecification fullyQualifiedName, string basePath, ExecutionContext context) { // first look in the loaded modules and then append the modules from gmo -Listavailable // Reason: gmo -li looks only the PSModulepath. There may be cases where a module // is imported directly from a path (that is not in PSModulePath). - List result = context.Modules.GetModules(new[] { fullyQualifiedName }, false); + List result = context.Modules.GetModules(basePath, context, new[] { fullyQualifiedName }, false); CommandInfo commandInfo = new CmdletInfo("Get-Module", typeof(GetModuleCommand), null, null, context); var getModuleCommand = new Runspaces.Command(commandInfo); diff --git a/src/System.Management.Automation/help/UpdatableHelpCommandBase.cs b/src/System.Management.Automation/help/UpdatableHelpCommandBase.cs index 237abe89e8..4d86d70b67 100644 --- a/src/System.Management.Automation/help/UpdatableHelpCommandBase.cs +++ b/src/System.Management.Automation/help/UpdatableHelpCommandBase.cs @@ -275,7 +275,7 @@ private Dictionary, UpdatableHelpModuleInfo> GetModuleInf else if (fullyQualifiedName != null) { moduleNamePattern = fullyQualifiedName.Name; - modules = Utils.GetModules(fullyQualifiedName, context); + modules = Utils.GetModules(fullyQualifiedName, SessionState.Path.CurrentLocation.Path, context); } var helpModules = new Dictionary, UpdatableHelpModuleInfo>(); From 8a843cefc112ce2287533fe533ada918d49bc609 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Fri, 9 Nov 2018 12:44:03 -0800 Subject: [PATCH 09/41] [Feature] Add copyright header --- .../Modules/Microsoft.PowerShell.Core/ModuleManifest.Tests.ps1 | 3 +++ 1 file changed, 3 insertions(+) diff --git a/test/powershell/Modules/Microsoft.PowerShell.Core/ModuleManifest.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Core/ModuleManifest.Tests.ps1 index d9ef82d1a8..8278c20559 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Core/ModuleManifest.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Core/ModuleManifest.Tests.ps1 @@ -1,3 +1,6 @@ +# Copyright (c) Microsoft Corporation. All rights reserved. +# Licensed under the MIT License. + function New-ModuleFromLayout { param( From fa950a1acbe8ef6a748527958bf10610e942d2e4 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Fri, 9 Nov 2018 12:46:00 -0800 Subject: [PATCH 10/41] [Feature] Remove unused vars from tests --- .../Microsoft.PowerShell.Core/ModuleManifest.Tests.ps1 | 4 ---- 1 file changed, 4 deletions(-) diff --git a/test/powershell/Modules/Microsoft.PowerShell.Core/ModuleManifest.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Core/ModuleManifest.Tests.ps1 index 8278c20559..dba71225fb 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Core/ModuleManifest.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Core/ModuleManifest.Tests.ps1 @@ -91,7 +91,6 @@ Describe "Manifest required module autoloading with relative path to dir" -Tags $requiredModule = 'reqmod' $mainModPath = Join-Path $TestDrive $mainModule - $reqModPath = Join-Path $TestDrive $requiredModule # Test to ensure that we treat backslashes as path separators on UNIX and vice-versa $altSep = [System.IO.Path]::AltDirectorySeparatorChar @@ -128,7 +127,6 @@ Describe "Manifest required module autoloading with relative path to manifest" - $requiredModule = 'reqmod' $mainModPath = Join-Path $TestDrive $mainModule "$mainModule.psd1" - $reqModPath = Join-Path $TestDrive $requiredModule "$requiredModule.psd1" # Test to ensure that we treat backslashes as path separators on UNIX and vice-versa $altSep = [System.IO.Path]::AltDirectorySeparatorChar @@ -166,7 +164,6 @@ Describe "Manifest required module autoloading with absolute path to dir" -Tags $requiredModule = 'reqmod' $mainModPath = Join-Path $TestDrive $mainModule "$mainModule.psd1" - $reqModPath = Join-Path $TestDrive $requiredModule "$requiredModule.psd1" # Test to ensure that we treat backslashes as path separators on UNIX and vice-versa $altSep = [System.IO.Path]::AltDirectorySeparatorChar @@ -204,7 +201,6 @@ Describe "Manifest required module autoloading with absolute path to manifest" - $requiredModule = 'reqmod' $mainModPath = Join-Path $TestDrive $mainModule "$mainModule.psd1" - $reqModPath = Join-Path $TestDrive $requiredModule "$requiredModule.psd1" # Test to ensure that we treat backslashes as path separators on UNIX and vice-versa $altSep = [System.IO.Path]::AltDirectorySeparatorChar From 2d9cf9cc191265c4939a4fda6c2bd0b98d5e4f53 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Fri, 9 Nov 2018 14:06:56 -0800 Subject: [PATCH 11/41] [Feature] Add more testing for relative paths in fullyqualifiednames --- .../Get-Module.Tests.ps1 | 29 ++++++++++++ .../ModuleConstraint.Tests.ps1 | 27 +++++++++++ .../ModuleManifest.Tests.ps1 | 4 ++ .../Remove-Module.Tests.ps1 | 46 ++++++++++++++++++- 4 files changed, 105 insertions(+), 1 deletion(-) 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 5e79478a48..d3f6bbfddb 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Core/Get-Module.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Core/Get-Module.Tests.ps1 @@ -23,6 +23,13 @@ Describe "Get-Module -ListAvailable" -Tags "CI" { New-Item -ItemType File -Path "$testdrive\Modules\Zoo\Zoo.psm1" > $null New-Item -ItemType File -Path "$testdrive\Modules\Zoo\Too\Zoo.psm1" > $null + $relativePathTestCases = @( + @{ Location = $TestDrive; ModPath = './Modules\Foo'; Name = 'Foo'; Version = '2.0' } + @{ Location = $TestDrive; ModPath = '.\Modules/Foo\1.1/Foo.psd1'; Name = 'Foo'; Version = '1.1' } + @{ Location = "$TestDrive/Bar"; ModPath = './Bar.psd1'; Name = 'Bar'; Version = '0.0.1' } + @{ Location = "$TestDrive/Bar/Download"; ModPath = '..\Bar.psd1'; Name = 'Bar'; Version = '0.0.1' } + ) + $env:PSModulePath = Join-Path $testdrive "Modules" } @@ -147,5 +154,27 @@ Describe "Get-Module -ListAvailable" -Tags "CI" { $modules | Should -HaveCount $ExpectedModule.Count $modules.Name | Sort-Object | Should -BeExactly $ExpectedModule } + + It "Get-Module respects relative paths in module specifications" -TestCases $relativePathTestCases { + param([string]$Location, [string]$ModPath, [string]$Name, [string]$Version) + + $modSpec = @{ + ModuleName = $ModPath + RequiredVersion = $Version + } + + Push-Location $Location + try + { + $modules = Get-Module -ListAvailable -FullyQualifiedName $modSpec + $modules | Should -HaveCount 1 + $modules[0].Name | Should -BeExactly $Name + $modules[0].Version | Should -Be $Version + } + finally + { + Pop-Location + } + } } } diff --git a/test/powershell/Modules/Microsoft.PowerShell.Core/ModuleConstraint.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Core/ModuleConstraint.Tests.ps1 index 42b54d1aa9..b353aacd13 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Core/ModuleConstraint.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Core/ModuleConstraint.Tests.ps1 @@ -740,6 +740,13 @@ Describe "Preloaded module specification checking" -Tags "Feature" { $env:PSModulePath += [System.IO.Path]::PathSeparator + $TestDrive Import-Module $modulePath + + $relativePathCases = @( + @{ Location = $TestDrive; ModPath = (Join-Path "." $moduleName) } + @{ Location = $TestDrive; ModPath = (Join-Path "." $moduleName "$moduleName.psd1") } + @{ Location = (Join-Path $TestDrive $moduleName); ModPath = (Join-Path "." "$moduleName.psd1") } + @{ Location = (Join-Path $TestDrive $moduleName); ModPath = (Join-Path ".." $moduleName) } + ) } AfterAll { @@ -761,6 +768,26 @@ Describe "Preloaded module specification checking" -Tags "Feature" { -RequiredVersion $RequiredVersion } + It "Gets the module when a relative path is used in a module specification" -TestCases $relativePathCases { + param([string]$Location, [string]$ModPath) + + Push-Location $Location + try + { + $modSpec = New-ModuleSpecification -ModuleName $ModPath -ModuleVersion $actualVersion + $mod = Get-Module -FullyQualifiedName $modSpec + Assert-ModuleIsCorrect ` + -Module $mod ` + -Name $moduleName + -Guid $actualGuid + -RequiredVersion $actualVersion + } + finally + { + Pop-Location + } + } + It "Loads the module by FullyQualifiedName from absolute path when ModuleVersion=, MaximumVersion=, RequiredVersion=, Guid=" -TestCases $guidSuccessCases { param($ModuleVersion, $MaximumVersion, $RequiredVersion, $Guid) diff --git a/test/powershell/Modules/Microsoft.PowerShell.Core/ModuleManifest.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Core/ModuleManifest.Tests.ps1 index dba71225fb..a9d111cf0d 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Core/ModuleManifest.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Core/ModuleManifest.Tests.ps1 @@ -1,6 +1,10 @@ # Copyright (c) Microsoft Corporation. All rights reserved. # Licensed under the MIT License. +# Recursively creates a module structure given a hashtable to describe it: +# - psd1 keys have values splatted to New-ModuleManifest +# - psm1 keys have values written to files +# - Other keys with hashtable values are treated as recursive module definitions function New-ModuleFromLayout { param( diff --git a/test/powershell/Modules/Microsoft.PowerShell.Core/Remove-Module.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Core/Remove-Module.Tests.ps1 index 428910ac06..63900f446e 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Core/Remove-Module.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Core/Remove-Module.Tests.ps1 @@ -1,6 +1,6 @@ # Copyright (c) Microsoft Corporation. All rights reserved. # Licensed under the MIT License. -Describe "Remove-Module" -Tags "CI" { +Describe "Remove-Module core module on module path by name" -Tags "CI" { $moduleName = "Microsoft.PowerShell.Security" BeforeEach { @@ -23,3 +23,47 @@ Describe "Remove-Module" -Tags "CI" { Import-Module -Name $moduleName -Force } } + +Describe "Remove-Module custom module with FullyQualifiedName" -Tags "Feature" { + BeforeAll { + $moduleName = 'Banana' + $moduleVersion = '1.0' + + New-Item -Path "$TestDrive/Modules/$moduleName" -ItemType Directory + New-Item -Path "$TestDrive/Modules/$moduleName/Subanana" -ItemType Directory + New-Item -Path "$TestDrive/Monkey" -ItemType Directory + + $modulePath = "$TestDrive/Modules/$moduleName" + $moduleName = 'Banana' + $moduleVersion = '1.0' + New-ModuleManifest -Path "$modulePath/$moduleName.psd1" -Version $moduleVersion + + $relativePathTestCases = @( + @{ Location = $TestDrive; ModPath = ".\Modules\$moduleName" } + @{ Location = "$TestDrive/Modules"; ModPath = ".\$moduleName" } + @{ Location = "$TestDrive/Modules"; ModPath = ".\$moduleName/$moduleName.psd1" } + @{ Location = "$TestDrive/Modules/$moduleName"; ModPath = "./" } + @{ Location = "$TestDrive/Modules/$moduleName"; ModPath = "./$moduleName.psd1" } + @{ Location = "$TestDrive/Modules/$moduleName/Subanana"; ModPath = "../$moduleName.psd1" } + @{ Location = "$TestDrive/Modules/$moduleName/Subanana"; ModPath = "../" } + ) + + BeforeEach { + Get-Module $moduleName | Remove-Module + } + + It "Removes a module with fully qualified name with path " -TestCases $relativePathTestCases { + param([string]$Location, [string]$ModPath) + + $m = Import-Module -Path $modulePath -PassThru + + $m.Name | Should -Be $moduleName + $m.Version | Should -Be $moduleVersion + $m.Path | Should -Be $modulePath + + Remove-Module -FullyQualifiedName @{ ModuleName = $ModPath; RequiredVersion = $moduleVersion } + + Get-Module $moduleName | Should -HaveCount 0 + } + } +} From 55f8a7de5f3f65c4d07eb8c3438ab88967638107 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Fri, 9 Nov 2018 15:26:26 -0800 Subject: [PATCH 12/41] [Feature] Ensure required module path is absolute at autoloading time --- .../engine/Modules/ModuleCmdletBase.cs | 20 ++----------------- .../ModuleManifest.Tests.ps1 | 10 +++++----- 2 files changed, 7 insertions(+), 23 deletions(-) diff --git a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs index a3f12d6565..de6e82b823 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs @@ -1960,24 +1960,8 @@ internal PSModuleInfo LoadModuleManifest( foreach (ModuleSpecification requiredModule in requiredModules) { - // The required module name is essentially raw user input. - // We need to deal with absolute and relative paths here. - string requiredName = requiredModule.Name; - if (requiredName != null - && (requiredName.Contains('/') || requiredName.Contains('\\'))) - { - // Normalize the path to the platform standard - requiredName = requiredName.Replace(Path.AltDirectorySeparatorChar, Path.DirectorySeparatorChar); - - // Root the path if needed - if (!IsRooted(requiredName, relativeRooted: false)) - { - requiredName = Path.Combine(moduleBase, requiredName); - } - - // Use the filesystem provider to resolve the path - requiredModule.Name = ResolveRootedFilePath(requiredName, Context); - } + // To make things easier, make the path absolute and platform-separator-normal here + requiredModule.Name = ModuleIntrinsics.NormalizeModuleName(requiredModule.Name, moduleBase, Context); ErrorRecord error = null; PSModuleInfo module = LoadRequiredModule(fakeManifestInfo, requiredModule, moduleManifestPath, diff --git a/test/powershell/Modules/Microsoft.PowerShell.Core/ModuleManifest.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Core/ModuleManifest.Tests.ps1 index a9d111cf0d..504984dfe3 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Core/ModuleManifest.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Core/ModuleManifest.Tests.ps1 @@ -80,7 +80,7 @@ Describe "Manifest required module autoloading from module path with simple name } It "Importing main module loads required modules successfully" { - $mainMod = Import-Module $mainModule -PassThru + $mainMod = Import-Module $mainModule -PassThru -ErrorAction Stop $reqMod = Get-Module -Name $requiredModule $mainMod.Name | Should -BeExactly $mainModule @@ -116,7 +116,7 @@ Describe "Manifest required module autoloading with relative path to dir" -Tags } It "Importing main module loads required modules successfully" { - $mainMod = Import-Module $mainModPath -PassThru + $mainMod = Import-Module $mainModPath -PassThru -ErrorAction Stop $reqMod = Get-Module -Name $requiredModule $mainMod.Name | Should -BeExactly $mainModule @@ -153,7 +153,7 @@ Describe "Manifest required module autoloading with relative path to manifest" - } It "Importing main module loads required modules successfully" { - $mainMod = Import-Module $mainModPath -PassThru + $mainMod = Import-Module $mainModPath -PassThru -ErrorAction Stop $reqMod = Get-Module -Name $requiredModule $mainMod.Name | Should -BeExactly $mainModule @@ -190,7 +190,7 @@ Describe "Manifest required module autoloading with absolute path to dir" -Tags } It "Importing main module loads required modules successfully" { - $mainMod = Import-Module $mainModPath -PassThru + $mainMod = Import-Module $mainModPath -PassThru -ErrorAction Stop $reqMod = Get-Module -Name $requiredModule $mainMod.Name | Should -BeExactly $mainModule @@ -227,7 +227,7 @@ Describe "Manifest required module autoloading with absolute path to manifest" - } It "Importing main module loads required modules successfully" { - $mainMod = Import-Module $mainModPath -PassThru + $mainMod = Import-Module $mainModPath -PassThru -ErrorAction Stop $reqMod = Get-Module -Name $requiredModule $mainMod.Name | Should -BeExactly $mainModule From d3b9651714dc0d65841b036e8b1aa7fd660375b9 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Fri, 9 Nov 2018 15:49:31 -0800 Subject: [PATCH 13/41] [Feature] Update Get-Module test to account for version ignoring quirk --- .../engine/Modules/ModuleIntrinsics.cs | 5 ++--- .../Get-Module.Tests.ps1 | 17 +++++++++-------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs index 96b9ab90cc..4c1070d40c 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs @@ -627,9 +627,8 @@ internal static bool AreModuleFieldsMatchingConstraints( Version minimumRequiredVersion, Version maximumRequiredVersion) { - // If a name is required, check it matches - // A required module name may also be an absolute path, - // so check the module path as well. + // If a name is required, check that it matches. + // A required module name may also be an absolute path, so check it against the given module's path as well. if ((requiredName != null && !requiredName.Equals(moduleName, StringComparison.OrdinalIgnoreCase)) && !MatchesModulePath(modulePath, requiredName, basePath, context)) { 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 d3f6bbfddb..9cc9ef44b7 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Core/Get-Module.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Core/Get-Module.Tests.ps1 @@ -24,10 +24,11 @@ Describe "Get-Module -ListAvailable" -Tags "CI" { New-Item -ItemType File -Path "$testdrive\Modules\Zoo\Too\Zoo.psm1" > $null $relativePathTestCases = @( - @{ Location = $TestDrive; ModPath = './Modules\Foo'; Name = 'Foo'; Version = '2.0' } - @{ Location = $TestDrive; ModPath = '.\Modules/Foo\1.1/Foo.psd1'; Name = 'Foo'; Version = '1.1' } - @{ Location = "$TestDrive/Bar"; ModPath = './Bar.psd1'; Name = 'Bar'; Version = '0.0.1' } - @{ Location = "$TestDrive/Bar/Download"; ModPath = '..\Bar.psd1'; Name = 'Bar'; Version = '0.0.1' } + # The current behaviour in PowerShell is that version gets ignored when using Get-Module -FullyQualifiedName with a path + @{ Location = $TestDrive; ModPath = './Modules\Foo'; Name = 'Foo'; Version = '2.0'; Count = 2 } + @{ Location = $TestDrive; ModPath = '.\Modules/Foo\1.1/Foo.psd1'; Name = 'Foo'; Version = '1.1'; Count = 1 } + @{ Location = "$TestDrive/Modules/Bar"; ModPath = './Bar.psd1'; Name = 'Bar'; Version = '0.0.1'; Count = 1 } + @{ Location = "$TestDrive/Modules/Bar/Download"; ModPath = '..\Bar.psd1'; Name = 'Bar'; Version = '0.0.1'; Count = 1 } ) $env:PSModulePath = Join-Path $testdrive "Modules" @@ -155,8 +156,8 @@ Describe "Get-Module -ListAvailable" -Tags "CI" { $modules.Name | Sort-Object | Should -BeExactly $ExpectedModule } - It "Get-Module respects relative paths in module specifications" -TestCases $relativePathTestCases { - param([string]$Location, [string]$ModPath, [string]$Name, [string]$Version) + It "Get-Module respects relative paths in module specifications: " -TestCases $relativePathTestCases { + param([string]$Location, [string]$ModPath, [string]$Name, [string]$Version, [int]$Count) $modSpec = @{ ModuleName = $ModPath @@ -167,9 +168,9 @@ Describe "Get-Module -ListAvailable" -Tags "CI" { try { $modules = Get-Module -ListAvailable -FullyQualifiedName $modSpec - $modules | Should -HaveCount 1 + $modules | Should -HaveCount $Count $modules[0].Name | Should -BeExactly $Name - $modules[0].Version | Should -Be $Version + $modules.Version | Should -Contain $Version } finally { From 980b8f052886c673501d4aa0941cc7eb331c5e3c Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Sat, 10 Nov 2018 14:10:37 -0800 Subject: [PATCH 14/41] [Feature] Reduce complexity of fix - normalize module spec path where needed --- .../engine/GetCommandCommand.cs | 6 +- .../engine/Modules/GetModuleCommand.cs | 2 +- .../engine/Modules/ModuleCmdletBase.cs | 15 ++-- .../engine/Modules/ModuleIntrinsics.cs | 72 +++++-------------- .../engine/Modules/ModuleSpecification.cs | 19 +++++ .../engine/Modules/RemoveModuleCommand.cs | 2 +- .../engine/Utils.cs | 5 +- .../help/UpdatableHelpCommandBase.cs | 2 +- 8 files changed, 50 insertions(+), 73 deletions(-) diff --git a/src/System.Management.Automation/engine/GetCommandCommand.cs b/src/System.Management.Automation/engine/GetCommandCommand.cs index 1e618650ad..536fb15d3e 100644 --- a/src/System.Management.Automation/engine/GetCommandCommand.cs +++ b/src/System.Management.Automation/engine/GetCommandCommand.cs @@ -589,7 +589,7 @@ private bool IsNounVerbMatch(CommandInfo command) { if (!_moduleSpecifications.Any( moduleSpecification => - ModuleIntrinsics.IsModuleMatchingModuleSpec(SessionState.Path.CurrentLocation.Path, Context, command.Module, moduleSpecification))) + ModuleIntrinsics.IsModuleMatchingModuleSpec(command.Module, moduleSpecification))) { break; } @@ -1113,7 +1113,7 @@ private bool IsCommandMatch(ref CommandInfo current, out bool isDuplicate) bool foundModuleMatch = false; foreach (var moduleSpecification in _moduleSpecifications) { - if (ModuleIntrinsics.IsModuleMatchingModuleSpec(SessionState.Path.CurrentLocation.Path, Context, current.Module, moduleSpecification)) + if (ModuleIntrinsics.IsModuleMatchingModuleSpec(current.Module, moduleSpecification)) { foundModuleMatch = true; break; @@ -1261,7 +1261,7 @@ private IEnumerable GetMatchingCommandsFromModules(string commandNa { isModuleMatch = SessionStateUtilities.MatchesAnyWildcardPattern(module.Name, _modulePatterns, true); } - else if (_moduleSpecifications.Any(moduleSpecification => ModuleIntrinsics.IsModuleMatchingModuleSpec(SessionState.Path.CurrentLocation.Path, Context, module, moduleSpecification))) + else if (_moduleSpecifications.Any(moduleSpecification => ModuleIntrinsics.IsModuleMatchingModuleSpec(module, moduleSpecification))) { isModuleMatch = true; } diff --git a/src/System.Management.Automation/engine/Modules/GetModuleCommand.cs b/src/System.Management.Automation/engine/Modules/GetModuleCommand.cs index c3cbadecbb..8830f5d654 100644 --- a/src/System.Management.Automation/engine/Modules/GetModuleCommand.cs +++ b/src/System.Management.Automation/engine/Modules/GetModuleCommand.cs @@ -548,7 +548,7 @@ private IEnumerable FilterModulesForSpecificationMatch( } // Modules with table entries only get returned if they match them - if (ModuleIntrinsics.IsModuleMatchingModuleSpec(SessionState.Path.CurrentLocation.Path, Context, module, moduleSpecification)) + if (ModuleIntrinsics.IsModuleMatchingModuleSpec(module, moduleSpecification)) { yield return module; } diff --git a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs index de6e82b823..d0c1d13be2 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs @@ -1656,8 +1656,6 @@ internal PSModuleInfo LoadModuleManifest( if (bailOnFirstError) return null; } else if (!ModuleIntrinsics.AreModuleFieldsMatchingConstraints( - moduleBase, - Context, moduleGuid: manifestGuid, moduleVersion: moduleVersion, requiredGuid: requiredModuleGuid, @@ -1960,11 +1958,12 @@ internal PSModuleInfo LoadModuleManifest( foreach (ModuleSpecification requiredModule in requiredModules) { - // To make things easier, make the path absolute and platform-separator-normal here - requiredModule.Name = ModuleIntrinsics.NormalizeModuleName(requiredModule.Name, moduleBase, Context); + // The required module name is essentially raw user input. + // We must process it so paths work. + ModuleSpecification normalizedRequiredModuleSpec = requiredModule?.CloneWithNormalizedName(Context, moduleBase); ErrorRecord error = null; - PSModuleInfo module = LoadRequiredModule(fakeManifestInfo, requiredModule, moduleManifestPath, + PSModuleInfo module = LoadRequiredModule(fakeManifestInfo, normalizedRequiredModuleSpec, moduleManifestPath, manifestProcessingFlags, containedErrors, out error); if (module == null && error != null) { @@ -3646,7 +3645,7 @@ internal static object IsModuleLoaded(string basePath, ExecutionContext context, foreach (PSModuleInfo module in context.Modules.GetModules(new string[] { "*" }, false)) { // Check that the module meets the module constraints give - if (ModuleIntrinsics.IsModuleMatchingModuleSpec(out matchFailure, basePath, context, module, requiredModule)) + if (ModuleIntrinsics.IsModuleMatchingModuleSpec(out matchFailure, module, requiredModule)) { result = module; loaded = true; @@ -4076,8 +4075,6 @@ internal static Collection GetModuleIfAvailable( foreach (var module in tempResult) { if (ModuleIntrinsics.IsModuleMatchingConstraints( - basePath: null, // basePath is not needed, since we don't need path resolution - context: null, // context is also only required for path resolution module, guid: requiredModule.Guid, requiredVersion: requiredModule.RequiredVersion, @@ -5047,8 +5044,6 @@ private bool ShouldModuleBeRemoved(PSModuleInfo module, string moduleNameInRemov internal bool DoesAlreadyLoadedModuleSatisfyConstraints(PSModuleInfo alreadyLoadedModule) { return ModuleIntrinsics.IsModuleMatchingConstraints( - basePath: null, // basePath is not needed here, since we are only checking the version - context: null, // context is not needed, since we don't need path resolution alreadyLoadedModule, guid: BaseGuid, requiredVersion: BaseRequiredVersion, diff --git a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs index 4c1070d40c..82985edbbc 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs @@ -352,8 +352,6 @@ private List GetModuleCore(string[] patterns, bool all, bool exact } internal List GetModules( - string basePath, - ExecutionContext context, ModuleSpecification[] fullyQualifiedName, bool all) { @@ -361,12 +359,12 @@ internal List GetModules( if (all) { - foreach (var moduleSpec in fullyQualifiedName) + foreach (ModuleSpecification moduleSpec in fullyQualifiedName) { foreach (PSModuleInfo module in ModuleTable.Values) { // See if this is the requested module... - if (IsModuleMatchingModuleSpec(basePath, context, module, moduleSpec)) + if (IsModuleMatchingModuleSpec(module, moduleSpec)) { modulesMatched.Add(module); } @@ -375,7 +373,7 @@ internal List GetModules( } else { - foreach (var moduleSpec in fullyQualifiedName) + foreach (ModuleSpecification moduleSpec in fullyQualifiedName) { // Create a joint list of local and global modules. Only report a module once. // Local modules are reported before global modules... @@ -385,7 +383,7 @@ internal List GetModules( string path = pair.Key; PSModuleInfo module = pair.Value; // See if this is the requested module... - if (IsModuleMatchingModuleSpec(basePath, context, module, moduleSpec)) + if (IsModuleMatchingModuleSpec(module, moduleSpec)) { modulesMatched.Add(module); found[path] = true; @@ -401,7 +399,7 @@ internal List GetModules( { PSModuleInfo module = pair.Value; // See if this is the requested module... - if (IsModuleMatchingModuleSpec(basePath, context, module, moduleSpec)) + if (IsModuleMatchingModuleSpec(module, moduleSpec)) { modulesMatched.Add(module); } @@ -418,21 +416,15 @@ internal List GetModules( /// /// Check if a given module info object matches a given module specification. /// - /// The base path to resolve relative required paths from. - /// The current execution context. /// The module info object to check. /// The module specification to match the module info object against. /// True if the module info object meets all the constraints on the module specification, false otherwise. internal static bool IsModuleMatchingModuleSpec( - string basePath, - ExecutionContext context, PSModuleInfo moduleInfo, ModuleSpecification moduleSpec) { return IsModuleMatchingModuleSpec( out ModuleMatchFailure matchFailureReason, - basePath, - context, moduleInfo, moduleSpec); } @@ -441,15 +433,11 @@ internal static bool IsModuleMatchingModuleSpec( /// Check if a given module info object matches a given module specification. /// /// The constraint that caused the match failure, if any. - /// The base path to resolve relative required paths against. - /// The current execution context. /// The module info object to check. /// The module specification to match the module info object against. /// True if the module info object meets all the constraints on the module specification, false otherwise. internal static bool IsModuleMatchingModuleSpec( out ModuleMatchFailure matchFailureReason, - string basePath, - ExecutionContext context, PSModuleInfo moduleInfo, ModuleSpecification moduleSpec) { @@ -461,8 +449,6 @@ internal static bool IsModuleMatchingModuleSpec( return IsModuleMatchingConstraints( out matchFailureReason, - basePath, - context, moduleInfo, moduleSpec.Name, moduleSpec.Guid, @@ -475,18 +461,14 @@ internal static bool IsModuleMatchingModuleSpec( /// Check if a given module info object matches the given constraints. /// Constraints given as null are ignored. /// - /// The base path to resolve required paths against. - /// The current execution context. /// The module info object to check. - /// The name of the expected module. + /// The name or normalized absolute path of the expected module. /// The guid of the expected module. /// The required version of the expected module. /// The minimum required version of the expected module. /// The maximum required version of the expected module. /// True if the module info object matches all given constraints, false otherwise. internal static bool IsModuleMatchingConstraints( - string basePath, - ExecutionContext context, PSModuleInfo moduleInfo, string name = null, Guid? guid = null, @@ -496,8 +478,6 @@ internal static bool IsModuleMatchingConstraints( { return IsModuleMatchingConstraints( out ModuleMatchFailure matchFailureReason, - basePath, - context, moduleInfo, name, guid, @@ -511,10 +491,8 @@ internal static bool IsModuleMatchingConstraints( /// Constraints given as null are ignored. /// /// The reason for the module constraint match failing. - /// The base path to resolve relative required paths against. - /// The current execution context. /// The module info object to check. - /// The name of the expected module. + /// The name or normalized absolute path of the expected module. /// The guid of the expected module. /// The required version of the expected module. /// The minimum required version of the expected module. @@ -522,8 +500,6 @@ internal static bool IsModuleMatchingConstraints( /// True if the module info object matches all given constraints, false otherwise. internal static bool IsModuleMatchingConstraints( out ModuleMatchFailure matchFailureReason, - string basePath, - ExecutionContext context, PSModuleInfo moduleInfo, string name, Guid? guid, @@ -540,8 +516,6 @@ internal static bool IsModuleMatchingConstraints( return AreModuleFieldsMatchingConstraints( out matchFailureReason, - basePath, - context, moduleInfo.Name, moduleInfo.Path, moduleInfo.Guid, @@ -557,21 +531,17 @@ internal static bool IsModuleMatchingConstraints( /// /// Check that given module fields meet any given constraints. /// - /// The base path to resolve relative required paths against. - /// The current execution context. /// The name of the module to check. /// The path of the module to check. /// The GUID of the module to check. /// The version of the module to check. - /// The name the module must have, if any. + /// The name or normalized absolute path the module must have, if any. /// The GUID the module must have, if any. /// The exact version the module must have, if any. /// The minimum version the module may have, if any. /// The maximum version the module may have, if any. /// True if the module parameters match all given constraints, false otherwise. internal static bool AreModuleFieldsMatchingConstraints( - string basePath, - ExecutionContext context, string moduleName = null, string modulePath = null, Guid? moduleGuid = null, @@ -584,8 +554,6 @@ internal static bool AreModuleFieldsMatchingConstraints( { return AreModuleFieldsMatchingConstraints( out ModuleMatchFailure matchFailureReason, - basePath, - context, moduleName, modulePath, moduleGuid, @@ -601,13 +569,11 @@ internal static bool AreModuleFieldsMatchingConstraints( /// Check that given module fields meet any given constraints. /// /// The reason the match failed, if any. - /// The base path to resolve relative required paths against. - /// The current execution context. /// The name of the module to check. /// The path of the module to check. /// The GUID of the module to check. /// The version of the module to check. - /// The name the module must have, if any. + /// The name or normalized absolute path the module must have, if any. /// The GUID the module must have, if any. /// The exact version the module must have, if any. /// The minimum version the module may have, if any. @@ -615,8 +581,6 @@ internal static bool AreModuleFieldsMatchingConstraints( /// True if the module parameters match all given constraints, false otherwise. internal static bool AreModuleFieldsMatchingConstraints( out ModuleMatchFailure matchFailureReason, - string basePath, - ExecutionContext context, string moduleName, string modulePath, Guid? moduleGuid, @@ -630,7 +594,7 @@ internal static bool AreModuleFieldsMatchingConstraints( // If a name is required, check that it matches. // A required module name may also be an absolute path, so check it against the given module's path as well. if ((requiredName != null && !requiredName.Equals(moduleName, StringComparison.OrdinalIgnoreCase)) - && !MatchesModulePath(modulePath, requiredName, basePath, context)) + && !MatchesModulePath(modulePath, requiredName)) { matchFailureReason = ModuleMatchFailure.Name; return false; @@ -719,8 +683,6 @@ internal static bool IsVersionMatchingConstraints( /// /// The path of the module whose path to check. /// The path of the required module. - /// The base path to resolve relative required module paths against. - /// The current execution context. /// True if the module path matches the required path, false otherwise. /// /// Because a module specification can contain a relative path, we are @@ -728,17 +690,16 @@ internal static bool IsVersionMatchingConstraints( /// A better solution would be to consolidate the module specification /// codepath to force path normalization in the caller. /// - internal static bool MatchesModulePath(string modulePath, string requiredPath, string basePath, ExecutionContext context) + internal static bool MatchesModulePath(string modulePath, string requiredPath) { Dbg.Assert(requiredPath != null, $"Caller to verify that {nameof(requiredPath)} is not null"); + Dbg.Assert(Path.IsPathFullyQualified(requiredPath), $"Caller to verify that {nameof(requiredPath)} is an absolute path."); - // If the module has no path, then it cannot match if (modulePath == null) { return false; } - requiredPath = NormalizeModuleName(requiredPath, basePath, context); #if UNIX StringComparison strcmp = StringComparison.Ordinal; #else @@ -775,7 +736,7 @@ internal static bool MatchesModulePath(string modulePath, string requiredPath, s /// and either returns it as a simple name (if it was a simple name) /// or a fully qualified, PowerShell-resolved path. /// - /// The name of the module from the specification. + /// The name or path of the module from the specification. /// The path to base relative paths off. /// The current execution context. /// @@ -799,8 +760,11 @@ internal static string NormalizeModuleName( return null; } - // The module name may be a simple name rather than a path - if (!moduleName.Contains(Path.DirectorySeparatorChar) && !moduleName.Contains(Path.AltDirectorySeparatorChar)) + // Check whether the module is a path -- if not, it is a simple name and we just return it. + if (!(moduleName.Contains(Path.DirectorySeparatorChar) + || moduleName.Contains(Path.AltDirectorySeparatorChar) + || moduleName.Equals(".") + || moduleName.Equals(".."))) { return moduleName; } diff --git a/src/System.Management.Automation/engine/Modules/ModuleSpecification.cs b/src/System.Management.Automation/engine/Modules/ModuleSpecification.cs index 775b14a555..fe32907803 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleSpecification.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleSpecification.cs @@ -247,6 +247,25 @@ public static bool TryParse(string input, out ModuleSpecification result) return false; } + /// + /// Copy the module specification while normalizing the name + /// so that paths become absolute and use the right directory separators. + /// + /// The current execution context. Used for path normalization. + /// The base path where a relative path should be interpreted with respect to. + /// A fresh module specification object with the name normalized for use internally. + internal ModuleSpecification CloneWithNormalizedName(ExecutionContext context, string basePath) + { + return new ModuleSpecification() + { + Guid = Guid, + MaximumVersion = MaximumVersion, + Version = Version, + RequiredVersion = RequiredVersion, + Name = ModuleIntrinsics.NormalizeModuleName(Name, basePath, context) + }; + } + /// /// The module name. /// diff --git a/src/System.Management.Automation/engine/Modules/RemoveModuleCommand.cs b/src/System.Management.Automation/engine/Modules/RemoveModuleCommand.cs index 2437b47064..bde9976d2e 100644 --- a/src/System.Management.Automation/engine/Modules/RemoveModuleCommand.cs +++ b/src/System.Management.Automation/engine/Modules/RemoveModuleCommand.cs @@ -84,7 +84,7 @@ protected override void ProcessRecord() if (FullyQualifiedName != null) { - foreach (var m in Context.Modules.GetModules(SessionState.Path.CurrentLocation.Path, Context, FullyQualifiedName, false)) + foreach (var m in Context.Modules.GetModules(FullyQualifiedName, false)) { modulesToRemove.Add(m, new List { m }); } diff --git a/src/System.Management.Automation/engine/Utils.cs b/src/System.Management.Automation/engine/Utils.cs index 585b6527ad..a940f3aaac 100644 --- a/src/System.Management.Automation/engine/Utils.cs +++ b/src/System.Management.Automation/engine/Utils.cs @@ -828,17 +828,16 @@ internal static List GetModules(string module, ExecutionContext co /// Uses Get-Module -ListAvailable cmdlet. /// /// - /// /// /// /// List of PSModuleInfo's or Null. /// - internal static List GetModules(ModuleSpecification fullyQualifiedName, string basePath, ExecutionContext context) + internal static List GetModules(ModuleSpecification fullyQualifiedName, ExecutionContext context) { // first look in the loaded modules and then append the modules from gmo -Listavailable // Reason: gmo -li looks only the PSModulepath. There may be cases where a module // is imported directly from a path (that is not in PSModulePath). - List result = context.Modules.GetModules(basePath, context, new[] { fullyQualifiedName }, false); + List result = context.Modules.GetModules(new[] { fullyQualifiedName }, false); CommandInfo commandInfo = new CmdletInfo("Get-Module", typeof(GetModuleCommand), null, null, context); var getModuleCommand = new Runspaces.Command(commandInfo); diff --git a/src/System.Management.Automation/help/UpdatableHelpCommandBase.cs b/src/System.Management.Automation/help/UpdatableHelpCommandBase.cs index 4d86d70b67..237abe89e8 100644 --- a/src/System.Management.Automation/help/UpdatableHelpCommandBase.cs +++ b/src/System.Management.Automation/help/UpdatableHelpCommandBase.cs @@ -275,7 +275,7 @@ private Dictionary, UpdatableHelpModuleInfo> GetModuleInf else if (fullyQualifiedName != null) { moduleNamePattern = fullyQualifiedName.Name; - modules = Utils.GetModules(fullyQualifiedName, SessionState.Path.CurrentLocation.Path, context); + modules = Utils.GetModules(fullyQualifiedName, context); } var helpModules = new Dictionary, UpdatableHelpModuleInfo>(); From 0a414fff276ca09dd203696e92bef7e4ce0c2f20 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Sat, 10 Nov 2018 14:14:33 -0800 Subject: [PATCH 15/41] [Feature] Revert further changes --- .../engine/Modules/GetModuleCommand.cs | 2 +- .../engine/Modules/ModuleCmdletBase.cs | 29 ++++++------------- 2 files changed, 10 insertions(+), 21 deletions(-) diff --git a/src/System.Management.Automation/engine/Modules/GetModuleCommand.cs b/src/System.Management.Automation/engine/Modules/GetModuleCommand.cs index 8830f5d654..c6745a4a44 100644 --- a/src/System.Management.Automation/engine/Modules/GetModuleCommand.cs +++ b/src/System.Management.Automation/engine/Modules/GetModuleCommand.cs @@ -531,7 +531,7 @@ private IEnumerable FilterModulesForEditionAndSpecification( /// The modules to filter by specification match. /// The specification lookup table to filter the modules on. /// The modules that match their corresponding table entry, or which have no table entry. - private IEnumerable FilterModulesForSpecificationMatch( + private static IEnumerable FilterModulesForSpecificationMatch( IEnumerable modules, IDictionary moduleSpecificationTable) { diff --git a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs index d0c1d13be2..f5b27cb5ba 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs @@ -3626,13 +3626,12 @@ private static ErrorRecord GenerateInvalidModuleMemberErrorRecord(string manifes /// If loadElements is false, we check requireModule for correctness but do /// not check if the modules are loaded. /// - /// Path to resolve relative module paths in the specification against. /// Execution Context. /// Either a string or a hash of ModuleName, optional Guid, and ModuleVersion. /// The reason the module failed to load, or null on success. /// Sets if the module/snapin is already present. /// null if the module is not loaded or loadElements is false, the loaded module otherwise. - internal static object IsModuleLoaded(string basePath, ExecutionContext context, ModuleSpecification requiredModule, out ModuleMatchFailure matchFailureReason, out bool loaded) + internal static object IsModuleLoaded(ExecutionContext context, ModuleSpecification requiredModule, out ModuleMatchFailure matchFailureReason, out bool loaded) { loaded = false; Dbg.Assert(requiredModule != null, "Caller should verify requiredModuleSpecification != null"); @@ -3719,7 +3718,7 @@ internal static PSModuleInfo LoadRequiredModule(ExecutionContext context, ModuleMatchFailure loadFailureReason = ModuleMatchFailure.None; bool loaded = false; - object loadedModule = IsModuleLoaded(currentModule.ModuleBase, context, requiredModuleSpecification, out loadFailureReason, out loaded); + object loadedModule = IsModuleLoaded(context, requiredModuleSpecification, out loadFailureReason, out loaded); if (loadedModule == null) { @@ -3751,7 +3750,7 @@ internal static PSModuleInfo LoadRequiredModule(ExecutionContext context, out error); if (!hasRequiredModulesCyclicReference) { - result = ImportRequiredModule(currentModule.ModuleBase, context, requiredModuleSpecification, out error); + result = ImportRequiredModule(context, requiredModuleSpecification, out error); } else { @@ -3876,7 +3875,6 @@ internal static PSModuleInfo LoadRequiredModule(ExecutionContext context, } private static PSModuleInfo ImportRequiredModule( - string basePath, ExecutionContext context, ModuleSpecification requiredModule, out ErrorRecord error) @@ -3937,7 +3935,7 @@ private static PSModuleInfo ImportRequiredModule( ModuleMatchFailure loadFailureReason; bool loaded = false; - object r = IsModuleLoaded(basePath, context, ms, out loadFailureReason, out loaded); + object r = IsModuleLoaded(context, ms, out loadFailureReason, out loaded); Dbg.Assert(r is PSModuleInfo, "The returned value should be PSModuleInfo"); @@ -4562,26 +4560,17 @@ internal string GetAbsolutePath(string moduleBase, string path) /// Check if a path is rooted or "relative rooted". /// /// The file path to check. - /// If true, paths like "./here" and "..\there" count as rooted ("relative rooted"). /// True if the path is rooted, false otherwise. - internal static bool IsRooted(string filePath, bool relativeRooted = true) + internal static bool IsRooted(string filePath) { - if (Path.IsPathRooted(filePath) || filePath.IndexOf(":", StringComparison.Ordinal) >= 0) - { - return true; - } - - if (!relativeRooted) - { - return false; - } - - return filePath.StartsWith(@".\", StringComparison.Ordinal) || + return Path.IsPathRooted(filePath) || + filePath.StartsWith(@".\", StringComparison.Ordinal) || filePath.StartsWith(@"./", StringComparison.Ordinal) || filePath.StartsWith(@"..\", StringComparison.Ordinal) || filePath.StartsWith(@"../", StringComparison.Ordinal) || filePath.StartsWith(@"~/", StringComparison.Ordinal) || - filePath.StartsWith(@"~\", StringComparison.Ordinal); + filePath.StartsWith(@"~\", StringComparison.Ordinal) || + filePath.IndexOf(":", StringComparison.Ordinal) >= 0; } /// From 9163bc0239387324ddede98557abbcc67025ab04 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Sat, 10 Nov 2018 14:31:33 -0800 Subject: [PATCH 16/41] [Feature] Add path normalization method to ModuleSpecification --- .../engine/Modules/GetModuleCommand.cs | 2 ++ .../engine/Modules/ModuleIntrinsics.cs | 18 ++++++++++++++---- .../engine/Modules/ModuleSpecification.cs | 8 +++++++- .../engine/Modules/RemoveModuleCommand.cs | 2 ++ 4 files changed, 25 insertions(+), 5 deletions(-) diff --git a/src/System.Management.Automation/engine/Modules/GetModuleCommand.cs b/src/System.Management.Automation/engine/Modules/GetModuleCommand.cs index c6745a4a44..43930682c8 100644 --- a/src/System.Management.Automation/engine/Modules/GetModuleCommand.cs +++ b/src/System.Management.Automation/engine/Modules/GetModuleCommand.cs @@ -374,6 +374,8 @@ protected override void ProcessRecord() var moduleSpecTable = new Dictionary(StringComparer.OrdinalIgnoreCase); if (FullyQualifiedName != null) { + // We need to normalize paths from user input. Relative paths are interpreted as relative to PWD. + FullyQualifiedName = FullyQualifiedName.Select(ModuleSpecification => ModuleSpecification?.WithNormalizedName(Context, SessionState.Path.CurrentLocation.Path)).ToArray(); moduleSpecTable = FullyQualifiedName.ToDictionary(moduleSpecification => moduleSpecification.Name, StringComparer.OrdinalIgnoreCase); strNames.AddRange(FullyQualifiedName.Select(spec => spec.Name)); } diff --git a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs index 82985edbbc..012df261c8 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs @@ -761,10 +761,7 @@ internal static string NormalizeModuleName( } // Check whether the module is a path -- if not, it is a simple name and we just return it. - if (!(moduleName.Contains(Path.DirectorySeparatorChar) - || moduleName.Contains(Path.AltDirectorySeparatorChar) - || moduleName.Equals(".") - || moduleName.Equals(".."))) + if (!IsModuleNamePath(moduleName)) { return moduleName; } @@ -782,6 +779,19 @@ internal static string NormalizeModuleName( return ModuleCmdletBase.GetResolvedPath(moduleName, executionContext).TrimEnd(Path.DirectorySeparatorChar); } + /// + /// Check if a given module name is a path to a module rather than a simple name. + /// + /// The module name to check. + /// True if the module name is a path, false otherwise. + internal static bool IsModuleNamePath(string moduleName) + { + return moduleName.Contains(Path.DirectorySeparatorChar) + || moduleName.Contains(Path.AltDirectorySeparatorChar) + || moduleName.Equals("..") + || moduleName.Equals("."); + } + internal static Version GetManifestModuleVersion(string manifestPath) { try diff --git a/src/System.Management.Automation/engine/Modules/ModuleSpecification.cs b/src/System.Management.Automation/engine/Modules/ModuleSpecification.cs index fe32907803..872385d845 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleSpecification.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleSpecification.cs @@ -254,8 +254,14 @@ public static bool TryParse(string input, out ModuleSpecification result) /// The current execution context. Used for path normalization. /// The base path where a relative path should be interpreted with respect to. /// A fresh module specification object with the name normalized for use internally. - internal ModuleSpecification CloneWithNormalizedName(ExecutionContext context, string basePath) + internal ModuleSpecification WithNormalizedName(ExecutionContext context, string basePath) { + // Save allocating a new module spec if we don't need to change anything + if (!ModuleIntrinsics.IsModuleNamePath(Name)) + { + return this; + } + return new ModuleSpecification() { Guid = Guid, diff --git a/src/System.Management.Automation/engine/Modules/RemoveModuleCommand.cs b/src/System.Management.Automation/engine/Modules/RemoveModuleCommand.cs index bde9976d2e..11ea78b68d 100644 --- a/src/System.Management.Automation/engine/Modules/RemoveModuleCommand.cs +++ b/src/System.Management.Automation/engine/Modules/RemoveModuleCommand.cs @@ -84,6 +84,8 @@ protected override void ProcessRecord() if (FullyQualifiedName != null) { + // Normalize the names of fully qualified module specifications so paths are absolute + FullyQualifiedName = FullyQualifiedName.Select(ms => ms.WithNormalizedName(Context, SessionState.Path.CurrentLocation.Path)).ToArray(); foreach (var m in Context.Modules.GetModules(FullyQualifiedName, false)) { modulesToRemove.Add(m, new List { m }); From 59cf3d1a1297dadc5de7b3c12a0943423a041a9b Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Sat, 10 Nov 2018 14:52:12 -0800 Subject: [PATCH 17/41] [Feature] Fix module specification method call --- .../engine/Modules/ModuleCmdletBase.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs index f5b27cb5ba..0d05187c1a 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs @@ -1960,7 +1960,7 @@ internal PSModuleInfo LoadModuleManifest( { // The required module name is essentially raw user input. // We must process it so paths work. - ModuleSpecification normalizedRequiredModuleSpec = requiredModule?.CloneWithNormalizedName(Context, moduleBase); + ModuleSpecification normalizedRequiredModuleSpec = requiredModule?.WithNormalizedName(Context, moduleBase); ErrorRecord error = null; PSModuleInfo module = LoadRequiredModule(fakeManifestInfo, normalizedRequiredModuleSpec, moduleManifestPath, From 5b4bd9293c9496fce89fb8eeb10a4324f53ac857 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Sat, 10 Nov 2018 15:06:40 -0800 Subject: [PATCH 18/41] [Feature] Revert unneeded changes - change to TODOs. --- .../engine/Modules/GetModuleCommand.cs | 5 +++-- .../engine/Modules/RemoveModuleCommand.cs | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/System.Management.Automation/engine/Modules/GetModuleCommand.cs b/src/System.Management.Automation/engine/Modules/GetModuleCommand.cs index 43930682c8..b69af5c340 100644 --- a/src/System.Management.Automation/engine/Modules/GetModuleCommand.cs +++ b/src/System.Management.Automation/engine/Modules/GetModuleCommand.cs @@ -374,8 +374,9 @@ protected override void ProcessRecord() var moduleSpecTable = new Dictionary(StringComparer.OrdinalIgnoreCase); if (FullyQualifiedName != null) { - // We need to normalize paths from user input. Relative paths are interpreted as relative to PWD. - FullyQualifiedName = FullyQualifiedName.Select(ModuleSpecification => ModuleSpecification?.WithNormalizedName(Context, SessionState.Path.CurrentLocation.Path)).ToArray(); + // TODO: Paths here will not match a module name. + // The table match logic will mean that a module with the right + // name but wrong path or version will still get returned. moduleSpecTable = FullyQualifiedName.ToDictionary(moduleSpecification => moduleSpecification.Name, StringComparer.OrdinalIgnoreCase); strNames.AddRange(FullyQualifiedName.Select(spec => spec.Name)); } diff --git a/src/System.Management.Automation/engine/Modules/RemoveModuleCommand.cs b/src/System.Management.Automation/engine/Modules/RemoveModuleCommand.cs index 11ea78b68d..393cb9a1be 100644 --- a/src/System.Management.Automation/engine/Modules/RemoveModuleCommand.cs +++ b/src/System.Management.Automation/engine/Modules/RemoveModuleCommand.cs @@ -84,8 +84,9 @@ protected override void ProcessRecord() if (FullyQualifiedName != null) { - // Normalize the names of fully qualified module specifications so paths are absolute - FullyQualifiedName = FullyQualifiedName.Select(ms => ms.WithNormalizedName(Context, SessionState.Path.CurrentLocation.Path)).ToArray(); + // TODO: Paths in the module name may fail here because + // they the wrong directory separator or are relative. + // Fix with fqn.WithNormalizedPath(Context, SessionState.Path.CurrentLocation.Path). foreach (var m in Context.Modules.GetModules(FullyQualifiedName, false)) { modulesToRemove.Add(m, new List { m }); From 29156061ba82b3b8acd593f799ec71b057ba27bd Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Sat, 10 Nov 2018 15:15:35 -0800 Subject: [PATCH 19/41] [Feature] Change tests to absolute path for gmo -List --- .../Microsoft.PowerShell.Core/Get-Module.Tests.ps1 | 14 +++++++------- .../ModuleConstraint.Tests.ps1 | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) 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 9cc9ef44b7..830f3eff74 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Core/Get-Module.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Core/Get-Module.Tests.ps1 @@ -23,12 +23,12 @@ Describe "Get-Module -ListAvailable" -Tags "CI" { New-Item -ItemType File -Path "$testdrive\Modules\Zoo\Zoo.psm1" > $null New-Item -ItemType File -Path "$testdrive\Modules\Zoo\Too\Zoo.psm1" > $null - $relativePathTestCases = @( + $fullyQualifiedPathTestCases = @( # The current behaviour in PowerShell is that version gets ignored when using Get-Module -FullyQualifiedName with a path - @{ Location = $TestDrive; ModPath = './Modules\Foo'; Name = 'Foo'; Version = '2.0'; Count = 2 } - @{ Location = $TestDrive; ModPath = '.\Modules/Foo\1.1/Foo.psd1'; Name = 'Foo'; Version = '1.1'; Count = 1 } - @{ Location = "$TestDrive/Modules/Bar"; ModPath = './Bar.psd1'; Name = 'Bar'; Version = '0.0.1'; Count = 1 } - @{ Location = "$TestDrive/Modules/Bar/Download"; ModPath = '..\Bar.psd1'; Name = 'Bar'; Version = '0.0.1'; Count = 1 } + @{ ModPath = "$TestDrive/Modules\Foo"; Name = 'Foo'; Version = '2.0'; Count = 2 } + @{ ModPath = "$TestDrive\Modules/Foo\1.1/Foo.psd1"; Name = 'Foo'; Version = '1.1'; Count = 1 } + @{ ModPath = "$TestDrive\Modules/Bar.psd1"; Name = 'Bar'; Version = '0.0.1'; Count = 1 } + @{ ModPath = "$TestDrive\Modules\Zoo\Too\Zoo.psm1"; Name = 'Zoo'; Version = '0.0.1'; Count = 1 } ) $env:PSModulePath = Join-Path $testdrive "Modules" @@ -156,8 +156,8 @@ Describe "Get-Module -ListAvailable" -Tags "CI" { $modules.Name | Sort-Object | Should -BeExactly $ExpectedModule } - It "Get-Module respects relative paths in module specifications: " -TestCases $relativePathTestCases { - param([string]$Location, [string]$ModPath, [string]$Name, [string]$Version, [int]$Count) + It "Get-Module respects absolute paths in module specifications: " -TestCases $relativePathTestCases { + param([string]$ModPath, [string]$Name, [string]$Version, [int]$Count) $modSpec = @{ ModuleName = $ModPath diff --git a/test/powershell/Modules/Microsoft.PowerShell.Core/ModuleConstraint.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Core/ModuleConstraint.Tests.ps1 index b353aacd13..6a16731796 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Core/ModuleConstraint.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Core/ModuleConstraint.Tests.ps1 @@ -768,7 +768,7 @@ Describe "Preloaded module specification checking" -Tags "Feature" { -RequiredVersion $RequiredVersion } - It "Gets the module when a relative path is used in a module specification" -TestCases $relativePathCases { + It "Gets the module when a relative path is used in a module specification: " -TestCases $relativePathCases -Pending { param([string]$Location, [string]$ModPath) Push-Location $Location From be9c5a4dc143310b32888e3178e289451d904e27 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Sat, 10 Nov 2018 17:29:21 -0800 Subject: [PATCH 20/41] [Feature] Add requires -Module tests --- .../Language/Scripting/Requires.Tests.ps1 | 120 +++++++++++++++++- 1 file changed, 113 insertions(+), 7 deletions(-) diff --git a/test/powershell/Language/Scripting/Requires.Tests.ps1 b/test/powershell/Language/Scripting/Requires.Tests.ps1 index 70586a578c..3fdb1b7257 100644 --- a/test/powershell/Language/Scripting/Requires.Tests.ps1 +++ b/test/powershell/Language/Scripting/Requires.Tests.ps1 @@ -4,13 +4,13 @@ Describe "Requires tests" -Tags "CI" { Context "Parser error" { $testcases = @( - @{command = "#requiresappID`r`n$foo = 1; $foo" ; testname = "appId with newline"} - @{command = "#requires -version A `r`n$foo = 1; $foo" ; testname = "version as character"} - @{command = "#requires -version 2b `r`n$foo = 1; $foo" ; testname = "alphanumeric version"} - @{command = "#requires -version 1. `r`n$foo = 1; $foo" ; testname = "version with dot"} - @{command = "#requires -version '' `r`n$foo = 1; $foo" ; testname = "empty version"} - @{command = "#requires -version 1.0. `r`n$foo = 1; $foo" ; testname = "version with two dots"} - @{command = "#requires -version 1.A `r`n$foo = 1; $foo" ; testname = "alphanumeric version with dots"} + @{command = "#requiresappID`r`n`$foo = 1; `$foo" ; testname = "appId with newline"} + @{command = "#requires -version A `r`n`$foo = 1; `$foo" ; testname = "version as character"} + @{command = "#requires -version 2b `r`n`$foo = 1; `$foo" ; testname = "alphanumeric version"} + @{command = "#requires -version 1. `r`n`$foo = 1; `$foo" ; testname = "version with dot"} + @{command = "#requires -version '' `r`n`$foo = 1; `$foo" ; testname = "empty version"} + @{command = "#requires -version 1.0. `r`n`$foo = 1; `$foo" ; testname = "version with two dots"} + @{command = "#requires -version 1.A `r`n`$foo = 1; `$foo" ; testname = "alphanumeric version with dots"} ) It "throws ParserException - " -TestCases $testcases { @@ -37,3 +37,109 @@ Describe "Requires tests" -Tags "CI" { } } } + +Describe "#requires -Modules" { + BeforeAll { + $success = 'SUCCESS' + + $moduleName = 'Banana' + $moduleVersion = '0.12.1' + $moduleDirPath = Join-Path $TestDrive 'modules' + $modulePath = Join-Path $moduleDirPath $moduleName + $manifestPath = Join-Path $modulePath "$moduleName.psd1" + $psm1Path = Join-Path $modulePath "$moduleName.psm1" + New-Item -Path $psm1Path -Value "function Test-RequiredModule { '$success' }" + New-ModuleManifest -Path $manifestPath -ModuleVersion $moduleVersion + } + + Context "Requiring non-existent modules" { + BeforeAll { + $badName = 'ModuleThatDoesNotExist' + $badPath = Join-Path $TestDrive 'ModuleThatDoesNotExist' + $version = '1.0' + $testCases = @( + @{ ModuleRequirement = "'$badName'"; Scenario = 'name' } + @{ ModuleRequirement = "'$badPath'"; Scenario = 'path' } + @{ ModuleRequirement = "@{ ModuleName = '$badName'; ModuleVersion = '$version' }"; Scenario = 'fully qualified name with name' } + @{ ModuleRequirement = "@{ ModuleName = '$badPath'; ModuleVersion = '$version' }"; Scenario = 'fully qualified name with path' } + ) + } + + It "Fails parsing a script that requires module by " -TestCases $testCases { + param([string]$ModuleRequirement, [string]$Scenario) + + $script = "#requires -Module $ModuleRequirement`n`nWrite-Output 'failed'" + { & $script } | Should -Throw -ErrorId 'ParseException' + } + } + + Context "Already loaded module" { + BeforeAll { + Import-Module $moduleName + $testCases = @( + @{ ModuleRequirement = "'$moduleName'"; Scenario = 'name' } + @{ ModuleRequirement = "'$modulePath'"; Scenario = 'path' } + @{ ModuleRequirement = "'$manifestPath'"; Scenario = 'manifest path' } + @{ ModuleRequirement = "@{ ModuleName='$moduleName'; ModuleVersion='$moduleVersion' }"; Scenario = 'fully qualified name with name' } + @{ ModuleRequirement = "@{ ModuleName='$modulePath'; ModuleVersion='$moduleVersion' }"; Scenario = 'fully qualified name with path' } + @{ ModuleRequirement = "@{ ModuleName='$manifestPath'; ModuleVersion='$moduleVersion' }"; Scenario = 'fully qualified name with manifest path' } + ) + } + + AfterAll { + Remove-Module $moduleName -ErrorAction SilentlyContinue + } + + It "Successfully runs a script requiring a loaded module by " -TestCases $testCases { + param([string]$ModuleRequirement, [string]$Scenario) + + $script = "#requires -Modules $ModuleRequirement`n`nTest-RequiredModule" + & $script | Should -BeExactly $success + } + } + + Context "Loading by name" { + BeforeAll { + $oldModulePath = $env:PSModulePath + $env:PSModulePath = $moduleDirPath + [System.IO.Path]::PathSeparator + $env:PSModulePath + + $testCases = @( + @{ ModuleRequirement = "'$moduleName'"; Scenario = 'name' } + @{ ModuleRequirement = "'$modulePath'"; Scenario = 'path' } + @{ ModuleRequirement = "'$manifestPath'"; Scenario = 'manifest path' } + @{ ModuleRequirement = "@{ ModuleName='$moduleName'; ModuleVersion='$moduleVersion' }"; Scenario = 'fully qualified name with name' } + @{ ModuleRequirement = "@{ ModuleName='$modulePath'; ModuleVersion='$moduleVersion' }"; Scenario = 'fully qualified name with path' } + @{ ModuleRequirement = "@{ ModuleName='$manifestPath'; ModuleVersion='$moduleVersion' }"; Scenario = 'fully qualified name with manifest path' } + ) + } + + AfterAll { + $env:PSModulePath = $oldModulePath + } + + It "Successfully runs a script requiring a module on the module path by " -TestCases $testCases { + param([string]$ModuleRequirement, [string]$Scenario) + + $script = "#requires -Modules $ModuleRequirement`n`nTest-RequiredModule" + & $script | Should -BeExactly $success + } + } + + Context "Loading by absolute path" { + BeforeAll { + $testCases = @( + @{ ModuleRequirement = "'$modulePath'"; Scenario = 'path' } + @{ ModuleRequirement = "'$manifestPath'"; Scenario = 'manifest path' } + @{ ModuleRequirement = "@{ ModuleName='$modulePath'; ModuleVersion='$moduleVersion' }"; Scenario = 'fully qualified name with path' } + @{ ModuleRequirement = "@{ ModuleName='$manifestPath'; ModuleVersion='$moduleVersion' }"; Scenario = 'fully qualified name with manifest path' } + ) + } + + It "Successfully runs a script requiring a module by absolute path by " -TestCases $testCases { + param([string]$ModuleRequirement, [string]$Scenario) + + $script = "#requires -Modules $ModuleRequirement`n`nTest-RequiredModule" + & $script | Should -BeExactly $success + } + } +} From b775900c8257dea677ff72ac16e45316380cb54e Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Sat, 10 Nov 2018 17:31:50 -0800 Subject: [PATCH 21/41] [Feature] Fix remove-module and get-module tests --- .../Get-Module.Tests.ps1 | 36 ++++++++----------- .../Remove-Module.Tests.ps1 | 2 +- 2 files changed, 15 insertions(+), 23 deletions(-) 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 830f3eff74..05b8e4d955 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Core/Get-Module.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Core/Get-Module.Tests.ps1 @@ -130,6 +130,20 @@ Describe "Get-Module -ListAvailable" -Tags "CI" { $modules.ExportedFunctions.Count | Should -Be 1 -Because 'We added a new function to export' } + It "Get-Module respects absolute paths in module specifications: " -TestCases $fullyQualifiedPathTestCases { + param([string]$ModPath, [string]$Name, [string]$Version, [int]$Count) + + $modSpec = @{ + ModuleName = $ModPath + RequiredVersion = $Version + } + + $modules = Get-Module -ListAvailable -FullyQualifiedName $modSpec + $modules | Should -HaveCount $Count + $modules[0].Name | Should -BeExactly $Name + $modules.Version | Should -Contain $Version + } + Context "PSEdition" { BeforeAll { @@ -155,27 +169,5 @@ Describe "Get-Module -ListAvailable" -Tags "CI" { $modules | Should -HaveCount $ExpectedModule.Count $modules.Name | Sort-Object | Should -BeExactly $ExpectedModule } - - It "Get-Module respects absolute paths in module specifications: " -TestCases $relativePathTestCases { - param([string]$ModPath, [string]$Name, [string]$Version, [int]$Count) - - $modSpec = @{ - ModuleName = $ModPath - RequiredVersion = $Version - } - - Push-Location $Location - try - { - $modules = Get-Module -ListAvailable -FullyQualifiedName $modSpec - $modules | Should -HaveCount $Count - $modules[0].Name | Should -BeExactly $Name - $modules.Version | Should -Contain $Version - } - finally - { - Pop-Location - } - } } } diff --git a/test/powershell/Modules/Microsoft.PowerShell.Core/Remove-Module.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Core/Remove-Module.Tests.ps1 index 63900f446e..3cd7ed07ca 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Core/Remove-Module.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Core/Remove-Module.Tests.ps1 @@ -36,7 +36,7 @@ Describe "Remove-Module custom module with FullyQualifiedName" -Tags "Feature" { $modulePath = "$TestDrive/Modules/$moduleName" $moduleName = 'Banana' $moduleVersion = '1.0' - New-ModuleManifest -Path "$modulePath/$moduleName.psd1" -Version $moduleVersion + New-ModuleManifest -Path "$modulePath/$moduleName.psd1" -ModuleVersion $moduleVersion $relativePathTestCases = @( @{ Location = $TestDrive; ModPath = ".\Modules\$moduleName" } From d786a68039e8aaaa88c16276c4149616bd2282b2 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Sat, 10 Nov 2018 17:41:39 -0800 Subject: [PATCH 22/41] [Feature] Fix requires tests --- test/powershell/Language/Scripting/Requires.Tests.ps1 | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/test/powershell/Language/Scripting/Requires.Tests.ps1 b/test/powershell/Language/Scripting/Requires.Tests.ps1 index 3fdb1b7257..d9a4c0126f 100644 --- a/test/powershell/Language/Scripting/Requires.Tests.ps1 +++ b/test/powershell/Language/Scripting/Requires.Tests.ps1 @@ -38,15 +38,18 @@ Describe "Requires tests" -Tags "CI" { } } -Describe "#requires -Modules" { +Describe "#requires -Modules" -Tags "CI" { BeforeAll { $success = 'SUCCESS' + $sep = [System.IO.Path]::DirectorySeparatorChar + $altSep = [System.IO.Path]::AltDirectorySeparatorChar + $moduleName = 'Banana' $moduleVersion = '0.12.1' $moduleDirPath = Join-Path $TestDrive 'modules' - $modulePath = Join-Path $moduleDirPath $moduleName - $manifestPath = Join-Path $modulePath "$moduleName.psd1" + $modulePath = "$moduleDirPath${sep}$moduleName" + $manifestPath = "$modulePath${altSep}$moduleName.psd1" $psm1Path = Join-Path $modulePath "$moduleName.psm1" New-Item -Path $psm1Path -Value "function Test-RequiredModule { '$success' }" New-ModuleManifest -Path $manifestPath -ModuleVersion $moduleVersion From 2540605160990bd41888a3f792f287fc93379d46 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Sat, 10 Nov 2018 19:01:31 -0800 Subject: [PATCH 23/41] [Feature] Fix tests, make Remove-Module not work with paths --- .../engine/Modules/ModuleIntrinsics.cs | 9 +------ .../engine/Modules/RemoveModuleCommand.cs | 8 +++--- .../Get-Module.Tests.ps1 | 4 +-- .../Remove-Module.Tests.ps1 | 26 ++++++++----------- 4 files changed, 19 insertions(+), 28 deletions(-) diff --git a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs index 012df261c8..30ae273321 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs @@ -682,18 +682,11 @@ internal static bool IsVersionMatchingConstraints( /// a required path. /// /// The path of the module whose path to check. - /// The path of the required module. + /// The path of the required module. Only normalized absolute paths will work for this. /// True if the module path matches the required path, false otherwise. - /// - /// Because a module specification can contain a relative path, we are - /// forced to pass a base path and context down to this point. - /// A better solution would be to consolidate the module specification - /// codepath to force path normalization in the caller. - /// internal static bool MatchesModulePath(string modulePath, string requiredPath) { Dbg.Assert(requiredPath != null, $"Caller to verify that {nameof(requiredPath)} is not null"); - Dbg.Assert(Path.IsPathFullyQualified(requiredPath), $"Caller to verify that {nameof(requiredPath)} is an absolute path."); if (modulePath == null) { diff --git a/src/System.Management.Automation/engine/Modules/RemoveModuleCommand.cs b/src/System.Management.Automation/engine/Modules/RemoveModuleCommand.cs index 393cb9a1be..6f4fa1992c 100644 --- a/src/System.Management.Automation/engine/Modules/RemoveModuleCommand.cs +++ b/src/System.Management.Automation/engine/Modules/RemoveModuleCommand.cs @@ -84,9 +84,11 @@ protected override void ProcessRecord() if (FullyQualifiedName != null) { - // TODO: Paths in the module name may fail here because - // they the wrong directory separator or are relative. - // Fix with fqn.WithNormalizedPath(Context, SessionState.Path.CurrentLocation.Path). + // TODO: + // Paths in the module name may fail here because + // they the wrong directory separator or are relative. + // Fix with the code below: + // FullyQualifiedName = FullyQualifiedName.Select(ms => ms.WithNormalizedName(Context, SessionState.Path.CurrentLocation.Path)).ToArray(); foreach (var m in Context.Modules.GetModules(FullyQualifiedName, false)) { modulesToRemove.Add(m, new List { m }); 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 05b8e4d955..17178faf6d 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Core/Get-Module.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Core/Get-Module.Tests.ps1 @@ -27,8 +27,8 @@ Describe "Get-Module -ListAvailable" -Tags "CI" { # The current behaviour in PowerShell is that version gets ignored when using Get-Module -FullyQualifiedName with a path @{ ModPath = "$TestDrive/Modules\Foo"; Name = 'Foo'; Version = '2.0'; Count = 2 } @{ ModPath = "$TestDrive\Modules/Foo\1.1/Foo.psd1"; Name = 'Foo'; Version = '1.1'; Count = 1 } - @{ ModPath = "$TestDrive\Modules/Bar.psd1"; Name = 'Bar'; Version = '0.0.1'; Count = 1 } - @{ ModPath = "$TestDrive\Modules\Zoo\Too\Zoo.psm1"; Name = 'Zoo'; Version = '0.0.1'; Count = 1 } + @{ ModPath = "$TestDrive\Modules/Bar.psd1"; Name = 'Bar'; Version = '0.0'; Count = 1 } + @{ ModPath = "$TestDrive\Modules\Zoo\Too\Zoo.psm1"; Name = 'Zoo'; Version = '0.0'; Count = 1 } ) $env:PSModulePath = Join-Path $testdrive "Modules" diff --git a/test/powershell/Modules/Microsoft.PowerShell.Core/Remove-Module.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Core/Remove-Module.Tests.ps1 index 3cd7ed07ca..70a5b544b5 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Core/Remove-Module.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Core/Remove-Module.Tests.ps1 @@ -36,34 +36,30 @@ Describe "Remove-Module custom module with FullyQualifiedName" -Tags "Feature" { $modulePath = "$TestDrive/Modules/$moduleName" $moduleName = 'Banana' $moduleVersion = '1.0' - New-ModuleManifest -Path "$modulePath/$moduleName.psd1" -ModuleVersion $moduleVersion + $manifestPath = Join-Path $modulePath "$moduleName.psd1" + New-ModuleManifest -Path $manifestPath -ModuleVersion $moduleVersion - $relativePathTestCases = @( - @{ Location = $TestDrive; ModPath = ".\Modules\$moduleName" } - @{ Location = "$TestDrive/Modules"; ModPath = ".\$moduleName" } - @{ Location = "$TestDrive/Modules"; ModPath = ".\$moduleName/$moduleName.psd1" } - @{ Location = "$TestDrive/Modules/$moduleName"; ModPath = "./" } - @{ Location = "$TestDrive/Modules/$moduleName"; ModPath = "./$moduleName.psd1" } - @{ Location = "$TestDrive/Modules/$moduleName/Subanana"; ModPath = "../$moduleName.psd1" } - @{ Location = "$TestDrive/Modules/$moduleName/Subanana"; ModPath = "../" } + $testCases = @( + @{ ModPath = "$TestDrive\Modules/$moduleName" } + @{ ModPath = "$TestDrive/Modules\$moduleName/$moduleName.psd1" } ) BeforeEach { Get-Module $moduleName | Remove-Module } - It "Removes a module with fully qualified name with path " -TestCases $relativePathTestCases { - param([string]$Location, [string]$ModPath) + It "Removes a module with fully qualified name with path " -TestCases $testCases -Pending { + param([string]$ModPath) - $m = Import-Module -Path $modulePath -PassThru + $m = Import-Module $modulePath -PassThru $m.Name | Should -Be $moduleName $m.Version | Should -Be $moduleVersion - $m.Path | Should -Be $modulePath + $m.Path | Should -Be $manifestPath - Remove-Module -FullyQualifiedName @{ ModuleName = $ModPath; RequiredVersion = $moduleVersion } + Remove-Module -FullyQualifiedName @{ ModuleName = $ModPath; RequiredVersion = $moduleVersion } -ErrorAction Stop - Get-Module $moduleName | Should -HaveCount 0 + Get-Module $moduleName | Should -HaveCount 0 -Because "The module should have been removed by its path" } } } From 817058429653f81f2fb3453108e4eb567abae79a Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Sat, 10 Nov 2018 19:07:15 -0800 Subject: [PATCH 24/41] [Feature] Remove extraneous style changes --- .../engine/Modules/ModuleCmdletBase.cs | 9 ++------- .../engine/Modules/ModuleIntrinsics.cs | 13 +++---------- 2 files changed, 5 insertions(+), 17 deletions(-) diff --git a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs index 0d05187c1a..5a82a5f14b 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs @@ -3874,10 +3874,7 @@ internal static PSModuleInfo LoadRequiredModule(ExecutionContext context, return result; } - private static PSModuleInfo ImportRequiredModule( - ExecutionContext context, - ModuleSpecification requiredModule, - out ErrorRecord error) + private static PSModuleInfo ImportRequiredModule(ExecutionContext context, ModuleSpecification requiredModule, out ErrorRecord error) { error = null; PSModuleInfo result = null; @@ -4033,9 +4030,7 @@ internal bool VerifyIfNestedModuleIsAvailable(ModuleSpecification nestedModuleSp // Checks if module is available to be loaded // ModuleName ---> checks if module can be loaded using Module loading rules // ModuleManifest --> checks if manifest is valid - internal static Collection GetModuleIfAvailable( - ModuleSpecification requiredModule, - Runspace rsToUse = null) + internal static Collection GetModuleIfAvailable(ModuleSpecification requiredModule, Runspace rsToUse = null) { Collection result = new Collection(); Collection tempResult = null; diff --git a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs index 30ae273321..41b4fc7b34 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs @@ -351,9 +351,7 @@ private List GetModuleCore(string[] patterns, bool all, bool exact return modulesMatched.OrderBy(m => m.Name).ToList(); } - internal List GetModules( - ModuleSpecification[] fullyQualifiedName, - bool all) + internal List GetModules(ModuleSpecification[] fullyQualifiedName, bool all) { List modulesMatched = new List(); @@ -419,9 +417,7 @@ internal List GetModules( /// The module info object to check. /// The module specification to match the module info object against. /// True if the module info object meets all the constraints on the module specification, false otherwise. - internal static bool IsModuleMatchingModuleSpec( - PSModuleInfo moduleInfo, - ModuleSpecification moduleSpec) + internal static bool IsModuleMatchingModuleSpec(PSModuleInfo moduleInfo, ModuleSpecification moduleSpec) { return IsModuleMatchingModuleSpec( out ModuleMatchFailure matchFailureReason, @@ -436,10 +432,7 @@ internal static bool IsModuleMatchingModuleSpec( /// The module info object to check. /// The module specification to match the module info object against. /// True if the module info object meets all the constraints on the module specification, false otherwise. - internal static bool IsModuleMatchingModuleSpec( - out ModuleMatchFailure matchFailureReason, - PSModuleInfo moduleInfo, - ModuleSpecification moduleSpec) + internal static bool IsModuleMatchingModuleSpec(out ModuleMatchFailure matchFailureReason, PSModuleInfo moduleInfo, ModuleSpecification moduleSpec) { if (moduleSpec == null) { From 226cbdc1fc84fcccc6b1d84ce12dfad3d156e3af Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Sat, 10 Nov 2018 19:09:55 -0800 Subject: [PATCH 25/41] [Feature] Remove more style changes --- .../engine/Modules/ModuleCmdletBase.cs | 3 ++- .../engine/Modules/ModuleIntrinsics.cs | 9 +++------ 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs index 5a82a5f14b..de0f2dae43 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs @@ -4030,7 +4030,8 @@ internal bool VerifyIfNestedModuleIsAvailable(ModuleSpecification nestedModuleSp // Checks if module is available to be loaded // ModuleName ---> checks if module can be loaded using Module loading rules // ModuleManifest --> checks if manifest is valid - internal static Collection GetModuleIfAvailable(ModuleSpecification requiredModule, Runspace rsToUse = null) + internal static Collection GetModuleIfAvailable(ModuleSpecification requiredModule, + Runspace rsToUse = null) { Collection result = new Collection(); Collection tempResult = null; diff --git a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs index 41b4fc7b34..bfcd1faeb7 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs @@ -357,7 +357,7 @@ internal List GetModules(ModuleSpecification[] fullyQualifiedName, if (all) { - foreach (ModuleSpecification moduleSpec in fullyQualifiedName) + foreach (var moduleSpec in fullyQualifiedName) { foreach (PSModuleInfo module in ModuleTable.Values) { @@ -371,7 +371,7 @@ internal List GetModules(ModuleSpecification[] fullyQualifiedName, } else { - foreach (ModuleSpecification moduleSpec in fullyQualifiedName) + foreach (var moduleSpec in fullyQualifiedName) { // Create a joint list of local and global modules. Only report a module once. // Local modules are reported before global modules... @@ -419,10 +419,7 @@ internal List GetModules(ModuleSpecification[] fullyQualifiedName, /// True if the module info object meets all the constraints on the module specification, false otherwise. internal static bool IsModuleMatchingModuleSpec(PSModuleInfo moduleInfo, ModuleSpecification moduleSpec) { - return IsModuleMatchingModuleSpec( - out ModuleMatchFailure matchFailureReason, - moduleInfo, - moduleSpec); + return IsModuleMatchingModuleSpec(out ModuleMatchFailure matchFailureReason, moduleInfo, moduleSpec); } /// From bbfaa49a21d63e61fd6bea856aaa0e610d65fd84 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Sat, 10 Nov 2018 19:14:25 -0800 Subject: [PATCH 26/41] [Feature] Add correct string comparer --- .../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 bfcd1faeb7..219c171341 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs @@ -711,7 +711,7 @@ internal static bool MatchesModulePath(string modulePath, string requiredPath) moduleDirPath = Path.GetDirectoryName(moduleDirPath); } - return moduleDirPath.Equals(requiredPath); + return moduleDirPath.Equals(requiredPath, strcmp); } /// From 012c4aadc015144394a697621955fb4db3b4f3c0 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Sat, 10 Nov 2018 20:11:22 -0800 Subject: [PATCH 27/41] [Feature] Create dir in requires test --- test/powershell/Language/Scripting/Requires.Tests.ps1 | 1 + 1 file changed, 1 insertion(+) diff --git a/test/powershell/Language/Scripting/Requires.Tests.ps1 b/test/powershell/Language/Scripting/Requires.Tests.ps1 index d9a4c0126f..445be95b9f 100644 --- a/test/powershell/Language/Scripting/Requires.Tests.ps1 +++ b/test/powershell/Language/Scripting/Requires.Tests.ps1 @@ -48,6 +48,7 @@ Describe "#requires -Modules" -Tags "CI" { $moduleName = 'Banana' $moduleVersion = '0.12.1' $moduleDirPath = Join-Path $TestDrive 'modules' + New-Item -Path $moduleDirPath -ItemType Directory $modulePath = "$moduleDirPath${sep}$moduleName" $manifestPath = "$modulePath${altSep}$moduleName.psd1" $psm1Path = Join-Path $modulePath "$moduleName.psm1" From c844113bd405fabaf0d41b77d1ecbb389ae97d36 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Sat, 10 Nov 2018 20:39:54 -0800 Subject: [PATCH 28/41] [Feature] Fix test dir creation --- test/powershell/Language/Scripting/Requires.Tests.ps1 | 1 + 1 file changed, 1 insertion(+) diff --git a/test/powershell/Language/Scripting/Requires.Tests.ps1 b/test/powershell/Language/Scripting/Requires.Tests.ps1 index 445be95b9f..4a3e63c633 100644 --- a/test/powershell/Language/Scripting/Requires.Tests.ps1 +++ b/test/powershell/Language/Scripting/Requires.Tests.ps1 @@ -50,6 +50,7 @@ Describe "#requires -Modules" -Tags "CI" { $moduleDirPath = Join-Path $TestDrive 'modules' New-Item -Path $moduleDirPath -ItemType Directory $modulePath = "$moduleDirPath${sep}$moduleName" + New-Item -Path $modulePath -ItemType Directory $manifestPath = "$modulePath${altSep}$moduleName.psd1" $psm1Path = Join-Path $modulePath "$moduleName.psm1" New-Item -Path $psm1Path -Value "function Test-RequiredModule { '$success' }" From fc2b46d58bbd26efc1aca8ae802bd33a782a6aba Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Sat, 10 Nov 2018 22:09:40 -0800 Subject: [PATCH 29/41] [Feature] Fix required module tests --- .../Language/Scripting/Requires.Tests.ps1 | 24 +++++++++++++------ 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/test/powershell/Language/Scripting/Requires.Tests.ps1 b/test/powershell/Language/Scripting/Requires.Tests.ps1 index 4a3e63c633..d86e683e26 100644 --- a/test/powershell/Language/Scripting/Requires.Tests.ps1 +++ b/test/powershell/Language/Scripting/Requires.Tests.ps1 @@ -45,6 +45,8 @@ Describe "#requires -Modules" -Tags "CI" { $sep = [System.IO.Path]::DirectorySeparatorChar $altSep = [System.IO.Path]::AltDirectorySeparatorChar + $scriptPath = Join-Path $TestDrive 'script.ps1' + $moduleName = 'Banana' $moduleVersion = '0.12.1' $moduleDirPath = Join-Path $TestDrive 'modules' @@ -54,7 +56,7 @@ Describe "#requires -Modules" -Tags "CI" { $manifestPath = "$modulePath${altSep}$moduleName.psd1" $psm1Path = Join-Path $modulePath "$moduleName.psm1" New-Item -Path $psm1Path -Value "function Test-RequiredModule { '$success' }" - New-ModuleManifest -Path $manifestPath -ModuleVersion $moduleVersion + New-ModuleManifest -Path $manifestPath -ModuleVersion $moduleVersion -RootModule "$moduleName.psm1" } Context "Requiring non-existent modules" { @@ -73,14 +75,16 @@ Describe "#requires -Modules" -Tags "CI" { It "Fails parsing a script that requires module by " -TestCases $testCases { param([string]$ModuleRequirement, [string]$Scenario) - $script = "#requires -Module $ModuleRequirement`n`nWrite-Output 'failed'" - { & $script } | Should -Throw -ErrorId 'ParseException' + $script = "#requires -Modules $ModuleRequirement`n`nWrite-Output 'failed'" + $null = New-Item -Path $scriptPath -Value $script -Force + + { & $scriptPath } | Should -Throw -ErrorId 'ScriptRequiresMissingModules' } } Context "Already loaded module" { BeforeAll { - Import-Module $moduleName + Import-Module $modulePath -ErrorAction Stop $testCases = @( @{ ModuleRequirement = "'$moduleName'"; Scenario = 'name' } @{ ModuleRequirement = "'$modulePath'"; Scenario = 'path' } @@ -99,7 +103,7 @@ Describe "#requires -Modules" -Tags "CI" { param([string]$ModuleRequirement, [string]$Scenario) $script = "#requires -Modules $ModuleRequirement`n`nTest-RequiredModule" - & $script | Should -BeExactly $success + [scriptblock]::Create($script).Invoke() | Should -BeExactly $success } } @@ -126,7 +130,10 @@ Describe "#requires -Modules" -Tags "CI" { param([string]$ModuleRequirement, [string]$Scenario) $script = "#requires -Modules $ModuleRequirement`n`nTest-RequiredModule" - & $script | Should -BeExactly $success + + $null = New-Item -Path $scriptPath -Value $script -Force + + & $scriptPath | Should -BeExactly $success } } @@ -144,7 +151,10 @@ Describe "#requires -Modules" -Tags "CI" { param([string]$ModuleRequirement, [string]$Scenario) $script = "#requires -Modules $ModuleRequirement`n`nTest-RequiredModule" - & $script | Should -BeExactly $success + + $null = New-Item -Path $scriptPath -Value $script -Force + + & $scriptPath | Should -BeExactly $success } } } From 5950e754944498b89854bb6fad80fbbd3cfc88d6 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Mon, 12 Nov 2018 18:48:36 -0800 Subject: [PATCH 30/41] Make required name null check clearer --- .../engine/Modules/ModuleIntrinsics.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs index 219c171341..d0d705176e 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs @@ -583,7 +583,8 @@ internal static bool AreModuleFieldsMatchingConstraints( { // If a name is required, check that it matches. // A required module name may also be an absolute path, so check it against the given module's path as well. - if ((requiredName != null && !requiredName.Equals(moduleName, StringComparison.OrdinalIgnoreCase)) + if (requiredName != null + && !requiredName.Equals(moduleName, StringComparison.OrdinalIgnoreCase) && !MatchesModulePath(modulePath, requiredName)) { matchFailureReason = ModuleMatchFailure.Name; From cbc650e76ab88591300b150380cc941316d6bc6b Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Mon, 12 Nov 2018 18:52:45 -0800 Subject: [PATCH 31/41] Protect against path resolution failure --- .../engine/Modules/ModuleIntrinsics.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs index d0d705176e..03bb5f0193 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs @@ -760,7 +760,10 @@ internal static string NormalizeModuleName( } // Use the PowerShell filesystem provider to fully resolve the path - return ModuleCmdletBase.GetResolvedPath(moduleName, executionContext).TrimEnd(Path.DirectorySeparatorChar); + // If there is a problem, null could be returned -- so default back to the pre-normalized path + string normalizedPath = ModuleCmdletBase.GetResolvedPath(moduleName, executionContext) ?? moduleName; + + return normalizedPath.TrimEnd(Path.DirectorySeparatorChar); } /// From 87a9704d6a6ed5fba3b81a88a55809ef387fbefe Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Mon, 12 Nov 2018 21:17:38 -0800 Subject: [PATCH 32/41] Fix Path.AltDirectorySeparatorChar misuse --- .../engine/Modules/ModuleIntrinsics.cs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs index 03bb5f0193..9d5ec4b007 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs @@ -750,8 +750,14 @@ internal static string NormalizeModuleName( return moduleName; } +#if UNIX + // On *nix, Path.AltDirectorySeparatorChar is '/', but PowerShell also supports '\\' as a dir separator + const char altDirSeparatorChar = '\\'; +#else + const char altDirSeparatorChar = Path.AltDirectorySeparatorChar; +#endif // Standardize directory separators - moduleName = moduleName.Replace(Path.AltDirectorySeparatorChar, Path.DirectorySeparatorChar); + moduleName = moduleName.Replace(altDirSeparatorChar, Path.DirectorySeparatorChar); // Path.IsRooted() is deprecated -- see https://github.com/dotnet/corefx/issues/22345 if (!Path.IsPathFullyQualified(moduleName)) From 2378bb808cd988d6fed20b5f633c5b2cce80b781 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Tue, 13 Nov 2018 09:56:33 -0800 Subject: [PATCH 33/41] [Feature] Fix non-const const assignment --- .../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 9d5ec4b007..d2f6775a2f 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs @@ -754,7 +754,7 @@ internal static string NormalizeModuleName( // On *nix, Path.AltDirectorySeparatorChar is '/', but PowerShell also supports '\\' as a dir separator const char altDirSeparatorChar = '\\'; #else - const char altDirSeparatorChar = Path.AltDirectorySeparatorChar; + char altDirSeparatorChar = Path.AltDirectorySeparatorChar; #endif // Standardize directory separators moduleName = moduleName.Replace(altDirSeparatorChar, Path.DirectorySeparatorChar); From 9faf21ed8471ae05d54fe79d971bf1ce783378ce Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Tue, 13 Nov 2018 09:57:41 -0800 Subject: [PATCH 34/41] [Feature] Undo change to ModuleCmdletBase.IsRooted --- .../engine/Modules/ModuleCmdletBase.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs index de0f2dae43..451f73acf6 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs @@ -4559,14 +4559,14 @@ internal string GetAbsolutePath(string moduleBase, string path) /// True if the path is rooted, false otherwise. internal static bool IsRooted(string filePath) { - return Path.IsPathRooted(filePath) || + return (Path.IsPathRooted(filePath) || filePath.StartsWith(@".\", StringComparison.Ordinal) || filePath.StartsWith(@"./", StringComparison.Ordinal) || filePath.StartsWith(@"..\", StringComparison.Ordinal) || filePath.StartsWith(@"../", StringComparison.Ordinal) || filePath.StartsWith(@"~/", StringComparison.Ordinal) || filePath.StartsWith(@"~\", StringComparison.Ordinal) || - filePath.IndexOf(":", StringComparison.Ordinal) >= 0; + filePath.IndexOf(":", StringComparison.Ordinal) >= 0); } /// From 60ece17bc52cffe6d2e966f066aeb0fc5b41267c Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Tue, 13 Nov 2018 11:27:38 -0800 Subject: [PATCH 35/41] [Feature] Improve TODOs in GetModuleCommand.cs --- .../engine/Modules/GetModuleCommand.cs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/System.Management.Automation/engine/Modules/GetModuleCommand.cs b/src/System.Management.Automation/engine/Modules/GetModuleCommand.cs index b69af5c340..5ab485b079 100644 --- a/src/System.Management.Automation/engine/Modules/GetModuleCommand.cs +++ b/src/System.Management.Automation/engine/Modules/GetModuleCommand.cs @@ -374,9 +374,10 @@ protected override void ProcessRecord() var moduleSpecTable = new Dictionary(StringComparer.OrdinalIgnoreCase); if (FullyQualifiedName != null) { - // TODO: Paths here will not match a module name. - // The table match logic will mean that a module with the right - // name but wrong path or version will still get returned. + // TODO: + // FullyQualifiedName.Name could be a path, in which case it will not match module.Name. + // This is potentially a bug (since version checks are ignored). + // We should normalize FullyQualifiedName.Name here with ModuleIntrinsics.NormalizeModuleName(). moduleSpecTable = FullyQualifiedName.ToDictionary(moduleSpecification => moduleSpecification.Name, StringComparer.OrdinalIgnoreCase); strNames.AddRange(FullyQualifiedName.Select(spec => spec.Name)); } @@ -543,6 +544,11 @@ private static IEnumerable FilterModulesForSpecificationMatch( foreach (PSModuleInfo module in modules) { + // TODO: + // moduleSpecification.Name may be a path and will not match module.Name when they refer to the same module. + // This actually causes the module to be returned always, so other specification checks are skipped erroneously. + // Instead we need to be able to look up or match modules by path as well (e.g. a new comparer for PSModuleInfo). + // No table entry means we return the module if (!moduleSpecificationTable.TryGetValue(module.Name, out ModuleSpecification moduleSpecification)) { From c21eb25e2713d25040f369260f77ae4a282d8169 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Tue, 13 Nov 2018 11:37:30 -0800 Subject: [PATCH 36/41] [Feature] Add comments to module path matching logic --- .../engine/Modules/ModuleIntrinsics.cs | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs index d2f6775a2f..4bb5601ea6 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs @@ -672,8 +672,8 @@ internal static bool IsVersionMatchingConstraints( /// Checks whether a given module path is the same as /// a required path. /// - /// The path of the module whose path to check. - /// The path of the required module. Only normalized absolute paths will work for this. + /// The path of the module whose path to check. This must be the path to the module file (.psd1, .psm1, .dll, etc.) + /// The path of the required module. This may be the module directory path or the file path. Only normalized absolute paths will work for this. /// True if the module path matches the required path, false otherwise. internal static bool MatchesModulePath(string modulePath, string requiredPath) { @@ -690,23 +690,28 @@ internal static bool MatchesModulePath(string modulePath, string requiredPath) StringComparison strcmp = StringComparison.OrdinalIgnoreCase; #endif - // If the required module just matches the module path, we are done + // We must check modulePath (e.g. /path/to/module/module.psd1) against several possibilities: + // 1. "/path/to/module" - Module dir path + // 2. "/path/to/module/module.psd1" - Module root file path + // 3. "/path/to/module/2.1/module.psd1" - Versioned module path + + // If the required module just matches the module path (case 1), we are done if (modulePath.Equals(requiredPath, strcmp)) { return true; } - // Save some allocations here if module path doesn't sit under the required path + // At this point we are looking for the module directory (case 2 or 3). + // We can some allocations here if module path doesn't sit under the required path // (the required path may still refer to some nested module though) if (!modulePath.StartsWith(requiredPath, strcmp)) { return false; } - // Otherwise, the required module may point to the module base directory string moduleDirPath = Path.GetDirectoryName(modulePath); - // The module itself may be in a versioned directory + // The module itself may be in a versioned directory (case 3) if (Version.TryParse(Path.GetFileName(moduleDirPath), out Version unused)) { moduleDirPath = Path.GetDirectoryName(moduleDirPath); From 336ba07c8dda18a590ff91c375acea81a52d6091 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Tue, 13 Nov 2018 11:53:45 -0800 Subject: [PATCH 37/41] [Feature] Fix up path normalization code --- .../engine/Modules/ModuleIntrinsics.cs | 22 ++++++++----------- 1 file changed, 9 insertions(+), 13 deletions(-) diff --git a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs index 4bb5601ea6..1b75dadb5a 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs @@ -755,26 +755,22 @@ internal static string NormalizeModuleName( return moduleName; } -#if UNIX - // On *nix, Path.AltDirectorySeparatorChar is '/', but PowerShell also supports '\\' as a dir separator - const char altDirSeparatorChar = '\\'; -#else - char altDirSeparatorChar = Path.AltDirectorySeparatorChar; -#endif - // Standardize directory separators - moduleName = moduleName.Replace(altDirSeparatorChar, Path.DirectorySeparatorChar); + // Standardize directory separators -- Path.IsPathRooted() will return false for "\path\here" on *nix and for "/path/there" on Windows + moduleName = moduleName.Replace(StringLiterals.AlternatePathSeparator, StringLiterals.DefaultPathSeparator); - // Path.IsRooted() is deprecated -- see https://github.com/dotnet/corefx/issues/22345 - if (!Path.IsPathFullyQualified(moduleName)) + // Note: Path.IsFullyQualified("\default\root") is false on Windows, but Path.IsPathRooted returns true + if (!Path.IsPathRooted(moduleName)) { moduleName = Path.Join(basePath, moduleName); } // Use the PowerShell filesystem provider to fully resolve the path // If there is a problem, null could be returned -- so default back to the pre-normalized path - string normalizedPath = ModuleCmdletBase.GetResolvedPath(moduleName, executionContext) ?? moduleName; - - return normalizedPath.TrimEnd(Path.DirectorySeparatorChar); + string normalizedPath = ModuleCmdletBase.GetResolvedPath(moduleName, executionContext)?.TrimEnd(StringLiterals.DefaultPathSeparator); + + // ModuleCmdletBase.GetResolvePath will return null in the unlikely event that it failed. + // If it does, we return the fully qualified path we return the fully qualified path generated before. + return normalizedPath ?? moduleName; } /// From 30c07b8ee1b91499a02245484928f492e9e50b86 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Tue, 13 Nov 2018 11:57:54 -0800 Subject: [PATCH 38/41] [Feature] Add extra fallback normalization --- .../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 1b75dadb5a..30c9f67fdd 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs @@ -770,7 +770,7 @@ internal static string NormalizeModuleName( // ModuleCmdletBase.GetResolvePath will return null in the unlikely event that it failed. // If it does, we return the fully qualified path we return the fully qualified path generated before. - return normalizedPath ?? moduleName; + return normalizedPath ?? Path.GetFullPath(moduleName); } /// From e381a50d1d7b074e5365311072e5ac8edc50d418 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Tue, 13 Nov 2018 12:07:20 -0800 Subject: [PATCH 39/41] [Feature] Fix IsModulePath check method --- .../engine/Modules/ModuleIntrinsics.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs index 30c9f67fdd..a22490e674 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs @@ -780,8 +780,8 @@ internal static string NormalizeModuleName( /// True if the module name is a path, false otherwise. internal static bool IsModuleNamePath(string moduleName) { - return moduleName.Contains(Path.DirectorySeparatorChar) - || moduleName.Contains(Path.AltDirectorySeparatorChar) + return moduleName.Contains(StringLiterals.DefaultPathSeparator) + || moduleName.Contains(StringLiterals.AlternatePathSeparator) || moduleName.Equals("..") || moduleName.Equals("."); } From 93443450c0281019d2c64bfc3c14d8c50851cf46 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Tue, 13 Nov 2018 13:20:36 -0800 Subject: [PATCH 40/41] [Feature] Add full stop to satisfy code factor --- .../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 a22490e674..cd578f6eea 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs @@ -672,7 +672,7 @@ internal static bool IsVersionMatchingConstraints( /// Checks whether a given module path is the same as /// a required path. /// - /// The path of the module whose path to check. This must be the path to the module file (.psd1, .psm1, .dll, etc.) + /// The path of the module whose path to check. This must be the path to the module file (.psd1, .psm1, .dll, etc). /// The path of the required module. This may be the module directory path or the file path. Only normalized absolute paths will work for this. /// True if the module path matches the required path, false otherwise. internal static bool MatchesModulePath(string modulePath, string requiredPath) From 389b245cb4b4d18f946e2e6ba6a4b25286370703 Mon Sep 17 00:00:00 2001 From: Robert Holt Date: Tue, 13 Nov 2018 13:54:20 -0800 Subject: [PATCH 41/41] [Feature] Fix comment typo --- .../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 cd578f6eea..40d4fd6e28 100644 --- a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs +++ b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs @@ -769,7 +769,7 @@ internal static string NormalizeModuleName( string normalizedPath = ModuleCmdletBase.GetResolvedPath(moduleName, executionContext)?.TrimEnd(StringLiterals.DefaultPathSeparator); // ModuleCmdletBase.GetResolvePath will return null in the unlikely event that it failed. - // If it does, we return the fully qualified path we return the fully qualified path generated before. + // If it does, we return the fully qualified path generated before. return normalizedPath ?? Path.GetFullPath(moduleName); }