diff options
-rw-r--r-- | luni-kernel/src/main/java/java/lang/ProcessManager.java | 42 | ||||
-rw-r--r-- | luni-kernel/src/main/java/java/lang/Runtime.java | 38 | ||||
-rw-r--r-- | luni-kernel/src/main/native/java_lang_ProcessManager.cpp | 29 | ||||
-rw-r--r-- | luni/src/main/java/java/lang/ProcessBuilder.java | 17 | ||||
-rw-r--r-- | luni/src/test/java/java/lang/AllTests.java | 1 | ||||
-rw-r--r-- | luni/src/test/java/java/lang/ProcessBuilderTest.java | 46 | ||||
-rw-r--r-- | support/src/test/java/tests/support/Support_Exec.java | 25 |
7 files changed, 131 insertions, 67 deletions
diff --git a/luni-kernel/src/main/java/java/lang/ProcessManager.java b/luni-kernel/src/main/java/java/lang/ProcessManager.java index 1e21b57..318fe9a 100644 --- a/luni-kernel/src/main/java/java/lang/ProcessManager.java +++ b/luni-kernel/src/main/java/java/lang/ProcessManager.java @@ -169,15 +169,45 @@ final class ProcessManager { * 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 @@ final class ProcessManager { 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/luni-kernel/src/main/java/java/lang/Runtime.java b/luni-kernel/src/main/java/java/lang/Runtime.java index 28cc96f..8560399 100644 --- a/luni-kernel/src/main/java/java/lang/Runtime.java +++ b/luni-kernel/src/main/java/java/lang/Runtime.java @@ -190,39 +190,11 @@ public class Runtime { * 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/luni-kernel/src/main/native/java_lang_ProcessManager.cpp b/luni-kernel/src/main/native/java_lang_ProcessManager.cpp index 8aa793c..045d980 100644 --- a/luni-kernel/src/main/native/java_lang_ProcessManager.cpp +++ b/luni-kernel/src/main/native/java_lang_ProcessManager.cpp @@ -181,7 +181,8 @@ static void closePipes(int pipes[], int skipFd) { /** 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 @@ static pid_t executeProcess(JNIEnv* env, char** commands, char** environment, // 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 void freeStrings(JNIEnv* env, jobjectArray javaArray, char** array) { 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 @@ static pid_t java_lang_ProcessManager_exec( 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 void java_lang_ProcessManager_staticInitialize(JNIEnv* env, } 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/luni/src/main/java/java/lang/ProcessBuilder.java b/luni/src/main/java/java/lang/ProcessBuilder.java index f649fec..245ed9c 100644 --- a/luni/src/main/java/java/lang/ProcessBuilder.java +++ b/luni/src/main/java/java/lang/ProcessBuilder.java @@ -191,24 +191,15 @@ public final class ProcessBuilder { * 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/luni/src/test/java/java/lang/AllTests.java b/luni/src/test/java/java/lang/AllTests.java index c37fb27..a40ef3a 100644 --- a/luni/src/test/java/java/lang/AllTests.java +++ b/luni/src/test/java/java/lang/AllTests.java @@ -23,6 +23,7 @@ public class AllTests { 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/luni/src/test/java/java/lang/ProcessBuilderTest.java b/luni/src/test/java/java/lang/ProcessBuilderTest.java new file mode 100644 index 0000000..53acf01 --- /dev/null +++ b/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/support/src/test/java/tests/support/Support_Exec.java b/support/src/test/java/tests/support/Support_Exec.java index 7b92d0d..2e709a5 100644 --- a/support/src/test/java/tests/support/Support_Exec.java +++ b/support/src/test/java/tests/support/Support_Exec.java @@ -80,8 +80,7 @@ public class Support_Exec extends TestCase { * <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 @@ public class Support_Exec extends TestCase { } } + /** + * 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 { |