Skip to content

Commit

Permalink
Unix: make UseShellExecute execute executables (dotnet/corefx#33052)
Browse files Browse the repository at this point in the history 
* Unix: make UseShellExecute execute executables

On Windows, UseShellExecute executes executables.
This gives it the same behavior on Unix.

* Add cross-platform tests

* fix Windows test build failure

* Add test for non-executable file with x-bit

* Skip tests on unsupported platforms

* Unix: only allow specific Verbs

* Limit Unix tests to run on Linux

* Fix test for verb null

* Improve verb check

* test: make working directory a temp location

* test: refactor WriteScriptFile

* test: refactor temp dir creation

* test: fix build failure

* Condense verb check

* Find the executable in the ProcessStartInfo.WorkingDirectory

* Don't pass Arguments when opening a document

* PR Feedback

* Fix verb check

* PR feedback


Commit migrated from dotnet/corefx@95619c8
  • Loading branch information
@tmds @stephentoub
tmds authored and stephentoub committed Dec 5, 2018
1 parent a336ed4 commit 656e5d4
Show file tree
Hide file tree
Showing 7 changed files with 422 additions and 77 deletions.
17 changes: 2 additions & 15 deletions src/libraries/Common/src/Interop/Unix/System.Native/Interop.ForkAndExecProcess.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ 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,
bool setUser, uint userId, uint groupId,
Expand All @@ -29,20 +29,7 @@ internal static partial class Sys
redirectStdin ? 1 : 0, redirectStdout ? 1 : 0, redirectStderr ? 1 :0,
setUser ? 1 : 0, userId, groupId,
out lpChildPid, out stdinFd, out stdoutFd, out stderrFd);
if (result != 0)
{
// Normally we'd simply make this method return the result of the native
// call and allow the caller to use GetLastWin32Error. However, we need
// to free the native arrays after calling the function, and doing so
// stomps on the runtime's captured last error. So we need to access the
// error here, and without SetLastWin32Error available, we can't propagate
// the error to the caller via the normal GetLastWin32Error mechanism. We could
// return 0 on success or the GetLastWin32Error value on failure, but that's
// 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();
}
return result == 0 ? 0 : Marshal.GetLastWin32Error();
}
finally
{
Expand Down
9 changes: 6 additions & 3 deletions src/libraries/System.Diagnostics.Process/src/System.Diagnostics.Process.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@
<Compile Include="..\..\Common\src\System\PasteArguments.cs">
<Link>Common\System\PasteArguments.cs</Link>
</Compile>
<Compile Include="$(CommonPath)\Interop\Windows\Interop.Errors.cs">
<Link>Common\Interop\Windows\Interop.Errors.cs</Link>
</Compile>
</ItemGroup>
<ItemGroup Condition=" '$(TargetsWindows)' == 'true' and '$(TargetGroup)' != 'uap'">
<Compile Include="$(CommonPath)\Interop\Windows\kernel32\Interop.EnumProcessModules.cs">
Expand Down Expand Up @@ -223,9 +226,6 @@
<Compile Include="$(CommonPath)\Interop\Windows\kernel32\Interop.CreatePipe_SafeFileHandle.cs">
<Link>Common\Interop\Windows\kernel32\Interop.CreatePipe.cs</Link>
</Compile>
<Compile Include="$(CommonPath)\Interop\Windows\Interop.Errors.cs">
<Link>Common\Interop\Windows\Interop.Errors.cs</Link>
</Compile>
<Compile Include="$(CommonPath)\Interop\Windows\kernel32\Interop.ThreadOptions.cs">
<Link>Common\Interop\Windows\kernel32\Interop.ThreadOptions.cs</Link>
</Compile>
Expand Down Expand Up @@ -346,6 +346,9 @@
<Compile Include="$(CommonPath)\Interop\Unix\System.Native\Interop.WaitPid.cs">
<Link>Common\Interop\Unix\Interop.WaitPid.cs</Link>
</Compile>
<Compile Include="$(CommonPath)\Interop\Unix\System.Native\Interop.Access.cs">
<Link>Common\Interop\Unix\System.Native\Interop.Access.cs</Link>
</Compile>
</ItemGroup>
<ItemGroup Condition=" '$(TargetsLinux)' == 'true'">
<Compile Include="System\Diagnostics\Process.Linux.cs" />
Expand Down
243 changes: 184 additions & 59 deletions src/libraries/System.Diagnostics.Process/src/System/Diagnostics/Process.Unix.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -291,15 +291,56 @@ private bool StartCore(ProcessStartInfo startInfo)
}
}

