Windows, subprocesses: correct flag escaping impl.
Add correct implementation of flag escaping for
subprocesses.
This is a follow-up to https://github.com/bazelbuild/bazel/pull/7413
Next steps:
- replace WindowsProcesses.quoteCommandLine with
the new logic
See https://github.com/bazelbuild/bazel/issues/7122
Closes #7420.
PiperOrigin-RevId: 233900149
diff --git a/src/main/java/com/google/devtools/build/lib/shell/ShellUtils.java b/src/main/java/com/google/devtools/build/lib/shell/ShellUtils.java
index 44006aa..bbad849 100644
--- a/src/main/java/com/google/devtools/build/lib/shell/ShellUtils.java
+++ b/src/main/java/com/google/devtools/build/lib/shell/ShellUtils.java
@@ -142,4 +142,94 @@
}
}
+ /**
+ * 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();
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/windows/jni/WindowsProcesses.java b/src/main/java/com/google/devtools/build/lib/windows/jni/WindowsProcesses.java
index 65b2620..7e00ddb 100644
--- a/src/main/java/com/google/devtools/build/lib/windows/jni/WindowsProcesses.java
+++ b/src/main/java/com/google/devtools/build/lib/windows/jni/WindowsProcesses.java
@@ -216,9 +216,8 @@
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++) {
diff --git a/src/test/java/com/google/devtools/build/lib/shell/ShellUtilsTest.java b/src/test/java/com/google/devtools/build/lib/shell/ShellUtilsTest.java
index 4ebe538..75433e6 100644
--- a/src/test/java/com/google/devtools/build/lib/shell/ShellUtilsTest.java
+++ b/src/test/java/com/google/devtools/build/lib/shell/ShellUtilsTest.java
@@ -134,4 +134,56 @@
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\\\\\\\"\"");
+ }
}
