diff --git a/src/main/cpp/BUILD b/src/main/cpp/BUILD index 7de595dddebf4f..9e18a2b19121e5 100644 --- a/src/main/cpp/BUILD +++ b/src/main/cpp/BUILD @@ -64,6 +64,7 @@ cc_library( "//src/main/cpp/util:logging", ] + select({ "//src/conditions:windows": [ + "//src/tools/launcher/util", "//src/main/native/windows:lib-file", "//src/main/native/windows:lib-process", ], diff --git a/src/main/cpp/blaze.cc b/src/main/cpp/blaze.cc index 5577d6a593835f..4b8e8f24d3ee6c 100644 --- a/src/main/cpp/blaze.cc +++ b/src/main/cpp/blaze.cc @@ -680,7 +680,7 @@ static void RunBatchMode( { WithEnvVars env_obj(PrepareEnvironmentForJvm()); - ExecuteProgram(server_exe, jvm_args_vector); + ExecuteServerJvm(server_exe, jvm_args_vector); BAZEL_DIE(blaze_exit_code::INTERNAL_ERROR) << "execv of '" << server_exe << "' failed: " << GetLastErrorString(); } @@ -2014,7 +2014,7 @@ unsigned int BlazeServer::Communicate( // Execute the requested program, but before doing so, flush everything // we still have to say. fflush(NULL); - ExecuteProgram(request.argv(0), argv); + ExecuteRunRequest(request.argv(0), argv); } // We'll exit with exit code SIGPIPE on Unixes due to PropagateSignalOnExit() diff --git a/src/main/cpp/blaze_util_platform.h b/src/main/cpp/blaze_util_platform.h index e7f5c3e5024ec0..c7c3aeefd2a7d8 100644 --- a/src/main/cpp/blaze_util_platform.h +++ b/src/main/cpp/blaze_util_platform.h @@ -140,11 +140,23 @@ std::string GetSystemJavabase(); // Return the path to the JVM binary relative to a javabase, e.g. "bin/java". std::string GetJavaBinaryUnderJavabase(); -// Replace the current process with the given program in the current working -// directory, using the given argument vector. +// Start the Bazel server's JVM in the current directory. +// +// Note on Windows: 'server_jvm_args' is NOT expected to be escaped for +// CreateProcessW. +// +// This function does not return on success. +void ExecuteServerJvm(const std::string& exe, + const std::vector& server_jvm_args); + +// Execute the "bazel run" request in the current directory. +// +// Note on Windows: 'run_request_args' IS expected to be escaped for +// CreateProcessW. +// // This function does not return on success. -void ExecuteProgram(const std::string& exe, - const std::vector& args_vector); +void ExecuteRunRequest(const std::string& exe, + const std::vector& run_request_args); class BlazeServerStartup { public: diff --git a/src/main/cpp/blaze_util_posix.cc b/src/main/cpp/blaze_util_posix.cc index c7ee7b6b3ffbff..87ba899180f1a5 100644 --- a/src/main/cpp/blaze_util_posix.cc +++ b/src/main/cpp/blaze_util_posix.cc @@ -276,7 +276,8 @@ class CharPP { char** charpp_; }; -void ExecuteProgram(const string& exe, const vector& args_vector) { +static void ExecuteProgram(const string& exe, + const vector& args_vector) { BAZEL_LOG(INFO) << "Invoking binary " << exe << " in " << blaze_util::GetCwd(); @@ -289,6 +290,16 @@ void ExecuteProgram(const string& exe, const vector& args_vector) { execv(exe.c_str(), argv.get()); } +void ExecuteServerJvm(const string& exe, + const std::vector& server_jvm_args) { + ExecuteProgram(exe, server_jvm_args); +} + +void ExecuteRunRequest(const string& exe, + const std::vector& run_request_args) { + ExecuteProgram(exe, run_request_args); +} + const char kListSeparator = ':'; bool SymlinkDirectories(const string &target, const string &link) { diff --git a/src/main/cpp/blaze_util_windows.cc b/src/main/cpp/blaze_util_windows.cc index 05dd37384a5335..a2751d3a9221ef 100644 --- a/src/main/cpp/blaze_util_windows.cc +++ b/src/main/cpp/blaze_util_windows.cc @@ -18,12 +18,12 @@ #include #include -#include // _open -#include // FOLDERID_Profile -#include // UNLEN -#include // CoTaskMemFree -#include // SHGetKnownFolderPath -#include // va_start, va_end, va_list +#include // _open +#include // FOLDERID_Profile +#include // UNLEN +#include // CoTaskMemFree +#include // SHGetKnownFolderPath +#include // va_start, va_end, va_list #include #include @@ -52,6 +52,7 @@ #include "src/main/native/windows/file.h" #include "src/main/native/windows/process.h" #include "src/main/native/windows/util.h" +#include "src/tools/launcher/util/launcher_util.h" namespace blaze { @@ -477,9 +478,6 @@ namespace { // Max command line length is per CreateProcess documentation // (https://msdn.microsoft.com/en-us/library/ms682425(VS.85).aspx) -// -// Quoting rules are described here: -// https://blogs.msdn.microsoft.com/twistylittlepassagesallalike/2011/04/23/everyone-quotes-command-line-arguments-the-wrong-way/ static const int MAX_CMDLINE_LENGTH = 32768; @@ -487,7 +485,7 @@ struct CmdLine { WCHAR cmdline[MAX_CMDLINE_LENGTH]; }; static void CreateCommandLine(CmdLine* result, const string& exe, - const std::vector& args_vector) { + const std::vector& wargs_vector) { std::wstringstream cmdline; string short_exe; if (!exe.empty()) { @@ -501,7 +499,7 @@ static void CreateCommandLine(CmdLine* result, const string& exe, } bool first = true; - for (const auto& s : args_vector) { + for (const std::wstring& wa : wargs_vector) { if (first) { // Skip first argument, it is equal to 'exe'. first = false; @@ -509,42 +507,7 @@ static void CreateCommandLine(CmdLine* result, const string& exe, } else { cmdline << L' '; } - - bool has_space = s.find(" ") != string::npos; - - if (has_space) { - cmdline << L'\"'; - } - - wstring ws = blaze_util::CstringToWstring(s.c_str()).get(); - std::wstring::const_iterator it = ws.begin(); - while (it != ws.end()) { - wchar_t ch = *it++; - switch (ch) { - case L'"': - // Escape double quotes - cmdline << L"\\\""; - break; - - case L'\\': - if (it == ws.end()) { - // Backslashes at the end of the string are quoted if we add quotes - cmdline << (has_space ? L"\\\\" : L"\\"); - } else { - // Backslashes everywhere else are quoted if they are followed by a - // quote or a backslash - cmdline << (*it == L'"' || *it == L'\\' ? L"\\\\" : L"\\"); - } - break; - - default: - cmdline << ch; - } - } - - if (has_space) { - cmdline << L'\"'; - } + cmdline << wa; } wstring cmdline_str = cmdline.str(); @@ -722,8 +685,16 @@ int ExecuteDaemon(const string& exe, STARTUPINFOEXW startupInfoEx = {0}; lpAttributeList->InitStartupInfoExW(&startupInfoEx); + std::vector wesc_args_vector; + wesc_args_vector.reserve(args_vector.size()); + for (const string& a : args_vector) { + std::wstring wa = blaze_util::CstringToWstring(a.c_str()).get(); + std::wstring wesc = bazel::launcher::WindowsEscapeArg2(wa); + wesc_args_vector.push_back(wesc); + } + CmdLine cmdline; - CreateCommandLine(&cmdline, exe, args_vector); + CreateCommandLine(&cmdline, exe, wesc_args_vector); BOOL ok; { @@ -771,11 +742,12 @@ int ExecuteDaemon(const string& exe, // Run the given program in the current working directory, using the given // argument vector, wait for it to finish, then exit ourselves with the exitcode // of that program. -void ExecuteProgram(const string& exe, const std::vector& args_vector) { +static void ExecuteProgram(const string& exe, + const std::vector& wargs_vector) { std::wstring wexe = blaze_util::CstringToWstring(exe.c_str()).get(); CmdLine cmdline; - CreateCommandLine(&cmdline, "", args_vector); + CreateCommandLine(&cmdline, "", wargs_vector); bazel::windows::WaitableProcess proc; std::wstring werror; @@ -796,6 +768,35 @@ void ExecuteProgram(const string& exe, const std::vector& args_vector) { exit(x); } +void ExecuteServerJvm(const string& exe, + const std::vector& server_jvm_args) { + std::vector wargs; + wargs.reserve(server_jvm_args.size()); + for (const string& a : server_jvm_args) { + std::wstring wa = blaze_util::CstringToWstring(a.c_str()).get(); + std::wstring wesc = bazel::launcher::WindowsEscapeArg2(wa); + wargs.push_back(wesc); + } + + ExecuteProgram(exe, wargs); +} + +void ExecuteRunRequest(const string& exe, + const std::vector& run_request_args) { + std::vector wargs; + wargs.reserve(run_request_args.size()); + std::wstringstream joined; + for (const string& a : run_request_args) { + std::wstring wa = blaze_util::CstringToWstring(a.c_str()).get(); + // The arguments are already escaped (Bash-style or Windows-style, depending + // on --[no]incompatible_windows_bashless_run_command). + wargs.push_back(wa); + joined << L' ' << wa; + } + + ExecuteProgram(exe, wargs); +} + const char kListSeparator = ';'; bool SymlinkDirectories(const string &posix_target, const string &posix_name) { diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java index de994231a0dbe6..47f4f68fa82390 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java @@ -519,9 +519,16 @@ public BlazeCommandResult exec(CommandEnvironment env, OptionsParsingResult opti return BlazeCommandResult.exitCode(ExitCode.COMMAND_LINE_ERROR); } + String shellEscaped = ShellEscaper.escapeJoinAll(cmdLine); + if (OS.getCurrent() == OS.WINDOWS) { + // On Windows, we run Bash as a subprocess of the client (via CreateProcessW). + // Bash uses its own (Bash-style) flag parsing logic, not the default logic for which + // ShellUtils.windowsEscapeArg escapes, so we escape the flags once again Bash-style. + shellEscaped = "\"" + shellEscaped.replace("\\", "\\\\").replace("\"", "\\\"") + "\""; + } + ImmutableList shellCmdLine = - ImmutableList.of( - shExecutable.getPathString(), "-c", ShellEscaper.escapeJoinAll(cmdLine)); + ImmutableList.of(shExecutable.getPathString(), "-c", shellEscaped); for (String arg : shellCmdLine) { execDescription.addArgv(ByteString.copyFrom(arg, StandardCharsets.ISO_8859_1)); diff --git a/src/test/shell/integration/py_args_escaping_test.sh b/src/test/shell/integration/py_args_escaping_test.sh index 59bf5b14bb2337..f3d4ed42c48d89 100755 --- a/src/test/shell/integration/py_args_escaping_test.sh +++ b/src/test/shell/integration/py_args_escaping_test.sh @@ -231,11 +231,41 @@ function test_args_escaping() { create_py_file_that_prints_args "$ws" create_build_file_with_many_args "$ws" - # On all platforms, the target prints good output. + # Batch mode, Bash-less run command. + if $is_windows; then + ( cd "$ws" + bazel --batch run --incompatible_windows_bashless_run_command \ + --verbose_failures :x &>"$TEST_log" || fail "expected success" + ) + assert_good_output_of_the_program_with_many_args + rm "$TEST_log" + fi + + # Batch mode, Bash-ful run command. + ( cd "$ws" + bazel --batch run --noincompatible_windows_bashless_run_command \ + --verbose_failures :x &>"$TEST_log" || fail "expected success" + ) + assert_good_output_of_the_program_with_many_args + rm "$TEST_log" + + # Server mode, Bash-less run command. + if $is_windows; then + ( cd "$ws" + bazel run --incompatible_windows_bashless_run_command \ + --verbose_failures :x &>"$TEST_log" || fail "expected success" + ) + assert_good_output_of_the_program_with_many_args + rm "$TEST_log" + fi + + # Server mode, Bash-ful run command. ( cd "$ws" - bazel run --verbose_failures :x &>"$TEST_log" || fail "expected success" + bazel run --noincompatible_windows_bashless_run_command \ + --verbose_failures :x &>"$TEST_log" || fail "expected success" ) assert_good_output_of_the_program_with_many_args + rm "$TEST_log" } function test_untokenizable_args() { diff --git a/src/tools/launcher/util/BUILD b/src/tools/launcher/util/BUILD index 70a4c05f1332cc..c4a66ef90a1901 100644 --- a/src/tools/launcher/util/BUILD +++ b/src/tools/launcher/util/BUILD @@ -20,6 +20,7 @@ win_cc_library( srcs = ["launcher_util.cc"], hdrs = ["launcher_util.h"], visibility = [ + "//src/main/cpp:__pkg__", "//src/tools/launcher:__subpackages__", "//tools/test:__pkg__", ],