From b7c875b0f2bd2d3e1c68c4173f536414ee035f08 Mon Sep 17 00:00:00 2001 From: Steve Lee Date: Mon, 18 Jul 2022 17:25:46 -0700 Subject: [PATCH] Remove `PSNativePSPathResolution` experimental feature (#17670) --- experimental-feature-linux.json | 1 - experimental-feature-windows.json | 1 - .../ExperimentalFeature.cs | 3 - .../engine/NativeCommandParameterBinder.cs | 242 ++++++------------ .../NativeCommandArguments.Tests.ps1 | 96 +------ 5 files changed, 78 insertions(+), 265 deletions(-) diff --git a/experimental-feature-linux.json b/experimental-feature-linux.json index 12df4c5db5..b0a5a69638 100644 --- a/experimental-feature-linux.json +++ b/experimental-feature-linux.json @@ -7,7 +7,6 @@ "PSLoadAssemblyFromNativeCode", "PSNativeCommandArgumentPassing", "PSNativeCommandErrorActionPreference", - "PSNativePSPathResolution", "PSRemotingSSHTransportErrorHandling", "PSStrictModeAssignment", "PSSubsystemPluginModel" diff --git a/experimental-feature-windows.json b/experimental-feature-windows.json index 12df4c5db5..b0a5a69638 100644 --- a/experimental-feature-windows.json +++ b/experimental-feature-windows.json @@ -7,7 +7,6 @@ "PSLoadAssemblyFromNativeCode", "PSNativeCommandArgumentPassing", "PSNativeCommandErrorActionPreference", - "PSNativePSPathResolution", "PSRemotingSSHTransportErrorHandling", "PSStrictModeAssignment", "PSSubsystemPluginModel" diff --git a/src/System.Management.Automation/engine/ExperimentalFeature/ExperimentalFeature.cs b/src/System.Management.Automation/engine/ExperimentalFeature/ExperimentalFeature.cs index f6e3632e16..8a3e428b9c 100644 --- a/src/System.Management.Automation/engine/ExperimentalFeature/ExperimentalFeature.cs +++ b/src/System.Management.Automation/engine/ExperimentalFeature/ExperimentalFeature.cs @@ -113,9 +113,6 @@ namespace System.Management.Automation new ExperimentalFeature( name: "PSCommandNotFoundSuggestion", description: "Recommend potential commands based on fuzzy search on a CommandNotFoundException"), - new ExperimentalFeature( - name: "PSNativePSPathResolution", - description: "Convert PSPath to filesystem path, if possible, for native commands"), new ExperimentalFeature( name: "PSSubsystemPluginModel", description: "A plugin model for registering and un-registering PowerShell subsystems"), diff --git a/src/System.Management.Automation/engine/NativeCommandParameterBinder.cs b/src/System.Management.Automation/engine/NativeCommandParameterBinder.cs index 200cfeee2e..1c2e835b95 100644 --- a/src/System.Management.Automation/engine/NativeCommandParameterBinder.cs +++ b/src/System.Management.Automation/engine/NativeCommandParameterBinder.cs @@ -83,7 +83,7 @@ namespace System.Management.Automation if (parameter.ParameterNameSpecified) { Diagnostics.Assert(!parameter.ParameterText.Contains(' '), "Parameters cannot have whitespace"); - PossiblyGlobArg(parameter.ParameterText, parameter, StringConstantType.BareWord); + PossiblyGlobArg(parameter.ParameterText, parameter, usedQuotes: false); if (parameter.SpaceAfterParameter) { @@ -108,30 +108,22 @@ namespace System.Management.Automation // windbg -k com:port=\\devbox\pipe\debug,pipe,resets=0,reconnect // The parser produced an array of strings but marked the parameter so we // can properly reconstruct the correct command line. - StringConstantType stringConstantType = StringConstantType.BareWord; + bool usedQuotes = false; ArrayLiteralAst arrayLiteralAst = null; switch (parameter?.ArgumentAst) { case StringConstantExpressionAst sce: - stringConstantType = sce.StringConstantType; + usedQuotes = sce.StringConstantType != StringConstantType.BareWord; break; case ExpandableStringExpressionAst ese: - stringConstantType = ese.StringConstantType; + usedQuotes = ese.StringConstantType != StringConstantType.BareWord; break; case ArrayLiteralAst ala: arrayLiteralAst = ala; break; } - // Prior to PSNativePSPathResolution experimental feature, a single quote worked the same as a double quote - // so if the feature is not enabled, we treat any quotes as double quotes. When this feature is no longer - // experimental, this code here needs to be removed. - if (!ExperimentalFeature.IsEnabled("PSNativePSPathResolution") && stringConstantType == StringConstantType.SingleQuoted) - { - stringConstantType = StringConstantType.DoubleQuoted; - } - - AppendOneNativeArgument(Context, parameter, argValue, arrayLiteralAst, sawVerbatimArgumentMarker, stringConstantType); + AppendOneNativeArgument(Context, parameter, argValue, arrayLiteralAst, sawVerbatimArgumentMarker, usedQuotes); } } } @@ -225,8 +217,8 @@ namespace System.Management.Automation /// The object to append. /// If the argument was an array literal, the Ast, otherwise null. /// True if the argument occurs after --%. - /// Bare, SingleQuoted, or DoubleQuoted. - private void AppendOneNativeArgument(ExecutionContext context, CommandParameterInternal parameter, object obj, ArrayLiteralAst argArrayAst, bool sawVerbatimArgumentMarker, StringConstantType stringConstantType) + /// True if the argument was a quoted string (single or double). + private void AppendOneNativeArgument(ExecutionContext context, CommandParameterInternal parameter, object obj, ArrayLiteralAst argArrayAst, bool sawVerbatimArgumentMarker, bool usedQuotes) { IEnumerator list = LanguagePrimitives.GetEnumerator(obj); @@ -291,20 +283,11 @@ namespace System.Management.Automation if (NeedQuotes(arg)) { _arguments.Append('"'); - - if (stringConstantType == StringConstantType.DoubleQuoted) - { - _arguments.Append(ResolvePath(arg, Context)); - AddToArgumentList(parameter, ResolvePath(arg, Context)); - } - else - { - _arguments.Append(arg); - AddToArgumentList(parameter, arg); - } + AddToArgumentList(parameter, arg); // need to escape all trailing backslashes so the native command receives it correctly // according to http://www.daviddeley.com/autohotkey/parameters/parameters.htm#WINCRULESDOC + _arguments.Append(arg); for (int i = arg.Length - 1; i >= 0 && arg[i] == '\\'; i--) { _arguments.Append('\\'); @@ -319,14 +302,14 @@ namespace System.Management.Automation // We have a literal array, so take the extent, break it on spaces and add them to the argument list. foreach (string element in argArrayAst.Extent.Text.Split(' ', StringSplitOptions.RemoveEmptyEntries)) { - PossiblyGlobArg(element, parameter, stringConstantType); + PossiblyGlobArg(element, parameter, usedQuotes); } break; } else { - PossiblyGlobArg(arg, parameter, stringConstantType); + PossiblyGlobArg(arg, parameter, usedQuotes); } } } @@ -346,101 +329,91 @@ namespace System.Management.Automation /// /// The argument that possibly needs expansion. /// The parameter associated with the operation. - /// Bare, SingleQuoted, or DoubleQuoted. - private void PossiblyGlobArg(string arg, CommandParameterInternal parameter, StringConstantType stringConstantType) + /// True if the argument was a quoted string (single or double). + private void PossiblyGlobArg(string arg, CommandParameterInternal parameter, bool usedQuotes) { var argExpanded = false; #if UNIX // On UNIX systems, we expand arguments containing wildcard expressions against // the file system just like bash, etc. - - if (stringConstantType == StringConstantType.BareWord) + if (!usedQuotes && WildcardPattern.ContainsWildcardCharacters(arg)) { - if (WildcardPattern.ContainsWildcardCharacters(arg)) + // See if the current working directory is a filesystem provider location + // We won't do the expansion if it isn't since native commands can only access the file system. + var cwdinfo = Context.EngineSessionState.CurrentLocation; + + // If it's a filesystem location then expand the wildcards + if (cwdinfo.Provider.Name.Equals(FileSystemProvider.ProviderName, StringComparison.OrdinalIgnoreCase)) { - // See if the current working directory is a filesystem provider location - // We won't do the expansion if it isn't since native commands can only access the file system. - var cwdinfo = Context.EngineSessionState.CurrentLocation; + // On UNIX, paths starting with ~ or absolute paths are not normalized + bool normalizePath = arg.Length == 0 || !(arg[0] == '~' || arg[0] == '/'); - // If it's a filesystem location then expand the wildcards - if (cwdinfo.Provider.Name.Equals(FileSystemProvider.ProviderName, StringComparison.OrdinalIgnoreCase)) + // See if there are any matching paths otherwise just add the pattern as the argument + Collection paths = null; + try { - // On UNIX, paths starting with ~ or absolute paths are not normalized - bool normalizePath = arg.Length == 0 || !(arg[0] == '~' || arg[0] == '/'); + paths = Context.EngineSessionState.InvokeProvider.ChildItem.Get(arg, false); + } + catch + { + // Fallthrough will append the pattern unchanged. + } - // See if there are any matching paths otherwise just add the pattern as the argument - Collection paths = null; - try + // Expand paths, but only from the file system. + if (paths?.Count > 0 && paths.All(p => p.BaseObject is FileSystemInfo)) + { + var sep = string.Empty; + foreach (var path in paths) { - paths = Context.EngineSessionState.InvokeProvider.ChildItem.Get(arg, false); - } - catch - { - // Fallthrough will append the pattern unchanged. - } - - // Expand paths, but only from the file system. - if (paths?.Count > 0 && paths.All(static p => p.BaseObject is FileSystemInfo)) - { - var sep = string.Empty; - foreach (var path in paths) + _arguments.Append(sep); + sep = " "; + var expandedPath = (path.BaseObject as FileSystemInfo).FullName; + if (normalizePath) { - _arguments.Append(sep); - sep = " "; - var expandedPath = (path.BaseObject as FileSystemInfo).FullName; - if (normalizePath) - { - expandedPath = - Context.SessionState.Path.NormalizeRelativePath(expandedPath, cwdinfo.ProviderPath); - } - // If the path contains spaces, then add quotes around it. - if (NeedQuotes(expandedPath)) - { - _arguments.Append('"'); - _arguments.Append(expandedPath); - _arguments.Append('"'); - AddToArgumentList(parameter, expandedPath); - } - else - { - _arguments.Append(expandedPath); - AddToArgumentList(parameter, expandedPath); - } - - argExpanded = true; + expandedPath = + Context.SessionState.Path.NormalizeRelativePath(expandedPath, cwdinfo.ProviderPath); } + // If the path contains spaces, then add quotes around it. + if (NeedQuotes(expandedPath)) + { + _arguments.Append('"'); + _arguments.Append(expandedPath); + _arguments.Append('"'); + } + else + { + _arguments.Append(expandedPath); + } + + AddToArgumentList(parameter, expandedPath); + argExpanded = true; } } } - else + } + else if (!usedQuotes) + { + // Even if there are no wildcards, we still need to possibly + // expand ~ into the filesystem provider home directory path + ProviderInfo fileSystemProvider = Context.EngineSessionState.GetSingleProvider(FileSystemProvider.ProviderName); + string home = fileSystemProvider.Home; + if (string.Equals(arg, "~")) { - // Even if there are no wildcards, we still need to possibly - // expand ~ into the filesystem provider home directory path - ProviderInfo fileSystemProvider = Context.EngineSessionState.GetSingleProvider(FileSystemProvider.ProviderName); - string home = fileSystemProvider.Home; - if (string.Equals(arg, "~")) - { - _arguments.Append(home); - AddToArgumentList(parameter, home); - argExpanded = true; - } - else if (arg.StartsWith("~/", StringComparison.OrdinalIgnoreCase)) - { - string replacementString = string.Concat(home, arg.AsSpan(1)); - _arguments.Append(replacementString); - AddToArgumentList(parameter, replacementString); - argExpanded = true; - } + _arguments.Append(home); + AddToArgumentList(parameter, home); + argExpanded = true; + } + else if (arg.StartsWith("~/", StringComparison.OrdinalIgnoreCase)) + { + var replacementString = string.Concat(home, arg.AsSpan(1)); + _arguments.Append(replacementString); + AddToArgumentList(parameter, replacementString); + argExpanded = true; } } #endif // UNIX - if (stringConstantType != StringConstantType.SingleQuoted) - { - arg = ResolvePath(arg, Context); - } - if (!argExpanded) { _arguments.Append(arg); @@ -448,71 +421,6 @@ namespace System.Management.Automation } } - /// - /// Check if string is prefixed by psdrive, if so, expand it if filesystem path. - /// - /// The potential PSPath to resolve. - /// The current ExecutionContext. - /// Resolved PSPath if applicable otherwise the original path - internal static string ResolvePath(string path, ExecutionContext context) - { - if (ExperimentalFeature.IsEnabled("PSNativePSPathResolution")) - { -#if !UNIX - // on Windows, we need to expand ~ to point to user's home path - if (string.Equals(path, "~", StringComparison.Ordinal) || path.StartsWith(TildeDirectorySeparator, StringComparison.Ordinal) || path.StartsWith(TildeAltDirectorySeparator, StringComparison.Ordinal)) - { - try - { - ProviderInfo fileSystemProvider = context.EngineSessionState.GetSingleProvider(FileSystemProvider.ProviderName); - return new StringBuilder(fileSystemProvider.Home) - .Append(path.AsSpan(1)) - .Replace(Path.AltDirectorySeparatorChar, Path.DirectorySeparatorChar) - .ToString(); - } - catch - { - return path; - } - } - - // check if the driveName is an actual disk drive on Windows, if so, no expansion - if (path.Length >= 2 && path[1] == ':') - { - foreach (var drive in DriveInfo.GetDrives()) - { - if (drive.Name.StartsWith(new string(path[0], 1), StringComparison.OrdinalIgnoreCase)) - { - return path; - } - } - } -#endif - - if (path.Contains(':')) - { - LocationGlobber globber = new LocationGlobber(context.SessionState); - try - { - ProviderInfo providerInfo; - - // replace the argument with resolved path if it's a filesystem path - string pspath = globber.GetProviderPath(path, out providerInfo); - if (string.Equals(providerInfo.Name, FileSystemProvider.ProviderName, StringComparison.OrdinalIgnoreCase)) - { - path = pspath; - } - } - catch - { - // if it's not a provider path, do nothing - } - } - } - - return path; - } - /// /// Check to see if the string contains spaces and therefore must be quoted. /// @@ -567,8 +475,6 @@ namespace System.Management.Automation /// The native command to bind to. /// private readonly NativeCommand _nativeCommand; - private static readonly string TildeDirectorySeparator = $"~{Path.DirectorySeparatorChar}"; - private static readonly string TildeAltDirectorySeparator = $"~{Path.AltDirectorySeparatorChar}"; #endregion private members } diff --git a/test/powershell/Language/Scripting/NativeExecution/NativeCommandArguments.Tests.ps1 b/test/powershell/Language/Scripting/NativeExecution/NativeCommandArguments.Tests.ps1 index c8e3c75cb1..b4e92d4317 100644 --- a/test/powershell/Language/Scripting/NativeExecution/NativeCommandArguments.Tests.ps1 +++ b/test/powershell/Language/Scripting/NativeExecution/NativeCommandArguments.Tests.ps1 @@ -285,99 +285,11 @@ foreach ( $argumentListValue in "Standard","Legacy","Windows" ) { } } - } -} -Describe 'PSPath to native commands' -tags "CI" { - BeforeAll { - $featureEnabled = $EnabledExperimentalFeatures.Contains('PSNativePSPathResolution') - $originalDefaultParameterValues = $PSDefaultParameterValues.Clone() - $PSDefaultParameterValues["it:skip"] = (-not $featureEnabled) - - if ($IsWindows) { - $cmd = "cmd" - $cmdArg1 = "/c" - $cmdArg2 = "type" - $dir = "cmd" - $dirArg1 = "/c" - $dirArg2 = "dir" - } - else { - $cmd = "cat" - $dir = "ls" + It 'Should treat a PSPath as literal' { + $lines = testexe -echoargs temp:/foo + $lines.Count | Should -Be 1 + $lines | Should -BeExactly 'Arg 0 is ' } - - Set-Content -Path testdrive:/test.txt -Value 'Hello' - Set-Content -Path "testdrive:/test file.txt" -Value 'Hello' - Set-Content -Path "env:/test var" -Value 'Hello' - $filePath = Join-Path -Path ~ -ChildPath (New-Guid) - Set-Content -Path $filePath -Value 'Home' - $complexDriveName = 'My test! ;+drive' - New-PSDrive -Name $complexDriveName -Root $testdrive -PSProvider FileSystem - } - - AfterAll { - $global:PSDefaultParameterValues = $originalDefaultParameterValues - - Remove-Item -Path "env:/test var" - Remove-Item -Path $filePath - Remove-PSDrive -Name $complexDriveName - } - - It 'PSPath with ~/path works' { - $out = & $cmd $cmdArg1 $cmdArg2 $filePath - $LASTEXITCODE | Should -Be 0 - $out | Should -BeExactly 'Home' - } - - It 'PSPath with ~ works' { - $out = & $dir $dirArg1 $dirArg2 ~ - $LASTEXITCODE | Should -Be 0 - $out | Should -Not -BeNullOrEmpty - } - - It 'PSPath that is file system path works with native commands: ' -TestCases @( - @{ path = "testdrive:/test.txt" } - @{ path = "testdrive:/test file.txt" } - ){ - param($path) - - $out = & $cmd $cmdArg1 $cmdArg2 "$path" - $LASTEXITCODE | Should -Be 0 - $out | Should -BeExactly 'Hello' - } - - It 'PSPath passed with single quotes should be treated as literal' { - $out = & $cmd $cmdArg1 $cmdArg2 'testdrive:/test.txt' - $LASTEXITCODE | Should -Not -Be 0 - $out | Should -BeNullOrEmpty - } - - It 'PSPath that is not a file system path fails with native commands: ' -TestCases @( - @{ path = "env:/PSModulePath" } - @{ path = "env:/test var" } - ){ - param($path) - - $out = & $cmd $cmdArg1 $cmdArg2 "$path" - $LASTEXITCODE | Should -Not -Be 0 - $out | Should -BeNullOrEmpty - } - - It 'Relative PSPath works' { - New-Item -Path $testdrive -Name TestFolder -ItemType Directory -ErrorAction Stop - $cwd = Get-Location - Set-Content -Path (Join-Path -Path $testdrive -ChildPath 'TestFolder' -AdditionalChildPath 'test.txt') -Value 'hello' - Set-Location -Path (Join-Path -Path $testdrive -ChildPath 'TestFolder') - Set-Location -Path $cwd - $out = & $cmd $cmdArg1 $cmdArg2 "TestDrive:test.txt" - $LASTEXITCODE | Should -Be 0 - $out | Should -BeExactly 'Hello' - } - - It 'Complex PSDrive name works' { - $out = & $cmd $cmdArg1 $cmdArg2 "${complexDriveName}:/test.txt" - $LASTEXITCODE | Should -Be 0 - $out | Should -BeExactly 'Hello' } }