From 8ac46c157c5da63e97a5825316ce6f0c3290e189 Mon Sep 17 00:00:00 2001
From: Laszlo Csomor <laszlocsomor@google.com>
Date: Tue, 26 Feb 2019 04:51:31 -0800
Subject: [PATCH] Windows: add --incompatible_windows_escape_jvm_flags

Add --incompatible_windows_escape_jvm_flags flag
(default: false). This flag has no effect on
platforms other than Windows.

This flag affects how Bazel builds the launcher of
java_binary and java_test targets. (The launcher
is the .exe file that sets up the environment for
the Java program and launches the JVM.)

In particular, the flag controls whether or not
Bazel will Bash-tokenize the jvm_flags that were
declared in the BUILD file, and whether or not
these jvm_flags are escaped by the launcher when
it invokes the JVM.

When the flag is enabled:

- Bazel will Bash-tokenize java_binary.jvm_flags
  and java_test.jvm_flags (as documented by the
  Build Encyclopedia) before embedding them into
  the launcher. The build fails if an entry cannot
  be Bash-tokenized, e.g. if it has unterminated
  quotes.

- The launcher will properly escape these flags
  when it runs the JVM as a subprocess (using
  launcher_util::WindowsEscapeArg2).

- Result: the jvm_flags declared in the BUILD file
  will arrive to the Java program as intended.
  However, due to the extra Bash-tokenization
  step, some ill-formed flags are no longer
  accepted, e.g.  `jvm_flags=["-Dfoo='a"]` now
  results in a build error.

When the flag is disabled:

- Bazel does not Bash-tokenize the jvm_flags
  before embedding them in the launcher. This
  preserves quoting meant to be stripped away, and
  it also means Bazel won't check whether the
  argument is properly quoted.

- The launcher escapes the flags with a Bash-like
  escaping logic (launcher_util::WindowsEscapeArg)
  which cannot properly quote and escape
  everything.

- Result: the jvm_flags declared in the BUILD file
  might get messed up as they are passed to the
  JVM, or the launcher may not even be able to run
  the JVM. However, due to the lack of
  Bash-tokenization, Bazel propagates some flags
  to the Java binary that it would no longer
  accept if the new
  `--incompatible_windows_escape_jvm_flags` were
  enabled, e.g. `jvm_flags=["'a"]` is fine.

Incompatible flag: https://github.com/bazelbuild/bazel/issues/7486

Related: https://github.com/bazelbuild/bazel/issues/7072

RELNOTES[NEW]: Added --incompatible_windows_escape_jvm_flags flag: enables correct java_binary.jvm_flags and java_test.jvm_flags tokenization and escaping on Windows. (No-op on other platforms.)

Change-Id: I531cc63cdfeccbe4b6d48876cb82870c1726a723

Closes #7490.

PiperOrigin-RevId: 235700143
---
 .bazelci/postsubmit.yml                       |   1 +
 .bazelci/presubmit.yml                        |   1 +
 .../java/com/google/devtools/build/lib/BUILD  |   1 +
 .../bazel/rules/java/BazelJavaSemantics.java  |  32 +-
 .../lib/rules/java/JavaConfiguration.java     |   6 +
 .../build/lib/rules/java/JavaOptions.java     |  23 ++
 src/test/shell/integration/BUILD              |   7 +
 .../integration/jvm_flags_escaping_test.sh    | 336 ++++++++++++++++++
 src/tools/launcher/java_launcher.cc           |   8 +-
 9 files changed, 409 insertions(+), 6 deletions(-)
 create mode 100755 src/test/shell/integration/jvm_flags_escaping_test.sh

diff --git a/.bazelci/postsubmit.yml b/.bazelci/postsubmit.yml
index 84bb31f68d6644..680f78158d4f01 100644
--- a/.bazelci/postsubmit.yml
+++ b/.bazelci/postsubmit.yml
@@ -105,6 +105,7 @@ platforms:
     - "-//src/test/shell/integration:discard_analysis_cache_test"
     - "-//src/test/shell/integration:discard_graph_edges_test"
     - "-//src/test/shell/integration:java_integration_test"
+    - "-//src/test/shell/integration:jvm_flags_escaping_test"
     - "-//src/test/shell/integration:minimal_jdk_test"
     - "-//src/test/shell/integration:nonincremental_builds_test"
     - "-//src/test/shell/integration:output_filter_test"
diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml
index 42fa9176961e43..2c9acd4ed5ead1 100644
--- a/.bazelci/presubmit.yml
+++ b/.bazelci/presubmit.yml
@@ -127,6 +127,7 @@ platforms:
     - "-//src/test/shell/integration:discard_analysis_cache_test"
     - "-//src/test/shell/integration:discard_graph_edges_test"
     - "-//src/test/shell/integration:java_integration_test"
+    - "-//src/test/shell/integration:jvm_flags_escaping_test"
     - "-//src/test/shell/integration:minimal_jdk_test"
     - "-//src/test/shell/integration:nonincremental_builds_test"
     - "-//src/test/shell/integration:output_filter_test"
diff --git a/src/main/java/com/google/devtools/build/lib/BUILD b/src/main/java/com/google/devtools/build/lib/BUILD
index 6a92c8b5e107b3..b36522d2917150 100644
--- a/src/main/java/com/google/devtools/build/lib/BUILD
+++ b/src/main/java/com/google/devtools/build/lib/BUILD
@@ -762,6 +762,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/rules/java:java-rules",
         "//src/main/java/com/google/devtools/build/lib/rules/objc",
         "//src/main/java/com/google/devtools/build/lib/rules/platform",
+        "//src/main/java/com/google/devtools/build/lib/shell",
         "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
         "//src/main/java/com/google/devtools/build/lib/skylarkbuildapi/android",
         "//src/main/java/com/google/devtools/build/lib/skylarkbuildapi/apple",
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java
index 399f7afb5c2a3e..d2cf3a15bbdd3a 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java
@@ -66,6 +66,8 @@
 import com.google.devtools.build.lib.rules.java.JavaToolchainProvider;
 import com.google.devtools.build.lib.rules.java.JavaUtil;
 import com.google.devtools.build.lib.rules.java.proto.GeneratedExtensionRegistryProvider;
+import com.google.devtools.build.lib.shell.ShellUtils;
+import com.google.devtools.build.lib.shell.ShellUtils.TokenizationException;
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import com.google.devtools.build.lib.syntax.Type;
 import com.google.devtools.build.lib.util.OS;
