Windows, refactor: move WindowsEscapeArg function
Move this function from the native
py/sh/java_binary launcher into the windows JNI
library.
The latter is slowly becoming Bazel's and its
tools' de-facto base library, so it's the right
place for this function.
Follow-up to https://github.com/bazelbuild/bazel/commit/da557f96c697102ad787e57bbf7db2460f6a60a8
Closes #9131.
PiperOrigin-RevId: 262540258
diff --git a/src/BUILD b/src/BUILD
index 795d830..500c9cd 100644
--- a/src/BUILD
+++ b/src/BUILD
@@ -446,7 +446,7 @@
"//src/test/cpp:srcs",
"//src/test/gen:srcs",
"//src/test/res:srcs",
- "//src/test/native:srcs",
+ "//src/test/native/windows:srcs",
"//src/test/starlark:srcs",
"//src/test/java/com/google/devtools/build/android:srcs",
"//src/test/java/com/google/devtools/build/docgen:srcs",
@@ -510,7 +510,7 @@
"//src/test/java/com/google/devtools/build/lib:all_windows_tests",
"//src/test/java/com/google/devtools/build/skyframe:all_windows_tests",
"//src/test/java/com/google/devtools/common/options:all_windows_tests",
- "//src/test/native:all_windows_tests",
+ "//src/test/native/windows:all_windows_tests",
"//src/test/py/bazel:all_windows_tests",
"//src/test/res:all_windows_tests",
"//src/test/shell:all_windows_tests",
diff --git a/src/main/cpp/BUILD b/src/main/cpp/BUILD
index 9e18a2b..7de595d 100644
--- a/src/main/cpp/BUILD
+++ b/src/main/cpp/BUILD
@@ -64,7 +64,6 @@
"//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_util_windows.cc b/src/main/cpp/blaze_util_windows.cc
index a2751d3..cc14a4a 100644
--- a/src/main/cpp/blaze_util_windows.cc
+++ b/src/main/cpp/blaze_util_windows.cc
@@ -52,7 +52,6 @@
#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 {
@@ -689,7 +688,7 @@
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);
+ std::wstring wesc = bazel::windows::WindowsEscapeArg(wa);
wesc_args_vector.push_back(wesc);
}
@@ -774,7 +773,7 @@
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);
+ std::wstring wesc = bazel::windows::WindowsEscapeArg(wa);
wargs.push_back(wesc);
}
diff --git a/src/main/cpp/util/BUILD b/src/main/cpp/util/BUILD
index c1094da..faafd9b 100644
--- a/src/main/cpp/util/BUILD
+++ b/src/main/cpp/util/BUILD
@@ -58,6 +58,7 @@
":ijar",
"//src/main/tools:__pkg__",
"//src/test/cpp/util:__pkg__",
+ "//src/test/native/windows:__pkg__",
"//src/tools/launcher:__subpackages__",
"//src/tools/singlejar:__pkg__",
"//third_party/def_parser:__pkg__",
diff --git a/src/main/native/windows/BUILD b/src/main/native/windows/BUILD
index 7b9c273..931db45 100644
--- a/src/main/native/windows/BUILD
+++ b/src/main/native/windows/BUILD
@@ -45,6 +45,8 @@
hdrs = ["process.h"],
visibility = [
"//src/main/cpp:__pkg__",
+ "//src/test/native:__subpackages__",
+ "//src/tools/launcher:__pkg__",
"//tools/test:__pkg__",
],
deps = [":lib-file"],
diff --git a/src/main/native/windows/process.cc b/src/main/native/windows/process.cc
index c863907..5f24c16 100644
--- a/src/main/native/windows/process.cc
+++ b/src/main/native/windows/process.cc
@@ -367,5 +367,95 @@
return true;
}
+// Escape arguments for CreateProcessW.
+//
+// This algorithm is based on information found in
+// http://daviddeley.com/autohotkey/parameters/parameters.htm
+//
+// The following source specifies a similar algorithm:
+// https://blogs.msdn.microsoft.com/twistylittlepassagesallalike/2011/04/23/everyone-quotes-command-line-arguments-the-wrong-way/
+// unfortunately I found this algorithm only after creating the one below, but
+// fortunately they seem to do the same.
+std::wstring WindowsEscapeArg(const std::wstring& s) {
+ if (s.empty()) {
+ return L"\"\"";
+ } else {
+ bool needs_escaping = false;
+ for (const auto& c : s) {
+ if (c == ' ' || c == '"') {
+ needs_escaping = true;
+ break;
+ }
+ }
+ if (!needs_escaping) {
+ return s;
+ }
+ }
+
+ std::wostringstream result;
+ result << L'"';
+ int start = 0;
+ for (int i = 0; i < s.size(); ++i) {
+ char c = s[i];
+ if (c == '"' || c == '\\') {
+ // Copy the segment since the last special character.
+ if (start >= 0) {
+ result << s.substr(start, i - start);
+ start = -1;
+ }
+
+ // Handle the current special character.
+ if (c == '"') {
+ // This is a quote character. Escape it with a single backslash.
+ result << L"\\\"";
+ } else {
+ // This is a backslash (or the first one in a run of backslashes).
+ // Whether we escape it depends on whether the run ends with a quote.
+ int run_len = 1;
+ int j = i + 1;
+ while (j < s.size() && s[j] == '\\') {
+ run_len++;
+ j++;
+ }
+ if (j == s.size()) {
+ // The run of backslashes goes to the end.
+ // We have to escape every backslash with another backslash.
+ for (int k = 0; k < run_len * 2; ++k) {
+ result << L'\\';
+ }
+ break;
+ } else if (j < s.size() && s[j] == '"') {
+ // The run of backslashes is terminated by a quote.
+ // We have to escape every backslash with another backslash, and
+ // escape the quote with one backslash.
+ for (int k = 0; k < run_len * 2; ++k) {
+ result << L'\\';
+ }
+ result << L"\\\"";
+ i += run_len; // 'i' is also increased in the loop iteration step
+ } else {
+ // No quote found. Each backslash counts for itself, they must not be
+ // escaped.
+ for (int k = 0; k < run_len; ++k) {
+ result << L'\\';
+ }
+ i += run_len - 1; // 'i' is also increased in the loop iteration step
+ }
+ }
+ } else {
+ // This is not a special character. Start the segment if necessary.
+ if (start < 0) {
+ start = i;
+ }
+ }
+ }
+ // Save final segment after the last special character.
+ if (start != -1) {
+ result << s.substr(start);
+ }
+ result << L'"';
+ return result.str();
+}
+
} // namespace windows
} // namespace bazel
diff --git a/src/main/native/windows/process.h b/src/main/native/windows/process.h
index 0f43eda..cceb780 100644
--- a/src/main/native/windows/process.h
+++ b/src/main/native/windows/process.h
@@ -67,6 +67,13 @@
DWORD pid_, exit_code_;
};
+// Escape a command line argument using Windows escaping syntax.
+//
+// This escaping lets us safely pass arguments to subprocesses created with
+// CreateProcessW. (The escaping rules are a bit complex, look at the function
+// implementation.)
+std::wstring WindowsEscapeArg(const std::wstring& arg);
+
} // namespace windows
} // namespace bazel
diff --git a/src/test/native/BUILD b/src/test/native/BUILD
deleted file mode 100644
index 868270d..0000000
--- a/src/test/native/BUILD
+++ /dev/null
@@ -1,50 +0,0 @@
-# Description:
-# C++ utility tests for Bazel
-package(default_visibility = ["//visibility:public"])
-
-load("@rules_cc//cc:defs.bzl", "cc_test")
-
-filegroup(
- name = "srcs",
- srcs = glob(["**"]),
- visibility = ["//src:__pkg__"],
-)
-
-cc_test(
- name = "windows_jni_test",
- size = "small",
- srcs = select({
- "//src/conditions:windows": [
- "windows/util_test.cc",
- "windows/file_test.cc",
- ],
- "//conditions:default": ["dummy_test.cc"],
- }),
- deps = select({
- "//src/conditions:windows": [
- "//src/main/native/windows:lib-file",
- "//src/test/cpp/util:windows_test_util",
- "@com_google_googletest//:gtest_main",
- ],
- "//conditions:default": [],
- }),
-)
-
-test_suite(name = "all_tests")
-
-test_suite(
- name = "windows_tests",
- tags = [
- "-no_windows",
- "-slow",
- ],
- visibility = ["//visibility:private"],
-)
-
-test_suite(
- name = "all_windows_tests",
- tests = [
- ":windows_tests",
- ],
- visibility = ["//src:__pkg__"],
-)
diff --git a/src/test/native/windows/BUILD b/src/test/native/windows/BUILD
new file mode 100644
index 0000000..dce5090
--- /dev/null
+++ b/src/test/native/windows/BUILD
@@ -0,0 +1,94 @@
+filegroup(
+ name = "srcs",
+ srcs = glob(["**"]),
+ visibility = ["//src:__pkg__"],
+)
+
+cc_binary(
+ name = "printarg",
+ testonly = 1,
+ srcs = ["printarg.cc"],
+)
+
+cc_library(
+ name = "test_deps",
+ testonly = 1,
+ deps = select({
+ "//src/conditions:windows": [
+ "//src/test/cpp/util:windows_test_util",
+ "@com_google_googletest//:gtest_main",
+ ],
+ "//conditions:default": [],
+ }),
+)
+
+cc_test(
+ name = "file_test",
+ size = "small",
+ srcs = select({
+ "//src/conditions:windows": ["file_test.cc"],
+ "//conditions:default": ["dummy_test.cc"],
+ }),
+ deps = select({
+ "//src/conditions:windows": [
+ "//src/main/native/windows:lib-file",
+ ":test_deps",
+ ],
+ "//conditions:default": [],
+ }),
+)
+
+cc_test(
+ name = "util_test",
+ size = "small",
+ srcs = select({
+ "//src/conditions:windows": ["util_test.cc"],
+ "//conditions:default": ["dummy_test.cc"],
+ }),
+ deps = select({
+ "//src/conditions:windows": [
+ "//src/main/native/windows:lib-file",
+ ":test_deps",
+ ],
+ "//conditions:default": [],
+ }),
+)
+
+cc_test(
+ name = "process_test",
+ size = "small",
+ srcs = select({
+ "//src/conditions:windows": ["process_test.cc"],
+ "//conditions:default": ["dummy_test.cc"],
+ }),
+ data = [":printarg"],
+ deps = select({
+ "//src/conditions:windows": [
+ ":test_deps",
+ "//src/main/native/windows:lib-file",
+ "//src/main/native/windows:lib-process",
+ "//src/main/cpp/util:filesystem",
+ "@bazel_tools//tools/cpp/runfiles",
+ ],
+ "//conditions:default": [],
+ }),
+)
+
+test_suite(name = "all_tests")
+
+test_suite(
+ name = "windows_tests",
+ tags = [
+ "-no_windows",
+ "-slow",
+ ],
+ visibility = ["//visibility:private"],
+)
+
+test_suite(
+ name = "all_windows_tests",
+ tests = [
+ ":windows_tests",
+ ],
+ visibility = ["//src/test/native:__pkg__"],
+)
diff --git a/src/test/native/dummy_test.cc b/src/test/native/windows/dummy_test.cc
similarity index 100%
rename from src/test/native/dummy_test.cc
rename to src/test/native/windows/dummy_test.cc
diff --git a/src/tools/launcher/util/printarg.cc b/src/test/native/windows/printarg.cc
similarity index 100%
rename from src/tools/launcher/util/printarg.cc
rename to src/test/native/windows/printarg.cc
diff --git a/src/test/native/windows/process_test.cc b/src/test/native/windows/process_test.cc
new file mode 100644
index 0000000..5b18c22
--- /dev/null
+++ b/src/test/native/windows/process_test.cc
@@ -0,0 +1,241 @@
+// Copyright 2019 The Bazel Authors. All rights reserved.
+//
+// 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 WIN32_LEAN_AND_MEAN
+#define WIN32_LEAN_AND_MEAN
+#endif
+#include "src/main/native/windows/process.h"
+
+#include <wchar.h>
+#include <windows.h>
+
+#include <memory>
+#include <string>
+#include <vector>
+
+#include "gtest/gtest.h"
+#include "src/main/cpp/util/path.h"
+#include "src/main/native/windows/util.h"
+#include "tools/cpp/runfiles/runfiles.h"
+
+namespace {
+
+// Asserts argument escaping for subprocesses.
+//
+// For each pair in 'args', this method:
+// 1. asserts that WindowsEscapeArg(pair.first) == pair.second
+// 2. asserts that passing pair.second to a subprocess results in the subprocess
+// receiving pair.first
+//
+// The method performs the second assertion by running "printarg.exe" (a
+// data-dependency of this test) once for each argument.
+void AssertSubprocessReceivesArgsAsIntended(
+ const std::vector<std::pair<std::wstring, std::wstring> >& args) {
+ // Assert that the WindowsEscapeArg produces what we expect.
+ for (const auto& i : args) {
+ ASSERT_EQ(bazel::windows::WindowsEscapeArg(i.first), i.second);
+ }
+
+ // Create a Runfiles object.
+ std::string error;
+ std::unique_ptr<bazel::tools::cpp::runfiles::Runfiles> runfiles(
+ bazel::tools::cpp::runfiles::Runfiles::CreateForTest(&error));
+ ASSERT_NE(runfiles.get(), nullptr) << error;
+
+ // Look up the path of the printarg.exe utility.
+ std::string printarg =
+ runfiles->Rlocation("io_bazel/src/test/native/windows/printarg.exe");
+ ASSERT_NE(printarg, "");
+
+ // Convert printarg.exe's path to a wchar_t Windows path.
+ std::wstring wprintarg;
+ bool success =
+ blaze_util::AsAbsoluteWindowsPath(printarg, &wprintarg, &error);
+ ASSERT_TRUE(success) << error;
+
+ // SECURITY_ATTRIBUTES for inheritable HANDLEs.
+ SECURITY_ATTRIBUTES sa;
+ sa.nLength = sizeof(sa);
+ sa.lpSecurityDescriptor = NULL;
+ sa.bInheritHandle = TRUE;
+
+ // Open /dev/null that will be redirected into the subprocess' stdin.
+ bazel::windows::AutoHandle devnull(
+ CreateFileW(L"NUL", GENERIC_READ,
+ FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, &sa,
+ OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL));
+ ASSERT_TRUE(devnull.IsValid());
+
+ // Create a pipe that the subprocess' stdout will be redirected to.
+ HANDLE pipe_read_h, pipe_write_h;
+ if (!CreatePipe(&pipe_read_h, &pipe_write_h, &sa, 0x10000)) {
+ DWORD err = GetLastError();
+ ASSERT_EQ(err, 0);
+ }
+ bazel::windows::AutoHandle pipe_read(pipe_read_h), pipe_write(pipe_write_h);
+
+ // Duplicate stderr, where the subprocess' stderr will be redirected to.
+ HANDLE stderr_h;
+ if (!DuplicateHandle(GetCurrentProcess(), GetStdHandle(STD_ERROR_HANDLE),
+ GetCurrentProcess(), &stderr_h, 0, TRUE,
+ DUPLICATE_SAME_ACCESS)) {
+ DWORD err = GetLastError();
+ ASSERT_EQ(err, 0);
+ }
+ bazel::windows::AutoHandle stderr_dup(stderr_h);
+
+ // Create the attribute object for the process creation. This object describes
+ // exactly which handles the subprocess shall inherit.
+ STARTUPINFOEXW startupInfo;
+ std::unique_ptr<bazel::windows::AutoAttributeList> attrs;
+ std::wstring werror;
+ ASSERT_TRUE(bazel::windows::AutoAttributeList::Create(
+ devnull, pipe_write, stderr_dup, &attrs, &werror));
+ attrs->InitStartupInfoExW(&startupInfo);
+
+ // MSDN says the maximum command line is 32767 characters, with a null
+ // terminator that is exactly 2^15 (= 0x8000).
+ static constexpr size_t kMaxCmdline = 0x8000;
+ wchar_t cmdline[kMaxCmdline];
+
+ // Copy printarg.exe's escaped path into the 'cmdline', and append a space.
+ // We will append arguments to this command line in the for-loop below.
+ wprintarg = bazel::windows::WindowsEscapeArg(wprintarg);
+ wcsncpy(cmdline, wprintarg.c_str(), wprintarg.size());
+ wchar_t* pcmdline = cmdline + wprintarg.size();
+ *pcmdline++ = L' ';
+
+ // Run a subprocess for each of the arguments and assert that the argument
+ // arrived to the subprocess as intended.
+ for (const auto& i : args) {
+ // We already asserted for every element that WindowsEscapeArg(i.first)
+ // produces the same output as i.second, so just use i.second instead of
+ // converting i.first again.
+ wcsncpy(pcmdline, i.second.c_str(), i.second.size());
+ pcmdline[i.second.size()] = 0;
+
+ // Run the subprocess.
+ PROCESS_INFORMATION processInfo;
+ BOOL ok = CreateProcessW(
+ NULL, cmdline, NULL, NULL, TRUE,
+ CREATE_UNICODE_ENVIRONMENT | EXTENDED_STARTUPINFO_PRESENT, NULL, NULL,
+ &startupInfo.StartupInfo, &processInfo);
+ if (!ok) {
+ DWORD err = GetLastError();
+ ASSERT_EQ(err, 0);
+ }
+ CloseHandle(processInfo.hThread);
+ bazel::windows::AutoHandle process(processInfo.hProcess);
+
+ // Wait for the subprocess to exit. Timeout is 5 seconds, which should be
+ // more than enough for the subprocess to finish.
+ ASSERT_EQ(WaitForSingleObject(process, 5000), WAIT_OBJECT_0);
+
+ // The subprocess printed its argv[1] (without a newline) to its stdout,
+ // which is redirected into the pipe.
+ // Let's write a null-terminator to the pipe to separate the output from the
+ // output of the subsequent subprocess. The null-terminator also yields
+ // null-terminated strings in the pipe, making it easy to read them out
+ // later.
+ DWORD dummy;
+ ASSERT_TRUE(WriteFile(pipe_write, "\0", 1, &dummy, NULL));
+ }
+
+ // Read the output of the subprocesses from the pipe. They are divided by
+ // null-terminators, so 'buf' will contain a sequence of null-terminated
+ // strings. We close the writing end so that ReadFile won't block until the
+ // desired amount of bytes is available.
+ DWORD total_output_len;
+ char buf[0x10000];
+ pipe_write = INVALID_HANDLE_VALUE;
+ if (!ReadFile(pipe_read, buf, 0x10000, &total_output_len, NULL)) {
+ DWORD err = GetLastError();
+ ASSERT_EQ(err, 0);
+ }
+
+ // Assert that the subprocesses produced exactly the *unescaped* arguments.
+ size_t start = 0;
+ for (const auto& arg : args) {
+ // Assert that there was enough data produced by the subprocesses.
+ ASSERT_LT(start, total_output_len);
+
+ // Find the output of the corresponding subprocess. Since all subprocesses
+ // printed into the same pipe and we added null-terminators between them,
+ // the output is already there, conveniently as a null-terminated string.
+ std::string actual_arg(buf + start);
+ start += actual_arg.size() + 1;
+
+ // 'args' contains wchar_t strings, but the subprocesses printed ASCII
+ // (char) strings. To compare, we convert arg.first to a char-string.
+ std::string expected_arg;
+ expected_arg.reserve(arg.first.size());
+ for (const auto& wc : arg.first) {
+ expected_arg.append(1, static_cast<char>(wc));
+ }
+
+ // Assert that the subprocess printed exactly the *unescaped* argument.
+ EXPECT_EQ(expected_arg, actual_arg);
+ }
+}
+
+TEST(ProcessTest, WindowsEscapeArgTest) {
+ AssertSubprocessReceivesArgsAsIntended({
+ {L"", L"\"\""},
+ {L" ", L"\" \""},
+ {L"\"", L"\"\\\"\""},
+ {L"\"\\", L"\"\\\"\\\\\""},
+ {L"\\", L"\\"},
+ {L"\\\"", L"\"\\\\\\\"\""},
+ {L"with space", L"\"with space\""},
+ {L"with^caret", L"with^caret"},
+ {L"space ^caret", L"\"space ^caret\""},
+ {L"caret^ space", L"\"caret^ space\""},
+ {L"with\"quote", L"\"with\\\"quote\""},
+ {L"with\\backslash", L"with\\backslash"},
+ {L"one\\ backslash and \\space", L"\"one\\ backslash and \\space\""},
+ {L"two\\\\backslashes", L"two\\\\backslashes"},
+ {L"two\\\\ backslashes \\\\and space",
+ L"\"two\\\\ backslashes \\\\and space\""},
+ {L"one\\\"x", L"\"one\\\\\\\"x\""},
+ {L"two\\\\\"x", L"\"two\\\\\\\\\\\"x\""},
+ {L"a \\ b", L"\"a \\ b\""},
+ {L"a \\\" b", L"\"a \\\\\\\" b\""},
+ {L"A", L"A"},
+ {L"\"a\"", L"\"\\\"a\\\"\""},
+ {L"B C", L"\"B C\""},
+ {L"\"b c\"", L"\"\\\"b c\\\"\""},
+ {L"D\"E", L"\"D\\\"E\""},
+ {L"\"d\"e\"", L"\"\\\"d\\\"e\\\"\""},
+ {L"C:\\F G", L"\"C:\\F G\""},
+ {L"\"C:\\f g\"", L"\"\\\"C:\\f g\\\"\""},
+ {L"C:\\H\"I", L"\"C:\\H\\\"I\""},
+ {L"\"C:\\h\"i\"", L"\"\\\"C:\\h\\\"i\\\"\""},
+ {L"C:\\J\\\"K", L"\"C:\\J\\\\\\\"K\""},
+ {L"\"C:\\j\\\"k\"", L"\"\\\"C:\\j\\\\\\\"k\\\"\""},
+ {L"C:\\L M ", L"\"C:\\L M \""},
+ {L"\"C:\\l m \"", L"\"\\\"C:\\l m \\\"\""},
+ {L"C:\\N O\\", L"\"C:\\N O\\\\\""},
+ {L"\"C:\\n o\\\"", L"\"\\\"C:\\n o\\\\\\\"\""},
+ {L"C:\\P Q\\ ", L"\"C:\\P Q\\ \""},
+ {L"\"C:\\p q\\ \"", L"\"\\\"C:\\p q\\ \\\"\""},
+ {L"C:\\R\\S\\", L"C:\\R\\S\\"},
+ {L"C:\\R x\\S\\", L"\"C:\\R x\\S\\\\\""},
+ {L"\"C:\\r\\s\\\"", L"\"\\\"C:\\r\\s\\\\\\\"\""},
+ {L"\"C:\\r x\\s\\\"", L"\"\\\"C:\\r x\\s\\\\\\\"\""},
+ {L"C:\\T U\\W\\", L"\"C:\\T U\\W\\\\\""},
+ {L"\"C:\\t u\\w\\\"", L"\"\\\"C:\\t u\\w\\\\\\\"\""},
+ });
+}
+
+} // namespace
diff --git a/src/tools/launcher/BUILD b/src/tools/launcher/BUILD
index 9383128..587dcc8 100644
--- a/src/tools/launcher/BUILD
+++ b/src/tools/launcher/BUILD
@@ -38,14 +38,20 @@
name = "java_launcher",
srcs = ["java_launcher.cc"],
hdrs = ["java_launcher.h"],
- deps = [":launcher_base"],
+ deps = [
+ ":launcher_base",
+ "//src/main/native/windows:lib-process",
+ ],
)
win_cc_library(
name = "python_launcher",
srcs = ["python_launcher.cc"],
hdrs = ["python_launcher.h"],
- deps = [":launcher_base"],
+ deps = [
+ ":launcher_base",
+ "//src/main/native/windows:lib-process",
+ ],
)
win_cc_library(
diff --git a/src/tools/launcher/java_launcher.cc b/src/tools/launcher/java_launcher.cc
index 01af0cf..2af0be2 100644
--- a/src/tools/launcher/java_launcher.cc
+++ b/src/tools/launcher/java_launcher.cc
@@ -12,6 +12,8 @@
// See the License for the specific language governing permissions and
// limitations under the License.
+#include "src/tools/launcher/java_launcher.h"
+
#include <memory>
#include <sstream>
#include <string>
@@ -23,7 +25,7 @@
#include "src/main/cpp/util/path_platform.h"
#include "src/main/cpp/util/strings.h"
#include "src/main/native/windows/file.h"
-#include "src/tools/launcher/java_launcher.h"
+#include "src/main/native/windows/process.h"
#include "src/tools/launcher/util/launcher_util.h"
namespace bazel {
@@ -411,7 +413,7 @@
vector<wstring> escaped_arguments;
// Quote the arguments if having spaces
for (const auto& arg : arguments) {
- escaped_arguments.push_back(WindowsEscapeArg2(arg));
+ escaped_arguments.push_back(bazel::windows::WindowsEscapeArg(arg));
}
ExitCode exit_code = this->LaunchProcess(java_bin, escaped_arguments);
diff --git a/src/tools/launcher/python_launcher.cc b/src/tools/launcher/python_launcher.cc
index b29173a..d96f208 100644
--- a/src/tools/launcher/python_launcher.cc
+++ b/src/tools/launcher/python_launcher.cc
@@ -12,10 +12,12 @@
// See the License for the specific language governing permissions and
// limitations under the License.
+#include "src/tools/launcher/python_launcher.h"
+
#include <string>
#include <vector>
-#include "src/tools/launcher/python_launcher.h"
+#include "src/main/native/windows/process.h"
#include "src/tools/launcher/util/launcher_util.h"
namespace bazel {
@@ -64,7 +66,7 @@
args[0] = python_file;
for (int i = 1; i < args.size(); i++) {
- args[i] = WindowsEscapeArg2(args[i]);
+ args[i] = bazel::windows::WindowsEscapeArg(args[i]);
}
return this->LaunchProcess(python_binary, args);
diff --git a/src/tools/launcher/util/BUILD b/src/tools/launcher/util/BUILD
index c4a66ef..74a5b77 100644
--- a/src/tools/launcher/util/BUILD
+++ b/src/tools/launcher/util/BUILD
@@ -19,20 +19,12 @@
name = "util",
srcs = ["launcher_util.cc"],
hdrs = ["launcher_util.h"],
- visibility = [
- "//src/main/cpp:__pkg__",
- "//src/tools/launcher:__subpackages__",
- "//tools/test:__pkg__",
- ],
deps = ["//src/main/cpp/util:filesystem"],
)
win_cc_test(
name = "util_test",
srcs = ["launcher_util_test.cc"],
- data = [
- ":printarg",
- ],
deps = [
":util",
"//src/main/native/windows:lib-file",
@@ -50,12 +42,6 @@
],
)
-win_cc_binary(
- name = "printarg",
- testonly = 1,
- srcs = ["printarg.cc"],
-)
-
test_suite(
name = "windows_tests",
tags = [
diff --git a/src/tools/launcher/util/launcher_util.cc b/src/tools/launcher/util/launcher_util.cc
index da51471..19ec6f2 100644
--- a/src/tools/launcher/util/launcher_util.cc
+++ b/src/tools/launcher/util/launcher_util.cc
@@ -170,96 +170,6 @@
return escaped_arg;
}
-// Escape arguments for CreateProcessW.
-//
-// This algorithm is based on information found in
-// http://daviddeley.com/autohotkey/parameters/parameters.htm
-//
-// The following source specifies a similar algorithm:
-// https://blogs.msdn.microsoft.com/twistylittlepassagesallalike/2011/04/23/everyone-quotes-command-line-arguments-the-wrong-way/
-// unfortunately I found this algorithm only after creating the one below, but
-// fortunately they seem to do the same.
-std::wstring WindowsEscapeArg2(const std::wstring& s) {
- if (s.empty()) {
- return L"\"\"";
- } else {
- bool needs_escaping = false;
- for (const auto& c : s) {
- if (c == ' ' || c == '"') {
- needs_escaping = true;
- break;
- }
- }
- if (!needs_escaping) {
- return s;
- }
- }
-
- std::wostringstream result;
- result << L'"';
- int start = 0;
- for (int i = 0; i < s.size(); ++i) {
- char c = s[i];
- if (c == '"' || c == '\\') {
- // Copy the segment since the last special character.
- if (start >= 0) {
- result << s.substr(start, i - start);
- start = -1;
- }
-
- // Handle the current special character.
- if (c == '"') {
- // This is a quote character. Escape it with a single backslash.
- result << L"\\\"";
- } else {
- // This is a backslash (or the first one in a run of backslashes).
- // Whether we escape it depends on whether the run ends with a quote.
- int run_len = 1;
- int j = i + 1;
- while (j < s.size() && s[j] == '\\') {
- run_len++;
- j++;
- }
- if (j == s.size()) {
- // The run of backslashes goes to the end.
- // We have to escape every backslash with another backslash.
- for (int k = 0; k < run_len * 2; ++k) {
- result << L'\\';
- }
- break;
- } else if (j < s.size() && s[j] == '"') {
- // The run of backslashes is terminated by a quote.
- // We have to escape every backslash with another backslash, and
- // escape the quote with one backslash.
- for (int k = 0; k < run_len * 2; ++k) {
- result << L'\\';
- }
- result << L"\\\"";
- i += run_len; // 'i' is also increased in the loop iteration step
- } else {
- // No quote found. Each backslash counts for itself, they must not be
- // escaped.
- for (int k = 0; k < run_len; ++k) {
- result << L'\\';
- }
- i += run_len - 1; // 'i' is also increased in the loop iteration step
- }
- }
- } else {
- // This is not a special character. Start the segment if necessary.
- if (start < 0) {
- start = i;
- }
- }
- }
- // Save final segment after the last special character.
- if (start != -1) {
- result << s.substr(start);
- }
- result << L'"';
- return result.str();
-}
-
// An environment variable has a maximum size limit of 32,767 characters
// https://msdn.microsoft.com/en-us/library/ms683188.aspx
static const int BUFFER_SIZE = 32767;
diff --git a/src/tools/launcher/util/launcher_util.h b/src/tools/launcher/util/launcher_util.h
index e641f42..7353612 100644
--- a/src/tools/launcher/util/launcher_util.h
+++ b/src/tools/launcher/util/launcher_util.h
@@ -48,13 +48,6 @@
// to \\).
std::wstring BashEscapeArg(const std::wstring& arg);
-// Escape a command line argument using Windows escaping syntax.
-//
-// This escaping lets us safely pass arguments to subprocesses created with
-// CreateProcessW. (The escaping rules are a bit complex, look at the function
-// implementation.)
-std::wstring WindowsEscapeArg2(const std::wstring& arg);
-
// Convert a path to an absolute Windows path with \\?\ prefix.
// This method will print an error and exit if it cannot convert the path.
std::wstring AsAbsoluteWindowsPath(const wchar_t* path);
diff --git a/src/tools/launcher/util/launcher_util_test.cc b/src/tools/launcher/util/launcher_util_test.cc
index 3d516cc..d2d9ce7 100644
--- a/src/tools/launcher/util/launcher_util_test.cc
+++ b/src/tools/launcher/util/launcher_util_test.cc
@@ -97,216 +97,6 @@
BashEscapeArg(L"C:\\foo foo\\bar\\"));
}
-// Asserts argument escaping for subprocesses.
-//
-// For each pair in 'args', this method:
-// 1. asserts that WindowsEscapeArg(pair.first) == pair.second
-// 2. asserts that passing pair.second to a subprocess results in the subprocess
-// receiving pair.first
-//
-// The method performs the second assertion by running "printarg.exe" (a
-// data-dependency of this test) once for each argument.
-void AssertSubprocessReceivesArgsAsIntended(
- std::wstring (*escape_func)(const std::wstring& s),
- const std::vector<std::pair<wstring, wstring> >& args) {
- // Assert that the WindowsEscapeArg produces what we expect.
- for (const auto& i : args) {
- ASSERT_EQ(escape_func(i.first), i.second);
- }
-
- // Create a Runfiles object.
- string error;
- std::unique_ptr<bazel::tools::cpp::runfiles::Runfiles> runfiles(
- bazel::tools::cpp::runfiles::Runfiles::CreateForTest(&error));
- ASSERT_NE(runfiles.get(), nullptr) << error;
-
- // Look up the path of the printarg.exe utility.
- string printarg =
- runfiles->Rlocation("io_bazel/src/tools/launcher/util/printarg.exe");
- ASSERT_NE(printarg, "");
-
- // Convert printarg.exe's path to a wchar_t Windows path.
- wstring wprintarg;
- bool success =
- blaze_util::AsAbsoluteWindowsPath(printarg, &wprintarg, &error);
- ASSERT_TRUE(success) << error;
-
- // SECURITY_ATTRIBUTES for inheritable HANDLEs.
- SECURITY_ATTRIBUTES sa;
- sa.nLength = sizeof(sa);
- sa.lpSecurityDescriptor = NULL;
- sa.bInheritHandle = TRUE;
-
- // Open /dev/null that will be redirected into the subprocess' stdin.
- bazel::windows::AutoHandle devnull(
- CreateFileW(L"NUL", GENERIC_READ,
- FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, &sa,
- OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL));
- ASSERT_TRUE(devnull.IsValid());
-
- // Create a pipe that the subprocess' stdout will be redirected to.
- HANDLE pipe_read_h, pipe_write_h;
- if (!CreatePipe(&pipe_read_h, &pipe_write_h, &sa, 0x10000)) {
- DWORD err = GetLastError();
- ASSERT_EQ(err, 0);
- }
- bazel::windows::AutoHandle pipe_read(pipe_read_h), pipe_write(pipe_write_h);
-
- // Duplicate stderr, where the subprocess' stderr will be redirected to.
- HANDLE stderr_h;
- if (!DuplicateHandle(GetCurrentProcess(), GetStdHandle(STD_ERROR_HANDLE),
- GetCurrentProcess(), &stderr_h, 0, TRUE,
- DUPLICATE_SAME_ACCESS)) {
- DWORD err = GetLastError();
- ASSERT_EQ(err, 0);
- }
- bazel::windows::AutoHandle stderr_dup(stderr_h);
-
- // Create the attribute object for the process creation. This object describes
- // exactly which handles the subprocess shall inherit.
- STARTUPINFOEXW startupInfo;
- std::unique_ptr<bazel::windows::AutoAttributeList> attrs;
- wstring werror;
- ASSERT_TRUE(bazel::windows::AutoAttributeList::Create(
- devnull, pipe_write, stderr_dup, &attrs, &werror));
- attrs->InitStartupInfoExW(&startupInfo);
-
- // MSDN says the maximum command line is 32767 characters, with a null
- // terminator that is exactly 2^15 (= 0x8000).
- static constexpr size_t kMaxCmdline = 0x8000;
- wchar_t cmdline[kMaxCmdline];
-
- // Copy printarg.exe's escaped path into the 'cmdline', and append a space.
- // We will append arguments to this command line in the for-loop below.
- wprintarg = escape_func(wprintarg);
- wcsncpy(cmdline, wprintarg.c_str(), wprintarg.size());
- wchar_t* pcmdline = cmdline + wprintarg.size();
- *pcmdline++ = L' ';
-
- // Run a subprocess for each of the arguments and assert that the argument
- // arrived to the subprocess as intended.
- for (const auto& i : args) {
- // We already asserted for every element that escape_func(i.first)
- // produces the same output as i.second, so just use i.second instead of
- // converting i.first again.
- wcsncpy(pcmdline, i.second.c_str(), i.second.size());
- pcmdline[i.second.size()] = 0;
-
- // Run the subprocess.
- PROCESS_INFORMATION processInfo;
- BOOL ok = CreateProcessW(
- NULL, cmdline, NULL, NULL, TRUE,
- CREATE_UNICODE_ENVIRONMENT | EXTENDED_STARTUPINFO_PRESENT, NULL, NULL,
- &startupInfo.StartupInfo, &processInfo);
- if (!ok) {
- DWORD err = GetLastError();
- ASSERT_EQ(err, 0);
- }
- CloseHandle(processInfo.hThread);
- bazel::windows::AutoHandle process(processInfo.hProcess);
-
- // Wait for the subprocess to exit. Timeout is 5 seconds, which should be
- // more than enough for the subprocess to finish.
- ASSERT_EQ(WaitForSingleObject(process, 5000), WAIT_OBJECT_0);
-
- // The subprocess printed its argv[1] (without a newline) to its stdout,
- // which is redirected into the pipe.
- // Let's write a null-terminator to the pipe to separate the output from the
- // output of the subsequent subprocess. The null-terminator also yields
- // null-terminated strings in the pipe, making it easy to read them out
- // later.
- DWORD dummy;
- ASSERT_TRUE(WriteFile(pipe_write, "\0", 1, &dummy, NULL));
- }
-
- // Read the output of the subprocesses from the pipe. They are divided by
- // null-terminators, so 'buf' will contain a sequence of null-terminated
- // strings. We close the writing end so that ReadFile won't block until the
- // desired amount of bytes is available.
- DWORD total_output_len;
- char buf[0x10000];
- pipe_write = INVALID_HANDLE_VALUE;
- if (!ReadFile(pipe_read, buf, 0x10000, &total_output_len, NULL)) {
- DWORD err = GetLastError();
- ASSERT_EQ(err, 0);
- }
-
- // Assert that the subprocesses produced exactly the *unescaped* arguments.
- size_t start = 0;
- for (const auto& arg : args) {
- // Assert that there was enough data produced by the subprocesses.
- ASSERT_LT(start, total_output_len);
-
- // Find the output of the corresponding subprocess. Since all subprocesses
- // printed into the same pipe and we added null-terminators between them,
- // the output is already there, conveniently as a null-terminated string.
- string actual_arg(buf + start);
- start += actual_arg.size() + 1;
-
- // 'args' contains wchar_t strings, but the subprocesses printed ASCII
- // (char) strings. To compare, we convert arg.first to a char-string.
- string expected_arg;
- expected_arg.reserve(arg.first.size());
- for (const auto& wc : arg.first) {
- expected_arg.append(1, static_cast<char>(wc));
- }
-
- // Assert that the subprocess printed exactly the *unescaped* argument.
- EXPECT_EQ(expected_arg, actual_arg);
- }
-}
-
-TEST_F(LaunchUtilTest, WindowsEscapeArg2Test) {
- AssertSubprocessReceivesArgsAsIntended(
- WindowsEscapeArg2,
- {
- {L"", L"\"\""},
- {L" ", L"\" \""},
- {L"\"", L"\"\\\"\""},
- {L"\"\\", L"\"\\\"\\\\\""},
- {L"\\", L"\\"},
- {L"\\\"", L"\"\\\\\\\"\""},
- {L"with space", L"\"with space\""},
- {L"with^caret", L"with^caret"},
- {L"space ^caret", L"\"space ^caret\""},
- {L"caret^ space", L"\"caret^ space\""},
- {L"with\"quote", L"\"with\\\"quote\""},
- {L"with\\backslash", L"with\\backslash"},
- {L"one\\ backslash and \\space", L"\"one\\ backslash and \\space\""},
- {L"two\\\\backslashes", L"two\\\\backslashes"},
- {L"two\\\\ backslashes \\\\and space",
- L"\"two\\\\ backslashes \\\\and space\""},
- {L"one\\\"x", L"\"one\\\\\\\"x\""},
- {L"two\\\\\"x", L"\"two\\\\\\\\\\\"x\""},
- {L"a \\ b", L"\"a \\ b\""},
- {L"a \\\" b", L"\"a \\\\\\\" b\""},
- {L"A", L"A"},
- {L"\"a\"", L"\"\\\"a\\\"\""},
- {L"B C", L"\"B C\""},
- {L"\"b c\"", L"\"\\\"b c\\\"\""},
- {L"D\"E", L"\"D\\\"E\""},
- {L"\"d\"e\"", L"\"\\\"d\\\"e\\\"\""},
- {L"C:\\F G", L"\"C:\\F G\""},
- {L"\"C:\\f g\"", L"\"\\\"C:\\f g\\\"\""},
- {L"C:\\H\"I", L"\"C:\\H\\\"I\""},
- {L"\"C:\\h\"i\"", L"\"\\\"C:\\h\\\"i\\\"\""},
- {L"C:\\J\\\"K", L"\"C:\\J\\\\\\\"K\""},
- {L"\"C:\\j\\\"k\"", L"\"\\\"C:\\j\\\\\\\"k\\\"\""},
- {L"C:\\L M ", L"\"C:\\L M \""},
- {L"\"C:\\l m \"", L"\"\\\"C:\\l m \\\"\""},
- {L"C:\\N O\\", L"\"C:\\N O\\\\\""},
- {L"\"C:\\n o\\\"", L"\"\\\"C:\\n o\\\\\\\"\""},
- {L"C:\\P Q\\ ", L"\"C:\\P Q\\ \""},
- {L"\"C:\\p q\\ \"", L"\"\\\"C:\\p q\\ \\\"\""},
- {L"C:\\R\\S\\", L"C:\\R\\S\\"},
- {L"C:\\R x\\S\\", L"\"C:\\R x\\S\\\\\""},
- {L"\"C:\\r\\s\\\"", L"\"\\\"C:\\r\\s\\\\\\\"\""},
- {L"\"C:\\r x\\s\\\"", L"\"\\\"C:\\r x\\s\\\\\\\"\""},
- {L"C:\\T U\\W\\", L"\"C:\\T U\\W\\\\\""},
- {L"\"C:\\t u\\w\\\"", L"\"\\\"C:\\t u\\w\\\\\\\"\""},
- });
-}
-
TEST_F(LaunchUtilTest, DoesFilePathExistTest) {
wstring file1 = GetTmpDir() + L"/foo";
wstring file2 = GetTmpDir() + L"/bar";
diff --git a/tools/test/BUILD b/tools/test/BUILD
index 8d33e18..f01ac67 100644
--- a/tools/test/BUILD
+++ b/tools/test/BUILD
@@ -82,7 +82,6 @@
"//src/main/cpp/util:strings",
"//src/main/native/windows:lib-file",
"//src/main/native/windows:lib-process",
- "//src/tools/launcher/util",
"//third_party/ijar:zip",
"@bazel_tools//tools/cpp/runfiles",
],
diff --git a/tools/test/windows/tw.cc b/tools/test/windows/tw.cc
index 2bd3e8e..d2c81d7 100644
--- a/tools/test/windows/tw.cc
+++ b/tools/test/windows/tw.cc
@@ -46,7 +46,6 @@
#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"
#include "third_party/ijar/common.h"
#include "third_party/ijar/platform_utils.h"
#include "third_party/ijar/zip.h"
@@ -1378,7 +1377,7 @@
*out_test_path_arg = argv[0];
std::wstringstream stm;
for (int i = 1; i < argc; i++) {
- stm << L' ' << bazel::launcher::WindowsEscapeArg2(argv[i]);
+ stm << L' ' << bazel::windows::WindowsEscapeArg(argv[i]);
}
*out_args = stm.str();
return true;
