From 99f9ef22d2b148c50ebf78b6d577e8569060be1a Mon Sep 17 00:00:00 2001 From: Dongbo Wang Date: Wed, 24 May 2017 13:30:54 -0700 Subject: [PATCH] Fix "Invoke-Item" to accept a file path with spaces on Unix platforms (#3850) Use the method NativeCommandParameterBinder.NeedQuotes, which is used by powershell native command processor, to check if quotes are needed. If yes, add quotes in the same way as our native command processor. Also, make 'Invoke-Item' on Linux and OSX able to invoke an executable properly. --- .../engine/NativeCommandParameterBinder.cs | 2 +- .../namespaces/FileSystemProvider.cs | 36 +++++-- .../Invoke-Item.Tests.ps1 | 94 +++++++++++-------- 3 files changed, 83 insertions(+), 49 deletions(-) diff --git a/src/System.Management.Automation/engine/NativeCommandParameterBinder.cs b/src/System.Management.Automation/engine/NativeCommandParameterBinder.cs index 3794f41c43..c00f62823e 100644 --- a/src/System.Management.Automation/engine/NativeCommandParameterBinder.cs +++ b/src/System.Management.Automation/engine/NativeCommandParameterBinder.cs @@ -306,7 +306,7 @@ namespace System.Management.Automation /// Check to see if the string contains spaces and therefore must be quoted. /// /// The string to check for spaces - private bool NeedQuotes(string stringToCheck) + internal static bool NeedQuotes(string stringToCheck) { bool needQuotes = false, followingBackslash = false; int quoteCount = 0; diff --git a/src/System.Management.Automation/namespaces/FileSystemProvider.cs b/src/System.Management.Automation/namespaces/FileSystemProvider.cs index d56977ade1..d5dde6ff71 100644 --- a/src/System.Management.Automation/namespaces/FileSystemProvider.cs +++ b/src/System.Management.Automation/namespaces/FileSystemProvider.cs @@ -1327,27 +1327,45 @@ namespace Microsoft.PowerShell.Commands { System.Diagnostics.Process invokeProcess = new System.Diagnostics.Process(); -#if UNIX - invokeProcess.StartInfo.FileName = Platform.IsLinux ? "xdg-open" : /* OS X */ "open"; - invokeProcess.StartInfo.Arguments = path; - invokeProcess.Start(); -#elif CORECLR try { - // Try Process.Start first. This works for executables even on headless SKUs. + // Try Process.Start first. + // - In FullCLR, this is all we need to do. + // - In CoreCLR, this works for executables on Win/Unix platforms invokeProcess.StartInfo.FileName = path; invokeProcess.Start(); } - catch (Win32Exception) +#if UNIX + catch (Win32Exception ex) when (ex.NativeErrorCode == 13) { + // Error code 13 -- Permission denied. + // The file is possibly not an executable, so we try invoking the default program that handles this file. + const string quoteFormat = "\"{0}\""; + invokeProcess.StartInfo.FileName = Platform.IsLinux ? "xdg-open" : /* OS X */ "open"; + if (NativeCommandParameterBinder.NeedQuotes(path)) + { + path = string.Format(CultureInfo.InvariantCulture, quoteFormat, path); + } + invokeProcess.StartInfo.Arguments = path; + invokeProcess.Start(); + } +#elif CORECLR + catch (Win32Exception ex) when (ex.NativeErrorCode == 193) + { + // Error code 193 -- BAD_EXE_FORMAT (not a valid Win32 application). // If it's headless SKUs, rethrow. if (Platform.IsNanoServer || Platform.IsIoT) { throw; } // If it's full Windows, then try ShellExecute. ShellExecuteHelper.Start(invokeProcess.StartInfo); } #else - invokeProcess.StartInfo.FileName = path; - invokeProcess.Start(); + finally + { + // Nothing to do in FullCLR. + // This empty 'finally' block is just to match the 'try' block above so that the code can be organized + // in a clean way without too many if/def's. + // Empty finally block will be ignored in release build, so there is no performance concern. + } #endif } } // InvokeDefaultAction diff --git a/test/powershell/Modules/Microsoft.PowerShell.Utility/Invoke-Item.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Utility/Invoke-Item.Tests.ps1 index ce25b0d34d..9954f5d94a 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Utility/Invoke-Item.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Utility/Invoke-Item.Tests.ps1 @@ -1,47 +1,63 @@ using namespace System.Diagnostics -Describe "Invoke-Item on non-Windows" -Tags "CI" { - - function NewProcessStartInfo([string]$CommandLine, [switch]$RedirectStdIn) - { - return [ProcessStartInfo]@{ - FileName = $powershell - Arguments = $CommandLine - RedirectStandardInput = $RedirectStdIn - RedirectStandardOutput = $true - RedirectStandardError = $true - UseShellExecute = $false - } - } - - function RunPowerShell([ProcessStartInfo]$debugfn) - { - $process = [Process]::Start($debugfn) - return $process - } - - function EnsureChildHasExited([Process]$process, [int]$WaitTimeInMS = 15000) - { - $process.WaitForExit($WaitTimeInMS) - - if (!$process.HasExited) - { - $process.HasExited | Should Be $true - $process.Kill() - } - } - +Describe "Invoke-Item basic tests" -Tags "CI" { BeforeAll { - $powershell = Join-Path -Path $PsHome -ChildPath "powershell" - Setup -File testfile.txt -Content "Hello World" - $testfile = Join-Path $TestDrive testfile.txt + $powershell = Join-Path $PSHOME -ChildPath powershell + + $testFile1 = Join-Path -Path $TestDrive -ChildPath "text1.txt" + New-Item -Path $testFile1 -ItemType File -Force > $null + + $testFolder = Join-Path -Path $TestDrive -ChildPath "My Folder" + New-Item -Path $testFolder -ItemType Directory -Force > $null + $testFile2 = Join-Path -Path $testFolder -ChildPath "text2.txt" + New-Item -Path $testFile2 -ItemType File -Force > $null + + $textFileTestCases = @( + @{ TestFile = $testFile1 }, + @{ TestFile = $testFile2 }) } - It "Should invoke a text file without error on non-Windows" -Skip:($IsWindows) { - $debugfn = NewProcessStartInfo "-noprofile ""``Invoke-Item $testfile`n" -RedirectStdIn - $process = RunPowerShell $debugfn - EnsureChildHasExited $process - $process.ExitCode | Should Be 0 + Context "Invoke a text file on Unix" { + BeforeEach { + $redirectErr = Join-Path -Path $TestDrive -ChildPath "error.txt" + } + + AfterEach { + Remove-Item -Path $redirectErr -Force -ErrorAction SilentlyContinue + } + + ## Run this test only on OSX because redirecting stderr of 'xdg-open' results in weird behavior in our Linux CI, + ## causing this test to fail or the build to hang. + It "Should invoke text file '' without error" -Skip:(!$IsOSX) -TestCases $textFileTestCases { + param($TestFile) + + ## Redirect stderr to a file. So if 'open' failed to open the text file, an error + ## message from 'open' would be written to the redirection file. + $proc = Start-Process -FilePath $powershell -ArgumentList "-noprofile Invoke-Item '$TestFile'" ` + -RedirectStandardError $redirectErr ` + -PassThru + $proc.WaitForExit(3000) > $null + if (!$proc.HasExited) { + try { $proc.Kill() } catch { } + } + ## If the text file was successfully opened, the redirection file should be empty since no error + ## message was written to it. + Get-Content $redirectErr -Raw | Should BeNullOrEmpty + } + } + + It "Should invoke an executable file without error" { + $executable = Get-Command "ping" -CommandType Application | ForEach-Object Source + $redirectFile = Join-Path -Path $TestDrive -ChildPath "redirect2.txt" + + if ($IsWindows) { + ## 'ping.exe' on Windows writes out usage to stdout. + & $powershell "-noprofile" "Invoke-Item '$executable'" > $redirectFile + } else { + ## 'ping' on Unix write out usage to stderr + & $powershell "-noprofile" "Invoke-Item '$executable'" 2> $redirectFile + } + Get-Content $redirectFile -Raw | Should Match "usage: ping" } }