From 47e832a95d390abf0e8e3e7a5cc251b99eaa1d06 Mon Sep 17 00:00:00 2001
From: mai93 <messa@google.com>
Date: Tue, 24 Nov 2020 01:26:48 -0800
Subject: [PATCH] Handle escaping command arguments in cmd_bat

This pull request stops arguments escaping for commands run using cmd.exe since the result of `ShellUtils.windowsEscapeArg()` method is not accepted by cmd.exe.

Fixes: #11975

Closes #12530.

PiperOrigin-RevId: 344010635
---
 .../lib/windows/WindowsSubprocessFactory.java | 19 ++++---
 src/test/py/bazel/genrule_test.py             | 50 +++++++++++++++++++
 2 files changed, 62 insertions(+), 7 deletions(-)

diff --git a/src/main/java/com/google/devtools/build/lib/windows/WindowsSubprocessFactory.java b/src/main/java/com/google/devtools/build/lib/windows/WindowsSubprocessFactory.java
index a2b58f13aaadbe..24f94a528a28dc 100644
--- a/src/main/java/com/google/devtools/build/lib/windows/WindowsSubprocessFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/windows/WindowsSubprocessFactory.java
@@ -27,9 +27,7 @@
 import java.util.Map;
 import java.util.TreeMap;
 
-/**
- * A subprocess factory that uses the Win32 API.
- */
+/** A subprocess factory that uses the Win32 API. */
 public class WindowsSubprocessFactory implements SubprocessFactory {
   public static final WindowsSubprocessFactory INSTANCE = new WindowsSubprocessFactory();
 
@@ -39,7 +37,10 @@ public Subprocess create(SubprocessBuilder builder) throws IOException {
 
     // DO NOT quote argv0, createProcess will do it for us.
     String argv0 = processArgv0(argv.get(0));
-    String argvRest = argv.size() > 1 ? escapeArgvRest(argv.subList(1, argv.size())) : "";
+    String argvRest =
+        argv.size() > 1
+            ? escapeArgvRest(argv.subList(1, argv.size()), argv0.equals("cmd.exe"))
+            : "";
     byte[] env = convertEnvToNative(builder.getEnv());
 
     String stdoutPath = getRedirectPath(builder.getStdout(), builder.getStdoutFile());
@@ -68,7 +69,7 @@ public Subprocess create(SubprocessBuilder builder) throws IOException {
         builder.getTimeoutMillis());
   }
 
-  private String escapeArgvRest(List<String> argv) {
+  private static String escapeArgvRest(List<String> argv, boolean isCmd) {
     StringBuilder result = new StringBuilder();
     boolean first = true;
     for (String arg : argv) {
@@ -77,7 +78,11 @@ private String escapeArgvRest(List<String> argv) {
       } else {
         result.append(" ");
       }
-      result.append(ShellUtils.windowsEscapeArg(arg));
+      if (isCmd) {
+        result.append(arg);
+      } else {
+        result.append(ShellUtils.windowsEscapeArg(arg));
+      }
     }
     return result.toString();
   }
@@ -98,7 +103,7 @@ public static String processArgv0(String argv0) {
   private static String getRedirectPath(StreamAction action, File file) {
     switch (action) {
       case DISCARD:
-        return "NUL";  // That's /dev/null on Windows
+        return "NUL"; // That's /dev/null on Windows
 
       case REDIRECT:
         return file.getPath();
diff --git a/src/test/py/bazel/genrule_test.py b/src/test/py/bazel/genrule_test.py
index 5a48b3098f8c32..16b1f61ba80017 100644
--- a/src/test/py/bazel/genrule_test.py
+++ b/src/test/py/bazel/genrule_test.py
@@ -148,6 +148,56 @@ def testCommandFailsEagerlyInPowershell(self):
         'The term \'command_not_exist\' is not recognized as the name of a cmdlet',
         ''.join(stderr))
 
+  def testCopyWithSpacesWithBatch(self):
+    if not self.IsWindows():
+      return
+    self.ScratchFile('WORKSPACE')
+    self.ScratchFile('foo/BUILD', [
+        'genrule(',
+        '  name = "x",',
+        '  srcs = ["hello source"],',
+        '  outs = ["hello copied"],',
+        '  cmd_bat = "copy \\"$<\\" \\"$@\\"",',
+        ')',
+    ])
+    self.ScratchFile('foo/hello source', ['hello world'])
+
+    exit_code, stdout, stderr = self.RunBazel(['info', 'bazel-bin'])
+    self.AssertExitCode(exit_code, 0, stderr)
+    bazel_bin = stdout[0]
+
+    exit_code, _, stderr = self.RunBazel(['build', '//foo:x'])
+    self.AssertExitCode(exit_code, 0, stderr)
+
+    copied = os.path.join(bazel_bin, 'foo', 'hello copied')
+    self.assertTrue(os.path.exists(copied))
+    self.AssertFileContentContains(copied, 'hello world')
+
+  def testCopyWithSpacesWithPowershell(self):
+    if not self.IsWindows():
+      return
+    self.ScratchFile('WORKSPACE')
+    self.ScratchFile('foo/BUILD', [
+        'genrule(',
+        '  name = "x",',
+        '  srcs = ["hello source"],',
+        '  outs = ["hello copied"],',
+        '  cmd_ps = "cp \\"$<\\" \\"$@\\"",',
+        ')',
+    ])
+    self.ScratchFile('foo/hello source', ['hello world'])
+
+    exit_code, stdout, stderr = self.RunBazel(['info', 'bazel-bin'])
+    self.AssertExitCode(exit_code, 0, stderr)
+    bazel_bin = stdout[0]
+
+    exit_code, _, stderr = self.RunBazel(['build', '//foo:x'])
+    self.AssertExitCode(exit_code, 0, stderr)
+
+    copied = os.path.join(bazel_bin, 'foo', 'hello copied')
+    self.assertTrue(os.path.exists(copied))
+    self.AssertFileContentContains(copied, 'hello world')
+
 
 if __name__ == '__main__':
   unittest.main()