Skip to content

Commit

Permalink
Windows, launcher: rename GetEscapedArgument
Browse files Browse the repository at this point in the history
Next step: implement correct escaping semantics
for subprocesses created with CreateProcessW.

See bazelbuild#7072
  • Loading branch information
laszlocsomor committed Feb 11, 2019
1 parent c0ba97e commit d004aac
Show file tree
Hide file tree
Showing 6 changed files with 42 additions and 29 deletions.
9 changes: 3 additions & 6 deletions src/tools/launcher/bash_launcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -45,18 +45,15 @@ ExitCode BashBinaryLauncher::Launch() {
vector<wstring> origin_args = this->GetCommandlineArguments();
wostringstream bash_command;
wstring bash_main_file = GetBinaryPathWithoutExtension(origin_args[0]);
bash_command << GetEscapedArgument(bash_main_file,
/*escape_backslash = */ true);
bash_command << BashEscapeArg(bash_main_file);
for (int i = 1; i < origin_args.size(); i++) {
bash_command << L' ';
bash_command << GetEscapedArgument(origin_args[i],
/*escape_backslash = */ true);
bash_command << BashEscapeArg(origin_args[i]);
}

vector<wstring> args;
args.push_back(L"-c");
args.push_back(
GetEscapedArgument(bash_command.str(), /*escape_backslash = */ true));
args.push_back(BashEscapeArg(bash_command.str()));
return this->LaunchProcess(bash_binary, args);
}

Expand Down
3 changes: 1 addition & 2 deletions src/tools/launcher/java_launcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -411,8 +411,7 @@ ExitCode JavaBinaryLauncher::Launch() {
vector<wstring> escaped_arguments;
// Quote the arguments if having spaces
for (const auto& arg : arguments) {
escaped_arguments.push_back(
GetEscapedArgument(arg, /*escape_backslash = */ false));
escaped_arguments.push_back(WindowsEscapeArg(arg));
}

ExitCode exit_code = this->LaunchProcess(java_bin, escaped_arguments);
Expand Down
2 changes: 1 addition & 1 deletion src/tools/launcher/python_launcher.cc
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ ExitCode PythonBinaryLauncher::Launch() {

// Escape arguments that has spaces
for (int i = 1; i < args.size(); i++) {
args[i] = GetEscapedArgument(args[i], /*escape_backslash = */ false);
args[i] = WindowsEscapeArg(args[i]);
}

return this->LaunchProcess(python_binary, args);
Expand Down
12 changes: 11 additions & 1 deletion src/tools/launcher/util/launcher_util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ wstring GetBinaryPathWithExtension(const wstring& binary) {
return GetBinaryPathWithoutExtension(binary) + L".exe";
}

wstring GetEscapedArgument(const wstring& argument, bool escape_backslash) {
static wstring GetEscapedArgument(const wstring& argument, bool escape_backslash) {
wstring escaped_arg;
// escaped_arg will be at least this long
escaped_arg.reserve(argument.size());
Expand Down Expand Up @@ -165,6 +165,16 @@ wstring GetEscapedArgument(const wstring& argument, bool escape_backslash) {
return escaped_arg;
}

std::wstring BashEscapeArg(const std::wstring& arg) {
return GetEscapedArgument(arg, /* escape_backslash */ true);
}

std::wstring WindowsEscapeArg(const std::wstring& arg);
// TODO(laszlocsomor): properly implement escaping syntax for CreateProcessW;
// it's different from Bash escaping syntax.
return GetEscapedArgument(arg, /* escape_backslash */ false);
}

// 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;
Expand Down
18 changes: 12 additions & 6 deletions src/tools/launcher/util/launcher_util.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,19 @@ std::wstring GetBinaryPathWithoutExtension(const std::wstring& binary);
// On Windows, if the binary path is foo/bar/bin then return foo/bar/bin.exe
std::wstring GetBinaryPathWithExtension(const std::wstring& binary);

// Escape a command line argument.
// Escape a command line argument using Bash escaping syntax.
//
// If the argument has space, then we quote it.
// Escape " to \"
// Escape \ to \\ if escape_backslash is true
std::wstring GetEscapedArgument(const std::wstring& argument,
bool escape_backslash);
// If the argument has space, then we quote it. We escape quote with a backslash
// (from " to \") and escape a single backslash with another backslash (from \
// 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 WindowsEscapeArg(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.
Expand Down
27 changes: 14 additions & 13 deletions src/tools/launcher/util/launcher_util_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -74,22 +74,23 @@ TEST_F(LaunchUtilTest, GetBinaryPathWithExtensionTest) {
ASSERT_EQ(L"foo.sh.exe", GetBinaryPathWithExtension(L"foo.sh"));
}

TEST_F(LaunchUtilTest, GetEscapedArgumentTest) {
ASSERT_EQ(L"\"\"", GetEscapedArgument(L"", true));
ASSERT_EQ(L"foo", GetEscapedArgument(L"foo", true));
ASSERT_EQ(L"\"foo bar\"", GetEscapedArgument(L"foo bar", true));
ASSERT_EQ(L"\"\\\"foo bar\\\"\"", GetEscapedArgument(L"\"foo bar\"", true));
ASSERT_EQ(L"foo\\\\bar", GetEscapedArgument(L"foo\\bar", true));
ASSERT_EQ(L"foo\\\"bar", GetEscapedArgument(L"foo\"bar", true));
TEST_F(LaunchUtilTest, BashEscapeArgTest) {
ASSERT_EQ(L"\"\"", BashEscapeArg(L""));
ASSERT_EQ(L"foo", BashEscapeArg(L"foo"));
ASSERT_EQ(L"\"foo bar\"", BashEscapeArg(L"foo bar"));
ASSERT_EQ(L"\"\\\"foo bar\\\"\"", BashEscapeArg(L"\"foo bar\""));
ASSERT_EQ(L"foo\\\\bar", BashEscapeArg(L"foo\\bar"));
ASSERT_EQ(L"foo\\\"bar", BashEscapeArg(L"foo\"bar"));
ASSERT_EQ(L"C:\\\\foo\\\\bar\\\\",
GetEscapedArgument(L"C:\\foo\\bar\\", true));
BashEscapeArg(L"C:\\foo\\bar\\"));
ASSERT_EQ(L"\"C:\\\\foo foo\\\\bar\\\\\"",
GetEscapedArgument(L"C:\\foo foo\\bar\\", true));
BashEscapeArg(L"C:\\foo foo\\bar\\"));
}

ASSERT_EQ(L"foo\\bar", GetEscapedArgument(L"foo\\bar", false));
ASSERT_EQ(L"C:\\foo\\bar\\", GetEscapedArgument(L"C:\\foo\\bar\\", false));
ASSERT_EQ(L"\"C:\\foo foo\\bar\\\"",
GetEscapedArgument(L"C:\\foo foo\\bar\\", false));
TEST_F(LaunchUtilTest, WindowsEscapeArgTest) {
ASSERT_EQ(L"foo\\bar", WindowsEscapeArg(L"foo\\bar"));
ASSERT_EQ(L"C:\\foo\\bar\\", WindowsEscapeArg(L"C:\\foo\\bar\\"));
ASSERT_EQ(L"\"C:\\foo foo\\bar\\\"", WindowsEscapeArg(L"C:\\foo foo\\bar\\"));
}

TEST_F(LaunchUtilTest, DoesFilePathExistTest) {
Expand Down

0 comments on commit d004aac

Please sign in to comment.