Skip to content

Commit ee9049b

Browse files
SteveL-MSFTmirichmo
SteveL-MSFT
authored and
mirichmo
committed
New-ModuleManifest was incorrectly checking if a Uri was well formed by using ToString() which just outputs the original (PowerShell#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.
1 parent f8f603a commit ee9049b

3 files changed

Lines changed: 64 additions & 6 deletions

File tree

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

Lines changed: 34 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -480,11 +480,35 @@ public string DefaultCommandPrefix
480480
/// </summary>
481481
/// <param name="name">The string to quote</param>
482482
/// <returns>The quoted string</returns>
483-
private string QuoteName(object name)
483+
private string QuoteName(string name)
484484
{
485485
if (name == null)
486486
return "''";
487-
return "'" + name.ToString().Replace("'", "''") + "'";
487+
return ("'" + name.ToString().Replace("'", "''") + "'");
488+
}
489+
490+
/// <summary>
491+
/// Return a single-quoted string using the AbsoluteUri member to ensure it is escaped correctly
492+
/// </summary>
493+
/// <param name="name">The Uri to quote</param>
494+
/// <returns>The quoted AbsoluteUri</returns>
495+
private string QuoteName(Uri name)
496+
{
497+
if (name == null)
498+
return "''";
499+
return QuoteName(name.AbsoluteUri);
500+
}
501+
502+
/// <summary>
503+
/// Return a single-quoted string from a Version object
504+
/// </summary>
505+
/// <param name="name">The Version object to quote</param>
506+
/// <returns>The quoted Version string</returns>
507+
private string QuoteName(Version name)
508+
{
509+
if (name == null)
510+
return "''";
511+
return QuoteName(name.ToString());
488512
}
489513

490514
/// <summary>
@@ -877,6 +901,10 @@ protected override void EndProcessing()
877901
ValidateUriParameterValue(ProjectUri, "ProjectUri");
878902
ValidateUriParameterValue(LicenseUri, "LicenseUri");
879903
ValidateUriParameterValue(IconUri, "IconUri");
904+
if (_helpInfoUri != null)
905+
{
906+
ValidateUriParameterValue(new Uri(_helpInfoUri), "HelpInfoUri");
907+
}
880908

881909
if (CompatiblePSEditions != null && (CompatiblePSEditions.Distinct(StringComparer.OrdinalIgnoreCase).Count() != CompatiblePSEditions.Count()))
882910
{
@@ -949,7 +977,7 @@ protected override void EndProcessing()
949977

950978
BuildModuleManifest(result, "RootModule", Modules.RootModule, !string.IsNullOrEmpty(_rootModule), () => QuoteName(_rootModule), streamWriter);
951979

952-
BuildModuleManifest(result, "ModuleVersion", Modules.ModuleVersion, _moduleVersion != null && !string.IsNullOrEmpty(_moduleVersion.ToString()), () => QuoteName(_moduleVersion.ToString()), streamWriter);
980+
BuildModuleManifest(result, "ModuleVersion", Modules.ModuleVersion, _moduleVersion != null && !string.IsNullOrEmpty(_moduleVersion.ToString()), () => QuoteName(_moduleVersion), streamWriter);
953981

954982
BuildModuleManifest(result, "CompatiblePSEditions", Modules.CompatiblePSEditions, _compatiblePSEditions != null && _compatiblePSEditions.Length > 0, () => QuoteNames(_compatiblePSEditions, streamWriter), streamWriter);
955983

@@ -973,7 +1001,7 @@ protected override void EndProcessing()
9731001

9741002
BuildModuleManifest(result, "CLRVersion", StringUtil.Format(Modules.CLRVersion, Modules.PrerequisiteForDesktopEditionOnly), _ClrVersion != null && !string.IsNullOrEmpty(_ClrVersion.ToString()), () => QuoteName(_ClrVersion), streamWriter);
9751003

976-
BuildModuleManifest(result, "ProcessorArchitecture", Modules.ProcessorArchitecture, _processorArchitecture.HasValue, () => QuoteName(_processorArchitecture), streamWriter);
1004+
BuildModuleManifest(result, "ProcessorArchitecture", Modules.ProcessorArchitecture, _processorArchitecture.HasValue, () => QuoteName(_processorArchitecture.ToString()), streamWriter);
9771005

9781006
BuildModuleManifest(result, "RequiredModules", Modules.RequiredModules, _requiredModules != null && _requiredModules.Length > 0, () => QuoteModules(_requiredModules, streamWriter), streamWriter);
9791007

@@ -1003,7 +1031,7 @@ protected override void EndProcessing()
10031031

10041032
BuildPrivateDataInModuleManifest(result, streamWriter);
10051033

1006-
BuildModuleManifest(result, "HelpInfoURI", Modules.HelpInfoURI, !string.IsNullOrEmpty(_helpInfoUri), () => QuoteName(_helpInfoUri), streamWriter);
1034+
BuildModuleManifest(result, "HelpInfoURI", Modules.HelpInfoURI, !string.IsNullOrEmpty(_helpInfoUri), () => QuoteName((_helpInfoUri != null) ? new Uri(_helpInfoUri) : null), streamWriter);
10071035

10081036
BuildModuleManifest(result, "DefaultCommandPrefix", Modules.DefaultCommandPrefix, !string.IsNullOrEmpty(_defaultCommandPrefix), () => QuoteName(_defaultCommandPrefix), streamWriter);
10091037

@@ -1128,7 +1156,7 @@ private void ValidateUriParameterValue(Uri uri, string parameterName)
11281156
{
11291157
Dbg.Assert(!String.IsNullOrWhiteSpace(parameterName), "parameterName should not be null or whitespace");
11301158

1131-
if (uri != null && !Uri.IsWellFormedUriString(uri.ToString(), UriKind.Absolute))
1159+
if (uri != null && !Uri.IsWellFormedUriString(uri.AbsoluteUri, UriKind.Absolute))
11321160
{
11331161
var message = StringUtil.Format(Modules.InvalidParameterValue, uri);
11341162
var ioe = new InvalidOperationException(message);

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

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
Import-Module $PSScriptRoot\..\..\Common\Test.Helpers.psm1
2+
3+
Describe "New-ModuleManifest tests" -tags "CI" {
4+
BeforeEach {
5+
New-Item -ItemType Directory -Path testdrive:/module
6+
$testModulePath = "testdrive:/module/test.psd1"
7+
}
8+
9+
AfterEach {
10+
Remove-Item -Recurse -Force -ErrorAction SilentlyContinue testdrive:/module
11+
}
12+
13+
It "Uris with spaces are allowed and escaped correctly" {
14+
$testUri = [Uri]"http://foo.com/hello world"
15+
$absoluteUri = $testUri.AbsoluteUri
16+
17+
New-ModuleManifest -Path $testModulePath -ProjectUri $testUri -LicenseUri $testUri -IconUri $testUri -HelpInfoUri $testUri
18+
$module = Test-ModuleManifest -Path $testModulePath
19+
$module.HelpInfoUri | Should BeExactly $absoluteUri
20+
$module.PrivateData.PSData.IconUri | Should BeExactly $absoluteUri
21+
$module.PrivateData.PSData.LicenseUri | Should BeExactly $absoluteUri
22+
$module.PrivateData.PSData.ProjectUri | Should BeExactly $absoluteUri
23+
}
24+
25+
It "Relative URIs are not allowed" {
26+
$testUri = [Uri]"../foo"
27+
28+
{ New-ModuleManifest -Path $testModulePath -ProjectUri $testUri -LicenseUri $testUri -IconUri $testUri } | ShouldBeErrorId "System.InvalidOperationException,Microsoft.PowerShell.Commands.NewModuleManifestCommand"
29+
}
30+
}

test/powershell/engine/Module/TestModuleManifest.ps1 renamed to test/powershell/engine/Module/TestModuleManifest.Tests.ps1

File renamed without changes.

0 commit comments

Comments
 (0)