From d5c1eee09959a4b820b5f10054419417c80181d3 Mon Sep 17 00:00:00 2001 From: Laszlo Csomor Date: Wed, 13 Feb 2019 04:52:45 -0800 Subject: [PATCH] Windows, subprocesses: add test for arg passing Add an integration test to assert that subprocesses created with WindowsSubprocessFactory receive arguments as intended. Next steps: - implement the same argument escaping logic in Bazel as windowsEscapeArg2 in https://github.com/bazelbuild/bazel/pull/7411 - replace WindowsProcesses.quoteCommandLine with the new escaper See https://github.com/bazelbuild/bazel/issues/7122 Closes #7413. PiperOrigin-RevId: 233730449 --- .../java/com/google/devtools/build/lib/BUILD | 7 ++ .../lib/windows/WindowsSubprocessTest.java | 74 +++++++++++++++++-- .../devtools/build/lib/windows/printarg.cc | 20 +++++ 3 files changed, 94 insertions(+), 7 deletions(-) create mode 100644 src/test/java/com/google/devtools/build/lib/windows/printarg.cc diff --git a/src/test/java/com/google/devtools/build/lib/BUILD b/src/test/java/com/google/devtools/build/lib/BUILD index 4480bf94663fe9..58c28c99045feb 100644 --- a/src/test/java/com/google/devtools/build/lib/BUILD +++ b/src/test/java/com/google/devtools/build/lib/BUILD @@ -514,6 +514,7 @@ java_test( srcs = WINDOWS_ON_WINDOWS_TESTS, data = [ ":MockSubprocess_deploy.jar", + ":printarg", ] + JNI_LIB, jvm_flags = [ "-Dbazel.windows_unix_root=C:/fake/msys", @@ -536,6 +537,12 @@ java_test( ], ) +cc_binary( + name = "printarg", + testonly = 1, + srcs = ["windows/printarg.cc"], +) + java_library( name = "actions_testutil", testonly = 1, diff --git a/src/test/java/com/google/devtools/build/lib/windows/WindowsSubprocessTest.java b/src/test/java/com/google/devtools/build/lib/windows/WindowsSubprocessTest.java index 4e4e1207bf0a36..4fb09688ef1992 100644 --- a/src/test/java/com/google/devtools/build/lib/windows/WindowsSubprocessTest.java +++ b/src/test/java/com/google/devtools/build/lib/windows/WindowsSubprocessTest.java @@ -15,15 +15,17 @@ package com.google.devtools.build.lib.windows; import static com.google.common.truth.Truth.assertThat; +import static java.nio.charset.StandardCharsets.UTF_8; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.shell.Subprocess; import com.google.devtools.build.lib.shell.SubprocessBuilder; import com.google.devtools.build.lib.testutil.TestSpec; import com.google.devtools.build.lib.util.OS; +import com.google.devtools.build.lib.windows.jni.WindowsProcesses; import com.google.devtools.build.runfiles.Runfiles; import java.io.File; -import java.nio.charset.Charset; import org.junit.After; import org.junit.Before; import org.junit.Test; @@ -36,14 +38,14 @@ @RunWith(JUnit4.class) @TestSpec(localOnly = true, supportedOs = OS.WINDOWS) public class WindowsSubprocessTest { - private static final Charset UTF8 = Charset.forName("UTF-8"); private String mockSubprocess; private String mockBinary; private Subprocess process; + private Runfiles runfiles; @Before public void loadJni() throws Exception { - Runfiles runfiles = Runfiles.create(); + runfiles = Runfiles.create(); mockSubprocess = runfiles.rlocation( "io_bazel/src/test/java/com/google/devtools/build/lib/MockSubprocess_deploy.jar"); @@ -73,7 +75,7 @@ public void testSystemRootIsSetByDefault() throws Exception { byte[] buf = new byte[11]; process.getInputStream().read(buf); - assertThat(new String(buf, UTF8).trim()).isEqualTo(System.getenv("SYSTEMROOT").trim()); + assertThat(new String(buf, UTF_8).trim()).isEqualTo(System.getenv("SYSTEMROOT").trim()); } @Test @@ -88,7 +90,7 @@ public void testSystemDriveIsSetByDefault() throws Exception { byte[] buf = new byte[3]; process.getInputStream().read(buf); - assertThat(new String(buf, UTF8).trim()).isEqualTo(System.getenv("SYSTEMDRIVE").trim()); + assertThat(new String(buf, UTF_8).trim()).isEqualTo(System.getenv("SYSTEMDRIVE").trim()); } @Test @@ -105,7 +107,7 @@ public void testSystemRootIsSet() throws Exception { byte[] buf = new byte[16]; process.getInputStream().read(buf); - assertThat(new String(buf, UTF8).trim()).isEqualTo("C:\\MySystemRoot"); + assertThat(new String(buf, UTF_8).trim()).isEqualTo("C:\\MySystemRoot"); } @Test @@ -122,6 +124,64 @@ public void testSystemDriveIsSet() throws Exception { byte[] buf = new byte[3]; process.getInputStream().read(buf); - assertThat(new String(buf, UTF8).trim()).isEqualTo("X:"); + assertThat(new String(buf, UTF_8).trim()).isEqualTo("X:"); + } + + /** + * An argument and its command-line-escaped counterpart. + * + *

