This is an automated email from the ASF dual-hosted git repository. bmahler pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/mesos.git
commit 570d80d06140035e311525e179156fc3fc67e667 Author: Benjamin Mahler <[email protected]> AuthorDate: Thu Mar 19 21:00:34 2020 -0400 Split out exec.hpp from shell.hpp in stout. shell.hpp included both shell command execution (i.e. `sh -c '...'` and `cmd.exe /c ...`) and non-shell command execution (i.e. fork/exec with explicit arguments and CreateProcess with explicit command line). This is rather confusing, since the latter of the two does not involve a shell. To make this clearer, we split the exec style utilities into their own header. This also helps set up some simpler diffs for fixing windows command execution, see MESOS-10093. Review: https://reviews.apache.org/r/72273 --- 3rdparty/stout/include/Makefile.am | 3 + 3rdparty/stout/include/stout/os.hpp | 1 + 3rdparty/stout/include/stout/os/exec.hpp | 24 ++ 3rdparty/stout/include/stout/os/posix/chown.hpp | 1 - 3rdparty/stout/include/stout/os/posix/copyfile.hpp | 2 +- 3rdparty/stout/include/stout/os/posix/exec.hpp | 99 +++++ 3rdparty/stout/include/stout/os/posix/shell.hpp | 46 --- .../stout/os/windows/{shell.hpp => exec.hpp} | 223 +++-------- 3rdparty/stout/include/stout/os/windows/shell.hpp | 420 +-------------------- 3rdparty/stout/include/stout/posix/os.hpp | 21 -- 3rdparty/stout/tests/os/process_tests.cpp | 4 +- 3rdparty/stout/tests/os/rmdir_tests.cpp | 1 + 3rdparty/stout/tests/os_tests.cpp | 5 +- 13 files changed, 188 insertions(+), 662 deletions(-) diff --git a/3rdparty/stout/include/Makefile.am b/3rdparty/stout/include/Makefile.am index 29dbf99..ad3f210 100644 --- a/3rdparty/stout/include/Makefile.am +++ b/3rdparty/stout/include/Makefile.am @@ -75,6 +75,7 @@ nobase_include_HEADERS = \ stout/os/copyfile.hpp \ stout/os/dup.hpp \ stout/os/environment.hpp \ + stout/os/exec.hpp \ stout/os/exists.hpp \ stout/os/fcntl.hpp \ stout/os/getenv.hpp \ @@ -130,6 +131,7 @@ nobase_include_HEADERS = \ stout/os/posix/copyfile.hpp \ stout/os/posix/dup.hpp \ stout/os/posix/environment.hpp \ + stout/os/posix/exec.hpp \ stout/os/posix/exists.hpp \ stout/os/posix/fcntl.hpp \ stout/os/posix/fork.hpp \ @@ -172,6 +174,7 @@ nobase_include_HEADERS = \ stout/os/windows/copyfile.hpp \ stout/os/windows/dup.hpp \ stout/os/windows/environment.hpp \ + stout/os/windows/exec.hpp \ stout/os/windows/exists.hpp \ stout/os/windows/fcntl.hpp \ stout/os/windows/fd.hpp \ diff --git a/3rdparty/stout/include/stout/os.hpp b/3rdparty/stout/include/stout/os.hpp index 15cbb50..2e1537c 100644 --- a/3rdparty/stout/include/stout/os.hpp +++ b/3rdparty/stout/include/stout/os.hpp @@ -47,6 +47,7 @@ #include <stout/os/chdir.hpp> #include <stout/os/chroot.hpp> #include <stout/os/dup.hpp> +#include <stout/os/exec.hpp> #include <stout/os/exists.hpp> #include <stout/os/fcntl.hpp> #include <stout/os/getenv.hpp> diff --git a/3rdparty/stout/include/stout/os/exec.hpp b/3rdparty/stout/include/stout/os/exec.hpp new file mode 100644 index 0000000..4ad130b --- /dev/null +++ b/3rdparty/stout/include/stout/os/exec.hpp @@ -0,0 +1,24 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef __STOUT_OS_EXEC_HPP__ +#define __STOUT_OS_EXEC_HPP__ + +// For readability, we minimize the number of #ifdef blocks in the code by +// splitting platform specific system calls into separate directories. +#ifdef __WINDOWS__ +#include <stout/os/windows/exec.hpp> +#else +#include <stout/os/posix/exec.hpp> +#endif // __WINDOWS__ + +#endif // __STOUT_OS_EXEC_HPP__ diff --git a/3rdparty/stout/include/stout/os/posix/chown.hpp b/3rdparty/stout/include/stout/os/posix/chown.hpp index 644129e..6fefa08 100644 --- a/3rdparty/stout/include/stout/os/posix/chown.hpp +++ b/3rdparty/stout/include/stout/os/posix/chown.hpp @@ -21,7 +21,6 @@ #include <stout/nothing.hpp> #include <stout/try.hpp> -#include <stout/os/shell.hpp> #include <stout/os/stat.hpp> namespace os { diff --git a/3rdparty/stout/include/stout/os/posix/copyfile.hpp b/3rdparty/stout/include/stout/os/posix/copyfile.hpp index 40b9e47..aa6c2d6 100644 --- a/3rdparty/stout/include/stout/os/posix/copyfile.hpp +++ b/3rdparty/stout/include/stout/os/posix/copyfile.hpp @@ -22,7 +22,7 @@ #include <stout/stringify.hpp> #include <stout/try.hpp> -#include <stout/os/shell.hpp> +#include <stout/os/exec.hpp> #include <stout/os/stat.hpp> namespace os { diff --git a/3rdparty/stout/include/stout/os/posix/exec.hpp b/3rdparty/stout/include/stout/os/posix/exec.hpp new file mode 100644 index 0000000..db57130 --- /dev/null +++ b/3rdparty/stout/include/stout/os/posix/exec.hpp @@ -0,0 +1,99 @@ +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef __STOUT_OS_POSIX_EXEC_HPP__ +#define __STOUT_OS_POSIX_EXEC_HPP__ + +#include <stdlib.h> // For exit. +#include <unistd.h> // For fork, exec*. + +#include <sys/wait.h> // For waitpid. + +#include <string> +#include <vector> + +#include <stout/none.hpp> +#include <stout/option.hpp> + +#include <stout/os/raw/argv.hpp> +#include <stout/os/raw/environment.hpp> + +namespace os { + +// Executes a command by calling "<command> <arguments...>", and returns after +// the command has been completed. Returns the exit code on success and `None` +// on error (e.g., fork/exec/waitpid failed). This function is async signal +// safe. We return an `Option<int>` instead of a `Try<int>`, because although +// `Try` does not dynamically allocate, `Error` uses `std::string`, which is +// not async signal safe. +inline Option<int> spawn( + const std::string& file, + const std::vector<std::string>& arguments) +{ + pid_t pid = ::fork(); + + if (pid == -1) { + return None(); + } else if (pid == 0) { + // In child process. + ::execvp(file.c_str(), os::raw::Argv(arguments)); + ::exit(127); + } else { + // In parent process. + int status; + while (::waitpid(pid, &status, 0) == -1) { + if (errno != EINTR) { + return None(); + } + } + + return status; + } +} + + +template<typename... T> +inline int execlp(const char* file, T... t) +{ + return ::execlp(file, t...); +} + + +inline int execvp(const char* file, char* const argv[]) +{ + return ::execvp(file, argv); +} + + +// This function is a portable version of execvpe ('p' means searching +// executable from PATH and 'e' means setting environments). We add +// this function because it is not available on all systems. +// +// NOTE: This function is not thread safe. It is supposed to be used +// only after fork (when there is only one thread). This function is +// async signal safe. +inline int execvpe(const char* file, char** argv, char** envp) +{ + char** saved = os::raw::environment(); + + *os::raw::environmentp() = envp; + + int result = execvp(file, argv); + + *os::raw::environmentp() = saved; + + return result; +} + +} // namespace os { + +#endif // __STOUT_OS_POSIX_EXEC_HPP__ diff --git a/3rdparty/stout/include/stout/os/posix/shell.hpp b/3rdparty/stout/include/stout/os/posix/shell.hpp index 2cc4e2c..96dd280 100644 --- a/3rdparty/stout/include/stout/os/posix/shell.hpp +++ b/3rdparty/stout/include/stout/os/posix/shell.hpp @@ -29,8 +29,6 @@ #include <stout/option.hpp> #include <stout/try.hpp> -#include <stout/os/raw/argv.hpp> - namespace os { namespace Shell { @@ -153,50 +151,6 @@ inline Option<int> system(const std::string& command) } } -// Executes a command by calling "<command> <arguments...>", and returns after -// the command has been completed. Returns the exit code on success and `None` -// on error (e.g., fork/exec/waitpid failed). This function is async signal -// safe. We return an `Option<int>` instead of a `Try<int>`, because although -// `Try` does not dynamically allocate, `Error` uses `std::string`, which is -// not async signal safe. -inline Option<int> spawn( - const std::string& command, - const std::vector<std::string>& arguments) -{ - pid_t pid = ::fork(); - - if (pid == -1) { - return None(); - } else if (pid == 0) { - // In child process. - ::execvp(command.c_str(), os::raw::Argv(arguments)); - ::exit(127); - } else { - // In parent process. - int status; - while (::waitpid(pid, &status, 0) == -1) { - if (errno != EINTR) { - return None(); - } - } - - return status; - } -} - - -template<typename... T> -inline int execlp(const char* file, T... t) -{ - return ::execlp(file, t...); -} - - -inline int execvp(const char* file, char* const argv[]) -{ - return ::execvp(file, argv); -} - } // namespace os { #endif // __STOUT_OS_POSIX_SHELL_HPP__ diff --git a/3rdparty/stout/include/stout/os/windows/shell.hpp b/3rdparty/stout/include/stout/os/windows/exec.hpp similarity index 72% copy from 3rdparty/stout/include/stout/os/windows/shell.hpp copy to 3rdparty/stout/include/stout/os/windows/exec.hpp index 2f76bba..89db8af 100644 --- a/3rdparty/stout/include/stout/os/windows/shell.hpp +++ b/3rdparty/stout/include/stout/os/windows/exec.hpp @@ -10,10 +10,14 @@ // See the License for the specific language governing permissions and // limitations under the License. -#ifndef __STOUT_OS_WINDOWS_SHELL_HPP__ -#define __STOUT_OS_WINDOWS_SHELL_HPP__ +#ifndef __STOUT_OS_WINDOWS_EXEC_HPP__ +#define __STOUT_OS_WINDOWS_EXEC_HPP__ #include <process.h> +#include <processthreadsapi.h> +#include <synchapi.h> +#include <userenv.h> + #include <stdarg.h> // For va_list, va_start, etc. #include <algorithm> @@ -35,8 +39,9 @@ #include <stout/internal/windows/inherit.hpp> -namespace internal { +namespace os { namespace windows { +namespace internal { // Retrieves system environment in a `std::map`, ignoring // the current process's environment variables. @@ -164,8 +169,10 @@ inline Option<std::wstring> create_process_env( inline std::wstring stringify_args(const std::vector<std::string>& argv) { std::wstring command; + for (auto argit = argv.cbegin(); argit != argv.cend(); ++argit) { std::wstring arg = wide_stringify(*argit); + // Don't quote empty arguments or those without troublesome characters. if (!arg.empty() && arg.find_first_of(L" \t\n\v\"") == arg.npos) { command.append(arg); @@ -204,13 +211,15 @@ inline std::wstring stringify_args(const std::vector<std::string>& argv) command.push_back(L' '); } } + // Append final null terminating character. command.push_back(L'\0'); return command; } -struct ProcessData { +struct ProcessData +{ SharedHandle process_handle; SharedHandle thread_handle; pid_t pid; @@ -220,9 +229,23 @@ struct ProcessData { // Provides an interface for creating a child process on Windows. // // The `command` argument is given for compatibility, and is ignored. This is -// because the `CreateProcess` will use `argv[0]` as the command to be executed, -// and will perform a `PATH` lookup. If `command` were to be used instead, -// `CreateProcess` would require an absolute path. +// because the `CreateProcess` will use the first part of `commandLine` as the +// module name to execute, and will perform a `PATH` lookup. If `command` were +// to be used instead, `CreateProcess` would require an absolute path. +// See the MSDN documentation for the complicated rules at play for parsing +// and locating the module to execute. +// +// This takes a string command line because Windows programs receive the +// entire command line as a string and perform their own parsing (unlike +// POSIX exec which allows you to explicitly pass arguments). However, +// many programs (e.g. those written with a libc `main(int argc, char** argv)`) +// will use `CommandLineToArgvW` to parse the command line. For callers +// of this function that want to pass arguments to such programs, we +// provide an overload of `create_process` that takes arguments +// explicitly and constructs an appropriate command line string for +// `CommandLineToArgvW` to parse it back out as arguments. Notably, +// cmd.exe has its own handling of the command line that differs from +// `CommandLineToArgvW`. // // If `create_suspended` is `true`, the process will not be started, and the // caller must use `ResumeThread` to start the process. @@ -286,7 +309,7 @@ inline Try<ProcessData> create_process( // Each of these handles must be inheritable. foreach (const int_fd& fd, pipes.get()) { handles.emplace_back(static_cast<HANDLE>(fd)); - const Try<Nothing> inherit = set_inherit(fd, true); + const Try<Nothing> inherit = ::internal::windows::set_inherit(fd, true); if (inherit.isError()) { return Error(inherit.error()); } @@ -306,14 +329,14 @@ inline Try<ProcessData> create_process( foreach (const int_fd& fd, whitelist_fds) { handles.emplace_back(static_cast<HANDLE>(fd)); - const Try<Nothing> inherit = set_inherit(fd, true); + const Try<Nothing> inherit = ::internal::windows::set_inherit(fd, true); if (inherit.isError()) { return Error(inherit.error()); } } - Result<std::shared_ptr<AttributeList>> attribute_list = - create_attributes_list_for_handles(handles); + Result<std::shared_ptr<::internal::windows::AttributeList>> attribute_list = + ::internal::windows::create_attributes_list_for_handles(handles); if (attribute_list.isError()) { return Error(attribute_list.error()); @@ -324,7 +347,7 @@ inline Try<ProcessData> create_process( } const BOOL result = ::CreateProcessW( - // This is replaced by the first token of `arg_buffer` string. + // This is replaced by the first token of `wideCommandLineCopy` string. static_cast<LPCWSTR>(nullptr), static_cast<LPWSTR>(arg_buffer.data()), static_cast<LPSECURITY_ATTRIBUTES>(nullptr), @@ -357,7 +380,7 @@ inline Try<ProcessData> create_process( // previous inheritance semantics of each `HANDLE`. It is assumed // that users of this function send non-inheritable handles. foreach (const int_fd& fd, pipes.get()) { - const Try<Nothing> inherit = set_inherit(fd, false); + const Try<Nothing> inherit = ::internal::windows::set_inherit(fd, false); if (inherit.isError()) { return Error(inherit.error()); } @@ -365,7 +388,7 @@ inline Try<ProcessData> create_process( } foreach (const int_fd& fd, whitelist_fds) { - const Try<Nothing> inherit = set_inherit(fd, false); + const Try<Nothing> inherit = ::internal::windows::set_inherit(fd, false); if (inherit.isError()) { return Error(inherit.error()); } @@ -382,148 +405,8 @@ inline Try<ProcessData> create_process( static_cast<pid_t>(process_info.dwProcessId)}; } -} // namespace windows { } // namespace internal { - -namespace os { -namespace Shell { - -// Canonical constants used as platform-dependent args to `exec` calls. -// `name` is the command name, `arg0` is the first argument received -// by the callee, usually the command name and `arg1` is the second -// command argument received by the callee. -constexpr const char* name = "cmd.exe"; -constexpr const char* arg0 = "cmd.exe"; -constexpr const char* arg1 = "/c"; - -} // namespace Shell { - -// Runs a shell command (with `cmd.exe`) with optional arguments. -// -// This assumes that a successful execution will result in the exit -// code for the command to be `0`; in this case, the contents of the -// `Try` will be the contents of `stdout`. -// -// If the exit code is non-zero, we will return an appropriate error -// message; but *not* `stderr`. -// -// If the caller needs to examine the contents of `stderr` it should -// be redirected to `stdout` (using, e.g., `2>&1 || exit /b 0` in the -// command string). The `|| exit /b 0` is required to obtain a success -// exit code in case of errors, and still obtain `stderr`, as piped to -// `stdout`. -template <typename... T> -Try<std::string> shell(const std::string& fmt, const T&... t) -{ - using std::array; - using std::string; - using std::vector; - - const Try<string> command = strings::format(fmt, t...); - if (command.isError()) { - return Error(command.error()); - } - - // This function is intended to pass the arguments to the default - // shell, so we first add the arguments `cmd.exe cmd.exe /c`, - // followed by the command and arguments given. - vector<string> args = {os::Shell::name, os::Shell::arg0, os::Shell::arg1}; - - { // Minimize the lifetime of the system allocated buffer. - // - // NOTE: This API returns a pointer to an array of `wchar_t*`, - // similar to `argv`. Each pointer to a null-terminated Unicode - // string represents an individual argument found on the command - // line. We use this because we cannot just split on whitespace. - int argc; - const std::unique_ptr<wchar_t*, decltype(&::LocalFree)> argv( - ::CommandLineToArgvW(wide_stringify(command.get()).data(), &argc), - &::LocalFree); - if (argv == nullptr) { - return WindowsError(); - } - - for (int i = 0; i < argc; ++i) { - args.push_back(stringify(std::wstring(argv.get()[i]))); - } - } - - // This function is intended to return only the `stdout` of the - // command; but since we have to redirect all of `stdin`, `stdout`, - // `stderr` if we want to redirect any one of them, we redirect - // `stdin` and `stderr` to `NUL`, and `stdout` to a pipe. - Try<int_fd> stdin_ = os::open(os::DEV_NULL, O_RDONLY); - if (stdin_.isError()) { - return Error(stdin_.error()); - } - - Try<array<int_fd, 2>> stdout_ = os::pipe(); - if (stdout_.isError()) { - return Error(stdout_.error()); - } - - Try<int_fd> stderr_ = os::open(os::DEV_NULL, O_WRONLY); - if (stderr_.isError()) { - return Error(stderr_.error()); - } - - // Ensure the file descriptors are closed when we leave this scope. - struct Closer - { - vector<int_fd> fds; - ~Closer() - { - foreach (int_fd& fd, fds) { - os::close(fd); - } - } - } closer = {{stdin_.get(), stdout_.get()[0], stderr_.get()}}; - - array<int_fd, 3> pipes = {stdin_.get(), stdout_.get()[1], stderr_.get()}; - - using namespace ::internal::windows; - - Try<ProcessData> process_data = - create_process(args.front(), args, None(), false, pipes); - - if (process_data.isError()) { - return Error(process_data.error()); - } - - // Close the child end of the stdout pipe and then read until EOF. - os::close(stdout_.get()[1]); - string out; - Result<string> part = None(); - do { - part = os::read(stdout_.get()[0], 1024); - if (part.isSome()) { - out += part.get(); - } - } while (part.isSome()); - - // Wait for the process synchronously. - ::WaitForSingleObject(process_data->process_handle.get_handle(), INFINITE); - - DWORD status; - if (!::GetExitCodeProcess( - process_data->process_handle.get_handle(), &status)) { - return Error("Failed to `GetExitCodeProcess`: " + command.get()); - } - - if (status == 0) { - return out; - } - - return Error( - "Failed to execute '" + command.get() + - "'; the command was either " - "not found or exited with a non-zero exit status: " + - stringify(status)); -} - - -template<typename... T> -inline int execlp(const char* file, T... t) = delete; +} // namespace windows { // Executes a command by calling "<command> <arguments...>", and @@ -534,7 +417,7 @@ inline Option<int> spawn( const std::vector<std::string>& arguments, const Option<std::map<std::string, std::string>>& environment = None()) { - using namespace ::internal::windows; + using namespace os::windows::internal; Try<ProcessData> process_data = create_process(command, arguments, environment); @@ -559,29 +442,19 @@ inline Option<int> spawn( } -// Executes a command by calling "cmd /c <command>", and returns -// after the command has been completed. Returns the process exit -// code on success and `None` on error. -// -// Note: Be cautious about shell injection -// (https://en.wikipedia.org/wiki/Code_injection#Shell_injection) -// when using this method and use proper validation and sanitization -// on the `command`. For this reason in general `os::spawn` is -// preferred if a shell is not required. -inline Option<int> system(const std::string& command) -{ - return os::spawn(Shell::name, {Shell::arg0, Shell::arg1, command}); -} +// TODO(bmahler): Why do we delete this on windows? +template<typename... T> +inline int execlp(const char* file, T... t) = delete; // In order to emulate the semantics of `execvp`, `os::spawn` waits for the new -// process to exit, and returns its error code, which is propogated back to the +// process to exit, and returns its error code, which is propagated back to the // parent via `exit` here. inline int execvp( - const std::string& command, + const std::string& file, const std::vector<std::string>& argv) { - exit(os::spawn(command, argv).getOrElse(-1)); + exit(os::spawn(file, argv).getOrElse(-1)); return -1; } @@ -590,14 +463,14 @@ inline int execvp( // explicit type conversions, but unlike the POSIX implementations, it cannot // accept the raw forms. inline int execvpe( - const std::string& command, + const std::string& file, const std::vector<std::string>& argv, const std::map<std::string, std::string>& envp) { - exit(os::spawn(command, argv, envp).getOrElse(-1)); + exit(os::spawn(file, argv, envp).getOrElse(-1)); return -1; } } // namespace os { -#endif // __STOUT_OS_WINDOWS_SHELL_HPP__ +#endif // __STOUT_OS_WINDOWS_EXEC_HPP__ diff --git a/3rdparty/stout/include/stout/os/windows/shell.hpp b/3rdparty/stout/include/stout/os/windows/shell.hpp index 2f76bba..3238fe1 100644 --- a/3rdparty/stout/include/stout/os/windows/shell.hpp +++ b/3rdparty/stout/include/stout/os/windows/shell.hpp @@ -14,6 +14,10 @@ #define __STOUT_OS_WINDOWS_SHELL_HPP__ #include <process.h> +#include <processthreadsapi.h> +#include <shellapi.h> +#include <synchapi.h> + #include <stdarg.h> // For va_list, va_start, etc. #include <algorithm> @@ -33,357 +37,7 @@ #include <stout/os/int_fd.hpp> #include <stout/os/pipe.hpp> -#include <stout/internal/windows/inherit.hpp> - -namespace internal { -namespace windows { - -// Retrieves system environment in a `std::map`, ignoring -// the current process's environment variables. -inline Option<std::map<std::wstring, std::wstring>> get_system_env() -{ - std::map<std::wstring, std::wstring> system_env; - wchar_t* env_entry = nullptr; - - // Get the system environment. - // The third parameter (bool) tells the function *not* to inherit - // variables from the current process. - if (!::CreateEnvironmentBlock((LPVOID*)&env_entry, nullptr, FALSE)) { - return None(); - } - - // Save the environment block in order to destroy it later. - wchar_t* env_block = env_entry; - - while (*env_entry != L'\0') { - // Each environment block contains the environment variables as follows: - // Var1=Value1\0 - // Var2=Value2\0 - // Var3=Value3\0 - // ... - // VarN=ValueN\0\0 - // The name of an environment variable cannot include an equal sign (=). - - // Construct a string from the pointer up to the first '\0', - // e.g. "Var1=Value1\0", then split into name and value. - std::wstring entry(env_entry); - std::wstring::size_type separator = entry.find(L"="); - std::wstring var_name(entry.substr(0, separator)); - std::wstring varVal(entry.substr(separator + 1)); - - // Mesos variables are upper case. Convert system variables to - // match the name provided by the scheduler in case of a collision. - // This is safe because Windows environment variables are case insensitive. - std::transform( - var_name.begin(), var_name.end(), var_name.begin(), ::towupper); - - // The system environment has priority. - system_env.insert_or_assign(var_name.data(), varVal.data()); - - // Advance the pointer the length of the entry string plus the '\0'. - env_entry += entry.length() + 1; - } - - ::DestroyEnvironmentBlock(env_block); - - return system_env; -} - - -// Creates a null-terminated array of null-terminated strings that will be -// passed to `CreateProcessW` as the `lpEnvironment` argument, as described by -// MSDN[1]. This array needs to be sorted in alphabetical order, but the `map` -// already takes care of that. Note that this function explicitly handles -// UTF-16 environments, so it must be used in conjunction with the -// `CREATE_UNICODE_ENVIRONMENT` flag. -// -// NOTE: This function will add the system's environment variables into -// the returned string. These variables take precedence over the provided -// `env` and are generally necessary in order to launch things on Windows. -// -// [1] https://msdn.microsoft.com/en-us/library/windows/desktop/ms682425(v=vs.85).aspx -inline Option<std::wstring> create_process_env( - const Option<std::map<std::string, std::string>>& env) -{ - if (env.isNone() || (env.isSome() && env.get().size() == 0)) { - return None(); - } - - Option<std::map<std::wstring, std::wstring>> system_env = get_system_env(); - - // The system environment must be non-empty. - // No subprocesses will be able to launch if the system environment is blank. - CHECK(system_env.isSome() && system_env.get().size() > 0); - - std::map<std::wstring, std::wstring> combined_env; - - // Populate the combined environment first with the system environment. - foreachpair (const std::wstring& key, - const std::wstring& value, - system_env.get()) { - combined_env[key] = value; - } - - // Now override with the supplied environment. - foreachpair (const std::string& key, - const std::string& value, - env.get()) { - combined_env[wide_stringify(key)] = wide_stringify(value); - } - - std::wstring env_string; - foreachpair (const std::wstring& key, - const std::wstring& value, - combined_env) { - env_string += key + L'=' + value + L'\0'; - } - - // Append final null terminating character. - env_string.push_back(L'\0'); - return env_string; -} - - -// Concatenates multiple command-line arguments and escapes the values. -// NOTE: This is necessary even when using Windows APIs that "appear" -// to take arguments as a list, because those APIs will themselves -// concatenate command-line arguments *without* escaping them. -// -// This function escapes arguments with the following rules: -// 1) Any argument with a space, tab, newline, vertical tab, -// or double-quote must be surrounded in double-quotes. -// 2) Backslashes at the very end of an argument must be escaped. -// 3) Backslashes that precede a double-quote must be escaped. -// The double-quote must also be escaped. -// -// NOTE: The below algorithm is adapted from Daniel Colascione's public domain -// algorithm for quoting command line arguments on Windows for `CreateProcess`. -// -// https://blogs.msdn.microsoft.com/twistylittlepassagesallalike/2011/04/23/everyone-quotes-command-line-arguments-the-wrong-way/ -// NOLINT(whitespace/line_length) -inline std::wstring stringify_args(const std::vector<std::string>& argv) -{ - std::wstring command; - for (auto argit = argv.cbegin(); argit != argv.cend(); ++argit) { - std::wstring arg = wide_stringify(*argit); - // Don't quote empty arguments or those without troublesome characters. - if (!arg.empty() && arg.find_first_of(L" \t\n\v\"") == arg.npos) { - command.append(arg); - } else { - // Beginning double quotation mark. - command.push_back(L'"'); - for (auto it = arg.cbegin(); it != arg.cend(); ++it) { - // Count existent backslashes in argument. - unsigned int backslashes = 0; - while (it != arg.cend() && *it == L'\\') { - ++it; - ++backslashes; - } - - if (it == arg.cend()) { - // Escape all backslashes, but let the terminating double quotation - // mark we add below be interpreted as a metacharacter. - command.append(backslashes * 2, L'\\'); - break; - } else if (*it == L'"') { - // Escape all backslashes and the following double quotation mark. - command.append(backslashes * 2 + 1, L'\\'); - command.push_back(*it); - } else { - // Backslashes aren't special here. - command.append(backslashes, L'\\'); - command.push_back(*it); - } - } - - // Terminating double quotation mark. - command.push_back(L'"'); - } - // Space separate arguments (but don't append at end). - if (argit != argv.cend() - 1) { - command.push_back(L' '); - } - } - // Append final null terminating character. - command.push_back(L'\0'); - return command; -} - - -struct ProcessData { - SharedHandle process_handle; - SharedHandle thread_handle; - pid_t pid; -}; - - -// Provides an interface for creating a child process on Windows. -// -// The `command` argument is given for compatibility, and is ignored. This is -// because the `CreateProcess` will use `argv[0]` as the command to be executed, -// and will perform a `PATH` lookup. If `command` were to be used instead, -// `CreateProcess` would require an absolute path. -// -// If `create_suspended` is `true`, the process will not be started, and the -// caller must use `ResumeThread` to start the process. -// -// The caller can specify explicit `stdin`, `stdout`, and `stderr` handles, -// in that order, for the process via the `pipes` argument. -// -// NOTE: If `pipes` are specified, they will be temporarily set to -// inheritable, and then set to uninheritable. This is a side effect -// on each `HANDLE`. -// -// The return value is a `ProcessData` struct, with the process and thread -// handles each saved in a `SharedHandle`, ensuring they are closed when struct -// goes out of scope. -inline Try<ProcessData> create_process( - const std::string& command, - const std::vector<std::string>& argv, - const Option<std::map<std::string, std::string>>& environment, - const bool create_suspended = false, - const Option<std::array<int_fd, 3>>& pipes = None(), - const std::vector<int_fd>& whitelist_fds = {}) -{ - // TODO(andschwa): Assert that `command` and `argv[0]` are the same. - const std::wstring arg_string = stringify_args(argv); - std::vector<wchar_t> arg_buffer(arg_string.begin(), arg_string.end()); - arg_buffer.push_back(L'\0'); - - // Create the process with a Unicode environment and extended - // startup info. - DWORD creation_flags = - CREATE_UNICODE_ENVIRONMENT | EXTENDED_STARTUPINFO_PRESENT; - if (create_suspended) { - creation_flags |= CREATE_SUSPENDED; - } - - // Construct the environment that will be passed to `::CreateProcessW`. - const Option<std::wstring> env_string = create_process_env(environment); - std::vector<wchar_t> env_buffer; - if (env_string.isSome()) { - // This string contains the necessary null characters. - env_buffer.assign(env_string.get().begin(), env_string.get().end()); - } - - wchar_t* process_env = env_buffer.empty() ? nullptr : env_buffer.data(); - - PROCESS_INFORMATION process_info = {}; - - STARTUPINFOEXW startup_info_ex = {}; - startup_info_ex.StartupInfo.cb = sizeof(startup_info_ex); - - // Windows provides a way to whitelist a set of handles to be - // inherited by the child process. - // https://blogs.msdn.microsoft.com/oldnewthing/20111216-00/?p=8873 - // (1) We're setting the pipe handles and whitelisted handles to be - // temporarily inheritable. - // (2) We're explicitly whitelisting the handles using a Windows API. - // (3) We're then setting the handles to back to non-inheritable - // after the child process has been created. - std::vector<HANDLE> handles; - if (pipes.isSome()) { - // Each of these handles must be inheritable. - foreach (const int_fd& fd, pipes.get()) { - handles.emplace_back(static_cast<HANDLE>(fd)); - const Try<Nothing> inherit = set_inherit(fd, true); - if (inherit.isError()) { - return Error(inherit.error()); - } - } - - // Hook up the stdin/out/err pipes and use the `STARTF_USESTDHANDLES` - // flag to instruct the child to use them [1]. - // A more user-friendly example can be found in [2]. - // - // [1] https://msdn.microsoft.com/en-us/library/windows/desktop/ms686331(v=vs.85).aspx - // [2] https://msdn.microsoft.com/en-us/library/windows/desktop/ms682499(v=vs.85).aspx - startup_info_ex.StartupInfo.dwFlags |= STARTF_USESTDHANDLES; - startup_info_ex.StartupInfo.hStdInput = std::get<0>(pipes.get()); - startup_info_ex.StartupInfo.hStdOutput = std::get<1>(pipes.get()); - startup_info_ex.StartupInfo.hStdError = std::get<2>(pipes.get()); - } - - foreach (const int_fd& fd, whitelist_fds) { - handles.emplace_back(static_cast<HANDLE>(fd)); - const Try<Nothing> inherit = set_inherit(fd, true); - if (inherit.isError()) { - return Error(inherit.error()); - } - } - - Result<std::shared_ptr<AttributeList>> attribute_list = - create_attributes_list_for_handles(handles); - - if (attribute_list.isError()) { - return Error(attribute_list.error()); - } - - if (attribute_list.isSome()) { - startup_info_ex.lpAttributeList = attribute_list->get(); - } - - const BOOL result = ::CreateProcessW( - // This is replaced by the first token of `arg_buffer` string. - static_cast<LPCWSTR>(nullptr), - static_cast<LPWSTR>(arg_buffer.data()), - static_cast<LPSECURITY_ATTRIBUTES>(nullptr), - static_cast<LPSECURITY_ATTRIBUTES>(nullptr), - TRUE, // Inherit parent process handles (such as those in `pipes`). - creation_flags, - static_cast<LPVOID>(process_env), - static_cast<LPCWSTR>(nullptr), // Inherit working directory. - &startup_info_ex.StartupInfo, - &process_info); - - // Save the error from the previous call so that we can proceed to - // always revert the inheritance of the handles, and then report - // this error, if there was one. - const DWORD create_process_error = ::GetLastError(); - - // NOTE: The MSDN documentation for `CreateProcess` states that it - // returns before the process has "finished initialization," but is - // not clear on precisely what initialization entails. It would seem - // that this does not affect inherited handles, as it stands to - // reason that the system call to `CreateProcess` causes inheritable - // handles to become inherited, and not some "initialization" of the - // child process. However, if an inheritance race condition - // manifests, this assumption should be re-evaluated. - if (pipes.isSome()) { - // These handles should no longer be inheritable. This prevents other child - // processes from accidentally inheriting the wrong handles. - // - // NOTE: This is explicit, and does not take into account the - // previous inheritance semantics of each `HANDLE`. It is assumed - // that users of this function send non-inheritable handles. - foreach (const int_fd& fd, pipes.get()) { - const Try<Nothing> inherit = set_inherit(fd, false); - if (inherit.isError()) { - return Error(inherit.error()); - } - } - } - - foreach (const int_fd& fd, whitelist_fds) { - const Try<Nothing> inherit = set_inherit(fd, false); - if (inherit.isError()) { - return Error(inherit.error()); - } - } - - if (result == FALSE) { - return WindowsError( - create_process_error, - "Failed to call `CreateProcess`: " + stringify(arg_string)); - } - - return ProcessData{SharedHandle{process_info.hProcess, ::CloseHandle}, - SharedHandle{process_info.hThread, ::CloseHandle}, - static_cast<pid_t>(process_info.dwProcessId)}; -} - -} // namespace windows { -} // namespace internal { +#include <stout/os/windows/exec.hpp> namespace os { namespace Shell { @@ -481,7 +135,7 @@ Try<std::string> shell(const std::string& fmt, const T&... t) array<int_fd, 3> pipes = {stdin_.get(), stdout_.get()[1], stderr_.get()}; - using namespace ::internal::windows; + using namespace os::windows::internal; Try<ProcessData> process_data = create_process(args.front(), args, None(), false, pipes); @@ -522,43 +176,6 @@ Try<std::string> shell(const std::string& fmt, const T&... t) } -template<typename... T> -inline int execlp(const char* file, T... t) = delete; - - -// Executes a command by calling "<command> <arguments...>", and -// returns after the command has been completed. Returns the process exit -// code on success and `None` on error. -inline Option<int> spawn( - const std::string& command, - const std::vector<std::string>& arguments, - const Option<std::map<std::string, std::string>>& environment = None()) -{ - using namespace ::internal::windows; - - Try<ProcessData> process_data = - create_process(command, arguments, environment); - - if (process_data.isError()) { - LOG(WARNING) << process_data.error(); - return None(); - } - - // Wait for the process synchronously. - ::WaitForSingleObject(process_data->process_handle.get_handle(), INFINITE); - - DWORD status; - if (!::GetExitCodeProcess( - process_data->process_handle.get_handle(), &status)) { - LOG(WARNING) << "Failed to `GetExitCodeProcess`: " << command; - return None(); - } - - // Return the child exit code. - return static_cast<int>(status); -} - - // Executes a command by calling "cmd /c <command>", and returns // after the command has been completed. Returns the process exit // code on success and `None` on error. @@ -573,31 +190,6 @@ inline Option<int> system(const std::string& command) return os::spawn(Shell::name, {Shell::arg0, Shell::arg1, command}); } - -// In order to emulate the semantics of `execvp`, `os::spawn` waits for the new -// process to exit, and returns its error code, which is propogated back to the -// parent via `exit` here. -inline int execvp( - const std::string& command, - const std::vector<std::string>& argv) -{ - exit(os::spawn(command, argv).getOrElse(-1)); - return -1; -} - - -// NOTE: This function can accept `Argv` and `Envp` constructs through their -// explicit type conversions, but unlike the POSIX implementations, it cannot -// accept the raw forms. -inline int execvpe( - const std::string& command, - const std::vector<std::string>& argv, - const std::map<std::string, std::string>& envp) -{ - exit(os::spawn(command, argv, envp).getOrElse(-1)); - return -1; -} - } // namespace os { #endif // __STOUT_OS_WINDOWS_SHELL_HPP__ diff --git a/3rdparty/stout/include/stout/posix/os.hpp b/3rdparty/stout/include/stout/posix/os.hpp index 91ef5bc..b98d5ab 100644 --- a/3rdparty/stout/include/stout/posix/os.hpp +++ b/3rdparty/stout/include/stout/posix/os.hpp @@ -192,27 +192,6 @@ inline void eraseenv(const std::string& key) } -// This function is a portable version of execvpe ('p' means searching -// executable from PATH and 'e' means setting environments). We add -// this function because it is not available on all systems. -// -// NOTE: This function is not thread safe. It is supposed to be used -// only after fork (when there is only one thread). This function is -// async signal safe. -inline int execvpe(const char* file, char** argv, char** envp) -{ - char** saved = os::raw::environment(); - - *os::raw::environmentp() = envp; - - int result = execvp(file, argv); - - *os::raw::environmentp() = saved; - - return result; -} - - inline Try<Nothing> chmod(const std::string& path, int mode) { if (::chmod(path.c_str(), mode) < 0) { diff --git a/3rdparty/stout/tests/os/process_tests.cpp b/3rdparty/stout/tests/os/process_tests.cpp index add8ed6..5abb2a7 100644 --- a/3rdparty/stout/tests/os/process_tests.cpp +++ b/3rdparty/stout/tests/os/process_tests.cpp @@ -224,8 +224,8 @@ TEST_F(ProcessTest, Pstree) 1u == total_children) << stringify(tree.get()); const bool conhost_spawned = total_children == 1; - Try<internal::windows::ProcessData> process_data = - internal::windows::create_process( + Try<os::windows::internal::ProcessData> process_data = + os::windows::internal::create_process( "powershell", {"powershell", "-NoProfile", "-Command", "Start-Sleep", "2"}, None()); diff --git a/3rdparty/stout/tests/os/rmdir_tests.cpp b/3rdparty/stout/tests/os/rmdir_tests.cpp index 9ca4d96..83f8ea4 100644 --- a/3rdparty/stout/tests/os/rmdir_tests.cpp +++ b/3rdparty/stout/tests/os/rmdir_tests.cpp @@ -19,6 +19,7 @@ #include <stout/os.hpp> #include <stout/path.hpp> +#include <stout/os/exec.hpp> #include <stout/os/getcwd.hpp> #include <stout/os/ls.hpp> #include <stout/os/mkdir.hpp> diff --git a/3rdparty/stout/tests/os_tests.cpp b/3rdparty/stout/tests/os_tests.cpp index f1cfe5c..4231303 100644 --- a/3rdparty/stout/tests/os_tests.cpp +++ b/3rdparty/stout/tests/os_tests.cpp @@ -51,6 +51,7 @@ #endif #include <stout/os/environment.hpp> +#include <stout/os/exec.hpp> #include <stout/os/int_fd.hpp> #include <stout/os/kill.hpp> #include <stout/os/killtree.hpp> @@ -87,12 +88,12 @@ using std::vector; #if defined(__WINDOWS__) // NOTE: These are used to partially implement tests which otherwise // relied on `os::Exec` and `os::Fork`. -using ::internal::windows::ProcessData; +using os::windows::internal::ProcessData; ProcessData windows_fork() { // This will unfortunately generate some output. - Try<ProcessData> process_data = ::internal::windows::create_process( + Try<ProcessData> process_data = ::os::windows::internal::create_process( "", {"ping.exe", "127.0.0.1", "-n", "10"}, None()); CHECK_SOME(process_data); return process_data.get();
