Skip to content

Commit

Permalink
Windows, subprocesses: correct flag escaping impl.
Browse files Browse the repository at this point in the history
Add correct implementation of flag escaping for
subprocesses.

This is a follow-up to bazelbuild#7413

Next steps:
- replace WindowsProcesses.quoteCommandLine with
  the new logic

See bazelbuild#7122
  • Loading branch information
laszlocsomor committed Feb 13, 2019
1 parent b7bd7a1 commit 73ac05d
Show file tree
Hide file tree
Showing 3 changed files with 144 additions and 3 deletions.
90 changes: 90 additions & 0 deletions src/main/java/com/google/devtools/build/lib/shell/ShellUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -142,4 +142,94 @@ public static void tokenize(List<String> options, String optionString)
}
}

/**
* Escape command line arguments for {@code CreateProcessW} on Windows.
*
* <p>This method implements the same algorithm as the native xx_binary launcher does (see
* https://github.com/bazelbuild/bazel/pull/7411).
*
* <p>A similar algorithm with lots of background information is described here:
* https://blogs.msdn.microsoft.com/twistylittlepassagesallalike/2011/04/23/
*/
public static String windowsEscapeArg(String s) {
if (s.isEmpty()) {
return "\"\"";
} else {
boolean needsEscape = false;
for (int i = 0; i < s.length(); ++i) {
char c = s.charAt(i);
if (c == ' ' || c == '"') {
needsEscape = true;
break;
}
}
if (!needsEscape) {
return s;
}
}

StringBuilder result = new StringBuilder();
result.append('"');
int start = 0;
for (int i = 0; i < s.length(); ++i) {
char c = s.charAt(i);
if (c == '"' || c == '\\') {
// Copy the segment since the last special character.
if (start >= 0) {
result.append(s, start, i);
start = -1;
}

// Handle the current special character.
if (c == '"') {
// This is a quote character. Escape it with a single backslash.
result.append("\\\"");
} 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 runLen = 1;
int j = i + 1;
while (j < s.length() && s.charAt(j) == '\\') {
runLen++;
j++;
}
if (j == s.length()) {
// The run of backslashes goes to the end.
// We have to escape every backslash with another backslash.
for (int k = 0; k < runLen * 2; ++k) {
result.append('\\');
}
break;
} else if (j < s.length() && s.charAt(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 < runLen * 2; ++k) {
result.append('\\');
}
result.append("\\\"");
i += runLen; // '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 < runLen; ++k) {
result.append('\\');
}
i += runLen - 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.append(s, start, s.length());
}
result.append('"');
return result.toString();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -216,9 +216,8 @@ public static int getpid() {

private static native int nativeGetpid();

// TODO(laszlocsomor): Audit this method and fix bugs. This method implements Bash quoting
// semantics but Windows quote semantics are different.
// More info: http://daviddeley.com/autohotkey/parameters/parameters.htm
// TODO(laszlocsomor): Replace this method with ShellUtils.windowsEscapeArg in order to fix
// https://github.com/bazelbuild/bazel/issues/7122
public static String quoteCommandLine(List<String> argv) {
StringBuilder result = new StringBuilder();
for (int iArg = 0; iArg < argv.size(); iArg++) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,4 +134,56 @@ public void testTokenizeFailsOnUnterminatedQuotation() {
assertTokenizeFails("-Dfoo='bar", "unterminated quotation");
assertTokenizeFails("-Dfoo=\"b'ar", "unterminated quotation");
}

private void assertWindowsEscapeArg(String arg, String expected) {
assertThat(ShellUtils.windowsEscapeArg(arg)).isEqualTo(expected);
}

@Test
public void testEscapeCreateProcessArg() {
assertWindowsEscapeArg("", "\"\"");
assertWindowsEscapeArg(" ", "\" \"");
assertWindowsEscapeArg("\"", "\"\\\"\"");
assertWindowsEscapeArg("\"\\", "\"\\\"\\\\\"");
assertWindowsEscapeArg("\\", "\\");
assertWindowsEscapeArg("\\\"", "\"\\\\\\\"\"");
assertWindowsEscapeArg("with space", "\"with space\"");
assertWindowsEscapeArg("with^caret", "with^caret");
assertWindowsEscapeArg("space ^caret", "\"space ^caret\"");
assertWindowsEscapeArg("caret^ space", "\"caret^ space\"");
assertWindowsEscapeArg("with\"quote", "\"with\\\"quote\"");
assertWindowsEscapeArg("with\\backslash", "with\\backslash");
assertWindowsEscapeArg("one\\ backslash and \\space", "\"one\\ backslash and \\space\"");
assertWindowsEscapeArg("two\\\\backslashes", "two\\\\backslashes");
assertWindowsEscapeArg(
"two\\\\ backslashes \\\\and space", "\"two\\\\ backslashes \\\\and space\"");
assertWindowsEscapeArg("one\\\"x", "\"one\\\\\\\"x\"");
assertWindowsEscapeArg("two\\\\\"x", "\"two\\\\\\\\\\\"x\"");
assertWindowsEscapeArg("a \\ b", "\"a \\ b\"");
assertWindowsEscapeArg("a \\\" b", "\"a \\\\\\\" b\"");
assertWindowsEscapeArg("A", "A");
assertWindowsEscapeArg("\"a\"", "\"\\\"a\\\"\"");
assertWindowsEscapeArg("B C", "\"B C\"");
assertWindowsEscapeArg("\"b c\"", "\"\\\"b c\\\"\"");
assertWindowsEscapeArg("D\"E", "\"D\\\"E\"");
assertWindowsEscapeArg("\"d\"e\"", "\"\\\"d\\\"e\\\"\"");
assertWindowsEscapeArg("C:\\F G", "\"C:\\F G\"");
assertWindowsEscapeArg("\"C:\\f g\"", "\"\\\"C:\\f g\\\"\"");
assertWindowsEscapeArg("C:\\H\"I", "\"C:\\H\\\"I\"");
assertWindowsEscapeArg("\"C:\\h\"i\"", "\"\\\"C:\\h\\\"i\\\"\"");
assertWindowsEscapeArg("C:\\J\\\"K", "\"C:\\J\\\\\\\"K\"");
assertWindowsEscapeArg("\"C:\\j\\\"k\"", "\"\\\"C:\\j\\\\\\\"k\\\"\"");
assertWindowsEscapeArg("C:\\L M ", "\"C:\\L M \"");
assertWindowsEscapeArg("\"C:\\l m \"", "\"\\\"C:\\l m \\\"\"");
assertWindowsEscapeArg("C:\\N O\\", "\"C:\\N O\\\\\"");
assertWindowsEscapeArg("\"C:\\n o\\\"", "\"\\\"C:\\n o\\\\\\\"\"");
assertWindowsEscapeArg("C:\\P Q\\ ", "\"C:\\P Q\\ \"");
assertWindowsEscapeArg("\"C:\\p q\\ \"", "\"\\\"C:\\p q\\ \\\"\"");
assertWindowsEscapeArg("C:\\R\\S\\", "C:\\R\\S\\");
assertWindowsEscapeArg("C:\\R x\\S\\", "\"C:\\R x\\S\\\\\"");
assertWindowsEscapeArg("\"C:\\r\\s\\\"", "\"\\\"C:\\r\\s\\\\\\\"\"");
assertWindowsEscapeArg("\"C:\\r x\\s\\\"", "\"\\\"C:\\r x\\s\\\\\\\"\"");
assertWindowsEscapeArg("C:\\T U\\W\\", "\"C:\\T U\\W\\\\\"");
assertWindowsEscapeArg("\"C:\\t u\\w\\\"", "\"\\\"C:\\t u\\w\\\\\\\"\"");
}
}

0 comments on commit 73ac05d

Please sign in to comment.