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.

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.
         /// </summary>
         /// <param name="stringToCheck">The string to check for spaces</param>
-        private bool NeedQuotes(string stringToCheck)
+        internal static bool NeedQuotes(string stringToCheck)
         {
             bool needQuotes = false, followingBackslash = false;
             int quoteCount = 0;

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

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 '<TestFile>' 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"
     }
 }