Implement ProcessBuilder.redirectErrorStream.
Also simplify and correct the security to ensure that the user can't modify
the command to be executed after the SecurityManager has approved it.
Bug: 2180063
diff --git a/libcore/luni-kernel/src/main/java/java/lang/ProcessManager.java b/libcore/luni-kernel/src/main/java/java/lang/ProcessManager.java
index 1e21b57..318fe9a 100644
--- a/libcore/luni-kernel/src/main/java/java/lang/ProcessManager.java
+++ b/libcore/luni-kernel/src/main/java/java/lang/ProcessManager.java
@@ -169,15 +169,45 @@
* Executes a native process. Fills in in, out, and err and returns the
* new process ID upon success.
*/
- static native int exec(String[] commands, String[] environment,
+ static native int exec(String[] command, String[] environment,
String workingDirectory, FileDescriptor in, FileDescriptor out,
- FileDescriptor err) throws IOException;
+ FileDescriptor err, boolean redirectErrorStream) throws IOException;
/**
* Executes a process and returns an object representing it.
*/
- Process exec(String[] commands, String[] environment,
- File workingDirectory) throws IOException {
+ Process exec(String[] taintedCommand, String[] taintedEnvironment, File workingDirectory,
+ boolean redirectErrorStream) throws IOException {
+ // Make sure we throw the same exceptions as the RI.
+ if (taintedCommand == null) {
+ throw new NullPointerException();
+ }
+ if (taintedCommand.length == 0) {
+ throw new IndexOutOfBoundsException();
+ }
+
+ // Handle security and safety by copying mutable inputs and checking them.
+ String[] command = taintedCommand.clone();
+ String[] environment = taintedEnvironment != null ? taintedEnvironment.clone() : null;
+ SecurityManager securityManager = System.getSecurityManager();
+ if (securityManager != null) {
+ securityManager.checkExec(command[0]);
+ }
+ // Check we're not passing null Strings to the native exec.
+ for (String arg : command) {
+ if (arg == null) {
+ throw new NullPointerException();
+ }
+ }
+ // The environment is allowed to be null or empty, but no element may be null.
+ if (environment != null) {
+ for (String env : environment) {
+ if (env == null) {
+ throw new NullPointerException();
+ }
+ }
+ }
+
FileDescriptor in = new FileDescriptor();
FileDescriptor out = new FileDescriptor();
FileDescriptor err = new FileDescriptor();
@@ -191,10 +221,10 @@
synchronized (processReferences) {
int pid;
try {
- pid = exec(commands, environment, workingPath, in, out, err);
+ pid = exec(command, environment, workingPath, in, out, err, redirectErrorStream);
} catch (IOException e) {
IOException wrapper = new IOException("Error running exec()."
- + " Commands: " + Arrays.toString(commands)
+ + " Command: " + Arrays.toString(command)
+ " Working Directory: " + workingDirectory
+ " Environment: " + Arrays.toString(environment));
wrapper.initCause(e);
diff --git a/libcore/luni-kernel/src/main/java/java/lang/Runtime.java b/libcore/luni-kernel/src/main/java/java/lang/Runtime.java
index 28cc96f..8560399 100644
--- a/libcore/luni-kernel/src/main/java/java/lang/Runtime.java
+++ b/libcore/luni-kernel/src/main/java/java/lang/Runtime.java
@@ -190,39 +190,11 @@
* execution.
* @see SecurityManager#checkExec
* @since Android 1.0
- */
- public Process exec(String[] progArray, String[] envp, File directory)
- throws java.io.IOException {
-
- // Sanity checks
- if (progArray == null) {
- throw new NullPointerException();
- } else if (progArray.length == 0) {
- throw new IndexOutOfBoundsException();
- } else {
- for (int i = 0; i < progArray.length; i++) {
- if (progArray[i] == null) {
- throw new NullPointerException();
- }
- }
- }
-
- if (envp != null) {
- for (int i = 0; i < envp.length; i++) {
- if (envp[i] == null) {
- throw new NullPointerException();
- }
- }
- }
-
- // Security checks
- SecurityManager smgr = System.getSecurityManager();
- if (smgr != null) {
- smgr.checkExec(progArray[0]);
- }
-
- // Delegate the execution
- return ProcessManager.getInstance().exec(progArray, envp, directory);
+ */
+ public Process exec(String[] progArray, String[] envp, File directory) throws IOException {
+ // BEGIN android-changed: push responsibility for argument checking into ProcessManager
+ return ProcessManager.getInstance().exec(progArray, envp, directory, false);
+ // END android-changed
}
/**
diff --git a/libcore/luni-kernel/src/main/native/java_lang_ProcessManager.cpp b/libcore/luni-kernel/src/main/native/java_lang_ProcessManager.cpp
index 8aa793c..045d980 100644
--- a/libcore/luni-kernel/src/main/native/java_lang_ProcessManager.cpp
+++ b/libcore/luni-kernel/src/main/native/java_lang_ProcessManager.cpp
@@ -181,7 +181,8 @@
/** Executes a command in a child process. */
static pid_t executeProcess(JNIEnv* env, char** commands, char** environment,
const char* workingDirectory, jobject inDescriptor,
- jobject outDescriptor, jobject errDescriptor) {
+ jobject outDescriptor, jobject errDescriptor,
+ jboolean redirectErrorStream) {
int i, result, error;
// Create 4 pipes: stdin, stdout, stderr, and an exec() status pipe.
@@ -222,7 +223,11 @@
// Replace stdin, out, and err with pipes.
dup2(stdinIn, 0);
dup2(stdoutOut, 1);
- dup2(stderrOut, 2);
+ if (redirectErrorStream) {
+ dup2(stdoutOut, 2);
+ } else {
+ dup2(stderrOut, 2);
+ }
// Close all but statusOut. This saves some work in the next step.
closePipes(pipes, statusOut);
@@ -333,7 +338,8 @@
static pid_t java_lang_ProcessManager_exec(
JNIEnv* env, jclass clazz, jobjectArray javaCommands,
jobjectArray javaEnvironment, jstring javaWorkingDirectory,
- jobject inDescriptor, jobject outDescriptor, jobject errDescriptor) {
+ jobject inDescriptor, jobject outDescriptor, jobject errDescriptor,
+ jboolean redirectErrorStream) {
// Copy commands into char*[].
char** commands = convertStrings(env, javaCommands);
@@ -349,7 +355,7 @@
pid_t result = executeProcess(
env, commands, environment, workingDirectory,
- inDescriptor, outDescriptor, errDescriptor);
+ inDescriptor, outDescriptor, errDescriptor, redirectErrorStream);
// Temporarily clear exception so we can clean up.
jthrowable exception = env->ExceptionOccurred();
@@ -402,15 +408,12 @@
}
static JNINativeMethod methods[] = {
- { "kill", "(I)V", (void*) java_lang_ProcessManager_kill },
- { "watchChildren", "()V", (void*) java_lang_ProcessManager_watchChildren },
- { "exec", "([Ljava/lang/String;[Ljava/lang/String;Ljava/lang/String;"
- "Ljava/io/FileDescriptor;Ljava/io/FileDescriptor;"
- "Ljava/io/FileDescriptor;)I", (void*) java_lang_ProcessManager_exec },
- { "staticInitialize", "()V",
- (void*) java_lang_ProcessManager_staticInitialize },
- { "close", "(Ljava/io/FileDescriptor;)V",
- (void*) java_lang_ProcessManager_close },
+ { "close", "(Ljava/io/FileDescriptor;)V", (void*) java_lang_ProcessManager_close },
+ { "kill", "(I)V", (void*) java_lang_ProcessManager_kill },
+ { "staticInitialize", "()V", (void*) java_lang_ProcessManager_staticInitialize },
+ { "watchChildren", "()V", (void*) java_lang_ProcessManager_watchChildren },
+ { "exec", "([Ljava/lang/String;[Ljava/lang/String;Ljava/lang/String;Ljava/io/FileDescriptor;Ljava/io/FileDescriptor;Ljava/io/FileDescriptor;Z)I",
+ (void*) java_lang_ProcessManager_exec },
};
int register_java_lang_ProcessManager(JNIEnv* env) {
diff --git a/libcore/luni/src/main/java/java/lang/ProcessBuilder.java b/libcore/luni/src/main/java/java/lang/ProcessBuilder.java
index f649fec..245ed9c 100644
--- a/libcore/luni/src/main/java/java/lang/ProcessBuilder.java
+++ b/libcore/luni/src/main/java/java/lang/ProcessBuilder.java
@@ -191,24 +191,15 @@
* if an I/O error happens.
*/
public Process start() throws IOException {
- if (command.isEmpty()) {
- throw new IndexOutOfBoundsException();
- }
- String[] cmdArray = new String[command.size()];
- for (int i = 0; i < cmdArray.length; i++) {
- if ((cmdArray[i] = command.get(i)) == null) {
- throw new NullPointerException();
- }
- }
+ // BEGIN android-changed: push responsibility for argument checking into ProcessManager
+ String[] cmdArray = command.toArray(new String[command.size()]);
String[] envArray = new String[environment.size()];
int i = 0;
for (Map.Entry<String, String> entry : environment.entrySet()) {
envArray[i++] = entry.getKey() + "=" + entry.getValue(); //$NON-NLS-1$
}
- Process process = Runtime.getRuntime().exec(cmdArray, envArray,
- directory);
- // TODO implement support for redirectErrorStream
- return process;
+ return ProcessManager.getInstance().exec(cmdArray, envArray, directory, redirectErrorStream);
+ // END android-changed
}
private static List<String> toList(String[] strings) {
diff --git a/libcore/luni/src/test/java/java/lang/AllTests.java b/libcore/luni/src/test/java/java/lang/AllTests.java
index c37fb27..a40ef3a 100644
--- a/libcore/luni/src/test/java/java/lang/AllTests.java
+++ b/libcore/luni/src/test/java/java/lang/AllTests.java
@@ -23,6 +23,7 @@
public static final Test suite() {
TestSuite suite = tests.TestSuiteFactory.createTestSuite();
suite.addTestSuite(java.lang.FloatTest.class);
+ suite.addTestSuite(java.lang.ProcessBuilderTest.class);
return suite;
}
}
diff --git a/libcore/luni/src/test/java/java/lang/ProcessBuilderTest.java b/libcore/luni/src/test/java/java/lang/ProcessBuilderTest.java
new file mode 100644
index 0000000..53acf01
--- /dev/null
+++ b/libcore/luni/src/test/java/java/lang/ProcessBuilderTest.java
@@ -0,0 +1,46 @@
+/*
+ * Copyright (C) 2009 The Android Open Source Project
+ *
+ * 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.
+ */
+
+package java.lang;
+
+import static tests.support.Support_Exec.execAndCheckOutput;
+
+import junit.framework.Test;
+import junit.framework.TestSuite;
+
+import java.io.InputStream;
+import java.util.Arrays;
+
+public class ProcessBuilderTest extends junit.framework.TestCase {
+ private static String shell() {
+ return "Dalvik".equals(System.getProperty("java.vm.name")) ? "/system/bin/sh" : "/bin/sh";
+ }
+
+ public void testRedirectErrorStream(boolean doRedirect,
+ String expectedOut, String expectedErr) throws Exception {
+ ProcessBuilder pb = new ProcessBuilder(shell(), "-c", "echo out; echo err 1>&2");
+ pb.redirectErrorStream(doRedirect);
+ execAndCheckOutput(pb, expectedOut, expectedErr);
+ }
+
+ public void test_redirectErrorStream_true() throws Exception {
+ testRedirectErrorStream(true, "out\nerr\n", "");
+ }
+
+ public void test_redirectErrorStream_false() throws Exception {
+ testRedirectErrorStream(false, "out\n", "err\n");
+ }
+}
diff --git a/libcore/support/src/test/java/tests/support/Support_Exec.java b/libcore/support/src/test/java/tests/support/Support_Exec.java
index 7b92d0d..2e709a5 100644
--- a/libcore/support/src/test/java/tests/support/Support_Exec.java
+++ b/libcore/support/src/test/java/tests/support/Support_Exec.java
@@ -80,8 +80,7 @@
* <p>This method assumes the target process will complete within ten
* seconds. If it does not, an AssertionFailedError will be thrown.
*/
- public static String execAndGetOutput(ProcessBuilder builder)
- throws IOException {
+ public static String execAndGetOutput(ProcessBuilder builder) throws IOException {
Process process = builder.start();
ExecutorService executorService = Executors.newFixedThreadPool(2);
try {
@@ -115,6 +114,28 @@
}
}
+ /**
+ * Starts the process described by 'builder', and asserts that it sees
+ * 'expectedOut' on stdout and 'expectedErr' on stderr. Times out after
+ * 10s.
+ */
+ public static void execAndCheckOutput(ProcessBuilder builder,
+ String expectedOut, String expectedErr) throws Exception {
+ Process process = builder.start();
+ ExecutorService executorService = Executors.newFixedThreadPool(2);
+ try {
+ Future<String> errFuture =
+ executorService.submit(streamToStringCallable(process.getErrorStream()));
+ Future<String> outFuture =
+ executorService.submit(streamToStringCallable(process.getInputStream()));
+ assertEquals(expectedOut, outFuture.get(10, TimeUnit.SECONDS));
+ assertEquals(expectedErr, errFuture.get(10, TimeUnit.SECONDS));
+ } finally {
+ executorService.shutdown();
+ process.waitFor();
+ }
+ }
+
private static Callable<String> streamToStringCallable(final InputStream in) {
return new Callable<String>() {
public String call() throws Exception {
