PowerShell · adityapatwardhan · Nov 13, 2018 · Nov 9, 2018 · Nov 9, 2018 · Nov 9, 2018
diff --git a/src/System.Management.Automation/engine/Modules/GetModuleCommand.cs b/src/System.Management.Automation/engine/Modules/GetModuleCommand.cs
@@ -374,6 +374,10 @@ protected override void ProcessRecord()
             var moduleSpecTable = new Dictionary<string, ModuleSpecification>(StringComparer.OrdinalIgnoreCase);
             if (FullyQualifiedName != null)
             {
+                // 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));
             }
@@ -540,6 +544,11 @@ private static IEnumerable<PSModuleInfo> 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))
                 {

diff --git a/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs b/src/System.Management.Automation/engine/Modules/ModuleCmdletBase.cs
@@ -1958,8 +1958,12 @@ internal PSModuleInfo LoadModuleManifest(
 
                     foreach (ModuleSpecification requiredModule in requiredModules)
                     {
+                        // The required module name is essentially raw user input.
+                        // We must process it so paths work.
+                        ModuleSpecification normalizedRequiredModuleSpec = requiredModule?.WithNormalizedName(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)
                         {
@@ -4059,10 +4063,17 @@ internal static Collection<PSModuleInfo> GetModuleIfAvailable(ModuleSpecificatio
                 tempResult = powerShell.Invoke<PSModuleInfo>();
             }
 
-            // 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: requiredModule.MaximumVersion == null ? null : GetMaximumVersion(requiredModule.MaximumVersion)))
                 {
                     result.Add(module);
                 }
@@ -4541,6 +4552,11 @@ internal string GetAbsolutePath(string moduleBase, string path)
             }
         }
 
+        /// <summary>
+        /// Check if a path is rooted or "relative rooted".
+        /// </summary>
+        /// <param name="filePath">The file path to check.</param>
+        /// <returns>True if the path is rooted, false otherwise.</returns>
         internal static bool IsRooted(string filePath)
         {
             return (Path.IsPathRooted(filePath) ||

diff --git a/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs b/src/System.Management.Automation/engine/Modules/ModuleIntrinsics.cs
@@ -452,7 +452,7 @@ internal static bool IsModuleMatchingModuleSpec(out ModuleMatchFailure matchFail
         /// Constraints given as null are ignored.
         /// </summary>
         /// <param name="moduleInfo">The module info object to check.</param>
-        /// <param name="name">The name of the expected module.</param>
+        /// <param name="name">The name or normalized absolute path of the expected module.</param>
         /// <param name="guid">The guid of the expected module.</param>
         /// <param name="requiredVersion">The required version of the expected module.</param>
         /// <param name="minimumVersion">The minimum required version of the expected module.</param>
@@ -482,7 +482,7 @@ internal static bool IsModuleMatchingConstraints(
         /// </summary>
         /// <param name="matchFailureReason">The reason for the module constraint match failing.</param>
         /// <param name="moduleInfo">The module info object to check.</param>
-        /// <param name="name">The name of the expected module.</param>
+        /// <param name="name">The name or normalized absolute path of the expected module.</param>
         /// <param name="guid">The guid of the expected module.</param>
         /// <param name="requiredVersion">The required version of the expected module.</param>
         /// <param name="minimumVersion">The minimum required version of the expected module.</param>
@@ -507,6 +507,7 @@ internal static bool IsModuleMatchingConstraints(
             return AreModuleFieldsMatchingConstraints(
                 out matchFailureReason,
                 moduleInfo.Name,
+                moduleInfo.Path,
                 moduleInfo.Guid,
                 moduleInfo.Version,
                 name,
@@ -521,16 +522,18 @@ internal static bool IsModuleMatchingConstraints(
         /// Check that given module fields meet any given constraints.
         /// </summary>
         /// <param name="moduleName">The name of the module to check.</param>
+        /// <param name="modulePath">The path of the module to check.</param>
         /// <param name="moduleGuid">The GUID of the module to check.</param>
         /// <param name="moduleVersion">The version of the module to check.</param>
-        /// <param name="requiredName">The name the module must have, if any.</param>
+        /// <param name="requiredName">The name or normalized absolute path the module must have, if any.</param>
         /// <param name="requiredGuid">The GUID the module must have, if any.</param>
         /// <param name="requiredVersion">The exact version the module must have, if any.</param>
         /// <param name="minimumRequiredVersion">The minimum version the module may have, if any.</param>
         /// <param name="maximumRequiredVersion">The maximum version the module may have, if any.</param>
         /// <returns>True if the module parameters match all given constraints, false otherwise.</returns>
         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,9 +560,10 @@ internal static bool AreModuleFieldsMatchingConstraints(
         /// </summary>
         /// <param name="matchFailureReason">The reason the match failed, if any.</param>
         /// <param name="moduleName">The name of the module to check.</param>
+        /// <param name="modulePath">The path of the module to check.</param>
         /// <param name="moduleGuid">The GUID of the module to check.</param>
         /// <param name="moduleVersion">The version of the module to check.</param>
-        /// <param name="requiredName">The name the module must have, if any.</param>
+        /// <param name="requiredName">The name or normalized absolute path the module must have, if any.</param>
         /// <param name="requiredGuid">The GUID the module must have, if any.</param>
         /// <param name="requiredVersion">The exact version the module must have, if any.</param>
         /// <param name="minimumRequiredVersion">The minimum version the module may have, if any.</param>
@@ -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,
@@ -575,8 +581,11 @@ internal static bool AreModuleFieldsMatchingConstraints(
             Version minimumRequiredVersion,
             Version maximumRequiredVersion)
         {
-            // If a name is required, check it matches
-            if (requiredName != null && !requiredName.Equals(moduleName, StringComparison.OrdinalIgnoreCase))
+            // 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))
             {
                 matchFailureReason = ModuleMatchFailure.Name;
                 return false;
@@ -659,6 +668,124 @@ internal static bool IsVersionMatchingConstraints(
             return true;
         }
 
+        /// <summary>
+        /// Checks whether a given module path is the same as
+        /// a required path.
+        /// </summary>
+        /// <param name="modulePath">The path of the module whose path to check. This must be the path to the module file (.psd1, .psm1, .dll, etc).</param>
+        /// <param name="requiredPath">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.</param>
+        /// <returns>True if the module path matches the required path, false otherwise.</returns>
+        internal static bool MatchesModulePath(string modulePath, string requiredPath)
+        {
+            Dbg.Assert(requiredPath != null, $"Caller to verify that {nameof(requiredPath)} is not null");
+
+            if (modulePath == null)
+            {
+                return false;
+            }
+
+#if UNIX
+            StringComparison strcmp = StringComparison.Ordinal;
+#else
+            StringComparison strcmp = StringComparison.OrdinalIgnoreCase;
+#endif
+
+            // 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;
+            }
+
+            // 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;
+            }
+
+            string moduleDirPath = Path.GetDirectoryName(modulePath);
+
+            // The module itself may be in a versioned directory (case 3)
+            if (Version.TryParse(Path.GetFileName(moduleDirPath), out Version unused))
+            {
+                moduleDirPath = Path.GetDirectoryName(moduleDirPath);
+            }
+
+            return moduleDirPath.Equals(requiredPath, strcmp);
+        }
+
+        /// <summary>
+        /// 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.
+        /// </summary>
+        /// <param name="moduleName">The name or path of the module from the specification.</param>
+        /// <param name="basePath">The path to base relative paths off.</param>
+        /// <param name="executionContext">The current execution context.</param>
+        /// <returns>
+        /// The simple module name if the given one was simple,
+        /// otherwise a fully resolved, absolute path to the module.
+        /// </returns>
+        /// <remarks>
+        /// 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.
+        /// </remarks>
+        internal static string NormalizeModuleName(
+            string moduleName,
+            string basePath,
+            ExecutionContext executionContext)
+        {
+            if (moduleName == null)
+            {
+                return null;
+            }
+
+            // Check whether the module is a path -- if not, it is a simple name and we just return it.
+            if (!IsModuleNamePath(moduleName))
+            {
+                return moduleName;
+            }
+
+            // 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);
+
+            // 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)?.TrimEnd(StringLiterals.DefaultPathSeparator);
+
+            // ModuleCmdletBase.GetResolvePath will return null in the unlikely event that it failed.
+            // If it does, we return the fully qualified path generated before.
+            return normalizedPath ?? Path.GetFullPath(moduleName);
+        }
+
+        /// <summary>
+        /// Check if a given module name is a path to a module rather than a simple name.
+        /// </summary>
+        /// <param name="moduleName">The module name to check.</param>
+        /// <returns>True if the module name is a path, false otherwise.</returns>
+        internal static bool IsModuleNamePath(string moduleName)
+        {
+            return moduleName.Contains(StringLiterals.DefaultPathSeparator)
+                || moduleName.Contains(StringLiterals.AlternatePathSeparator)
+                || 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
@@ -247,6 +247,31 @@ public static bool TryParse(string input, out ModuleSpecification result)
             return false;
         }
 
+        /// <summary>
+        /// Copy the module specification while normalizing the name
+        /// so that paths become absolute and use the right directory separators.
+        /// </summary>
+        /// <param name="context">The current execution context. Used for path normalization.</param>
+        /// <param name="basePath">The base path where a relative path should be interpreted with respect to.</param>
+        /// <returns>A fresh module specification object with the name normalized for use internally.</returns>
+        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,
+                MaximumVersion = MaximumVersion,
+                Version = Version,
+                RequiredVersion = RequiredVersion,
+                Name = ModuleIntrinsics.NormalizeModuleName(Name, basePath, context)
+            };
+        }
+
         /// <summary>
         /// The module name.
         /// </summary>

diff --git a/src/System.Management.Automation/engine/Modules/RemoveModuleCommand.cs b/src/System.Management.Automation/engine/Modules/RemoveModuleCommand.cs
@@ -84,6 +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 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<PSModuleInfo> { m });