New-ModuleManifest was incorrectly checking if a Uri was well formed by using ToString() which just outputs the original (#3631)

string.  If the string was a uri with spaces, ToString() doesn't return the escaped version.  The AbsoluteUri property
should be used instead which returns an escaped absolute uri (if valid).

Also renamed TestModuleManfest.ps1 to TestModuleManifest.Tests.ps1 so that it gets picked up correctly as Pester test.

Since HelpInfoUri is just a string, ensure it is a valid absolute uri and escaped correctly whereas before it was just
an opaque string that wasn't validated.

src/System.Management.Automation/engine/Modules/NewModuleManifestCommand.cs

@@ -480,11 +480,35 @@ namespace Microsoft.PowerShell.Commands
         /// </summary>
         /// <param name="name">The string to quote</param>
         /// <returns>The quoted string</returns>
-        private string QuoteName(object name)
+        private string QuoteName(string name)
         {
             if (name == null)
                 return "''";
-            return "'" + name.ToString().Replace("'", "''") + "'";
+            return ("'" + name.ToString().Replace("'", "''") + "'");
+        }
+
+        /// <summary>
+        /// Return a single-quoted string using the AbsoluteUri member to ensure it is escaped correctly
+        /// </summary>
+        /// <param name="name">The Uri to quote</param>
+        /// <returns>The quoted AbsoluteUri</returns>
+        private string QuoteName(Uri name)
+        {
+            if (name == null)
+                return "''";
+            return QuoteName(name.AbsoluteUri);
+        }
+
+        /// <summary>
+        /// Return a single-quoted string from a Version object
+        /// </summary>
+        /// <param name="name">The Version object to quote</param>
+        /// <returns>The quoted Version string</returns>
+        private string QuoteName(Version name)
+        {
+            if (name == null)
+                return "''";
+            return QuoteName(name.ToString());
         }
 
         /// <summary>
@@ -877,6 +901,10 @@ namespace Microsoft.PowerShell.Commands
             ValidateUriParameterValue(ProjectUri, "ProjectUri");
             ValidateUriParameterValue(LicenseUri, "LicenseUri");
             ValidateUriParameterValue(IconUri, "IconUri");
+            if (_helpInfoUri != null)
+            {
+                ValidateUriParameterValue(new Uri(_helpInfoUri), "HelpInfoUri");
+            }
 
             if (CompatiblePSEditions != null && (CompatiblePSEditions.Distinct(StringComparer.OrdinalIgnoreCase).Count() != CompatiblePSEditions.Count()))
             {
@@ -949,7 +977,7 @@ namespace Microsoft.PowerShell.Commands
 
                     BuildModuleManifest(result, "RootModule", Modules.RootModule, !string.IsNullOrEmpty(_rootModule), () => QuoteName(_rootModule), streamWriter);
 
-                    BuildModuleManifest(result, "ModuleVersion", Modules.ModuleVersion, _moduleVersion != null && !string.IsNullOrEmpty(_moduleVersion.ToString()), () => QuoteName(_moduleVersion.ToString()), streamWriter);
+                    BuildModuleManifest(result, "ModuleVersion", Modules.ModuleVersion, _moduleVersion != null && !string.IsNullOrEmpty(_moduleVersion.ToString()), () => QuoteName(_moduleVersion), streamWriter);
 
                     BuildModuleManifest(result, "CompatiblePSEditions", Modules.CompatiblePSEditions, _compatiblePSEditions != null && _compatiblePSEditions.Length > 0, () => QuoteNames(_compatiblePSEditions, streamWriter), streamWriter);
 
@@ -973,7 +1001,7 @@ namespace Microsoft.PowerShell.Commands
 
                     BuildModuleManifest(result, "CLRVersion", StringUtil.Format(Modules.CLRVersion, Modules.PrerequisiteForDesktopEditionOnly), _ClrVersion != null && !string.IsNullOrEmpty(_ClrVersion.ToString()), () => QuoteName(_ClrVersion), streamWriter);
 
-                    BuildModuleManifest(result, "ProcessorArchitecture", Modules.ProcessorArchitecture, _processorArchitecture.HasValue, () => QuoteName(_processorArchitecture), streamWriter);
+                    BuildModuleManifest(result, "ProcessorArchitecture", Modules.ProcessorArchitecture, _processorArchitecture.HasValue, () => QuoteName(_processorArchitecture.ToString()), streamWriter);
 
                     BuildModuleManifest(result, "RequiredModules", Modules.RequiredModules, _requiredModules != null && _requiredModules.Length > 0, () => QuoteModules(_requiredModules, streamWriter), streamWriter);
 
@@ -1003,7 +1031,7 @@ namespace Microsoft.PowerShell.Commands
 
                     BuildPrivateDataInModuleManifest(result, streamWriter);
 
-                    BuildModuleManifest(result, "HelpInfoURI", Modules.HelpInfoURI, !string.IsNullOrEmpty(_helpInfoUri), () => QuoteName(_helpInfoUri), streamWriter);
+                    BuildModuleManifest(result, "HelpInfoURI", Modules.HelpInfoURI, !string.IsNullOrEmpty(_helpInfoUri), () => QuoteName((_helpInfoUri != null) ? new Uri(_helpInfoUri) : null), streamWriter);
 
                     BuildModuleManifest(result, "DefaultCommandPrefix", Modules.DefaultCommandPrefix, !string.IsNullOrEmpty(_defaultCommandPrefix), () => QuoteName(_defaultCommandPrefix), streamWriter);
 
@@ -1128,7 +1156,7 @@ namespace Microsoft.PowerShell.Commands
         {
             Dbg.Assert(!String.IsNullOrWhiteSpace(parameterName), "parameterName should not be null or whitespace");
 
-            if (uri != null && !Uri.IsWellFormedUriString(uri.ToString(), UriKind.Absolute))
+            if (uri != null && !Uri.IsWellFormedUriString(uri.AbsoluteUri, UriKind.Absolute))
             {
                 var message = StringUtil.Format(Modules.InvalidParameterValue, uri);
                 var ioe = new InvalidOperationException(message);

test/powershell/engine/Module/NewModuleManifest.Tests.ps1

@@ -0,0 +1,30 @@
+Import-Module $PSScriptRoot\..\..\Common\Test.Helpers.psm1
+
+Describe "New-ModuleManifest tests" -tags "CI" {
+    BeforeEach {
+        New-Item -ItemType Directory -Path testdrive:/module
+        $testModulePath = "testdrive:/module/test.psd1"
+    }
+
+    AfterEach {
+        Remove-Item -Recurse -Force -ErrorAction SilentlyContinue testdrive:/module
+    }
+
+    It "Uris with spaces are allowed and escaped correctly" {
+        $testUri = [Uri]"http://foo.com/hello world"
+        $absoluteUri = $testUri.AbsoluteUri
+
+        New-ModuleManifest -Path $testModulePath -ProjectUri $testUri -LicenseUri $testUri -IconUri $testUri -HelpInfoUri $testUri
+        $module = Test-ModuleManifest -Path $testModulePath
+        $module.HelpInfoUri | Should BeExactly $absoluteUri
+        $module.PrivateData.PSData.IconUri | Should BeExactly $absoluteUri
+        $module.PrivateData.PSData.LicenseUri | Should BeExactly $absoluteUri
+        $module.PrivateData.PSData.ProjectUri | Should BeExactly $absoluteUri
+    }
+
+    It "Relative URIs are not allowed" {
+        $testUri = [Uri]"../foo"
+
+        { New-ModuleManifest -Path $testModulePath -ProjectUri $testUri -LicenseUri $testUri -IconUri $testUri } | ShouldBeErrorId "System.InvalidOperationException,Microsoft.PowerShell.Commands.NewModuleManifestCommand"
+    }
+}