From ee9049b61f7dfd314cbb53e811ff661af638292b Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Tue, 25 Apr 2017 17:45:24 -0700 Subject: [PATCH] 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. --- .../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 c903b5b622..8afdfcd9b8 100644 --- a/src/System.Management.Automation/engine/Modules/NewModuleManifestCommand.cs +++ b/src/System.Management.Automation/engine/Modules/NewModuleManifestCommand.cs @@ -480,11 +480,35 @@ namespace Microsoft.PowerShell.Commands /// /// 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 @@ 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); diff --git a/test/powershell/engine/Module/NewModuleManifest.Tests.ps1 b/test/powershell/engine/Module/NewModuleManifest.Tests.ps1 new file mode 100644 index 0000000000..dcd750ebc3 --- /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