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/bazel#7413

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

    See bazelbuild/bazel#7122

    Closes #7420.

    PiperOrigin-RevId: 233900149
  • Loading branch information
Luca Di Grazia committed Sep 4, 2022
1 parent e5bd4b9 commit 3d5eaeb
Show file tree
Hide file tree
Showing 3 changed files with 97 additions and 1 deletion.
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,13 +216,19 @@ public static int getpid() {

private static native int nativeGetpid();

// 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++) {
if (iArg != 0) {
result.append(" ");
}
String arg = argv.get(iArg);
if (arg.isEmpty()) {
result.append("\"\"");
continue;
}
boolean hasSpace = arg.contains(" ");
if (!arg.contains("\"") && !arg.contains("\\") && !hasSpace) {
// fast path. Just append the input string.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ private void assertTokenizeFails(String copts, String expectedError) {
tokenize(new ArrayList<String>(), copts);
fail();
} catch (ShellUtils.TokenizationException e) {
assertThat(e).hasMessageThat().isEqualTo(expectedError);
assertThat(e).hasMessage(expectedError);
}
}

Expand Down

0 comments on commit 3d5eaeb

Please sign in to comment.