Improving unix implementation for UseShellExecute

Making sure we are coherent with mono implementation
Added unit tests per linux and osx

src/System.Diagnostics.Process/src/System/Diagnostics/Process.Linux.cs

@@ -57,6 +57,54 @@ internal DateTime StartTimeCore
             }
         }
 
+        /// <summary>Gets arguments.</summary>
+        /// <param name="startInfo">The start info with which to start the process.</param>
+        private string[] GetArgs(ProcessStartInfo startInfo)
+        {
+            if (!startInfo.UseShellExecute)
+            {
+                return ParseArgv(startInfo);
+            }
+
+            string shellArgs = string.IsNullOrEmpty(startInfo.Arguments) ? startInfo.FileName : startInfo.FileName + " " + startInfo.Arguments;
+            return new string[2] { GetExecPath(), shellArgs };
+        }
+
+        /// <summary>Gets execution path</summary>
+        private string GetExecPath()
+        {
+            string[] allowedProgramsToRun = { "xdg-open", "gnome-open", "kfmclient" };
+            foreach (var program in allowedProgramsToRun)
+            {
+                string pathToProgram = GetPathToProgram(program);
+                if (!string.IsNullOrEmpty(pathToProgram))
+                {
+                    return Path.Combine(pathToProgram, program);
+                }
+            }
+            return string.Empty;
+        }
+
+        /// <summary>
+        /// Gets the path to the program
+        /// </summary>
+        /// <param name="program"></param>
+        /// <returns></returns>
+        private string GetPathToProgram(string program)
+        {
+            string path = Environment.GetEnvironmentVariable("PATH");
+            string[] dirs = path.Split(":");
+            foreach (var dir in dirs)
+            {
+                string[] files = Directory.GetFiles(dir, program);
+                if (files.Length != 0)
+                {
+                    return dir;
+                }
+            }
+            return string.Empty;
+        }
+
         /// <summary>
         /// Gets the amount of time the associated process has spent utilizing the CPU.
         /// It is the sum of the <see cref='System.Diagnostics.Process.UserProcessorTime'/> and

src/System.Diagnostics.Process/src/System/Diagnostics/Process.OSX.cs

@@ -88,6 +88,24 @@ internal DateTime StartTimeCore
             }
         }
 
+        /// <summary>Gets arguments.</summary>
+        /// <param name="startInfo">The start info with which to start the process.</param>
+        private string[] GetArgs(ProcessStartInfo startInfo)
+        {
+            if (!startInfo.UseShellExecute)
+            {
+                return ParseArgv(startInfo);
+            }
+
+            return new string[3] { GetExecPath(), "--args", startInfo.FileName + " " + startInfo.Arguments };
+        }
+
+        /// <summary>Gets execution path</summary>
+        private string GetExecPath()
+        {
+            return "/usr/bin/open";
+        }
+
         /// <summary>
         /// Gets the amount of time the associated process has spent utilizing the CPU.
         /// It is the sum of the <see cref='System.Diagnostics.Process.UserProcessorTime'/> and

src/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs

@@ -232,18 +232,11 @@ private bool StartCore(ProcessStartInfo startInfo)
                 {
                     throw new InvalidOperationException(SR.CantRedirectStreams);
                 }
-
-                const string ShellPath = "/bin/sh";
-
-                filename = ShellPath;
-                argv = new string[3] { ShellPath, "-c", startInfo.FileName + " " + startInfo.Arguments};
-            }
-            else
-            {
-                filename = ResolvePath(startInfo.FileName);
-                argv = ParseArgv(startInfo);
             }
 
+            filename = ResolvePath(startInfo.FileName);
+            argv = ParseArgv(startInfo);
+
             string[] envp = CreateEnvp(startInfo);
             string cwd = !string.IsNullOrWhiteSpace(startInfo.WorkingDirectory) ? startInfo.WorkingDirectory : null;
 
@@ -253,11 +246,33 @@ private bool StartCore(ProcessStartInfo startInfo)
             // is used to fork/execve as executing managed code in a forked process is not safe (only
             // the calling thread will transfer, thread IDs aren't stable across the fork, etc.)
             int childPid, stdinFd, stdoutFd, stderrFd;
