Windows, subprocesses: correct flag escaping impl.

src/main/java/com/google/devtools/build/lib/shell/ShellUtils.java

@@ -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();
+  }
 }

src/main/java/com/google/devtools/build/lib/windows/jni/WindowsProcesses.java

@@ -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++) {

src/test/java/com/google/devtools/build/lib/shell/ShellUtilsTest.java

@@ -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\\\\\\\"\"");
+  }
 }