java_launcher.cc parsing of JVM flags doesn't use shell tokenization

[nfelt]

The java_binary() rule's jvm_flags argument says that it will be subject to "Bourne shell tokenization" but this doesn't happen on Windows, when apparently java_launcher.cc is used as the launcher instead of the normal stub shell script from java_stub_template.txt.

As a consequence, jvm_flags values that use quoting or backslash-escaping to include values with spaces don't work as expected.

I confirmed this on Windows 10 with bazel 0.21.0. Here's a minimal repro case:

File structure:
/WORKSPACE (empty)
/java/repro/BUILD
/java/repro/Repro.java

contents of BUILD:

java_binary(
  name = "Repro",
  srcs = ["Repro.java",
  jvm_flags = ["-Duser.name='Ada Lovelace'"],
)

contents of Repro.java

package repro;

public class Repro {
  public static void main(String[] args) {
    System.out.printf("Hello %s\n", System.getProperty("user.name"));
  }
}

Command to reproduce: bazel run //java/repro:Repro
Expected output: Hello Ada Lovelace
Actual output: Error: Could not find or load main class Lovelace'

This appears to occur because java_launcher.h splits the jvm_flags argument list up by spaces, without protecting spaces within quotes (and note that it also leaves the quotes behind). It produces a cryptic error because evidently the "extra" jvm_flags argument is just tacked on as an extra "flag" but since it's not formatted like a flag java interprets it as the name of the main class.

bazel/src/tools/launcher/java_launcher.cc

Lines 84 to 89 in c10b108

	} else if (GetFlagValue(argument, L"--jvm_flags=", &flag_value)) {
	wstringstream flag_value_ss(flag_value);
	wstring item;
	while (getline(flag_value_ss, item, L' ')) {
	this->jvm_flags_cmdline.push_back(item);
	}

[laszlocsomor]

Thanks for the bug report! Your culprit analysis is absolutely correct.

The bad news is, on Linux/macOS the quoting support is implemented by simply embedding the argument in a shell script (the Java stub script) and letting Bash interpret it. On Windows we have to implement it ourselves. That means implementing Bash de-qouting to get the same string the java process would (on Linux), and proper escaping for CreateProcessW that runs java.exe.

The good news is, I have a prototype implementation of the de-quoting + escaping code (which needs a lot of testing before I can upstream it) and it seems to work as expected, even with nasty inputs:

    jvm_flags = ["-Duser.name=\"$(COMPILATION_MODE)\\\\\"\ '\\\\\"Ada Lovelace'"],

produces on both Linux and Windows (with my patch):

Hello fastbuild\ \\"Ada Lovelace

[laszlocsomor]

Closing this, because it's fixed at HEAD, see #7486. That bug will be closed when the flag's default value is flipped to true at HEAD.

[nfelt]

Thank you for fixing this!