--- old/src/share/classes/java/lang/ProcessBuilder.java Tue Sep 13 17:07:08 2011 +++ new/src/share/classes/java/lang/ProcessBuilder.java Tue Sep 13 17:07:07 2011 @@ -1011,26 +1011,12 @@ // Throws IndexOutOfBoundsException if command is empty String prog = cmdarray[0]; - SecurityManager security = System.getSecurityManager(); - if (security != null) - security.checkExec(prog); - String dir = directory == null ? null : directory.toString(); - try { - return ProcessImpl.start(cmdarray, - environment, - dir, - redirects, - redirectErrorStream); - } catch (IOException e) { - // It's much easier for us to create a high-quality error - // message than the low-level C code which found the problem. - throw new IOException( - "Cannot run program \"" + prog + "\"" - + (dir == null ? "" : " (in directory \"" + dir + "\")") - + ": " + e.getMessage(), - e); - } + return ProcessImpl.start(cmdarray, + environment, + dir, + redirects, + redirectErrorStream); } } --- old/src/windows/classes/java/lang/ProcessImpl.java Tue Sep 13 17:07:11 2011 +++ new/src/windows/classes/java/lang/ProcessImpl.java Tue Sep 13 17:07:10 2011 @@ -37,6 +37,7 @@ import java.lang.ProcessBuilder.Redirect; import java.security.AccessController; import java.security.PrivilegedAction; +import java.util.StringTokenizer; /* This class is for the exclusive use of ProcessBuilder.start() to * create new processes. @@ -155,71 +156,174 @@ final boolean redirectErrorStream) throws IOException { - // Win32 CreateProcess requires cmd[0] to be normalized - cmd[0] = new File(cmd[0]).getPath(); + String prog = ""; - StringBuilder cmdbuf = new StringBuilder(80); - for (int i = 0; i < cmd.length; i++) { - if (i > 0) { - cmdbuf.append(' '); - } - String s = cmd[i]; - if (s.indexOf(' ') >= 0 || s.indexOf('\t') >= 0) { - if (s.charAt(0) != '"') { - cmdbuf.append('"'); - cmdbuf.append(s); - if (s.endsWith("\\")) { - cmdbuf.append("\\"); + try { + // Win32 CreateProcess requires cmd[0] to be normalized + cmd[0] = new File(cmd[0]).getPath(); + + StringBuilder cmdbuf = new StringBuilder(80); + for (int i = 0; i < cmd.length; i++) { + if (i > 0) { + cmdbuf.append(' '); + } + String s = cmd[i]; + if (s.indexOf(' ') >= 0 || s.indexOf('\t') >= 0) { + if (s.charAt(0) != '"') { + cmdbuf.append('"'); + cmdbuf.append(s); + if (s.endsWith("\\")) { + cmdbuf.append("\\"); + } + cmdbuf.append('"'); + } else if (s.endsWith("\"")) { + /* The argument has already been quoted. */ + cmdbuf.append(s); + } else { + /* Unmatched quote for the argument. */ + throw new IllegalArgumentException(); } - cmdbuf.append('"'); - } else if (s.endsWith("\"")) { - /* The argument has already been quoted. */ - cmdbuf.append(s); } else { - /* Unmatched quote for the argument. */ - throw new IllegalArgumentException(); + cmdbuf.append(s); } - } else { - cmdbuf.append(s); } - } - String cmdstr = cmdbuf.toString(); + String cmdstr = cmdbuf.toString(); - handle = create(cmdstr, envblock, path, - stdHandles, redirectErrorStream); - - java.security.AccessController.doPrivileged( - new java.security.PrivilegedAction() { - public Void run() { - if (stdHandles[0] == -1L) - stdin_stream = ProcessBuilder.NullOutputStream.INSTANCE; - else { - FileDescriptor stdin_fd = new FileDescriptor(); - fdAccess.setHandle(stdin_fd, stdHandles[0]); - stdin_stream = new BufferedOutputStream( - new FileOutputStream(stdin_fd)); + if (cmdstr.length() > 0 && cmdstr.charAt(0) == '"') { + // quoted string must contain the prog name + int end = cmdstr.indexOf('"', 1); + if (end == -1 || cmdstr.indexOf('"', end+1) != -1) { + // either unterminated string or has embedded string + throw new IllegalArgumentException("Invalid string"); + } + prog = cmdstr.substring(1, end); + } else { + prog = getProgramPath(cmdstr); } - if (stdHandles[1] == -1L) - stdout_stream = ProcessBuilder.NullInputStream.INSTANCE; - else { - FileDescriptor stdout_fd = new FileDescriptor(); - fdAccess.setHandle(stdout_fd, stdHandles[1]); - stdout_stream = new BufferedInputStream( - new FileInputStream(stdout_fd)); + SecurityManager security = System.getSecurityManager(); + if (security != null) { + try { + security.checkExec(prog); + } catch (SecurityException e) { + // We support invocation of "foo" or "foo.exe" assuming "foo.exe" exists. + // So, the security check should allow the same latitude. + String lprog = prog.toLowerCase(); + if (lprog.endsWith(".exe")) { + // check without the .exe extension + int len = lprog.length(); + String mprog = prog.substring(0, len-4); + security.checkExec(mprog); + } else { + throw e; + } + } } - if (stdHandles[2] == -1L) - stderr_stream = ProcessBuilder.NullInputStream.INSTANCE; - else { - FileDescriptor stderr_fd = new FileDescriptor(); - fdAccess.setHandle(stderr_fd, stdHandles[2]); - stderr_stream = new FileInputStream(stderr_fd); + handle = create(cmdstr, envblock, path, + stdHandles, redirectErrorStream); + + java.security.AccessController.doPrivileged( + new java.security.PrivilegedAction() { + public Void run() { + if (stdHandles[0] == -1L) + stdin_stream = ProcessBuilder.NullOutputStream.INSTANCE; + else { + FileDescriptor stdin_fd = new FileDescriptor(); + fdAccess.setHandle(stdin_fd, stdHandles[0]); + stdin_stream = new BufferedOutputStream( + new FileOutputStream(stdin_fd)); + } + + if (stdHandles[1] == -1L) + stdout_stream = ProcessBuilder.NullInputStream.INSTANCE; + else { + FileDescriptor stdout_fd = new FileDescriptor(); + fdAccess.setHandle(stdout_fd, stdHandles[1]); + stdout_stream = new BufferedInputStream( + new FileInputStream(stdout_fd)); + } + + if (stdHandles[2] == -1L) + stderr_stream = ProcessBuilder.NullInputStream.INSTANCE; + else { + FileDescriptor stderr_fd = new FileDescriptor(); + fdAccess.setHandle(stderr_fd, stdHandles[2]); + stderr_stream = new FileInputStream(stderr_fd); + } + + return null; } + }); + } catch (IOException e) { + throw new IOException ( + "Cannot run program \"" + prog + "\"" + + (path == null ? "" : " (in directory \"" + path + "\")") + + ": " + e.getMessage(), e); + } + } + + /** + * Use the same algorithm as Windows CreateProcess to determine the pathname to + * the given command. Is complicated by the presence of whitespace in the pathname, + * which makes it hard(er) to distinguish between command line parameters and + * whitespace in the command path. We have to check for the existence of files + * to disambiguate. Note, this means when the target does *not* exist, the path + * remains ambiguous, and security checks in particular, will probably fail + * because the whole command string (including args) will be passed to the security + * check. + * + * C:\A B\C D\E F G + * + * We check in the following order (for files existing) + * 1. C:\A.exe + * 2. C:\A + * 3. C:\A B\C.exe + * 4. C:\A B\C + * 5. C:\A B\C D\E.exe + * 6. C:\A B\C D\E (and so on) + * Any token ending in '\\' can't be a valid filename. So it isn't checked. + * If no file is found, then the entire original string is returned. + */ + static String getProgramPath(final String command) { + return java.security.AccessController.doPrivileged( + new java.security.PrivilegedAction() { + public String run() { + StringBuilder sb = new StringBuilder(); + StringTokenizer tokenizer = new StringTokenizer(command, " \t", true); + while (tokenizer.hasMoreTokens()) { + String t = tokenizer.nextToken(); + sb.append(t); + if (t.equals(" ") || t.equals("\t") || t.endsWith("\\")) { + continue; + } + t = sb.toString(); + if (fileHasNoDot(t)) { + // try appending .exe + String t1 = t + ".exe"; + if (exists(t1)) { + return t1; + } + } + if (exists(t)) { + return t; + } + } + return command; } + }); + } - return null; }}); + // final component of path has no '.' in it + private static boolean fileHasNoDot(String name) { + int n = name.lastIndexOf('\\') + 1; + return name.length() > n ? name.indexOf('.', n) == -1 : false; } + private static boolean exists(String name) { + File f = new File (name); + return f.exists(); + } + public OutputStream getOutputStream() { return stdin_stream; } --- old/src/solaris/classes/java/lang/ProcessImpl.java Tue Sep 13 17:07:14 2011 +++ new/src/solaris/classes/java/lang/ProcessImpl.java Tue Sep 13 17:07:12 2011 @@ -65,83 +65,95 @@ throws IOException { assert cmdarray != null && cmdarray.length > 0; + String prog = cmdarray[0]; - // Convert arguments to a contiguous block; it's easier to do - // memory management in Java than in C. - byte[][] args = new byte[cmdarray.length-1][]; - int size = args.length; // For added NUL bytes - for (int i = 0; i < args.length; i++) { - args[i] = cmdarray[i+1].getBytes(); - size += args[i].length; - } - byte[] argBlock = new byte[size]; - int i = 0; - for (byte[] arg : args) { - System.arraycopy(arg, 0, argBlock, i, arg.length); - i += arg.length + 1; - // No need to write NUL bytes explicitly - } + try { + // Convert arguments to a contiguous block; it's easier to do + // memory management in Java than in C. + byte[][] args = new byte[cmdarray.length-1][]; + int size = args.length; // For added NUL bytes + for (int i = 0; i < args.length; i++) { + args[i] = cmdarray[i+1].getBytes(); + size += args[i].length; + } + byte[] argBlock = new byte[size]; + int i = 0; + for (byte[] arg : args) { + System.arraycopy(arg, 0, argBlock, i, arg.length); + i += arg.length + 1; + // No need to write NUL bytes explicitly + } - int[] envc = new int[1]; - byte[] envBlock = ProcessEnvironment.toEnvironmentBlock(environment, envc); + int[] envc = new int[1]; + byte[] envBlock = ProcessEnvironment.toEnvironmentBlock(environment, envc); - int[] std_fds; + int[] std_fds; - FileInputStream f0 = null; - FileOutputStream f1 = null; - FileOutputStream f2 = null; + FileInputStream f0 = null; + FileOutputStream f1 = null; + FileOutputStream f2 = null; - try { - if (redirects == null) { - std_fds = new int[] { -1, -1, -1 }; - } else { - std_fds = new int[3]; + try { + if (redirects == null) { + std_fds = new int[] { -1, -1, -1 }; + } else { + std_fds = new int[3]; - if (redirects[0] == Redirect.PIPE) - std_fds[0] = -1; - else if (redirects[0] == Redirect.INHERIT) - std_fds[0] = 0; - else { - f0 = new FileInputStream(redirects[0].file()); - std_fds[0] = fdAccess.get(f0.getFD()); - } + if (redirects[0] == Redirect.PIPE) + std_fds[0] = -1; + else if (redirects[0] == Redirect.INHERIT) + std_fds[0] = 0; + else { + f0 = new FileInputStream(redirects[0].file()); + std_fds[0] = fdAccess.get(f0.getFD()); + } - if (redirects[1] == Redirect.PIPE) - std_fds[1] = -1; - else if (redirects[1] == Redirect.INHERIT) - std_fds[1] = 1; - else { - f1 = new FileOutputStream(redirects[1].file(), + if (redirects[1] == Redirect.PIPE) + std_fds[1] = -1; + else if (redirects[1] == Redirect.INHERIT) + std_fds[1] = 1; + else { + f1 = new FileOutputStream(redirects[1].file(), redirects[1].append()); - std_fds[1] = fdAccess.get(f1.getFD()); + std_fds[1] = fdAccess.get(f1.getFD()); + } + + if (redirects[2] == Redirect.PIPE) + std_fds[2] = -1; + else if (redirects[2] == Redirect.INHERIT) + std_fds[2] = 2; + else { + f2 = new FileOutputStream(redirects[2].file(), + redirects[2].append()); + std_fds[2] = fdAccess.get(f2.getFD()); + } } - if (redirects[2] == Redirect.PIPE) - std_fds[2] = -1; - else if (redirects[2] == Redirect.INHERIT) - std_fds[2] = 2; - else { - f2 = new FileOutputStream(redirects[2].file(), - redirects[2].append()); - std_fds[2] = fdAccess.get(f2.getFD()); + SecurityManager security = System.getSecurityManager(); + if (security != null) + security.checkExec(prog); + + return new UNIXProcess + (toCString(prog), + argBlock, args.length, + envBlock, envc[0], + toCString(dir), + std_fds, + redirectErrorStream); + } finally { + // In theory, close() can throw IOException + // (although it is rather unlikely to happen here) + try { if (f0 != null) f0.close(); } + finally { + try { if (f1 != null) f1.close(); } + finally { if (f2 != null) f2.close(); } } } - - return new UNIXProcess - (toCString(cmdarray[0]), - argBlock, args.length, - envBlock, envc[0], - toCString(dir), - std_fds, - redirectErrorStream); - } finally { - // In theory, close() can throw IOException - // (although it is rather unlikely to happen here) - try { if (f0 != null) f0.close(); } - finally { - try { if (f1 != null) f1.close(); } - finally { if (f2 != null) f2.close(); } - } + } catch (IOException e) { + throw new IOException ( + "Cannot run program \"" + prog + "\"" + + (dir == null ? "" : " (in directory \"" + dir + "\")") + + ": " + e.getMessage(), e); } } }