diff --git a/src/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.cs b/src/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.cs index 1e01ac23ae6d..be94dbee19f1 100644 --- a/src/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.cs +++ b/src/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.cs @@ -12,17 +12,18 @@ internal static partial class Interop { internal static partial class Sys { - internal static unsafe void ForkAndExecProcess( + internal static unsafe int ForkAndExecProcess( string filename, string[] argv, string[] envp, string cwd, bool redirectStdin, bool redirectStdout, bool redirectStderr, - out int lpChildPid, out int stdinFd, out int stdoutFd, out int stderrFd) + out int lpChildPid, out int stdinFd, out int stdoutFd, out int stderrFd, bool shouldThrow = true) { byte** argvPtr = null, envpPtr = null; + int result = -1; try { AllocNullTerminatedArray(argv, ref argvPtr); AllocNullTerminatedArray(envp, ref envpPtr); - int result = ForkAndExecProcess( + result = ForkAndExecProcess( filename, argvPtr, envpPtr, cwd, redirectStdin ? 1 : 0, redirectStdout ? 1 : 0, redirectStderr ? 1 :0, out lpChildPid, out stdinFd, out stdoutFd, out stderrFd); @@ -38,7 +39,8 @@ internal static unsafe void ForkAndExecProcess( // technically ambiguous, in the case of a failure with a 0 errno. Simplest // solution then is just to throw here the same exception the Process caller // would have. This can be revisited if we ever have another call site. - throw new Win32Exception(); + if (shouldThrow) + throw new Win32Exception(); } } finally @@ -46,6 +48,7 @@ internal static unsafe void ForkAndExecProcess( FreeArray(envpPtr, envp.Length); FreeArray(argvPtr, argv.Length); } + return result; } [DllImport(Libraries.SystemNative, EntryPoint = "SystemNative_ForkAndExecProcess", SetLastError = true)] diff --git a/src/System.Diagnostics.Process/src/Resources/Strings.resx b/src/System.Diagnostics.Process/src/Resources/Strings.resx index 71dc8266c78b..b7ff33eb4df5 100644 --- a/src/System.Diagnostics.Process/src/Resources/Strings.resx +++ b/src/System.Diagnostics.Process/src/Resources/Strings.resx @@ -210,6 +210,9 @@ The Process object must have the UseShellExecute property set to false in order to redirect IO streams. + + The FileName property should not be a directory. + An async read operation has already been started on the stream. diff --git a/src/System.Diagnostics.Process/src/System/Diagnostics/Process.Linux.cs b/src/System.Diagnostics.Process/src/System/Diagnostics/Process.Linux.cs index bf05c4254695..d46cda85f4d2 100644 --- a/src/System.Diagnostics.Process/src/System/Diagnostics/Process.Linux.cs +++ b/src/System.Diagnostics.Process/src/System/Diagnostics/Process.Linux.cs @@ -57,6 +57,21 @@ internal DateTime StartTimeCore } } + /// Gets execution path + private string GetPathToOpenFile() + { + string[] allowedProgramsToRun = { "xdg-open", "gnome-open", "kfmclient" }; + foreach (var program in allowedProgramsToRun) + { + string pathToProgram = FindProgramInPath(program); + if (!string.IsNullOrEmpty(pathToProgram)) + { + return pathToProgram; + } + } + return null; + } + /// /// Gets the amount of time the associated process has spent utilizing the CPU. /// It is the sum of the and diff --git a/src/System.Diagnostics.Process/src/System/Diagnostics/Process.OSX.cs b/src/System.Diagnostics.Process/src/System/Diagnostics/Process.OSX.cs index 2870fc7ae393..560d04c4cb3b 100644 --- a/src/System.Diagnostics.Process/src/System/Diagnostics/Process.OSX.cs +++ b/src/System.Diagnostics.Process/src/System/Diagnostics/Process.OSX.cs @@ -88,6 +88,12 @@ internal DateTime StartTimeCore } } + /// Gets execution path + private string GetPathToOpenFile() + { + return "/usr/bin/open"; + } + /// /// Gets the amount of time the associated process has spent utilizing the CPU. /// It is the sum of the and diff --git a/src/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs b/src/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs index 2d2c3c2feab0..987a8272f638 100644 --- a/src/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs +++ b/src/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs @@ -45,7 +45,7 @@ public static Process Start(string fileName, string userName, SecureString passw [CLSCompliant(false)] public static Process Start(string fileName, string arguments, string userName, SecureString password, string domain) - { + { throw new PlatformNotSupportedException(SR.ProcessStartIdentityNotSupported); } @@ -163,7 +163,7 @@ private ProcessPriorityClass PriorityClassCore } Debug.Assert(pri >= -20 && pri <= 20); - return + return pri < -15 ? ProcessPriorityClass.RealTime : pri < -10 ? ProcessPriorityClass.High : pri < -5 ? ProcessPriorityClass.AboveNormal : @@ -219,45 +219,72 @@ private SafeProcessHandle GetProcessHandle() return new SafeProcessHandle(_processId); } - /// Starts the process using the supplied start info. + /// + /// Starts the process using the supplied start info. + /// Even with UseShellExecute option, we try first running fileName just in case the caller is giving executable which we should run + /// Then if we couldn't run it we'll try the shell tools to launch it(e.g. "open fileName") + /// /// The start info with which to start the process. private bool StartCore(ProcessStartInfo startInfo) { string filename; string[] argv; + if (Directory.Exists(startInfo.FileName)) + { + throw new Win32Exception(SR.DirectoryNotValidAsInput); + } + if (startInfo.UseShellExecute) { if (startInfo.RedirectStandardInput || startInfo.RedirectStandardOutput || startInfo.RedirectStandardError) { throw new InvalidOperationException(SR.CantRedirectStreams); } + } - const string ShellPath = "/bin/sh"; + int childPid = -1, stdinFd = -1, stdoutFd = -1, stderrFd = -1, result = -1; + string[] envp = CreateEnvp(startInfo); + string cwd = !string.IsNullOrWhiteSpace(startInfo.WorkingDirectory) ? startInfo.WorkingDirectory : null; - filename = ShellPath; - argv = new string[3] { ShellPath, "-c", startInfo.FileName + " " + startInfo.Arguments}; - } - else + filename = ResolvePath(startInfo.FileName); + if (!string.IsNullOrEmpty(filename)) { - filename = ResolvePath(startInfo.FileName); argv = ParseArgv(startInfo); + + // Invoke the shim fork/execve routine. It will create pipes for all requested + // redirects, fork a child process, map the pipe ends onto the appropriate stdin/stdout/stderr + // descriptors, and execve to execute the requested process. The shim implementation + // 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.) + result = Interop.Sys.ForkAndExecProcess( + filename, argv, envp, cwd, + startInfo.RedirectStandardInput, startInfo.RedirectStandardOutput, startInfo.RedirectStandardError, + out childPid, + out stdinFd, out stdoutFd, out stderrFd, shouldThrow: !startInfo.UseShellExecute); } - string[] envp = CreateEnvp(startInfo); - string cwd = !string.IsNullOrWhiteSpace(startInfo.WorkingDirectory) ? startInfo.WorkingDirectory : null; + if (result != 0) + { + if (!startInfo.UseShellExecute) + { + // Could not find the file + Debug.Assert(string.IsNullOrEmpty(filename)); + throw new Win32Exception(Interop.Error.ENOENT.Info().RawErrno); + } + + // this time, set the filename as default program to open file/url + filename = GetPathToOpenFile(); + argv = ParseArgv(startInfo, GetPathToOpenFile()); - // Invoke the shim fork/execve routine. It will create pipes for all requested - // redirects, fork a child process, map the pipe ends onto the appropriate stdin/stdout/stderr - // descriptors, and execve to execute the requested process. The shim implementation - // 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); + result = Interop.Sys.ForkAndExecProcess( + filename, argv, envp, cwd, + startInfo.RedirectStandardInput, startInfo.RedirectStandardOutput, startInfo.RedirectStandardError, + out childPid, + out stdinFd, out stdoutFd, out stderrFd); + + Debug.Assert(result == 0); + } // Store the child's information into this Process object. Debug.Assert(childPid >= 0); @@ -300,23 +327,31 @@ private bool StartCore(ProcessStartInfo startInfo) /// Size to use for redirect streams and stream readers/writers. private const int StreamBufferSize = 4096; + /// Converts the filename and arguments information from a ProcessStartInfo into an argv array. /// The ProcessStartInfo. + /// alternative resolved path to use as first argument /// The argv array. - private static string[] ParseArgv(ProcessStartInfo psi) + private static string[] ParseArgv(ProcessStartInfo psi, string alternativePath = null) { - string argv0 = psi.FileName; // pass filename (instead of resolved path) as argv[0], to match what caller supplied - if (string.IsNullOrEmpty(psi.Arguments)) + string argv0 = psi.FileName; // when no alternative path exists, pass filename (instead of resolved path) as argv[0], to match what caller supplied + if (string.IsNullOrEmpty(psi.Arguments) && string.IsNullOrEmpty(alternativePath)) { return new string[] { argv0 }; } - else + + var argvList = new List(); + if (!string.IsNullOrEmpty(alternativePath)) { - var argvList = new List(); - argvList.Add(argv0); - ParseArgumentsIntoList(psi.Arguments, argvList); - return argvList.ToArray(); + argvList.Add(alternativePath); + if (alternativePath.Contains("kfmclient")) + { + argvList.Add("openURL"); // kfmclient needs OpenURL + } } + argvList.Add(argv0); + ParseArgumentsIntoList(psi.Arguments, argvList); + return argvList.ToArray(); } /// Converts the environment variables information from a ProcessStartInfo into an envp array. @@ -333,9 +368,9 @@ private static string[] CreateEnvp(ProcessStartInfo psi) return envp; } - /// Resolves a path to the filename passed to ProcessStartInfo. + /// Resolves a path to the filename passed to ProcessStartInfo. /// The filename. - /// The resolved path. + /// The resolved path. It can return null in case of URLs. private static string ResolvePath(string filename) { // Follow the same resolution that Windows uses with CreateProcess: @@ -377,6 +412,17 @@ private static string ResolvePath(string filename) } // Then check each directory listed in the PATH environment variables + return FindProgramInPath(filename); + } + + /// + /// Gets the path to the program + /// + /// + /// + private static string FindProgramInPath(string program) + { + string path; string pathEnvVar = Environment.GetEnvironmentVariable("PATH"); if (pathEnvVar != null) { @@ -384,16 +430,14 @@ private static string ResolvePath(string filename) while (pathParser.MoveNext()) { string subPath = pathParser.ExtractCurrent(); - path = Path.Combine(subPath, filename); + path = Path.Combine(subPath, program); if (File.Exists(path)) { return path; } } } - - // Could not find the file - throw new Win32Exception(Interop.Error.ENOENT.Info().RawErrno); + return null; } /// Convert a number of "jiffies", or ticks, to a TimeSpan. @@ -436,7 +480,7 @@ private static FileStream OpenStream(int fd, FileAccess access) { Debug.Assert(fd >= 0); return new FileStream( - new SafeFileHandle((IntPtr)fd, ownsHandle: true), + new SafeFileHandle((IntPtr)fd, ownsHandle: true), access, StreamBufferSize, isAsync: false); } diff --git a/src/System.Diagnostics.Process/tests/ProcessTests.Unix.cs b/src/System.Diagnostics.Process/tests/ProcessTests.Unix.cs index e8470a92e8ed..9f7b7fe68a8d 100644 --- a/src/System.Diagnostics.Process/tests/ProcessTests.Unix.cs +++ b/src/System.Diagnostics.Process/tests/ProcessTests.Unix.cs @@ -16,6 +16,9 @@ namespace System.Diagnostics.Tests { public partial class ProcessTests : ProcessTestBase { + private const string s_xdg_open = "xdg-open"; + private const int s_exit_code_kill = 137; // using exit code 137 to show the process was killed + [Fact] private void TestWindowApisUnix() { @@ -61,13 +64,148 @@ public void TestRootGetProcessById() Assert.Equal(1, p.Id); } - [Fact] - public void TestUseShellExecute_Unix_Succeeds() + [Theory, InlineData(false), InlineData(true)] // Expected behavior varies on Windows and Unix. Refer to #23969 + public void ProcessStart_TryOpenFolder_ThrowsWin32Exception(bool useShellExecute) + { + Win32Exception e = Assert.Throws(() => Process.Start(new ProcessStartInfo { UseShellExecute = useShellExecute, FileName = Path.GetTempPath() })); + } + + [Fact, PlatformSpecific(TestPlatforms.Linux)] + [OuterLoop("Opens program")] + public void ProcessStart_UseShellExecuteTrue_OpenFile_ThrowsIfNoDefaultProgramInstalledSucceedsOtherwise() + { + string fileToOpen = GetTestFilePath() + ".txt"; + File.WriteAllText(fileToOpen, $"{nameof(ProcessStart_UseShellExecuteTrue_OpenFile_ThrowsIfNoDefaultProgramInstalledSucceedsOtherwise)}"); + + string[] allowedProgramsToRun = { s_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)) + { + Assert.NotNull(px); + Console.WriteLine($"{nameof(ProcessStart_UseShellExecuteTrue_OpenFile_ThrowsIfNoDefaultProgramInstalledSucceedsOtherwise)}(): {program} was used to open file on this machine. ProcessName: {px.ProcessName}"); + Assert.Equal(program, px.ProcessName); + px.Kill(); + px.WaitForExit(); + Assert.True(px.HasExited); + Assert.Equal(s_exit_code_kill, px.ExitCode); + } + return; + } + } + + Win32Exception e = Assert.Throws(() => Process.Start(new ProcessStartInfo { UseShellExecute = true, FileName = fileToOpen })); + } + + [Theory, InlineData("nano"), InlineData("vi")] + [PlatformSpecific(TestPlatforms.Linux)] + [OuterLoop("Opens program")] + public void ProcessStart_OpenFileOnLinux_UsesSpecifiedProgram(string programToOpenWith) + { + if (IsProgramInstalled(programToOpenWith)) + { + string fileToOpen = GetTestFilePath() + ".txt"; + File.WriteAllText(fileToOpen, $"{nameof(ProcessStart_OpenFileOnLinux_UsesSpecifiedProgram)}"); + using (var px = Process.Start(programToOpenWith, fileToOpen)) + { + Assert.Equal(programToOpenWith, px.ProcessName); + px.Kill(); + px.WaitForExit(); + Assert.True(px.HasExited); + Assert.Equal(s_exit_code_kill, px.ExitCode); + } + } + else + { + Console.WriteLine($"Program specified to open file with {programToOpenWith} is not installed on this machine."); + } + } + + [Fact, PlatformSpecific(TestPlatforms.Linux)] + [OuterLoop("Opens program")] + public void ProcessStart_UseShellExecuteTrue_OpenMissingFile_XdgOpenReturnsExitCode2() + { + // The exit code is coming from xdg-open. Which is why I split this test for OSX and Linux to assert against two different exit code values. + if (IsProgramInstalled(s_xdg_open)) + { + string fileToOpen = Path.Combine(Environment.CurrentDirectory, "_no_such_file.TXT"); + using (var p = Process.Start(new ProcessStartInfo { UseShellExecute = true, FileName = fileToOpen })) + { + Assert.NotNull(p); + Assert.Equal(s_xdg_open, p.ProcessName); + p.WaitForExit(); + Assert.True(p.HasExited); + Assert.Equal(2, p.ExitCode); // Exit Code 2 from xdg-open means file was not found + } + } + else + { + Console.WriteLine($"{nameof(ProcessStart_UseShellExecuteTrue_OpenMissingFile_XdgOpenReturnsExitCode2)}(): {s_xdg_open} is not installed on this machine."); + } + } + + [Fact, PlatformSpecific(TestPlatforms.OSX)] + [OuterLoop("Opens program")] + public void ProcessStart_UseShellExecuteTrue_TryOpenFileThatDoesntExist_ReturnsExitCode1() { - using (var p = Process.Start(new ProcessStartInfo { UseShellExecute = true, FileName = "exit", Arguments = "42" })) + // The exit code is coming from open. Which is why I split this test for OSX and Linux to assert against two different exit code values. + 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(42, p.ExitCode); + Assert.Equal(1, p.ExitCode); // Exit Code 1 from open means something went wrong + } + } + + [Theory, InlineData("/usr/bin/open"), InlineData("/usr/bin/nano")] + [PlatformSpecific(TestPlatforms.OSX)] + [OuterLoop("Opens program")] + public void ProcessStart_OpenFileOnOsx_UsesSpecifiedProgram(string programToOpenWith) + { + string fileToOpen = GetTestFilePath() + ".txt"; + File.WriteAllText(fileToOpen, $"{nameof(ProcessStart_OpenFileOnOsx_UsesSpecifiedProgram)}"); + using (var px = Process.Start(programToOpenWith, fileToOpen)) + { + Console.WriteLine($"in OSX, {nameof(programToOpenWith)} is {programToOpenWith}, while {nameof(px.ProcessName)} is {px.ProcessName}."); + // Assert.Equal(programToOpenWith, px.ProcessName); // on OSX, process name is dotnet for some reason. Refer to #23972 + px.Kill(); + px.WaitForExit(); + Assert.True(px.HasExited); + Assert.Equal(s_exit_code_kill, px.ExitCode); + } + } + + [Theory, InlineData("Safari"), InlineData("\"Google Chrome\"")] + [PlatformSpecific(TestPlatforms.OSX)] + [OuterLoop("Opens browser")] + public void ProcessStart_OpenUrl_UsesSpecifiedApplication(string applicationToOpenWith) + { + using (var px = Process.Start("/usr/bin/open", "https://github.com/dotnet/corefx -a " + applicationToOpenWith)) + { + Assert.NotNull(px); + px.Kill(); + px.WaitForExit(); + Assert.True(px.HasExited); + Assert.Equal(s_exit_code_kill, px.ExitCode); + } + } + + [Theory, InlineData("-a Safari"), InlineData("-a \"Google Chrome\"")] + [PlatformSpecific(TestPlatforms.OSX)] + [OuterLoop("Opens browser")] + public void ProcessStart_UseShellExecuteTrue_OpenUrl_SuccessfullyReadsArgument(string arguments) + { + var startInfo = new ProcessStartInfo { UseShellExecute = true, FileName = "https://github.com/dotnet/corefx", Arguments = arguments }; + using (var px = Process.Start(startInfo)) + { + Assert.NotNull(px); + px.Kill(); + px.WaitForExit(); + Assert.True(px.HasExited); + Assert.Equal(s_exit_code_kill, px.ExitCode); } } @@ -148,5 +286,30 @@ public void TestStartOnUnixWithBadFormat() [DllImport("libc")] private static extern int chmod(string path, int mode); + + /// + /// Checks if the program is installed + /// + /// + /// + private bool IsProgramInstalled(string program) + { + string path; + string pathEnvVar = Environment.GetEnvironmentVariable("PATH"); + if (pathEnvVar != null) + { + var pathParser = new StringParser(pathEnvVar, ':', skipEmpty: true); + while (pathParser.MoveNext()) + { + string subPath = pathParser.ExtractCurrent(); + path = Path.Combine(subPath, program); + if (File.Exists(path)) + { + return true; + } + } + } + return false; + } } } diff --git a/src/System.Diagnostics.Process/tests/ProcessTests.cs b/src/System.Diagnostics.Process/tests/ProcessTests.cs index 4eed35edee33..e452dc88c0d6 100644 --- a/src/System.Diagnostics.Process/tests/ProcessTests.cs +++ b/src/System.Diagnostics.Process/tests/ProcessTests.cs @@ -2,7 +2,6 @@ // 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; using System.Collections.Generic; using System.ComponentModel; using System.IO; @@ -13,7 +12,6 @@ using System.Text; using System.Threading; using Xunit; -using Xunit.NetCore.Extensions; namespace System.Diagnostics.Tests { @@ -120,6 +118,36 @@ public void TestEnableRaiseEvents(bool? enable) } } + [Fact] + public void ProcessStart_TryExitCommandAsFileName_ThrowsWin32Exception() + { + Win32Exception e = Assert.Throws(() => Process.Start(new ProcessStartInfo { UseShellExecute = false, FileName = "exit", Arguments = "42" })); + } + + [Fact] + public void ProcessStart_UseShellExecuteFalse_FilenameIsUrl_ThrowsWin32Exception() + { + Win32Exception e = Assert.Throws(() => Process.Start(new ProcessStartInfo { UseShellExecute = false, FileName = "https://www.github.com/corefx" })); + } + + [Fact] + public void ProcessStart_TryOpenFolder_UseShellExecuteIsFalse_ThrowsWin32Exception() + { + Win32Exception e = Assert.Throws(() => Process.Start(new ProcessStartInfo { UseShellExecute = false, FileName = Path.GetTempPath() })); + } + + [PlatformSpecific(TestPlatforms.Windows)] // Expected behavior varies on Windows and Unix. Refer to #23969 + [ConditionalFact(typeof(PlatformDetection), nameof(PlatformDetection.IsNotWindowsNanoServer))] // Nano does not support UseShellExecute + [SkipOnTargetFramework(TargetFrameworkMonikers.Uap, "not supported on UAP")] + [OuterLoop("Launches File Explorer")] + public void ProcessStart_TryOpenFolder_UseShellExecuteIsTrue_DoesNotThrow() + { + using (var px = Process.Start(new ProcessStartInfo { UseShellExecute = true, FileName = Path.GetTempPath() })) + { + Assert.Null(px); // Not sure why px returned is null. but the call does not throw and opens folder successfully. + } + } + [Fact] public void TestExitCode() { diff --git a/src/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj b/src/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj index ae37c17dbdb9..84eb2b4c5637 100644 --- a/src/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj +++ b/src/System.Diagnostics.Process/tests/System.Diagnostics.Process.Tests.csproj @@ -15,6 +15,9 @@ Common\System\PasteArguments.cs + + Common\System\IO\StringParser.cs + Common\System\ShouldNotBeInvokedException.cs