From 3095fc8990cf13d2626ba15e6c19c1fffeefce37 Mon Sep 17 00:00:00 2001 From: Daniel Kroening Date: Sat, 8 Sep 2018 18:41:56 +0100 Subject: [PATCH 1/2] run(): whitespace quoting for windows --- src/util/run.cpp | 66 +++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 60 insertions(+), 6 deletions(-) diff --git a/src/util/run.cpp b/src/util/run.cpp index ee509c19ec1..6bc8034f9ef 100644 --- a/src/util/run.cpp +++ b/src/util/run.cpp @@ -75,6 +75,64 @@ static int stdio_redirection(int fd, const std::string &file) } #endif +#ifdef _WIN32 +// Read +// https://blogs.msdn.microsoft.com/twistylittlepassagesallalike/2011/04/23/everyone-quotes-command-line-arguments-the-wrong-way/ +std::wstring quote_windows_arg(const std::wstring &src) +{ + if(src.find_first_of(L" \t\n\v\"") == src.npos) + return src; + + std::wstring result = L"\""; + + for(auto it = src.begin();; ++it) + { + std::size_t NumberBackslashes = 0; + + while(it != src.end() && *it == L'\\') + { + ++it; + ++NumberBackslashes; + } + + if(it == src.end()) + { + // + // Escape all backslashes, but let the terminating + // double quotation mark we add below be interpreted + // as a metacharacter. + // + + result.append(NumberBackslashes * 2, L'\\'); + break; + } + else if(*it == L'"') + { + // + // Escape all backslashes and the following + // double quotation mark. + // + + result.append(NumberBackslashes * 2 + 1, L'\\'); + result.push_back(*it); + } + else + { + // + // Backslashes aren't special here. + // + + result.append(NumberBackslashes, L'\\'); + result.push_back(*it); + } + } + + result.push_back(L'"'); + + return result; +} +#endif + int run( const std::string &what, const std::vector &argv, @@ -112,13 +170,13 @@ int run( return run(new_argv[0], new_argv, "", "", ""); } - // unicode version of the arguments + // unicode and whitespace-quoted version of the arguments std::vector wargv; wargv.resize(argv.size()); for(std::size_t i=0; i _argv(argv.size()+1); @@ -127,10 +185,6 @@ int run( _argv[argv.size()]=NULL; - // warning: the arguments may still need escaping, - // as windows will concatenate the argv strings back together, - // separating them with spaces - std::wstring wide_what=widen(what); int status=_wspawnvp(_P_WAIT, wide_what.c_str(), _argv.data()); return status; From fd9e35c785de2664e41a2e37c33eeda31e89bc6f Mon Sep 17 00:00:00 2001 From: Daniel Kroening Date: Sat, 8 Sep 2018 18:43:26 +0100 Subject: [PATCH 2/2] replace _wspawnvp() by CreateProcessW() --- src/util/run.cpp | 190 ++++++++++++++++++++++++++++++++++++----------- 1 file changed, 147 insertions(+), 43 deletions(-) diff --git a/src/util/run.cpp b/src/util/run.cpp index 6bc8034f9ef..9ab46282cbc 100644 --- a/src/util/run.cpp +++ b/src/util/run.cpp @@ -12,6 +12,7 @@ Date: August 2012 #ifdef _WIN32 #include +#include #else #include @@ -37,14 +38,85 @@ int run(const std::string &what, const std::vector &argv) return run(what, argv, "", "", ""); } -#ifndef _WIN32 +#ifdef _WIN32 +#define STDIN_FILENO 0 +#define STDOUT_FILENO 1 +#define STDERR_FILENO 2 +using fdt = HANDLE; +#else +using fdt = int; +#endif + /// open given file to replace either stdin, stderr, stdout -static int stdio_redirection(int fd, const std::string &file) +static fdt stdio_redirection(int fd, const std::string &file) { - int result_fd = fd; + fdt result_fd; + +#ifdef _WIN32 + std::string name; + + SECURITY_ATTRIBUTES SecurityAttributes; + ZeroMemory(&SecurityAttributes, sizeof SecurityAttributes); + SecurityAttributes.bInheritHandle = true; + + switch(fd) + { + case STDIN_FILENO: + name = "stdin"; + if(file.empty()) + result_fd = GetStdHandle(STD_INPUT_HANDLE); + else + result_fd = CreateFileW( + widen(file).c_str(), + GENERIC_READ, + 0, + &SecurityAttributes, + OPEN_EXISTING, + FILE_ATTRIBUTE_READONLY, + NULL); + break; + + case STDOUT_FILENO: + name = "stdout"; + if(file.empty()) + result_fd = GetStdHandle(STD_OUTPUT_HANDLE); + else + result_fd = CreateFileW( + widen(file).c_str(), + GENERIC_WRITE, + 0, + &SecurityAttributes, + CREATE_ALWAYS, + FILE_ATTRIBUTE_NORMAL, + NULL); + break; + + case STDERR_FILENO: + name = "stderr"; + if(file.empty()) + result_fd = GetStdHandle(STD_ERROR_HANDLE); + else + result_fd = CreateFileW( + widen(file).c_str(), + GENERIC_WRITE, + 0, + &SecurityAttributes, + CREATE_ALWAYS, + FILE_ATTRIBUTE_NORMAL, + NULL); + break; + + default: + UNREACHABLE; + } + + if(result_fd == INVALID_HANDLE_VALUE) + perror(("Failed to open " + name + " file " + file).c_str()); + +#else if(file.empty()) - return result_fd; + return fd; int flags = 0, mode = 0; std::string name; @@ -68,12 +140,13 @@ static int stdio_redirection(int fd, const std::string &file) } result_fd = open(file.c_str(), flags, mode); + if(result_fd == -1) perror(("Failed to open " + name + " file " + file).c_str()); +#endif return result_fd; } -#endif #ifdef _WIN32 // Read @@ -140,56 +213,87 @@ int run( const std::string &std_output, const std::string &std_error) { - #ifdef _WIN32 - // we use the cmd.exe shell to do stdin/stdout/stderr redirection on Windows - if(!std_input.empty() || !std_output.empty() || !std_error.empty()) +#ifdef _WIN32 + // unicode commandline, quoted + std::wstring cmdline; + + // we replace argv[0] by what + cmdline = quote_windows_arg(widen(what)); + + for(std::size_t i = 1; i < argv.size(); i++) { - std::vector new_argv = argv; - new_argv.insert(new_argv.begin(), "cmd.exe"); - new_argv.insert(new_argv.begin() + 1, "/c"); + cmdline += L" "; + cmdline += quote_windows_arg(widen(argv[i])); + } - if(!std_input.empty()) - { - new_argv.push_back("<"); - new_argv.push_back(std_input); - } + PROCESS_INFORMATION piProcInfo; + STARTUPINFOW siStartInfo; - if(!std_output.empty()) - { - new_argv.push_back(">"); - new_argv.push_back(std_output); - } + ZeroMemory(&piProcInfo, sizeof piProcInfo); + ZeroMemory(&siStartInfo, sizeof siStartInfo); - if(!std_error.empty()) - { - new_argv.push_back("2>"); - new_argv.push_back(std_error); - } + siStartInfo.cb = sizeof siStartInfo; - // this is recursive - return run(new_argv[0], new_argv, "", "", ""); - } + siStartInfo.hStdInput = stdio_redirection(STDIN_FILENO, std_input); + siStartInfo.hStdOutput = stdio_redirection(STDOUT_FILENO, std_output); + siStartInfo.hStdError = stdio_redirection(STDERR_FILENO, std_error); - // unicode and whitespace-quoted version of the arguments - std::vector wargv; + siStartInfo.dwFlags |= STARTF_USESTDHANDLES; - wargv.resize(argv.size()); + // CreateProcessW wants to modify the command line + std::vector mutable_cmdline(cmdline.begin(), cmdline.end()); + mutable_cmdline.push_back(0); // zero termination + wchar_t *cmdline_ptr = mutable_cmdline.data(); - for(std::size_t i=0; i _argv(argv.size()+1); + if(!bSuccess) + { + if(!std_input.empty()) + CloseHandle(siStartInfo.hStdInput); + if(!std_output.empty()) + CloseHandle(siStartInfo.hStdOutput); + if(!std_error.empty()) + CloseHandle(siStartInfo.hStdError); + return -1; + } - for(std::size_t i=0; i