diff --git a/CHANGELOG.md b/CHANGELOG.md index 3400c1f..4a1891b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -172,5 +172,10 @@ The format is based on - Fix a `FormatException` in the cmdlets for a path with braces, such as `C:\Data\{Archive}`: their messages formatted the path a second time ([#3](https://github.com/raandree/NTFSSecurity/issues/3)) +- Fix a `NullReferenceException` in every cmdlet when a variable named + `PWD` in the scope of the caller, such as a loop variable, hid the + automatic variable; the cmdlets now read the current location from the + session, and only for a relative path + ([#86](https://github.com/raandree/NTFSSecurity/issues/86)) [Unreleased]: https://github.com/raandree/NTFSSecurity/compare/4.2.6...HEAD diff --git a/NTFSSecurity/BaseCmdlets.cs b/NTFSSecurity/BaseCmdlets.cs index 2aaf5da..36aad2f 100644 --- a/NTFSSecurity/BaseCmdlets.cs +++ b/NTFSSecurity/BaseCmdlets.cs @@ -26,32 +26,7 @@ namespace NTFSSecurity #region GetFileSystemInfo protected System.IO.FileSystemInfo GetFileSystemInfo(string path) { - string currentLocation = GetVariableValue("PWD").ToString(); - - if (path == ".") - { - path = currentLocation; - } - if (path.StartsWith("..")) - { - path = System.IO.Path.Combine( - string.Join("\\", currentLocation.Split('\\').Take(currentLocation.Split('\\').Count() - path.Split('\\').Count(s => s == "..")).ToArray()), - string.Join("\\", path.Split('\\').Where(e => e != "..").ToArray())); - } - else if (path.StartsWith(".")) - { - //combine . and .\path\subpath - path = System.IO.Path.Combine(currentLocation, path.Substring(2)); - } - else if (path.StartsWith("\\")) - { - //do nothing - } - else - { - ////combine . and \path\subpath or path\subpath - path = System.IO.Path.Combine(currentLocation, path.Substring(0)); - } + path = GetRelativePath(path); if (System.IO.File.Exists(path)) { @@ -114,18 +89,17 @@ namespace NTFSSecurity #region GetRelativePath protected string GetRelativePath(string path) { - string currentLocation = GetVariableValue("PWD").ToString(); - if (string.IsNullOrEmpty(path)) { - path = currentLocation; + path = GetCurrentLocation(); } else if (path == ".") { - path = currentLocation; + path = GetCurrentLocation(); } else if (path.StartsWith("..")) { + var currentLocation = GetCurrentLocation(); path = System.IO.Path.Combine( string.Join("\\", currentLocation.Split('\\').Take(currentLocation.Split('\\').Count() - path.Split('\\').Count(s => s == "..")).ToArray()), string.Join("\\", path.Split('\\').Where(e => e != "..").ToArray())); @@ -133,20 +107,28 @@ namespace NTFSSecurity else if (path.StartsWith(".")) { //combine . and .\path\subpath - path = System.IO.Path.Combine(currentLocation, path.Substring(2)); + path = System.IO.Path.Combine(GetCurrentLocation(), path.Substring(2)); } - else if (path.StartsWith("\\")) + else if (path.StartsWith("\\") || System.IO.Path.IsPathRooted(path)) { - //do nothing + //an absolute path needs no location } else { - ////combine . and \path\subpath or path\subpath - path = System.IO.Path.Combine(currentLocation, path); + ////combine . and path\subpath + path = System.IO.Path.Combine(GetCurrentLocation(), path); } return path; } + + // Read from the session state, not from $PWD: a variable named PWD in the caller's scope shadows the + // automatic variable and can be $null (#86). In a location of another provider, such as the registry, this + // is the last file system location. + private string GetCurrentLocation() + { + return SessionState.Path.CurrentFileSystemLocation.ProviderPath; + } #endregion #region InvokeAsOwner @@ -353,4 +335,4 @@ namespace NTFSSecurity base.WriteDebug(args == null || args.Length == 0 ? text : string.Format(text, args)); } } -} \ No newline at end of file +} diff --git a/Tests/Owner.Tests.ps1 b/Tests/Owner.Tests.ps1 index 9c5d993..7ad1608 100644 --- a/Tests/Owner.Tests.ps1 +++ b/Tests/Owner.Tests.ps1 @@ -4,6 +4,9 @@ [Diagnostics.CodeAnalysis.SuppressMessageAttribute( 'PSUseDeclaredVarsMoreThanAssignments', '', Justification = 'Pester shares variables between blocks.' )] +[Diagnostics.CodeAnalysis.SuppressMessageAttribute( + 'PSAvoidAssignmentToAutomaticVariable', '', Justification = 'A test shadows $PWD on purpose (#86).' +)] param () BeforeDiscovery { @@ -52,3 +55,33 @@ Describe 'Get-NTFSOwner' { } } } + +Describe 'Current location' { + BeforeAll { + function Invoke-WithShadowedPwd { + param ([scriptblock] $Command) + + $PWD = $null + & $Command + } + } + + # Before 5.0.0, a variable named PWD in the scope of the caller, such as a loop variable, made every cmdlet + # fail with a NullReferenceException, also for an absolute path (#86). + It 'Should ignore a variable named PWD for an absolute path' { + $file = New-TestSandboxItem -Sandbox $sandbox -Name 'Pwd' + + $result = Invoke-WithShadowedPwd -Command { Get-NTFSOwner -Path $file -ErrorAction Stop } + + $result.FullName | Should -Be $file + } + + It 'Should resolve a relative path against the current location despite a variable named PWD' { + $file = New-TestSandboxItem -Sandbox $sandbox -Name 'PwdRelative' + $name = Split-Path -Path $file -Leaf + + $result = Invoke-WithShadowedPwd -Command { Get-NTFSOwner -Path $name -ErrorAction Stop } + + $result.FullName | Should -Be $file + } +}