Skip to content

Commit

Permalink
Windows, subprocesses: add test for arg passing
Browse files Browse the repository at this point in the history
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 #7411
- replace WindowsProcesses.quoteCommandLine with
  the new escaper

See #7122

Closes #7413.

PiperOrigin-RevId: 233730449
  • Loading branch information
laszlocsomor authored and Copybara-Service committed Feb 13, 2019
1 parent 6d90948 commit d5c1eee
Show file tree
Hide file tree
Showing 3 changed files with 94 additions and 7 deletions.
7 changes: 7 additions & 0 deletions src/test/java/com/google/devtools/build/lib/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -536,6 +537,12 @@ java_test(
],
)

cc_binary(
name = "printarg",
testonly = 1,
srcs = ["windows/printarg.cc"],
)

java_library(
name = "actions_testutil",
testonly = 1,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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");
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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.
*
* <p>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.
}
}
20 changes: 20 additions & 0 deletions src/test/java/com/google/devtools/build/lib/windows/printarg.cc
Original file line number Diff line number Diff line change
@@ -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 <stdio.h>

int main(int argc, char** argv) {
printf("(%s)", argv[1]);
return 0;
}

0 comments on commit d5c1eee

Please sign in to comment.