From 5017f7956b6daa2cc2ef210d25147a880cd0de9f Mon Sep 17 00:00:00 2001 From: "Steve Lee (POWERSHELL)" Date: Fri, 21 Apr 2017 16:31:52 -0700 Subject: [PATCH] New-ModuleManifest was incorrectly checking if a Uri was well formed by using ToString() which just outputs the original 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. --- .../Modules/NewModuleManifestCommand.cs | 40 ++++++++++++++++--- .../engine/Module/NewModuleManifest.Tests.ps1 | 30 ++++++++++++++ ...ifest.ps1 => TestModuleManifest.Tests.ps1} | 0 3 files changed, 64 insertions(+), 6 deletions(-) create mode 100644 test/powershell/engine/Module/NewModuleManifest.Tests.ps1 rename test/powershell/engine/Module/{TestModuleManifest.ps1 => TestModuleManifest.Tests.ps1} (100%) diff --git a/src/System.Management.Automation/engine/Modules/NewModuleManifestCommand.cs b/src/System.Management.Automation/engine/Modules/NewModuleManifestCommand.cs index c903b5b622f..8afdfcd9b8c 100644 --- a/src/System.Management.Automation/engine/Modules/NewModuleManifestCommand.cs +++ b/src/System.Management.Automation/engine/Modules/NewModuleManifestCommand.cs @@ -480,11 +480,35 @@ public string DefaultCommandPrefix /// /// The string to quote /// The quoted string - private string QuoteName(object name) + private string QuoteName(string name) { if (name == null) return "''"; - return "'" + name.ToString().Replace("'", "''") + "'"; + return ("'" + name.ToString().Replace("'", "''") + "'"); + } + + /// + /// Return a single-quoted string using the AbsoluteUri member to ensure it is escaped correctly + /// + /// The Uri to quote + /// The quoted AbsoluteUri + private string QuoteName(Uri name) + { + if (name == null) + return "''"; + return QuoteName(name.AbsoluteUri); + } + + /// + /// Return a single-quoted string from a Version object + /// + /// The Version object to quote + /// The quoted Version string + private string QuoteName(Version name) + { + if (name == null) + return "''"; + return QuoteName(name.ToString()); } /// @@ -877,6 +901,10 @@ protected override void EndProcessing() 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 @@ protected override void EndProcessing() 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 @@ protected override void EndProcessing() 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 @@ protected override void EndProcessing() 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 @@ private void ValidateUriParameterValue(Uri uri, string parameterName) { 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); diff --git a/test/powershell/engine/Module/NewModuleManifest.Tests.ps1 b/test/powershell/engine/Module/NewModuleManifest.Tests.ps1 new file mode 100644 index 00000000000..dcd750ebc32 --- /dev/null +++ b/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" + } +} diff --git a/test/powershell/engine/Module/TestModuleManifest.ps1 b/test/powershell/engine/Module/TestModuleManifest.Tests.ps1 similarity index 100% rename from test/powershell/engine/Module/TestModuleManifest.ps1 rename to test/powershell/engine/Module/TestModuleManifest.Tests.ps1