-            Interop.Sys.ForkAndExecProcess(
-                filename, argv, envp, cwd,
-                startInfo.RedirectStandardInput, startInfo.RedirectStandardOutput, startInfo.RedirectStandardError,
-                out childPid,
-                out stdinFd, out stdoutFd, out stderrFd);
+
+            try
+            {
+                Interop.Sys.ForkAndExecProcess(
+                    filename, argv, envp, cwd,
+                    startInfo.RedirectStandardInput, startInfo.RedirectStandardOutput, startInfo.RedirectStandardError,
+                    out childPid,
+                    out stdinFd, out stdoutFd, out stderrFd);
+            }
+            catch (Win32Exception e)
+            {
+                if (!startInfo.UseShellExecute)
+                {
+                    throw e;
+                }
+                else
+                {
+                    filename = GetExecPath();
+                    argv = GetArgs(startInfo);
+
+                    Interop.Sys.ForkAndExecProcess(
+                    filename, argv, envp, cwd,
+                    startInfo.RedirectStandardInput, startInfo.RedirectStandardOutput, startInfo.RedirectStandardError,
+                    out childPid,
+                    out stdinFd, out stdoutFd, out stderrFd);
+                }
+            }
 
             // Store the child's information into this Process object.
             Debug.Assert(childPid >= 0);

src/System.Diagnostics.Process/tests/Configurations.props

@@ -4,8 +4,9 @@
     <BuildConfigurations>
       netstandard-Windows_NT;
       netstandard-Unix;
+      netstandard-OSX;
       uap-Windows_NT;
       uapaot-Windows_NT;
     </BuildConfigurations>
   </PropertyGroup>
-</Project>
+</Project>

src/System.Diagnostics.Process/tests/ProcessTests.Linux.cs

@@ -0,0 +1,86 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System.Collections.Generic;
+using System.ComponentModel;
+using System.IO;
+using System.Linq;
+using System.Runtime.InteropServices;
+using System.Threading;
+using System.Security;
+using Xunit;
+using Xunit.NetCore.Extensions;
+
+namespace System.Diagnostics.Tests
+{
+    public partial class ProcessTests : ProcessTestBase
+    {
+        [Fact]
+        public void ProcessStart_UseShellExecuteTrue_OpenFile_ThrowsIfNoDefaultProgramInstalledSucceedsOtherwise()
+        {
+            string fileToOpen = GetTestFilePath() + ".txt";
+            File.WriteAllText(fileToOpen, $"{nameof(ProcessStart_UseShellExecuteTrue_OpenFile_ThrowsIfNoDefaultProgramInstalledSucceedsOtherwise)}");
+
+            string[] allowedProgramsToRun = { "xdg-open", "gnome-open", "kfmclient" };
+            foreach (var program in allowedProgramsToRun)
+            {
+                if (IsProgramInstalled(program))
+                {
+                    var startInfo = new ProcessStartInfo { UseShellExecute = true, FileName = fileToOpen };
+                    using (var px = Process.Start(startInfo))
+                    {
+                        if (px != null)
+                        {
+                            Assert.Equal(program, px.ProcessName);
+                            px.Kill();
+                            px.WaitForExit();
+                            Assert.True(px.HasExited);
+                            Assert.Equal(137, px.ExitCode); // 137 means the process was killed
+                        }
+                    }
+                    return;
+                }
+            }
+
+            Win32Exception e = Assert.Throws<Win32Exception>(() => Process.Start(new ProcessStartInfo { UseShellExecute = true, FileName = fileToOpen }));
+        }
+
+        [Fact]
+        [OuterLoop("Returns failure exit code when default program, xdg-open, is installed")]
+        public void ProcessStart_UseShellExecuteTrue_OpenMissingFile_DefaultProgramInstalled_ReturnsFailureExitCode()
+        {
+            string fileToOpen = Path.Combine(Environment.CurrentDirectory, "_no_such_file.TXT");
+            using (var p = Process.Start(new ProcessStartInfo { UseShellExecute = true, FileName = fileToOpen }))
+            {
+                if (p != null)
+                {
+                    Assert.Equal("xdg-open", p.ProcessName);
+                    p.WaitForExit();
+                    Assert.True(p.HasExited);
+                    Assert.Equal(2, p.ExitCode);
+                }
+            }
+        }
+
+        /// <summary>
+        /// Gets the path to the program
+        /// </summary>
+        /// <param name="program"></param>
+        /// <returns></returns>
+        private bool IsProgramInstalled(string program)
+        {
+            string path = Environment.GetEnvironmentVariable("PATH");
+            string[] dirs = path.Split(':');
+            foreach (var dir in dirs)
+            {
+                string[] files = Directory.GetFiles(dir, program);
+                if (files.Length != 0)
+                {
+                    return true;
+                }
+            }
+            return false;
+        }
+    }
+}

src/System.Diagnostics.Process/tests/ProcessTests.OSX.cs

