mirror of
https://github.com/aphrody-labs/PowerShell.git
synced 2026-10-14 05:39:11 +00:00
Fix PATH caching in CommandDiscovery code (#27809)
The method `GetLookupDirectoryPaths` always returns the cached instance of path collection. However, the returned collection gets mutated in `CommandPathSearch.ResolveCurrentDirectoryInLookupPaths` to resolve the relative paths such as `.\tools` based on the user's current working directory `$PWD`, so for example, `.\tools` gets replaced with `cwd-1\tools` in the cached instance. Then, when the user changes to a different working directory `cwd-2`, command discovery won't find the executable or ps1 script under `cwd-2/tools` as expected, but those executables under `cwd-1\tools` will always be discoverable no matter what the `$PWD` is. That behavior is incorrect. This pull request improves the caching logic of `GetLookupDirectoryPaths` and makes it return a copy of the path collection, so the mutation happens to the returned copy, and the cached instance is kept intact. So, for every command search, the `CommandPathSearch` will resolve relative paths against the `$PWD` that the user is located at that time.
This commit is contained in:
Dongbo Wang authored and GitHub committed
1 parent
f83ccddf80
commit
bac40c877b
2 files changed
+84
-46
No files matched your search
@@ -1199,81 +1199,68 @@ namespace System.Management.Automation
|
||||
/// </remarks>
|
||||
internal LookupPathCollection GetLookupDirectoryPaths()
|
||||
{
|
||||
LookupPathCollection result = new LookupPathCollection();
|
||||
|
||||
string path = Environment.GetEnvironmentVariable("PATH");
|
||||
discoveryTracer.WriteLine("PATH: {0}", path);
|
||||
|
||||
discoveryTracer.WriteLine(
|
||||
"PATH: {0}",
|
||||
path);
|
||||
|
||||
bool isPathCacheValid =
|
||||
path != null &&
|
||||
string.Equals(_pathCacheKey, path, StringComparison.OrdinalIgnoreCase) &&
|
||||
_cachedPath != null;
|
||||
bool isPathCacheValid = _cachedLookupPaths is not null
|
||||
&& string.Equals(_pathCacheKey, path, StringComparison.OrdinalIgnoreCase);
|
||||
|
||||
if (!isPathCacheValid)
|
||||
{
|
||||
// Reset the cached lookup paths
|
||||
_pathCacheKey = path;
|
||||
_cachedLookupPaths = null;
|
||||
|
||||
// Tokenize the path and cache it
|
||||
|
||||
_pathCacheKey = path;
|
||||
|
||||
if (_pathCacheKey != null)
|
||||
if (string.IsNullOrEmpty(path))
|
||||
{
|
||||
// Cache an empty collection when PATH is null (unset) or an empty string.
|
||||
_cachedLookupPaths = new List<string>();
|
||||
}
|
||||
else
|
||||
{
|
||||
// Tokenize the path and cache it
|
||||
string[] tokenizedPath = _pathCacheKey.Split(Path.PathSeparator, StringSplitOptions.RemoveEmptyEntries);
|
||||
_cachedPath = new Collection<string>();
|
||||
_cachedLookupPaths = new List<string>(capacity: tokenizedPath.Length);
|
||||
|
||||
foreach (string directory in tokenizedPath)
|
||||
{
|
||||
string tempDir = directory.TrimStart();
|
||||
if (tempDir.EqualsOrdinalIgnoreCase("~"))
|
||||
string tempDir = directory.Trim();
|
||||
if (tempDir.StartsWith('~'))
|
||||
{
|
||||
tempDir = Environment.GetFolderPath(
|
||||
Environment.SpecialFolder.UserProfile,
|
||||
Environment.SpecialFolderOption.DoNotVerify);
|
||||
}
|
||||
else if (tempDir.StartsWith("~" + Path.DirectorySeparatorChar))
|
||||
{
|
||||
tempDir = Environment.GetFolderPath(
|
||||
Environment.SpecialFolder.UserProfile,
|
||||
Environment.SpecialFolderOption.DoNotVerify)
|
||||
+ Path.DirectorySeparatorChar
|
||||
+ tempDir.Substring(2);
|
||||
if (tempDir.Length is 1)
|
||||
{
|
||||
tempDir = Environment.GetFolderPath(
|
||||
Environment.SpecialFolder.UserProfile,
|
||||
Environment.SpecialFolderOption.DoNotVerify);
|
||||
}
|
||||
else if (tempDir[1] == Path.DirectorySeparatorChar)
|
||||
{
|
||||
string homeDir = Environment.GetFolderPath(
|
||||
Environment.SpecialFolder.UserProfile,
|
||||
Environment.SpecialFolderOption.DoNotVerify);
|
||||
tempDir = $"{homeDir}{Path.DirectorySeparatorChar}{tempDir.AsSpan(2)}";
|
||||
}
|
||||
}
|
||||
|
||||
_cachedPath.Add(tempDir);
|
||||
result.Add(tempDir);
|
||||
_cachedLookupPaths.Add(tempDir);
|
||||
}
|
||||
}
|
||||
}
|
||||
else
|
||||
{
|
||||
result.AddRange(_cachedPath);
|
||||
}
|
||||
|
||||
// Cache the new lookup paths
|
||||
return _cachedLookupPaths ??= result;
|
||||
// The returned instance will be mutated in 'CommandPathSearch.ResolveCurrentDirectoryInLookupPaths' when resolving relative paths,
|
||||
// which depends on user's current working directory. So, we need to return a copy of the lookup paths to keep the cache intact.
|
||||
return new LookupPathCollection(_cachedLookupPaths);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// The cached list of lookup paths. It can be invalidated by
|
||||
/// the PATH changing.
|
||||
/// The cached list of lookup paths. It can be invalidated by the PATH changing.
|
||||
/// </summary>
|
||||
private LookupPathCollection _cachedLookupPaths;
|
||||
private List<string> _cachedLookupPaths;
|
||||
|
||||
/// <summary>
|
||||
/// The key that determines if the cached PATH can be used.
|
||||
/// </summary>
|
||||
private string _pathCacheKey;
|
||||
|
||||
/// <summary>
|
||||
/// The cache of the tokenized PATH directories.
|
||||
/// </summary>
|
||||
private Collection<string> _cachedPath;
|
||||
|
||||
#endregion internal members
|
||||
|
||||
#region environment variable helpers
|
||||
|
||||
@@ -275,3 +275,54 @@ Describe "Get-Command Tests" -Tags "CI" {
|
||||
$result | Should -Be $null
|
||||
}
|
||||
}
|
||||
|
||||
Describe "Test relative path in PATH env var" -Tags "CI" {
|
||||
|
||||
BeforeAll {
|
||||
$originalPath = $env:PATH
|
||||
$originalLocation = Get-Location
|
||||
|
||||
$subDir1 = Join-Path $TestDrive -ChildPath "subdir1"
|
||||
$subDir2 = Join-Path $TestDrive -ChildPath "subdir2"
|
||||
$toolsUnderSubDir1 = Join-Path $subDir1 -ChildPath "tools"
|
||||
$toolsUnderSubDir2 = Join-Path $subDir2 -ChildPath "tools"
|
||||
|
||||
$null = New-Item -Path $subDir1 -ItemType Directory -Force
|
||||
$null = New-Item -Path $subDir2 -ItemType Directory -Force
|
||||
$null = New-Item -Path $toolsUnderSubDir1 -ItemType Directory -Force
|
||||
$null = New-Item -Path $toolsUnderSubDir2 -ItemType Directory -Force
|
||||
|
||||
$helloScript = Join-Path $toolsUnderSubDir1 -ChildPath "hello.ps1"
|
||||
$byeScript = Join-Path $toolsUnderSubDir2 -ChildPath "bye.ps1"
|
||||
$null = New-Item -Path $helloScript -ItemType File -Force
|
||||
$null = New-Item -Path $byeScript -ItemType File -Force
|
||||
}
|
||||
|
||||
AfterAll {
|
||||
Set-Location $originalLocation
|
||||
$env:PATH = $originalPath
|
||||
}
|
||||
|
||||
It "Get-Command should resolve relative path in PATH env var based on user's current working directory" {
|
||||
$dirSep = [System.IO.Path]::DirectorySeparatorChar
|
||||
$pathSep = [System.IO.Path]::PathSeparator
|
||||
|
||||
Set-Location $subDir1
|
||||
$env:PATH = ".${dirSep}tools${pathSep}$env:PATH"
|
||||
|
||||
$result = Get-Command "hello.ps1" -ErrorAction silentlycontinue
|
||||
$result | Should -Not -BeNullOrEmpty -Because "CWD is $subDir1, so '.${dirSep}tools' in PATH should be resolved to '$toolsUnderSubDir1', which contains 'hello.ps1'"
|
||||
$result.Path | Should -BeExactly $helloScript
|
||||
|
||||
$result = Get-Command "bye.ps1" -ErrorAction silentlycontinue
|
||||
$result | Should -BeNullOrEmpty -Because "'bye.ps1' is not in '$toolsUnderSubDir1'"
|
||||
|
||||
Set-Location $subDir2
|
||||
$result = Get-Command "hello.ps1" -ErrorAction silentlycontinue
|
||||
$result | Should -BeNullOrEmpty -Because "CWD is $subDir2, so '.${dirSep}tools' in PATH should be resolved to '$toolsUnderSubDir2', which contains 'bye.ps1'"
|
||||
|
||||
$result = Get-Command "bye.ps1" -ErrorAction silentlycontinue
|
||||
$result | Should -Not -BeNullOrEmpty
|
||||
$result.Path | Should -BeExactly $byeScript
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user