int childPid, stdinFd, stdoutFd, stderrFd;
int stdinFd = -1, stdoutFd = -1, stderrFd = -1;
string[] envp = CreateEnvp(startInfo);
string cwd = !string.IsNullOrWhiteSpace(startInfo.WorkingDirectory) ? startInfo.WorkingDirectory : null;

bool setCredentials = !string.IsNullOrEmpty(startInfo.UserName);
uint userId = 0;
uint groupId = 0;
if (setCredentials)
{
(userId, groupId) = GetUserAndGroupIds(startInfo);
}

if (startInfo.UseShellExecute)
{
string verb = startInfo.Verb;
if (verb != string.Empty &&
!string.Equals(verb, "open", StringComparison.OrdinalIgnoreCase))
{
throw new Win32Exception(Interop.Errors.ERROR_NO_ASSOCIATION);
}

// On Windows, UseShellExecute of executables and scripts causes those files to be executed.
// To achieve this on Unix, we check if the file is executable (x-bit).
// Some files may have the x-bit set even when they are not executable. This happens for example
// when a Windows filesystem is mounted on Linux. To handle that, treat it as a regular file
// when exec returns ENOEXEC (file format cannot be executed).
bool isExecuting = false;
filename = ResolveExecutableForShellExecute(startInfo.FileName, cwd);
if (filename != null)
{
argv = ParseArgv(startInfo);

isExecuting = ForkAndExecProcess(filename, argv, envp, cwd,
startInfo.RedirectStandardInput, startInfo.RedirectStandardOutput, startInfo.RedirectStandardError,
setCredentials, userId, groupId,
out stdinFd, out stdoutFd, out stderrFd,
throwOnNoExec: false); // return false instead of throwing on ENOEXEC
}

// use default program to open file/url
filename = GetPathToOpenFile();
argv = ParseArgv(startInfo, filename);
if (!isExecuting)
{
filename = GetPathToOpenFile();
argv = ParseArgv(startInfo, filename, ignoreArguments: true);

ForkAndExecProcess(filename, argv, envp, cwd,
startInfo.RedirectStandardInput, startInfo.RedirectStandardOutput, startInfo.RedirectStandardError,
setCredentials, userId, groupId,
out stdinFd, out stdoutFd, out stderrFd);
}
}
else
{
Expand All @@ -309,50 +350,11 @@ private bool StartCore(ProcessStartInfo startInfo)
{
throw new Win32Exception(SR.DirectoryNotValidAsInput);
}
}

if (string.IsNullOrEmpty(filename))
{
throw new Win32Exception(Interop.Error.ENOENT.Info().RawErrno);
}

bool setCredentials = !string.IsNullOrEmpty(startInfo.UserName);
uint userId = 0;
uint groupId = 0;
if (setCredentials)
{
(userId, groupId) = GetUserAndGroupIds(startInfo);
}

// Lock to avoid races with OnSigChild
// By using a ReaderWriterLock we allow multiple processes to start concurrently.
s_processStartLock.EnterReadLock();
try
{
// 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.)
Interop.Sys.ForkAndExecProcess(
filename, argv, envp, cwd,
ForkAndExecProcess(filename, argv, envp, cwd,
startInfo.RedirectStandardInput, startInfo.RedirectStandardOutput, startInfo.RedirectStandardError,
setCredentials, userId, groupId,
out childPid,
setCredentials, userId, groupId,
out stdinFd, out stdoutFd, out stderrFd);

// Ensure we'll reap this process.
// note: SetProcessId will set this if we don't set it first.
_waitStateHolder = new ProcessWaitState.Holder(childPid, isNewChild: true);

// Store the child's information into this Process object.
Debug.Assert(childPid >= 0);
SetProcessId(childPid);
SetProcessHandle(new SafeProcessHandle(childPid));
}
finally
{
s_processStartLock.ExitReadLock();
}

// Configure the parent's ends of the redirection streams.
Expand Down Expand Up @@ -381,6 +383,67 @@ private bool StartCore(ProcessStartInfo startInfo)
return true;
}

private bool ForkAndExecProcess(
string filename, string[] argv, string[] envp, string cwd,
bool redirectStdin, bool redirectStdout, bool redirectStderr,
bool setCredentials, uint userId, uint groupId,
out int stdinFd, out int stdoutFd, out int stderrFd,
bool throwOnNoExec = true)
{
if (string.IsNullOrEmpty(filename))
{
throw new Win32Exception(Interop.Error.ENOENT.Info().RawErrno);
}

// Lock to avoid races with OnSigChild
// By using a ReaderWriterLock we allow multiple processes to start concurrently.
s_processStartLock.EnterReadLock();
try
{
int childPid;

// 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 errno = Interop.Sys.ForkAndExecProcess(
filename, argv, envp, cwd,
redirectStdin, redirectStdout, redirectStderr,
setCredentials, userId, groupId,
out childPid,
out stdinFd, out stdoutFd, out stderrFd);

if (errno == 0)
{
// Ensure we'll reap this process.
// note: SetProcessId will set this if we don't set it first.
_waitStateHolder = new ProcessWaitState.Holder(childPid, isNewChild: true);

// Store the child's information into this Process object.
Debug.Assert(childPid >= 0);
SetProcessId(childPid);
SetProcessHandle(new SafeProcessHandle(childPid));

return true;
}
else
{
if (!throwOnNoExec &&
new Interop.ErrorInfo(errno).Error == Interop.Error.ENOEXEC)
{
return false;
}

throw new Win32Exception(errno);
}
}
finally
{
s_processStartLock.ExitReadLock();
}
}

