From ea329fc7b90ef2844a8ba44023260b24ed84b785 Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Fri, 3 Feb 2017 19:17:34 -0800 Subject: [PATCH 1/7] fixing #2610 changed hard coded Windows directory separator and resolved path so the slashes are correct added test --- .../engine/Modules/TestModuleManifestCommand.cs | 11 ++++++++++- .../powershell/engine/Module/TestModuleManifest.ps1 | 13 +++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) create mode 100644 test/powershell/engine/Module/TestModuleManifest.ps1 diff --git a/src/System.Management.Automation/engine/Modules/TestModuleManifestCommand.cs b/src/System.Management.Automation/engine/Modules/TestModuleManifestCommand.cs index 7f387a7e516..f466bb0a5ef 100644 --- a/src/System.Management.Automation/engine/Modules/TestModuleManifestCommand.cs +++ b/src/System.Management.Automation/engine/Modules/TestModuleManifestCommand.cs @@ -288,10 +288,19 @@ private bool IsValidFilePath(string path, PSModuleInfo module, bool verifyPathSc { try { + // resolve the path so slashes are in the right direction + CmdletProviderContext cmdContext = new CmdletProviderContext(this); + Collection pathInfos = SessionState.Path.GetResolvedPSPathFromPSPath(path, cmdContext); + + foreach (PathInfo pathInfo in pathInfos) + { + path = pathInfo.Path; + } + if (!System.IO.Path.IsPathRooted(path)) { // we assume the relative path is under module scope, otherwise we will throw error anyway. - path = System.IO.Path.GetFullPath(module.ModuleBase + "\\" + path); + path = System.IO.Path.GetFullPath(module.ModuleBase + System.IO.Path.DirectorySeparatorChar + path); } // First, we validate if the path does exist. diff --git a/test/powershell/engine/Module/TestModuleManifest.ps1 b/test/powershell/engine/Module/TestModuleManifest.ps1 new file mode 100644 index 00000000000..6d3d12cc789 --- /dev/null +++ b/test/powershell/engine/Module/TestModuleManifest.ps1 @@ -0,0 +1,13 @@ +Describe "Test-ModuleManifest tests" -tags "CI" { + + It "module manifest containing paths with backslashes or forwardslashes are resolved correctly" { + + New-Item -ItemType Directory -Path testdrive:/module + New-Item -ItemType Directory -Path testdrive:/module/foo + New-Item -ItemType Directory -Path testdrive:/module/bar + "" > testdrive:/module/foo/bar.psm1 + "" > testdrive:/module/bar/foo.psm1 + New-ModuleManifest -NestedModules foo\bar.psm1,bar/foo.psm1 -RootModule foo\bar.psm1 -RequiredAssemblies foo/bar.psm1,bar\foo.psm1 -Path testdrive:/module/test.psd1 + Test-ModuleManifest -Path testdrive:/module/test.psd1 -ErrorAction Stop | Should BeOfType System.Management.Automation.PSModuleInfo + } +} From 5657bfdeccd5c6765ef77ec6d614a454497dce7d Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Tue, 7 Feb 2017 13:49:28 -0800 Subject: [PATCH 2/7] throw if resolving file path returns more than one result added negative test cases that do file path checks --- .../Modules/TestModuleManifestCommand.cs | 10 ++-- .../engine/Module/TestModuleManifest.ps1 | 50 ++++++++++++++++++- 2 files changed, 55 insertions(+), 5 deletions(-) diff --git a/src/System.Management.Automation/engine/Modules/TestModuleManifestCommand.cs b/src/System.Management.Automation/engine/Modules/TestModuleManifestCommand.cs index f466bb0a5ef..4f0e0586b70 100644 --- a/src/System.Management.Automation/engine/Modules/TestModuleManifestCommand.cs +++ b/src/System.Management.Automation/engine/Modules/TestModuleManifestCommand.cs @@ -291,12 +291,16 @@ private bool IsValidFilePath(string path, PSModuleInfo module, bool verifyPathSc // resolve the path so slashes are in the right direction CmdletProviderContext cmdContext = new CmdletProviderContext(this); Collection pathInfos = SessionState.Path.GetResolvedPSPathFromPSPath(path, cmdContext); - - foreach (PathInfo pathInfo in pathInfos) + if (pathInfos.Count != 1) { - path = pathInfo.Path; + string message = StringUtil.Format(Modules.InvalidModuleManifestPath, path); + InvalidOperationException ioe = new InvalidOperationException(message); + ErrorRecord er = new ErrorRecord(ioe, "Modules_InvalidModuleManifestPath", ErrorCategory.InvalidArgument, path); + ThrowTerminatingError(er); } + path = pathInfos[0].Path; + if (!System.IO.Path.IsPathRooted(path)) { // we assume the relative path is under module scope, otherwise we will throw error anyway. diff --git a/test/powershell/engine/Module/TestModuleManifest.ps1 b/test/powershell/engine/Module/TestModuleManifest.ps1 index 6d3d12cc789..2561fbff8fc 100644 --- a/test/powershell/engine/Module/TestModuleManifest.ps1 +++ b/test/powershell/engine/Module/TestModuleManifest.ps1 @@ -1,5 +1,11 @@ Describe "Test-ModuleManifest tests" -tags "CI" { + AfterEach { + if (Test-Path testdrive:/module) { + Remove-Item -Recurse -Force testdrive:/module + } + } + It "module manifest containing paths with backslashes or forwardslashes are resolved correctly" { New-Item -ItemType Directory -Path testdrive:/module @@ -7,7 +13,47 @@ Describe "Test-ModuleManifest tests" -tags "CI" { New-Item -ItemType Directory -Path testdrive:/module/bar "" > testdrive:/module/foo/bar.psm1 "" > testdrive:/module/bar/foo.psm1 - New-ModuleManifest -NestedModules foo\bar.psm1,bar/foo.psm1 -RootModule foo\bar.psm1 -RequiredAssemblies foo/bar.psm1,bar\foo.psm1 -Path testdrive:/module/test.psd1 - Test-ModuleManifest -Path testdrive:/module/test.psd1 -ErrorAction Stop | Should BeOfType System.Management.Automation.PSModuleInfo + $testModulePath = "testdrive:/module/test.psd1" + $fileList = "foo\bar.psm1","bar/foo.psm1" + + New-ModuleManifest -NestedModules $fileList -RootModule foo\bar.psm1 -RequiredAssemblies $fileList -Path $testModulePath -TypesToProcess $fileList -FormatsToProcess $fileList -ScriptsToProcess $fileList -FileList $fileList -ModuleList $fileList + + Test-Path $testModulePath | Should Be $true + + Test-ModuleManifest -Path $testModulePath -ErrorAction Stop | Should BeOfType System.Management.Automation.PSModuleInfo + } + + It "module manifest containing missing files returns error" { + + New-Item -ItemType Directory -Path testdrive:/module + New-Item -ItemType Directory -Path testdrive:/module/foo + "" > testdrive:/module/foo/bar.psm1 + $testModulePath = "testdrive:/module/test.psd1" + + $parametersAndErrors = @{"RequiredAssemblies"="Modules_InvalidRequiredAssembliesInModuleManifest"; + "NestedModules"="Modules_InvalidNestedModuleinModuleManifest"; + "RequiredModules"="Modules_InvalidRequiredModulesinModuleManifest"; + "FileList"="Modules_InvalidFilePathinModuleManifest"; + "ModuleList"="Modules_InvalidModuleListinModuleManifest"; + "TypesToProcess"="Modules_InvalidManifest"; + "FormatsToProcess"="Modules_InvalidManifest"; + "RootModule"="Modules_InvalidManifest"; + "ScriptsToProcess"="Modules_InvalidManifest" + } + foreach ($parameter in $parametersAndExecptions.Keys) { + $args = @{$parameter = "doesnotexist.psm1"} + New-ModuleManifest -Path $testModulePath @args + Test-Path $testModulePath | Should Be $true + $errorId = $parametersAndErrors[$parameter] + + try { + Test-ModuleManifest -Path $testModulePath -ErrorAction Stop | Should BeOfType System.Management.Automation.PSModuleInfo + throw "Test-ModuleManifest did not throw $errorId" + } + catch { + $_.FullQulaifiedErrorId | Should Match "$errorId" + } + Remove-Item -Recurse -Force $testModulePath + } } } From 6d3a83cea3170af260a0ebeb582fe4b3eab5a538 Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Wed, 22 Feb 2017 14:58:11 -0800 Subject: [PATCH 3/7] changes to tests based on review feedback to make it simpler and more readable fixed typo in test that exposed issue in cmdlet for validating nestedmodules --- .../Modules/TestModuleManifestCommand.cs | 33 ++++++++----------- .../engine/Module/TestModuleManifest.ps1 | 30 ++++++++--------- 2 files changed, 26 insertions(+), 37 deletions(-) diff --git a/src/System.Management.Automation/engine/Modules/TestModuleManifestCommand.cs b/src/System.Management.Automation/engine/Modules/TestModuleManifestCommand.cs index 4f0e0586b70..16234571cc0 100644 --- a/src/System.Management.Automation/engine/Modules/TestModuleManifestCommand.cs +++ b/src/System.Management.Automation/engine/Modules/TestModuleManifestCommand.cs @@ -147,18 +147,8 @@ protected override void ProcessRecord() && !IsValidFilePath(nestedModule.Name + StringLiterals.PowerShellModuleFileExtension, module, true) && !IsValidGacAssembly(nestedModule.Name)) { - // The nested module could be dependencies. We compare if it can be loaded by loadmanifest - bool isDependency = false; - foreach (PSModuleInfo loadedNestedModule in module.NestedModules) - { - if (string.Equals(loadedNestedModule.Name, nestedModule.Name, StringComparison.OrdinalIgnoreCase)) - { - isDependency = true; - break; - } - } - - if (!isDependency) + Collection modules = GetModuleIfAvailable(nestedModule); + if (0 == modules.Count) { string errorMsg = StringUtil.Format(Modules.InvalidNestedModuleinModuleManifest, nestedModule.Name, filePath); var errorRecord = new ErrorRecord(new DirectoryNotFoundException(errorMsg), "Modules_InvalidNestedModuleinModuleManifest", @@ -288,6 +278,12 @@ private bool IsValidFilePath(string path, PSModuleInfo module, bool verifyPathSc { try { + if (!System.IO.Path.IsPathRooted(path)) + { + // we assume the relative path is under module scope, otherwise we will throw error anyway. + path = System.IO.Path.GetFullPath(module.ModuleBase + System.IO.Path.DirectorySeparatorChar + path); + } + // resolve the path so slashes are in the right direction CmdletProviderContext cmdContext = new CmdletProviderContext(this); Collection pathInfos = SessionState.Path.GetResolvedPSPathFromPSPath(path, cmdContext); @@ -298,15 +294,8 @@ private bool IsValidFilePath(string path, PSModuleInfo module, bool verifyPathSc ErrorRecord er = new ErrorRecord(ioe, "Modules_InvalidModuleManifestPath", ErrorCategory.InvalidArgument, path); ThrowTerminatingError(er); } - path = pathInfos[0].Path; - if (!System.IO.Path.IsPathRooted(path)) - { - // we assume the relative path is under module scope, otherwise we will throw error anyway. - path = System.IO.Path.GetFullPath(module.ModuleBase + System.IO.Path.DirectorySeparatorChar + path); - } - // First, we validate if the path does exist. if (!File.Exists(path) && !Directory.Exists(path)) { @@ -321,7 +310,7 @@ private bool IsValidFilePath(string path, PSModuleInfo module, bool verifyPathSc } catch (Exception exception) { - if (exception is ArgumentException || exception is ArgumentNullException || exception is NotSupportedException || exception is PathTooLongException) + if (exception is ArgumentException || exception is ArgumentNullException || exception is NotSupportedException || exception is PathTooLongException || exception is ItemNotFoundException) { return false; } @@ -337,6 +326,9 @@ private bool IsValidFilePath(string path, PSModuleInfo module, bool verifyPathSc /// private bool IsValidGacAssembly(string assemblyName) { +#if UNIX + return false; +#else string gacPath = System.Environment.GetEnvironmentVariable("windir") + "\\Microsoft.NET\\assembly"; string assemblyFile = assemblyName; string ngenAssemblyFile = assemblyName; @@ -364,6 +356,7 @@ private bool IsValidGacAssembly(string assemblyName) } return true; +#endif } } diff --git a/test/powershell/engine/Module/TestModuleManifest.ps1 b/test/powershell/engine/Module/TestModuleManifest.ps1 index 2561fbff8fc..2a24ef5789b 100644 --- a/test/powershell/engine/Module/TestModuleManifest.ps1 +++ b/test/powershell/engine/Module/TestModuleManifest.ps1 @@ -1,9 +1,9 @@ +Import-Module $PSScriptRoot\..\..\Common\Test.Helpers.psm1 + Describe "Test-ModuleManifest tests" -tags "CI" { AfterEach { - if (Test-Path testdrive:/module) { - Remove-Item -Recurse -Force testdrive:/module - } + Remove-Item -Recurse -Force -ErrorAction SilentlyContinue testdrive:/module } It "module manifest containing paths with backslashes or forwardslashes are resolved correctly" { @@ -11,8 +11,8 @@ Describe "Test-ModuleManifest tests" -tags "CI" { New-Item -ItemType Directory -Path testdrive:/module New-Item -ItemType Directory -Path testdrive:/module/foo New-Item -ItemType Directory -Path testdrive:/module/bar - "" > testdrive:/module/foo/bar.psm1 - "" > testdrive:/module/bar/foo.psm1 + Set-Content -Value "" -Path testdrive:/module/foo/bar.psm1 + Set-Content -Value "" -Path testdrive:/module/bar/foo.psm1 $testModulePath = "testdrive:/module/test.psd1" $fileList = "foo\bar.psm1","bar/foo.psm1" @@ -20,6 +20,7 @@ Describe "Test-ModuleManifest tests" -tags "CI" { Test-Path $testModulePath | Should Be $true + # use -ErrorAction Stop to cause test to fail if Test-ModuleManifest writes to error stream Test-ModuleManifest -Path $testModulePath -ErrorAction Stop | Should BeOfType System.Management.Automation.PSModuleInfo } @@ -27,7 +28,7 @@ Describe "Test-ModuleManifest tests" -tags "CI" { New-Item -ItemType Directory -Path testdrive:/module New-Item -ItemType Directory -Path testdrive:/module/foo - "" > testdrive:/module/foo/bar.psm1 + Set-Content -Value "" -Path testdrive:/module/foo/bar.psm1 $testModulePath = "testdrive:/module/test.psd1" $parametersAndErrors = @{"RequiredAssemblies"="Modules_InvalidRequiredAssembliesInModuleManifest"; @@ -37,22 +38,17 @@ Describe "Test-ModuleManifest tests" -tags "CI" { "ModuleList"="Modules_InvalidModuleListinModuleManifest"; "TypesToProcess"="Modules_InvalidManifest"; "FormatsToProcess"="Modules_InvalidManifest"; - "RootModule"="Modules_InvalidManifest"; + #"RootModule"="Modules_InvalidManifest"; "ScriptsToProcess"="Modules_InvalidManifest" } - foreach ($parameter in $parametersAndExecptions.Keys) { + foreach ($parameter in $parametersAndErrors.Keys) { + Write-Warning "Testing $parameter" $args = @{$parameter = "doesnotexist.psm1"} New-ModuleManifest -Path $testModulePath @args Test-Path $testModulePath | Should Be $true - $errorId = $parametersAndErrors[$parameter] - - try { - Test-ModuleManifest -Path $testModulePath -ErrorAction Stop | Should BeOfType System.Management.Automation.PSModuleInfo - throw "Test-ModuleManifest did not throw $errorId" - } - catch { - $_.FullQulaifiedErrorId | Should Match "$errorId" - } + [string]$errorId = $parametersAndErrors[$parameter] + ",Microsoft.PowerShell.Commands.TestModuleManifestCommand" + + { Test-ModuleManifest -Path $testModulePath -ErrorAction Stop } | ShouldBeErrorId $errorId Remove-Item -Recurse -Force $testModulePath } } From 9ba78951a51b30efc1815cb0f39ecc77bd359c05 Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Thu, 23 Feb 2017 13:11:40 -0800 Subject: [PATCH 4/7] fixed issue with validation of rootmodule used TestCases feature of Pester for tests added tests for valid and invalid rootmodules --- .../Modules/TestModuleManifestCommand.cs | 16 ++++ .../engine/Module/TestModuleManifest.ps1 | 86 ++++++++++++++----- 2 files changed, 80 insertions(+), 22 deletions(-) diff --git a/src/System.Management.Automation/engine/Modules/TestModuleManifestCommand.cs b/src/System.Management.Automation/engine/Modules/TestModuleManifestCommand.cs index 16234571cc0..b82e9effe71 100644 --- a/src/System.Management.Automation/engine/Modules/TestModuleManifestCommand.cs +++ b/src/System.Management.Automation/engine/Modules/TestModuleManifestCommand.cs @@ -131,6 +131,22 @@ protected override void ProcessRecord() } } + //RootModule can be null, empty string or point to a valid .psm1, or .dll. Anything else is invalid. + if (module.RootModule != null && module.RootModule != "") + { + string rootModuleExt = System.IO.Path.GetExtension(module.RootModule); + if ((!IsValidFilePath(module.RootModule, module, true) && !IsValidGacAssembly(module.RootModule)) || + (!rootModuleExt.Equals(StringLiterals.PowerShellModuleFileExtension, StringComparison.OrdinalIgnoreCase) && + !rootModuleExt.Equals(".dll", StringComparison.OrdinalIgnoreCase)) + ) + { + string errorMsg = StringUtil.Format(Modules.InvalidModuleManifest, module.RootModule, filePath); + var errorRecord = new ErrorRecord(new ArgumentException(errorMsg), "Modules_InvalidRootModuleInModuleManifest", + ErrorCategory.InvalidArgument, _path); + WriteError(errorRecord); + } + } + Hashtable data = null; Hashtable localizedData = null; bool containerErrors = false; diff --git a/test/powershell/engine/Module/TestModuleManifest.ps1 b/test/powershell/engine/Module/TestModuleManifest.ps1 index 2a24ef5789b..016922548c2 100644 --- a/test/powershell/engine/Module/TestModuleManifest.ps1 +++ b/test/powershell/engine/Module/TestModuleManifest.ps1 @@ -11,8 +11,8 @@ Describe "Test-ModuleManifest tests" -tags "CI" { New-Item -ItemType Directory -Path testdrive:/module New-Item -ItemType Directory -Path testdrive:/module/foo New-Item -ItemType Directory -Path testdrive:/module/bar - Set-Content -Value "" -Path testdrive:/module/foo/bar.psm1 - Set-Content -Value "" -Path testdrive:/module/bar/foo.psm1 + New-Item -ItemType File -Path testdrive:/module/foo/bar.psm1 + New-Item -ItemType File -Path testdrive:/module/bar/foo.psm1 $testModulePath = "testdrive:/module/test.psd1" $fileList = "foo\bar.psm1","bar/foo.psm1" @@ -24,32 +24,74 @@ Describe "Test-ModuleManifest tests" -tags "CI" { Test-ModuleManifest -Path $testModulePath -ErrorAction Stop | Should BeOfType System.Management.Automation.PSModuleInfo } - It "module manifest containing missing files returns error" { + It "module manifest containing missing files returns error" -TestCases ` + @{parameter = "RequiredAssemblies"; error = "Modules_InvalidRequiredAssembliesInModuleManifest"}, + @{parameter = "NestedModules"; error = "Modules_InvalidNestedModuleinModuleManifest"}, + @{parameter = "RequiredModules"; error = "Modules_InvalidRequiredModulesinModuleManifest"}, + @{parameter = "FileList"; error = "Modules_InvalidFilePathinModuleManifest"}, + @{parameter = "ModuleList"; error = "Modules_InvalidModuleListinModuleManifest"}, + @{parameter = "TypesToProcess"; error = "Modules_InvalidManifest"}, + @{parameter = "FormatsToProcess"; error = "Modules_InvalidManifest"}, + @{parameter = "RootModule"; error = "Modules_InvalidRootModuleInModuleManifest"}, + @{parameter = "ScriptsToProcess"; error = "Modules_InvalidManifest"} { + + param ($parameter, $error) New-Item -ItemType Directory -Path testdrive:/module New-Item -ItemType Directory -Path testdrive:/module/foo - Set-Content -Value "" -Path testdrive:/module/foo/bar.psm1 + New-Item -ItemType File -Path testdrive:/module/foo/bar.psm1 $testModulePath = "testdrive:/module/test.psd1" - $parametersAndErrors = @{"RequiredAssemblies"="Modules_InvalidRequiredAssembliesInModuleManifest"; - "NestedModules"="Modules_InvalidNestedModuleinModuleManifest"; - "RequiredModules"="Modules_InvalidRequiredModulesinModuleManifest"; - "FileList"="Modules_InvalidFilePathinModuleManifest"; - "ModuleList"="Modules_InvalidModuleListinModuleManifest"; - "TypesToProcess"="Modules_InvalidManifest"; - "FormatsToProcess"="Modules_InvalidManifest"; - #"RootModule"="Modules_InvalidManifest"; - "ScriptsToProcess"="Modules_InvalidManifest" + $args = @{$parameter = "doesnotexist.psm1"} + New-ModuleManifest -Path $testModulePath @args + Test-Path $testModulePath | Should Be $true + [string]$errorId = "$error,Microsoft.PowerShell.Commands.TestModuleManifestCommand" + + { Test-ModuleManifest -Path $testModulePath -ErrorAction Stop } | ShouldBeErrorId $errorId + } + + It "module manifest containing valid rootmodule succeeds" -TestCases ` + @{rootModuleValue = $null}, + @{rootModuleValue = ""}, + @{rootModuleValue = "foo.psm1"}, + @{rootModuleValue = "foo.dll"} { + + param($rootModuleValue) + + New-Item -ItemType Directory -Path testdrive:/module + $testModulePath = "testdrive:/module/test.psd1" + + if ($rootModuleValue -ne $null -and $rootModuleValue -ne "") + { + New-Item -ItemType File -Path testdrive:/module/$rootModuleValue } - foreach ($parameter in $parametersAndErrors.Keys) { - Write-Warning "Testing $parameter" - $args = @{$parameter = "doesnotexist.psm1"} - New-ModuleManifest -Path $testModulePath @args - Test-Path $testModulePath | Should Be $true - [string]$errorId = $parametersAndErrors[$parameter] + ",Microsoft.PowerShell.Commands.TestModuleManifestCommand" - - { Test-ModuleManifest -Path $testModulePath -ErrorAction Stop } | ShouldBeErrorId $errorId - Remove-Item -Recurse -Force $testModulePath + New-ModuleManifest -Path $testModulePath -RootModule $rootModuleValue + Test-Path $testModulePath | Should Be $true + $moduleManifest = Test-ModuleManifest -Path $testModulePath -ErrorAction Stop + $moduleManifest | Should BeOfType System.Management.Automation.PSModuleInfo + if ($rootModuleValue -eq $null -or $rootModuleValue -eq "") { + $moduleManifest.RootModule | Should BeNullOrEmpty } + else { + $moduleManifest.RootModule | Should Be $rootModuleValue + } + } + + It "module manifest containing invalid rootmodule returns error" -TestCases ` + @{rootModuleValue = "foo.psd1"; error = "Modules_InvalidManifest"}, + @{rootModuleValue = "doesnotexist.psm1"; error = "Modules_InvalidRootModuleInModuleManifest"} { + + param($rootModuleValue, $error) + + New-Item -ItemType Directory -Path testdrive:/module + $testModulePath = "testdrive:/module/test.psd1" + + if ($rootModuleValue -ne "doesnotexist.psm1") + { + New-Item -ItemType File -Path testdrive:/module/$rootModuleValue + } + New-ModuleManifest -Path $testModulePath -RootModule $rootModuleValue + Test-Path $testModulePath | Should Be $true + { Test-ModuleManifest -Path $testModulePath -ErrorAction Stop } | ShouldBeErrorId "$error,Microsoft.PowerShell.Commands.TestModuleManifestCommand" } } From c915ff07dcf95f1c2f4d4c40aeb31b1482c227d6 Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Fri, 24 Feb 2017 11:15:11 -0800 Subject: [PATCH 5/7] split tests to make them more readable, removed unnecessary checks for manifest creation --- .../engine/Module/TestModuleManifest.ps1 | 54 +++++++++++-------- 1 file changed, 32 insertions(+), 22 deletions(-) diff --git a/test/powershell/engine/Module/TestModuleManifest.ps1 b/test/powershell/engine/Module/TestModuleManifest.ps1 index 016922548c2..72305d91d38 100644 --- a/test/powershell/engine/Module/TestModuleManifest.ps1 +++ b/test/powershell/engine/Module/TestModuleManifest.ps1 @@ -44,15 +44,12 @@ Describe "Test-ModuleManifest tests" -tags "CI" { $args = @{$parameter = "doesnotexist.psm1"} New-ModuleManifest -Path $testModulePath @args - Test-Path $testModulePath | Should Be $true [string]$errorId = "$error,Microsoft.PowerShell.Commands.TestModuleManifestCommand" { Test-ModuleManifest -Path $testModulePath -ErrorAction Stop } | ShouldBeErrorId $errorId } - It "module manifest containing valid rootmodule succeeds" -TestCases ` - @{rootModuleValue = $null}, - @{rootModuleValue = ""}, + It "module manifest containing valid rootmodule file succeeds" -TestCases ` @{rootModuleValue = "foo.psm1"}, @{rootModuleValue = "foo.dll"} { @@ -61,37 +58,50 @@ Describe "Test-ModuleManifest tests" -tags "CI" { New-Item -ItemType Directory -Path testdrive:/module $testModulePath = "testdrive:/module/test.psd1" - if ($rootModuleValue -ne $null -and $rootModuleValue -ne "") - { - New-Item -ItemType File -Path testdrive:/module/$rootModuleValue - } + New-Item -ItemType File -Path testdrive:/module/$rootModuleValue + New-ModuleManifest -Path $testModulePath -RootModule $rootModuleValue + $moduleManifest = Test-ModuleManifest -Path $testModulePath -ErrorAction Stop + $moduleManifest | Should BeOfType System.Management.Automation.PSModuleInfo + $moduleManifest.RootModule | Should Be $rootModuleValue + } + + It "module manifest containing empty rootmodule succeeds" -TestCases ` + @{rootModuleValue = $null}, + @{rootModuleValue = ""} { + + param($rootModuleValue) + + New-Item -ItemType Directory -Path testdrive:/module + $testModulePath = "testdrive:/module/test.psd1" + New-ModuleManifest -Path $testModulePath -RootModule $rootModuleValue - Test-Path $testModulePath | Should Be $true $moduleManifest = Test-ModuleManifest -Path $testModulePath -ErrorAction Stop $moduleManifest | Should BeOfType System.Management.Automation.PSModuleInfo - if ($rootModuleValue -eq $null -or $rootModuleValue -eq "") { - $moduleManifest.RootModule | Should BeNullOrEmpty - } - else { - $moduleManifest.RootModule | Should Be $rootModuleValue - } + $moduleManifest.RootModule | Should BeNullOrEmpty } It "module manifest containing invalid rootmodule returns error" -TestCases ` - @{rootModuleValue = "foo.psd1"; error = "Modules_InvalidManifest"}, - @{rootModuleValue = "doesnotexist.psm1"; error = "Modules_InvalidRootModuleInModuleManifest"} { + @{rootModuleValue = "foo.psd1"; error = "Modules_InvalidManifest"} { param($rootModuleValue, $error) + $testModulePath = "testdrive:/module/test.psd1" New-Item -ItemType Directory -Path testdrive:/module + New-Item -ItemType File -Path testdrive:/module/$rootModuleValue + + New-ModuleManifest -Path $testModulePath -RootModule $rootModuleValue + { Test-ModuleManifest -Path $testModulePath -ErrorAction Stop } | ShouldBeErrorId "$error,Microsoft.PowerShell.Commands.TestModuleManifestCommand" + } + + It "module manifest containing non-existing rootmodule returns error" -TestCases ` + @{rootModuleValue = "doesnotexist.psm1"; error = "Modules_InvalidRootModuleInModuleManifest"} { + + param($rootModuleValue, $error) + $testModulePath = "testdrive:/module/test.psd1" + New-Item -ItemType Directory -Path testdrive:/module - if ($rootModuleValue -ne "doesnotexist.psm1") - { - New-Item -ItemType File -Path testdrive:/module/$rootModuleValue - } New-ModuleManifest -Path $testModulePath -RootModule $rootModuleValue - Test-Path $testModulePath | Should Be $true { Test-ModuleManifest -Path $testModulePath -ErrorAction Stop } | ShouldBeErrorId "$error,Microsoft.PowerShell.Commands.TestModuleManifestCommand" } } From ae5ab4c7ed193e2602f199674b101a8f261d2f9a Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Tue, 28 Feb 2017 14:08:43 -0800 Subject: [PATCH 6/7] added check for cdxml and xaml rootmodules which are allowed added tests updated InvalidModuleManifest error message to be about module manifests --- .../engine/Modules/TestModuleManifestCommand.cs | 6 ++++-- .../resources/Modules.resx | 3 +-- .../engine/Module/TestModuleManifest.ps1 | 16 +++++++++++++++- 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/src/System.Management.Automation/engine/Modules/TestModuleManifestCommand.cs b/src/System.Management.Automation/engine/Modules/TestModuleManifestCommand.cs index b82e9effe71..8115c840a31 100644 --- a/src/System.Management.Automation/engine/Modules/TestModuleManifestCommand.cs +++ b/src/System.Management.Automation/engine/Modules/TestModuleManifestCommand.cs @@ -131,13 +131,15 @@ protected override void ProcessRecord() } } - //RootModule can be null, empty string or point to a valid .psm1, or .dll. Anything else is invalid. + //RootModule can be null, empty string or point to a valid .psm1, , .cdxml, .xaml or .dll. Anything else is invalid. if (module.RootModule != null && module.RootModule != "") { string rootModuleExt = System.IO.Path.GetExtension(module.RootModule); if ((!IsValidFilePath(module.RootModule, module, true) && !IsValidGacAssembly(module.RootModule)) || (!rootModuleExt.Equals(StringLiterals.PowerShellModuleFileExtension, StringComparison.OrdinalIgnoreCase) && - !rootModuleExt.Equals(".dll", StringComparison.OrdinalIgnoreCase)) + !rootModuleExt.Equals(".dll", StringComparison.OrdinalIgnoreCase) && + !rootModuleExt.Equals(".cdxml", StringComparison.OrdinalIgnoreCase) && + !rootModuleExt.Equals(".xaml", StringComparison.OrdinalIgnoreCase)) ) { string errorMsg = StringUtil.Format(Modules.InvalidModuleManifest, module.RootModule, filePath); diff --git a/src/System.Management.Automation/resources/Modules.resx b/src/System.Management.Automation/resources/Modules.resx index c8febde1990..782ab5e99d6 100644 --- a/src/System.Management.Automation/resources/Modules.resx +++ b/src/System.Management.Automation/resources/Modules.resx @@ -154,8 +154,7 @@ No custom object was returned for module '{0}' because the -AsCustomObject parameter can only be used with script modules. - The module manifest '{0}' could not be processed because it is not a valid Windows PowerShell restricted language file. Remove the elements that are not permitted by the restricted language: -{1} + The module manifest '{0}' could not be processed because it is not a valid PowerShell module manifest file. Remove the elements that are not permitted: {1} Processing the module manifest file '{0}' did not result in a valid manifest object. Update the file to contain a valid Windows PowerShell module manifest. A valid manifest can be created using the New-ModuleManifest cmdlet. diff --git a/test/powershell/engine/Module/TestModuleManifest.ps1 b/test/powershell/engine/Module/TestModuleManifest.ps1 index 72305d91d38..99d73d709c0 100644 --- a/test/powershell/engine/Module/TestModuleManifest.ps1 +++ b/test/powershell/engine/Module/TestModuleManifest.ps1 @@ -49,7 +49,7 @@ Describe "Test-ModuleManifest tests" -tags "CI" { { Test-ModuleManifest -Path $testModulePath -ErrorAction Stop } | ShouldBeErrorId $errorId } - It "module manifest containing valid rootmodule file succeeds" -TestCases ` + It "module manifest containing valid unprocessed rootmodule file type succeeds" -TestCases ` @{rootModuleValue = "foo.psm1"}, @{rootModuleValue = "foo.dll"} { @@ -65,6 +65,20 @@ Describe "Test-ModuleManifest tests" -tags "CI" { $moduleManifest.RootModule | Should Be $rootModuleValue } + It "module manifest containing valid processed empty rootmodule file type fails" -TestCases ` + @{rootModuleValue = "foo.cdxml"; error = "System.Xml.XmlException"}, # fails when cmdlet tries to read it as XML + @{rootModuleValue = "foo.xaml"; error = "NotSupported"} { # not supported on PowerShell Core + + param($rootModuleValue, $error) + + New-Item -ItemType Directory -Path testdrive:/module + $testModulePath = "testdrive:/module/test.psd1" + + New-Item -ItemType File -Path testdrive:/module/$rootModuleValue + New-ModuleManifest -Path $testModulePath -RootModule $rootModuleValue + { Test-ModuleManifest -Path $testModulePath -ErrorAction Stop } | ShouldBeErrorId "$error,Microsoft.PowerShell.Commands.TestModuleManifestCommand" + } + It "module manifest containing empty rootmodule succeeds" -TestCases ` @{rootModuleValue = $null}, @{rootModuleValue = ""} { From cb5ca175e90f1832e80ff68a78625d10cab9a444 Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Tue, 28 Feb 2017 14:08:43 -0800 Subject: [PATCH 7/7] added check for cdxml and xaml rootmodules which are allowed added tests updated InvalidModuleManifest error message to be about module manifests changed -TestCases usage to parens than backtick for readability --- .../engine/Modules/TestModuleManifestCommand.cs | 6 ++++-- .../resources/Modules.resx | 3 +-- .../engine/Module/TestModuleManifest.ps1 | 16 +++++++++++++++- 3 files changed, 20 insertions(+), 5 deletions(-) diff --git a/src/System.Management.Automation/engine/Modules/TestModuleManifestCommand.cs b/src/System.Management.Automation/engine/Modules/TestModuleManifestCommand.cs index b82e9effe71..8115c840a31 100644 --- a/src/System.Management.Automation/engine/Modules/TestModuleManifestCommand.cs +++ b/src/System.Management.Automation/engine/Modules/TestModuleManifestCommand.cs @@ -131,13 +131,15 @@ protected override void ProcessRecord() } } - //RootModule can be null, empty string or point to a valid .psm1, or .dll. Anything else is invalid. + //RootModule can be null, empty string or point to a valid .psm1, , .cdxml, .xaml or .dll. Anything else is invalid. if (module.RootModule != null && module.RootModule != "") { string rootModuleExt = System.IO.Path.GetExtension(module.RootModule); if ((!IsValidFilePath(module.RootModule, module, true) && !IsValidGacAssembly(module.RootModule)) || (!rootModuleExt.Equals(StringLiterals.PowerShellModuleFileExtension, StringComparison.OrdinalIgnoreCase) && - !rootModuleExt.Equals(".dll", StringComparison.OrdinalIgnoreCase)) + !rootModuleExt.Equals(".dll", StringComparison.OrdinalIgnoreCase) && + !rootModuleExt.Equals(".cdxml", StringComparison.OrdinalIgnoreCase) && + !rootModuleExt.Equals(".xaml", StringComparison.OrdinalIgnoreCase)) ) { string errorMsg = StringUtil.Format(Modules.InvalidModuleManifest, module.RootModule, filePath); diff --git a/src/System.Management.Automation/resources/Modules.resx b/src/System.Management.Automation/resources/Modules.resx index c8febde1990..782ab5e99d6 100644 --- a/src/System.Management.Automation/resources/Modules.resx +++ b/src/System.Management.Automation/resources/Modules.resx @@ -154,8 +154,7 @@ No custom object was returned for module '{0}' because the -AsCustomObject parameter can only be used with script modules. - The module manifest '{0}' could not be processed because it is not a valid Windows PowerShell restricted language file. Remove the elements that are not permitted by the restricted language: -{1} + The module manifest '{0}' could not be processed because it is not a valid PowerShell module manifest file. Remove the elements that are not permitted: {1} Processing the module manifest file '{0}' did not result in a valid manifest object. Update the file to contain a valid Windows PowerShell module manifest. A valid manifest can be created using the New-ModuleManifest cmdlet. diff --git a/test/powershell/engine/Module/TestModuleManifest.ps1 b/test/powershell/engine/Module/TestModuleManifest.ps1 index 72305d91d38..99d73d709c0 100644 --- a/test/powershell/engine/Module/TestModuleManifest.ps1 +++ b/test/powershell/engine/Module/TestModuleManifest.ps1 @@ -49,7 +49,7 @@ Describe "Test-ModuleManifest tests" -tags "CI" { { Test-ModuleManifest -Path $testModulePath -ErrorAction Stop } | ShouldBeErrorId $errorId } - It "module manifest containing valid rootmodule file succeeds" -TestCases ` + It "module manifest containing valid unprocessed rootmodule file type succeeds" -TestCases ` @{rootModuleValue = "foo.psm1"}, @{rootModuleValue = "foo.dll"} { @@ -65,6 +65,20 @@ Describe "Test-ModuleManifest tests" -tags "CI" { $moduleManifest.RootModule | Should Be $rootModuleValue } + It "module manifest containing valid processed empty rootmodule file type fails" -TestCases ` + @{rootModuleValue = "foo.cdxml"; error = "System.Xml.XmlException"}, # fails when cmdlet tries to read it as XML + @{rootModuleValue = "foo.xaml"; error = "NotSupported"} { # not supported on PowerShell Core + + param($rootModuleValue, $error) + + New-Item -ItemType Directory -Path testdrive:/module + $testModulePath = "testdrive:/module/test.psd1" + + New-Item -ItemType File -Path testdrive:/module/$rootModuleValue + New-ModuleManifest -Path $testModulePath -RootModule $rootModuleValue + { Test-ModuleManifest -Path $testModulePath -ErrorAction Stop } | ShouldBeErrorId "$error,Microsoft.PowerShell.Commands.TestModuleManifestCommand" + } + It "module manifest containing empty rootmodule succeeds" -TestCases ` @{rootModuleValue = $null}, @{rootModuleValue = ""} {