Such escaping ensures that Bazel correctly forwards arguments to subprocesses. + */ + private static final class ArgPair { + public final String original; + public final String escaped; + + public ArgPair(String original, String escaped) { + this.original = original; + this.escaped = escaped; + } + }; + + /** Asserts that a subprocess correctly receives command line arguments. */ + private void assertSubprocessReceivesArgsAsIntended(ArgPair... args) throws Exception { + // Look up the path of the printarg.exe utility. + String printArgExe = + runfiles.rlocation("io_bazel/src/test/java/com/google/devtools/build/lib/printarg.exe"); + assertThat(printArgExe).isNotEmpty(); + + for (ArgPair arg : args) { + // Assert that the command-line encoding logic works as intended. + assertThat(WindowsProcesses.quoteCommandLine(ImmutableList.of(arg.original))) + .isEqualTo(arg.escaped); + + // Create a separate subprocess just for this argument. + SubprocessBuilder subprocessBuilder = new SubprocessBuilder(); + subprocessBuilder.setWorkingDirectory(new File(".")); + subprocessBuilder.setSubprocessFactory(WindowsSubprocessFactory.INSTANCE); + subprocessBuilder.setArgv(printArgExe, arg.original); + process = subprocessBuilder.start(); + process.waitFor(); + assertThat(process.exitValue()).isEqualTo(0); + + // The subprocess printed its argv[1] in parentheses, e.g. (foo). + // Assert that it printed exactly the *original* argument in parentheses. + byte[] buf = new byte[1000]; + process.getInputStream().read(buf); + String actual = new String(buf, UTF_8).trim(); + assertThat(actual).isEqualTo("(" + arg.original + ")"); + } + } + + @Test + public void testSubprocessReceivesArgsAsIntended() throws Exception { + assertSubprocessReceivesArgsAsIntended( + new ArgPair("", "\"\""), + new ArgPair(" ", "\" \""), + new ArgPair("foo", "foo"), + new ArgPair("foo\\bar", "foo\\bar"), + new ArgPair("foo bar", "\"foo bar\"")); + // TODO(laszlocsomor): the escaping logic in WindowsProcesses.quoteCommandLine is wrong, because + // it fails to properly escape things like a single backslash followed by a quote, e.g. a\"b + // Fix the escaping logic and add more test here. } } diff --git a/src/test/java/com/google/devtools/build/lib/windows/printarg.cc b/src/test/java/com/google/devtools/build/lib/windows/printarg.cc new file mode 100644 index 00000000000000..9ecd6582f61c89 --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/windows/printarg.cc @@ -0,0 +1,20 @@ +// 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. + +#include + +int main(int argc, char** argv) { + printf("(%s)", argv[1]); + return 0; +}