diff --git a/src/System.Management.Automation/engine/SessionState.cs b/src/System.Management.Automation/engine/SessionState.cs index 937bebc061..516828c743 100644 --- a/src/System.Management.Automation/engine/SessionState.cs +++ b/src/System.Management.Automation/engine/SessionState.cs @@ -7,6 +7,7 @@ using System.Collections; using System.Collections.Generic; using System.Collections.ObjectModel; using System.Diagnostics; +using System.Management.Automation.Internal; using System.Management.Automation.Runspaces; using Dbg = System.Management.Automation; using System.Diagnostics.CodeAnalysis; @@ -66,6 +67,10 @@ namespace System.Management.Automation _workingLocationStack = new Dictionary>(StringComparer.OrdinalIgnoreCase); + // Conservative choice to limit the Set-Location history in order to limit memory impact in case of a regression. + const int locationHistoryLimit = 20; + _SetLocationHistory = new BoundedStack(locationHistoryLimit); + GlobalScope = new SessionStateScope(null); ModuleScope = GlobalScope; _currentScope = GlobalScope; diff --git a/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs b/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs index c05460c3bb..60f9f8affa 100644 --- a/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs +++ b/src/System.Management.Automation/engine/SessionStateLocationAPIs.cs @@ -6,6 +6,7 @@ using System.Collections.Generic; using System.Collections.ObjectModel; using System.IO; +using System.Management.Automation.Internal; using System.Management.Automation.Provider; using Dbg = System.Management.Automation; @@ -230,10 +231,28 @@ namespace System.Management.Automation ProviderInfo provider = null; string providerId = null; + // Replace path with last working directory when '-' was passed. + bool pushNextLocation = true; + if (originalPath.Equals("-", StringComparison.OrdinalIgnoreCase)) + { + if (_SetLocationHistory.Count <= 0) + { + throw new InvalidOperationException(SessionStateStrings.SetContentToLastLocationWhenHistoryIsEmpty); + } + var previousLocation = _SetLocationHistory.Pop(); + path = previousLocation.Path; + pushNextLocation = false; + } + + if (pushNextLocation) + { + var newPushPathInfo = GetNewPushPathInfo(); + _SetLocationHistory.Push(newPushPathInfo); + } + PSDriveInfo previousWorkingDrive = CurrentDrive; // First check to see if the path is a home path - if (LocationGlobber.IsHomePath(path)) { path = Globber.GetHomeRelativePath(path); @@ -783,6 +802,11 @@ namespace System.Management.Automation #region push-Pop current working directory + /// + /// A bounded stack for the location history of Set-Location + /// + private BoundedStack _SetLocationHistory; + /// /// A stack of the most recently pushed locations /// @@ -811,8 +835,23 @@ namespace System.Management.Automation stackName = _defaultStackName; } - // Create a new instance of the directory/drive pair + // Get the location stack from the hashtable + Stack locationStack = null; + if (!_workingLocationStack.TryGetValue(stackName, out locationStack)) + { + locationStack = new Stack(); + _workingLocationStack[stackName] = locationStack; + } + + // Push the directory/drive pair onto the stack + var newPushPathInfo = GetNewPushPathInfo(); + locationStack.Push(newPushPathInfo); + } + + private PathInfo GetNewPushPathInfo() + { + // Create a new instance of the directory/drive pair ProviderInfo provider = CurrentDrive.Provider; string mshQualifiedPath = LocationGlobber.GetMshQualifiedPath(CurrentDrive.CurrentLocation, CurrentDrive); @@ -829,19 +868,7 @@ namespace System.Management.Automation CurrentDrive.Name, mshQualifiedPath); - // Get the location stack from the hashtable - - Stack locationStack = null; - - if (!_workingLocationStack.TryGetValue(stackName, out locationStack)) - { - locationStack = new Stack(); - _workingLocationStack[stackName] = locationStack; - } - - // Push the directory/drive pair onto the stack - - locationStack.Push(newPushLocation); + return newPushLocation; } /// diff --git a/src/System.Management.Automation/engine/Utils.cs b/src/System.Management.Automation/engine/Utils.cs index 9f735d26ef..2cc4b9602c 100644 --- a/src/System.Management.Automation/engine/Utils.cs +++ b/src/System.Management.Automation/engine/Utils.cs @@ -1459,4 +1459,53 @@ namespace System.Management.Automation.Internal } } } + + /// + /// An bounded stack based on a linked list. + /// + internal class BoundedStack : LinkedList + { + private readonly int _capacity; + + /// + /// Lazy initialisation, i.e. it sets only its limit but does not allocate the memory for the given capacity. + /// + /// + internal BoundedStack(int capacity) + { + _capacity = capacity; + } + + /// + /// Push item. + /// + /// + internal void Push(T item) + { + this.AddFirst(item); + + if(this.Count > _capacity) + { + this.RemoveLast(); + } + } + + /// + /// Pop item. + /// + /// + internal T Pop() + { + var item = this.First.Value; + try + { + this.RemoveFirst(); + } + catch (InvalidOperationException) + { + throw new InvalidOperationException(SessionStateStrings.BoundedStackIsEmpty); + } + return item; + } + } } diff --git a/src/System.Management.Automation/resources/SessionStateStrings.resx b/src/System.Management.Automation/resources/SessionStateStrings.resx index d7a7afcec7..7fe2a4fee8 100644 --- a/src/System.Management.Automation/resources/SessionStateStrings.resx +++ b/src/System.Management.Automation/resources/SessionStateStrings.resx @@ -279,6 +279,12 @@ The dynamic parameters for the GetContentWriter operation cannot be retrieved for the '{0}' provider for path '{1}'. {2} + + There is no location history left to navigate backwards. + + + The BoundedStack is empty. + Attempting to perform the ClearContent operation on the '{0}' provider failed for path '{1}'. {2} diff --git a/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Location.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Location.Tests.ps1 index 74f59ffa60..a69c4677df 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Location.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Management/Set-Location.Tests.ps1 @@ -93,4 +93,38 @@ Describe "Set-Location" -Tags "CI" { Remove-PSDrive -Name 'Z' } } + + Context 'Set-Location with last location history' { + + It 'Should go to last location when specifying minus as a path' { + $initialLocation = Get-Location + Set-Location ([System.IO.Path]::GetTempPath()) + Set-Location - + (Get-Location).Path | Should -Be ($initialLocation).Path + } + + It 'Should go back to previous locations when specifying minus twice' { + $initialLocation = (Get-Location).Path + Set-Location ([System.IO.Path]::GetTempPath()) + $firstLocationChange = (Get-Location).Path + Set-Location ([System.Environment]::GetFolderPath("user")) + Set-Location - + (Get-Location).Path | Should -Be $firstLocationChange + Set-Location - + (Get-Location).Path | Should -Be $initialLocation + } + + It 'Location History is limited' { + $initialLocation = (Get-Location).Path + $maximumLocationHistory = 20 + foreach ($i in 1..$maximumLocationHistory) { + Set-Location ([System.IO.Path]::GetTempPath()) + } + foreach ($i in 1..$maximumLocationHistory) { + Set-Location - + } + (Get-Location).Path | Should Be $initialLocation + { Set-Location - } | Should -Throw -ErrorId 'System.InvalidOperationException,Microsoft.PowerShell.Commands.SetLocationCommand' + } + } }