@@ -407,13 +409,32 @@ public Artifact createStubAction(
     arguments.add(Substitution.ofSpaceSeparatedList("%jvm_flags%", jvmFlagsList));
 
     if (OS.getCurrent() == OS.WINDOWS) {
+      boolean windowsEscapeJvmFlags =
+          ruleContext
+              .getConfiguration()
+              .getFragment(JavaConfiguration.class)
+              .windowsEscapeJvmFlags();
+
+      List<String> jvmFlagsForLauncher = jvmFlagsList;
+      if (windowsEscapeJvmFlags) {
+        try {
+          jvmFlagsForLauncher = new ArrayList<>(jvmFlagsList.size());
+          for (String f : jvmFlagsList) {
+            ShellUtils.tokenize(jvmFlagsForLauncher, f);
+          }
+        } catch (TokenizationException e) {
+          ruleContext.attributeError("jvm_flags", "could not Bash-tokenize flag: " + e);
+        }
+      }
+
       return createWindowsExeLauncher(
           ruleContext,
           javaExecutable,
           classpath,
           javaStartClass,
-          jvmFlagsList,
-          executable);
+          jvmFlagsForLauncher,
+          executable,
+          windowsEscapeJvmFlags);
     }
 
     ruleContext.registerAction(new TemplateExpansionAction(
@@ -426,9 +447,9 @@ private static Artifact createWindowsExeLauncher(
       String javaExecutable,
       NestedSet<Artifact> classpath,
       String javaStartClass,
-      ImmutableList<String> jvmFlags,
-      Artifact javaLauncher) {
-
+      List<String> jvmFlags,
+      Artifact javaLauncher,
+      boolean windowsEscapeJvmFlags) {
     LaunchInfo launchInfo =
         LaunchInfo.builder()
             .addKeyValuePair("binary_type", "Java")
@@ -448,6 +469,7 @@ private static Artifact createWindowsExeLauncher(
                 "classpath",
                 ";",
                 Iterables.transform(classpath, Artifact.ROOT_RELATIVE_PATH_STRING))
+            .addKeyValuePair("escape_jvmflags", windowsEscapeJvmFlags ? "1" : "0")
             // TODO(laszlocsomor): Change the Launcher to accept multiple jvm_flags entries. As of
             // 2019-02-13 the Launcher accepts just one jvm_flags entry, which contains all the
             // flags, joined by TAB characters. The Launcher splits up the string to get the
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaConfiguration.java
index cc30c415e66e92..550ba51e03ecea 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaConfiguration.java
@@ -178,6 +178,7 @@ public boolean alwaysGenerateOutputMapping() {
   private final ImmutableList<Label> pluginList;
   private final boolean requireJavaToolchainHeaderCompilerDirect;
   private final boolean disallowResourceJars;
+  private final boolean windowsEscapeJvmFlags;
 
   // TODO(dmarting): remove once we have a proper solution for #2539
   private final boolean useLegacyBazelJavaTest;
@@ -215,6 +216,7 @@ public boolean alwaysGenerateOutputMapping() {
     this.isJlplStrictDepsEnforced = javaOptions.isJlplStrictDepsEnforced;
     this.disallowResourceJars = javaOptions.disallowResourceJars;
     this.addTestSupportToCompileTimeDeps = javaOptions.addTestSupportToCompileTimeDeps;
+    this.windowsEscapeJvmFlags = javaOptions.windowsEscapeJvmFlags;
 
     ImmutableList.Builder<Label> translationsBuilder = ImmutableList.builder();
     for (String s : javaOptions.translationTargets) {
@@ -461,4 +463,8 @@ public boolean requireJavaToolchainHeaderCompilerDirect() {
   public boolean disallowResourceJars() {
     return disallowResourceJars;
   }
+
+  public boolean windowsEscapeJvmFlags() {
+    return windowsEscapeJvmFlags;
+  }
 }
diff --git a/src/main/java/com/google/devtools/build/lib/rules/java/JavaOptions.java b/src/main/java/com/google/devtools/build/lib/rules/java/JavaOptions.java
index a50f2f4bf4eb04..83a31a5149a735 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/java/JavaOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/java/JavaOptions.java
@@ -641,6 +641,27 @@ public ImportDepsCheckingLevelConverter() {
           "Disables the resource_jars attribute; use java_import and deps or runtime_deps instead.")
   public boolean disallowResourceJars;
 
+  @Option(
+      name = "incompatible_windows_escape_jvm_flags",
+      defaultValue = "false",
+      documentationCategory = OptionDocumentationCategory.OUTPUT_PARAMETERS,
+      effectTags = {
+        OptionEffectTag.ACTION_COMMAND_LINES,
+        OptionEffectTag.AFFECTS_OUTPUTS,
+      },
+      metadataTags = {
+        OptionMetadataTag.INCOMPATIBLE_CHANGE,
+        OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
+      },
+      help =
+          "On Linux/macOS/non-Windows: no-op. On Windows: this flag affects how java_binary and"
+              + " java_test targets are built; in particular, how the launcher of these targets"
+              + " escapes flags at the time of running the java_binary/java_test. When the flag is"
+              + " true, the launcher escapes the JVM flags using Windows-style escaping (correct"
+              + " behavior). When the flag is false, the launcher uses Bash-style escaping"
+              + " (buggy behavior). See https://github.com/bazelbuild/bazel/issues/7072")
+  public boolean windowsEscapeJvmFlags;
+
   private Label getHostJavaBase() {
     if (hostJavaBase == null) {
       if (useJDK11AsHostJavaBase) {
@@ -715,6 +736,8 @@ public FragmentOptions getHost() {
 
     host.disallowResourceJars = disallowResourceJars;
 
+    host.windowsEscapeJvmFlags = windowsEscapeJvmFlags;
+
     return host;
   }
 
diff --git a/src/test/shell/integration/BUILD b/src/test/shell/integration/BUILD
index 837ecae2bd75f1..60d96b4991a757 100644
--- a/src/test/shell/integration/BUILD
+++ b/src/test/shell/integration/BUILD
@@ -583,6 +583,13 @@ sh_test(
     tags = ["no_windows"],
 )
 
+sh_test(
+    name = "jvm_flags_escaping_test",
+    srcs = ["jvm_flags_escaping_test.sh"],
+    data = [":test-deps"],
+    deps = ["@bazel_tools//tools/bash/runfiles"],
+)
+
 ########################################################################
 # Test suites.
 
diff --git a/src/test/shell/integration/jvm_flags_escaping_test.sh b/src/test/shell/integration/jvm_flags_escaping_test.sh
new file mode 100755
index 00000000000000..9d8041204122da
--- /dev/null
+++ b/src/test/shell/integration/jvm_flags_escaping_test.sh
@@ -0,0 +1,336 @@
+#!/bin/bash
+#
+# Copyright 2019 The Bazel Authors. All rights reserved.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#    http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# --- begin runfiles.bash initialization ---
+# Copy-pasted from Bazel's Bash runfiles library (tools/bash/runfiles/runfiles.bash).
+set -euo pipefail
+if [[ ! -d "${RUNFILES_DIR:-/dev/null}" && ! -f "${RUNFILES_MANIFEST_FILE:-/dev/null}" ]]; then
+  if [[ -f "$0.runfiles_manifest" ]]; then
+    export RUNFILES_MANIFEST_FILE="$0.runfiles_manifest"
+  elif [[ -f "$0.runfiles/MANIFEST" ]]; then
+    export RUNFILES_MANIFEST_FILE="$0.runfiles/MANIFEST"
+  elif [[ -f "$0.runfiles/bazel_tools/tools/bash/runfiles/runfiles.bash" ]]; then
+    export RUNFILES_DIR="$0.runfiles"
+  fi
+fi
+if [[ -f "${RUNFILES_DIR:-/dev/null}/bazel_tools/tools/bash/runfiles/runfiles.bash" ]]; then
+  source "${RUNFILES_DIR}/bazel_tools/tools/bash/runfiles/runfiles.bash"
+elif [[ -f "${RUNFILES_MANIFEST_FILE:-/dev/null}" ]]; then
+  source "$(grep -m1 "^bazel_tools/tools/bash/runfiles/runfiles.bash " \
+            "$RUNFILES_MANIFEST_FILE" | cut -d ' ' -f 2-)"
+else
+  echo >&2 "ERROR: cannot find @bazel_tools//tools/bash/runfiles:runfiles.bash"
+  exit 1
+fi
+# --- end runfiles.bash initialization ---
+
+source "$(rlocation "io_bazel/src/test/shell/integration_test_setup.sh")" \
+  || { echo "integration_test_setup.sh not found!" >&2; exit 1; }
+
+# `uname` returns the current platform, e.g "MSYS_NT-10.0" or "Linux".
+# `tr` converts all upper case letters to lower case.
+# `case` matches the result if the `uname | tr` expression to string prefixes
+# that use the same wildcards as names do in Bash, i.e. "msys*" matches strings
+# starting with "msys", and "*" matches everything (it's the default case).
+case "$(uname -s | tr [:upper:] [:lower:])" in
+msys*)
+  # As of 2019-02-20, Bazel on Windows only supports MSYS Bash.
+  declare -r is_windows=true
+  ;;
+*)
+  declare -r is_windows=false
+  ;;
+esac
+
+if "$is_windows"; then
+  # Disable MSYS path conversion that converts path-looking command arguments to
+  # Windows paths (even if they arguments are not in fact paths).
+  export MSYS_NO_PATHCONV=1
+  export MSYS2_ARG_CONV_EXCL="*"
+  declare -r EXE_EXT=".exe"
+else
+  declare -r EXE_EXT=""
+fi
+
+# ----------------------------------------------------------------------
+# HELPER FUNCTIONS
+# ----------------------------------------------------------------------
+
+# Writes a java file that prints System.getProperty(argN).
+#
+# The program prints every JVM definition of the form argN, where N >= 0, until
+# the first N is found for which argN is empty.
+#
+# Args:
+# $1: directory (package path) where the file will be written
+function create_java_file_that_prints_jvm_args() {
+  local -r pkg="$1"; shift
+  mkdir -p "$pkg" || fail "mkdir -p $pkg"
+  cat >"$pkg/A.java" <<'eof'
+public class A {
+  public static void main(String[] args) {
+    for (int i = 0; ; ++i) {
+      String value = System.getProperty("arg" + i);
+      if (value == null) {
+        break;
+      } else {
+        System.out.printf("arg%d=(%s)%n", i, value);
+      }
+    }
+  }
+}
+eof
+}
+
+# Writes a BUILD file for a java_binary with an untokenizable jvm_flags entry.
+#
+# Args:
+# $1: directory (package path) where the file will be written
+function create_build_file_for_untokenizable_flag() {
+  local -r pkg="$1"; shift
+  mkdir -p "$pkg" || fail "mkdir -p $pkg"
+  cat >"$pkg/BUILD" <<'eof'
+java_binary(
+    name = "cannot_tokenize",
+    srcs = ["A.java"],
+    main_class = "A",
+    jvm_flags = ["-Darg0='abc"],
+)
+eof
+}
+
+# Writes a BUILD file for a java_binary with many different jvm_flags entries.
+#
+# Use this together with assert_output_of_the_program_with_many_jvm_flags().
+#
+# Args:
+# $1: directory (package path) where the file will be written
+function create_build_file_with_many_jvm_flags() {
+  local -r pkg="$1"; shift
+  mkdir -p "$pkg" || fail "mkdir -p $pkg"
+  cat >"$pkg/BUILD" <<'eof'
+java_binary(
+    name = "x",
+    srcs = ["A.java"],
+    main_class = "A",
+    jvm_flags = [
+        "-Darg0=''",
+        "-Darg1=' '",
+        "-Darg2='\"'",
+        "-Darg3='\"\\'",
+        "-Darg4='\\'",
+        "-Darg5='\\\"'",
+        "-Darg6='with space'",
+        "-Darg7='with^caret'",
+        "-Darg8='space ^caret'",
+        "-Darg9='caret^ space'",
+        "-Darg10='with\"quote'",
+        "-Darg11='with\\backslash'",
+        "-Darg12='one\\ backslash and \\space'",
+        "-Darg13='two\\\\backslashes'",
+        "-Darg14='two\\\\ backslashes \\\\and space'",
+        "-Darg15='one\\\"x'",
+        "-Darg16='two\\\\\"x'",
+        "-Darg17='a \\ b'",
+        "-Darg18='a \\\" b'",
+        "-Darg19='A'",
+        "-Darg20='\"a\"'",
+        "-Darg21='B C'",
+        "-Darg22='\"b c\"'",
+        "-Darg23='D\"E'",
+        "-Darg24='\"d\"e\"'",
+        "-Darg25='C:\\F G'",
+        "-Darg26='\"C:\\f g\"'",
+        "-Darg27='C:\\H\"I'",
+        "-Darg28='\"C:\\h\"i\"'",
+        "-Darg29='C:\\J\\\"K'",
+        "-Darg30='\"C:\\j\\\"k\"'",
+        "-Darg31='C:\\L M '",
+        "-Darg32='\"C:\\l m \"'",
+        "-Darg33='C:\\N O\\'",
+        "-Darg34='\"C:\\n o\\\"'",
+        "-Darg35='C:\\P Q\\ '",
+        "-Darg36='\"C:\\p q\\ \"'",
+        "-Darg37='C:\\R\\S\\'",
+        "-Darg38='C:\\R x\\S\\'",
+        "-Darg39='\"C:\\r\\s\\\"'",
+        "-Darg40='\"C:\\r x\\s\\\"'",
+        "-Darg41='C:\\T U\\W\\'",
+        "-Darg42='\"C:\\t u\\w\\\"'",
+    ],
+)
+eof
+}
+
+# Asserts that the $TEST_log contains all JVM definitions of the form argN.
+#
+# See create_build_file_with_many_jvm_flags() and
+# create_java_file_that_prints_jvm_args().
+function assert_output_of_the_program_with_many_jvm_flags() {
+  expect_log 'arg0=()'
+  expect_log 'arg1=( )'
+  expect_log 'arg2=(")'
+  expect_log 'arg3=("\\)'
+  expect_log 'arg4=(\\)'
+  expect_log 'arg5=(\\")'
+  expect_log 'arg6=(with space)'
+  expect_log 'arg7=(with^caret)'
+  expect_log 'arg8=(space ^caret)'
+  expect_log 'arg9=(caret^ space)'
+  expect_log 'arg10=(with"quote)'
+  expect_log 'arg11=(with\\backslash)'
+  expect_log 'arg12=(one\\ backslash and \\space)'
+  expect_log 'arg13=(two\\\\backslashes)'
+  expect_log 'arg14=(two\\\\ backslashes \\\\and space)'
+  expect_log 'arg15=(one\\"x)'
+  expect_log 'arg16=(two\\\\"x)'
+  expect_log 'arg17=(a \\ b)'
+  expect_log 'arg18=(a \\" b)'
+  expect_log 'arg19=(A)'
+  expect_log 'arg20=("a")'
+  expect_log 'arg21=(B C)'
+  expect_log 'arg22=("b c")'
+  expect_log 'arg23=(D"E)'
+  expect_log 'arg24=("d"e")'
+  expect_log 'arg25=(C:\\F G)'
+  expect_log 'arg26=("C:\\f g")'
+  expect_log 'arg27=(C:\\H"I)'
+  expect_log 'arg28=("C:\\h"i")'
+  expect_log 'arg29=(C:\\J\\"K)'
+  expect_log 'arg30=("C:\\j\\"k")'
+  expect_log 'arg31=(C:\\L M )'
+  expect_log 'arg32=("C:\\l m ")'
+  expect_log 'arg33=(C:\\N O\\)'
+  expect_log 'arg34=("C:\\n o\\")'
+  expect_log 'arg35=(C:\\P Q\\ )'
+  expect_log 'arg36=("C:\\p q\\ ")'
+  expect_log 'arg37=(C:\\R\\S\\)'
+  expect_log 'arg38=(C:\\R x\\S\\)'
+  expect_log 'arg39=("C:\\r\\s\\")'
+  expect_log 'arg40=("C:\\r x\\s\\")'
+  expect_log 'arg41=(C:\\T U\\W\\)'
+  expect_log 'arg42=("C:\\t u\\w\\")'
+}
+
+# Runs a program, expecting it to succeed. Redirects all output to $TEST_log.
+#
+# Args:
+# $1: path of the program
+function expect_program_runs() {
+  local -r path="$1"; shift
+  (RUNFILES_DIR= \
+   RUNFILES_MANIFEST_FILE= \
+   RUNFILES_MANIFEST_ONLY= \
+   "$path" >&"$TEST_log" ; ) \
+   || fail "Expected running '$path' succeed but failed with exit code $?"
+}
+
+# Runs a program, expecting it to fail. Redirects all output to $TEST_log.
+#
+# Args:
+# $1: path of the program
+function expect_program_cannot_run() {
+  local -r path="$1"; shift
+  (RUNFILES_DIR= \
+   RUNFILES_MANIFEST_FILE= \
+   RUNFILES_MANIFEST_ONLY= \
+   "$path" >&"$TEST_log" ; ) \
+   && fail "Expected running '$path' to fail but succeeded" || true
+}
+
+# ----------------------------------------------------------------------
+# TESTS
+# ----------------------------------------------------------------------
+
+function test_jvm_flags_escaping_when_it_is_disabled_on_windows() {
+  local -r pkg="${FUNCNAME[0]}"  # unique package name for this test
+
+  create_java_file_that_prints_jvm_args "$pkg"
+  create_build_file_with_many_jvm_flags "$pkg"
+
+  # On all platforms, Bazel can build the target.
+  bazel build --verbose_failures --noincompatible_windows_escape_jvm_flags \
+    "${pkg}:x" &>"$TEST_log" || fail "expected success"
+  if "$is_windows"; then
+    # On Windows, the target cannot run.
+    expect_program_cannot_run "bazel-bin/$pkg/x${EXE_EXT}"
+    expect_log "Could not find or load main class"
+  else
+    # On other platforms, the program can run fine.
+    expect_program_runs "bazel-bin/$pkg/x${EXE_EXT}"
+    assert_output_of_the_program_with_many_jvm_flags
+  fi
+}
+
+function test_jvm_flags_escaping() {
+  local -r pkg="${FUNCNAME[0]}"  # unique package name for this test
+
+  create_java_file_that_prints_jvm_args "$pkg"
+  create_build_file_with_many_jvm_flags "$pkg"
+
+  # On all platforms, Bazel can build and run the target.
+  bazel build --verbose_failures --incompatible_windows_escape_jvm_flags \
+    "${pkg}:x" &>"$TEST_log" || fail "expected success"
+  expect_program_runs "bazel-bin/$pkg/x${EXE_EXT}"
+  assert_output_of_the_program_with_many_jvm_flags
+}
+
+function test_untokenizable_jvm_flag_when_escaping_is_disabled() {
+  local -r pkg="${FUNCNAME[0]}"  # unique package name for this test
+
+  create_java_file_that_prints_jvm_args "$pkg"
+  create_build_file_for_untokenizable_flag "$pkg"
+
+  # On all platforms, Bazel can build the target.
+  bazel build --verbose_failures --noincompatible_windows_escape_jvm_flags \
+    "${pkg}:cannot_tokenize" 2>"$TEST_log" || fail "expected success"
+  if "$is_windows"; then
+    # On Windows, Bazel will not check the flag. It will just propagate the flag
+    # to the launcher, which also just passes it to the JVM.  This is bad, the
+    # flag should have been rejected because it cannot be Bash-tokenized.
+    expect_program_runs "bazel-bin/$pkg/cannot_tokenize${EXE_EXT}"
+    expect_log "arg0=('abc)"
+  else
+    # On other platforms, Bazel will build the target but it fails to run.
+    expect_program_cannot_run "bazel-bin/$pkg/cannot_tokenize${EXE_EXT}"
+    expect_log "syntax error"
+  fi
+}
+
+function test_untokenizable_jvm_flag_when_escaping_is_enabled() {
+  local -r pkg="${FUNCNAME[0]}"  # unique package name for this test
+
+  create_java_file_that_prints_jvm_args "$pkg"
+  create_build_file_for_untokenizable_flag "$pkg"
+
+  local -r flag="--incompatible_windows_escape_jvm_flags"
+  if "$is_windows"; then
+    # On Windows, Bazel will check the flag. It will just propagate the flag
+    # to the launcher, which also just passes the flag to the JVM. This is bad,
+    # the flag should have been rejected because it cannot be Bash-tokenized.
+    bazel build --verbose_failures "$flag" "${pkg}:cannot_tokenize" \
+      2>"$TEST_log" && fail "expected failure" || true
+    expect_log "ERROR:.*in jvm_flags attribute of java_binary rule"
+  else
+    # On other platforms, Bazel will build the target but it fails to run.
+    bazel build --verbose_failures "$flag" "${pkg}:cannot_tokenize" \
+      2>"$TEST_log" || fail "expected success"
+    expect_program_cannot_run "bazel-bin/$pkg/cannot_tokenize${EXE_EXT}"
+    expect_log "syntax error"
+  fi
+}
+
+run_suite "Tests about how Bazel passes java_binary.jvm_flags to the binary"
diff --git a/src/tools/launcher/java_launcher.cc b/src/tools/launcher/java_launcher.cc
index d653dfe298bc51..6e99cca4f813dd 100644
--- a/src/tools/launcher/java_launcher.cc
+++ b/src/tools/launcher/java_launcher.cc
@@ -43,6 +43,7 @@ static constexpr const char* JAR_BIN_PATH = "jar_bin_path";
 static constexpr const char* CLASSPATH = "classpath";
 static constexpr const char* JAVA_START_CLASS = "java_start_class";
 static constexpr const char* JVM_FLAGS = "jvm_flags";
+static constexpr const char* WINDOWS_STYLE_ESCAPE_JVM_FLAGS = "escape_jvmflags";
 
 // Check if a string start with a certain prefix.
 // If it's true, store the substring without the prefix in value.
@@ -408,10 +409,15 @@ ExitCode JavaBinaryLauncher::Launch() {
     arguments.push_back(arg);
   }
 
+  wstring (*const escape_arg_func)(const wstring&) =
+      this->GetLaunchInfoByKey(WINDOWS_STYLE_ESCAPE_JVM_FLAGS) == L"1"
+          ? WindowsEscapeArg2
+          : WindowsEscapeArg;
+
   vector<wstring> escaped_arguments;
   // Quote the arguments if having spaces
   for (const auto& arg : arguments) {
-    escaped_arguments.push_back(WindowsEscapeArg(arg));
+    escaped_arguments.push_back(escape_arg_func(arg));
   }
 
   ExitCode exit_code = this->LaunchProcess(java_bin, escaped_arguments);