// -----------------------------
// ---- PAL layer ends here ----
// -----------------------------
Expand All @@ -393,34 +456,39 @@ private bool StartCore(ProcessStartInfo startInfo)

/// <summary>Converts the filename and arguments information from a ProcessStartInfo into an argv array.</summary>
/// <param name="psi">The ProcessStartInfo.</param>
/// <param name="alternativePath">alternative resolved path to use as first argument</param>
/// <param name="resolvedExe">Resolved executable to open ProcessStartInfo.FileName</param>
/// <param name="ignoreArguments">Don't pass ProcessStartInfo.Arguments</param>
/// <returns>The argv array.</returns>
private static string[] ParseArgv(ProcessStartInfo psi, string alternativePath = null)
private static string[] ParseArgv(ProcessStartInfo psi, string resolvedExe = null, bool ignoreArguments = false)
{
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) && psi.ArgumentList.Count == 0)
if (string.IsNullOrEmpty(resolvedExe) &&
(ignoreArguments || (string.IsNullOrEmpty(psi.Arguments) && psi.ArgumentList.Count == 0)))
{
return new string[] { argv0 };
return new string[] { psi.FileName };
}

var argvList = new List<string>();
if (!string.IsNullOrEmpty(alternativePath))
if (!string.IsNullOrEmpty(resolvedExe))
{
argvList.Add(alternativePath);
if (alternativePath.Contains("kfmclient"))
argvList.Add(resolvedExe);
if (resolvedExe.Contains("kfmclient"))
{
argvList.Add("openURL"); // kfmclient needs OpenURL
}
}

argvList.Add(argv0);
if (!string.IsNullOrEmpty(psi.Arguments))
{
ParseArgumentsIntoList(psi.Arguments, argvList);
}
else
argvList.Add(psi.FileName);

if (!ignoreArguments)
{
argvList.AddRange(psi.ArgumentList);
if (!string.IsNullOrEmpty(psi.Arguments))
{
ParseArgumentsIntoList(psi.Arguments, argvList);
}
else
{
argvList.AddRange(psi.ArgumentList);
}
}
return argvList.ToArray();
}
Expand All @@ -439,6 +507,63 @@ private static string[] CreateEnvp(ProcessStartInfo psi)
return envp;
}

private static string ResolveExecutableForShellExecute(string filename, string workingDirectory)
{
// Determine if filename points to an executable file.
// filename may be an absolute path, a relative path or a uri.

string resolvedFilename = null;
// filename is an absolute path
if (Path.IsPathRooted(filename))
{
if (File.Exists(filename))
{
resolvedFilename = filename;
}
}
// filename is a uri
else if (Uri.TryCreate(filename, UriKind.Absolute, out Uri uri))
{
if (uri.IsFile && uri.Host == "" && File.Exists(uri.LocalPath))
{
resolvedFilename = uri.LocalPath;
}
}
// filename is relative
else
{
// The WorkingDirectory property specifies the location of the executable.
// If WorkingDirectory is an empty string, the current directory is understood to contain the executable.
workingDirectory = workingDirectory != null ? Path.GetFullPath(workingDirectory) :
Directory.GetCurrentDirectory();
string filenameInWorkingDirectory = Path.Combine(workingDirectory, filename);
// filename is a relative path in the working directory
if (File.Exists(filenameInWorkingDirectory))
{
resolvedFilename = filenameInWorkingDirectory;
}
// find filename on PATH
else
{
resolvedFilename = FindProgramInPath(filename);
}
}

if (resolvedFilename == null)
{
return null;
}

if (Interop.Sys.Access(resolvedFilename, Interop.Sys.AccessMode.X_OK) == 0)
{
return resolvedFilename;
}
else
{
return null;
}
}

/// <summary>Resolves a path to the filename passed to ProcessStartInfo. </summary>
/// <param name="filename">The filename.</param>
/// <returns>The resolved path. It can return null in case of URLs.</returns>
Expand Down
Loading

0 comments on commit 656e5d4

Please sign in to comment.