@@ -0,0 +1,58 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System.Collections.Generic;
+using System.ComponentModel;
+using System.IO;
+using System.Linq;
+using System.Runtime.InteropServices;
+using System.Threading;
+using System.Security;
+using Xunit;
+using Xunit.NetCore.Extensions;
+
+namespace System.Diagnostics.Tests
+{
+    public partial class ProcessTests : ProcessTestBase
+    {
+        [Fact]
+        [OuterLoop("Launches default application")]
+        public void TestWithFilename_ShouldUseOpenWithDefaultApp()
+        {
+            string file = Path.Combine(Environment.CurrentDirectory, "..", "..", "..", "..", "PATENTS.TXT");
+            using (var px = Process.Start("/usr/bin/open", file))
+            {
+                Assert.False(px.HasExited);
+                px.WaitForExit();
+                Assert.True(px.HasExited);
+                Assert.Equal(0, px.ExitCode); // Exit Code 0 from open means success
+            }
+        }
+
+        [Fact]
+        [OuterLoop("Launches default browser")]
+        public void TestWithUrl_ShouldUseOpenWithDefaultApp()
+        {
+            using (var px = Process.Start("/usr/bin/open", "http://www.google.com"))
+            {
+                Assert.False(px.HasExited);
+                px.WaitForExit();
+                Assert.True(px.HasExited);
+                Assert.Equal(0, px.ExitCode); // Exit Code 0 from open means success
+            }
+        }
+
+        [Fact]
+        // TODO fix behavior to ThrowWin32Exception instead?
+        public void ProcessStart_TryOpenFileThatDoesntExist_UseShellExecuteIsTrue_ThrowsWin32Exception()
+        {
+            string file = Path.Combine(Environment.CurrentDirectory, "..", "..", "..", "..", "_no_such_file.TXT");
+            using (var p = Process.Start(new ProcessStartInfo { UseShellExecute = true, FileName = file }))
+            {
+                Assert.True(p.WaitForExit(WaitInMS));
+                Assert.Equal(1, p.ExitCode); // Exit Code 1 from open means something went wrong
+            }
+        }
+    }
+}

src/System.Diagnostics.Process/tests/ProcessTests.Unix.cs

@@ -61,13 +61,27 @@ public void TestRootGetProcessById()
             Assert.Equal(1, p.Id);
         }
 
+        [Theory, InlineData(true), InlineData(false)]
+        public void ProcessStart_TryExitCommandAsFileName_ThrowsWin32Exception(bool useShellExecute)
+        {
+            Win32Exception e = Assert.Throws<Win32Exception>(() => Process.Start(new ProcessStartInfo { UseShellExecute = useShellExecute, FileName = "exit", Arguments = "42" }));
+        }
+
         [Fact]
-        public void TestUseShellExecute_Unix_Succeeds()
+        public void ProcessStart_UseShellExecuteTrue_OpenNano_OpensNano()
         {
-            using (var p = Process.Start(new ProcessStartInfo { UseShellExecute = true, FileName = "exit", Arguments = "42" }))
+            string appToOpen = "nano";
+            var startInfo = new ProcessStartInfo { UseShellExecute = true, FileName = appToOpen };
+            using (var px = Process.Start(startInfo))
             {
-                Assert.True(p.WaitForExit(WaitInMS));
-                Assert.Equal(42, p.ExitCode);
+                if (px != null)
+                {
+                    Assert.Equal(appToOpen, px.ProcessName);
+                    px.Kill();
+                    px.WaitForExit();
+                    Assert.True(px.HasExited);
+                    Assert.Equal(137, px.ExitCode);
+                }
             }
         }
 

src/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj

@@ -7,6 +7,8 @@
     <DefineConstants Condition="'$(TargetsWindows)' == 'true'">$(DefineConstants);TargetsWindows</DefineConstants>
   </PropertyGroup>
   <!-- Default configurations to help VS understand the configurations -->
+  <PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'netstandard-OSX-Debug|AnyCPU'" />
+  <PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'netstandard-OSX-Release|AnyCPU'" />
   <PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'netstandard-Unix-Debug|AnyCPU'" />
   <PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'netstandard-Unix-Release|AnyCPU'" />
   <PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'netstandard-Windows_NT-Debug|AnyCPU'" />
@@ -32,6 +34,8 @@
     <Compile Condition="'$(TargetGroup)' != 'uap'" Include="ProcessTestBase.NonUap.cs" />
     <Compile Condition="'$(TargetGroup)' == 'uap'" Include="ProcessTestBase.Uap.cs" />
     <Compile Include="ProcessTests.cs" />
+    <Compile Include="ProcessTests.Linux.cs" Condition=" '$(TargetsLinux)' == 'true'" />
+    <Compile Include="ProcessTests.OSX.cs" Condition=" '$(TargetsOSX)' == 'true'" />
     <Compile Include="ProcessTests.Unix.cs" Condition="'$(TargetsWindows)' != 'true'" />
     <Compile Include="ProcessThreadTests.cs" />
     <Compile Include="ProcessWaitingTests.cs" />