7034570: java.lang.Runtime.exec(String[] cmd, String[] env) can not work properly if SystemRoot not inherited
Reviewed-by: dholmes, alanb
--- a/jdk/src/share/classes/java/lang/ProcessBuilder.java Mon Apr 18 21:44:03 2011 -0700
+++ b/jdk/src/share/classes/java/lang/ProcessBuilder.java Wed Apr 20 12:03:30 2011 +0100
@@ -938,6 +938,11 @@
* but at the very least the command must be a non-empty list of
* non-null strings.
*
+ * <p>A minimal set of system dependent environment variables may
+ * be required to start a process on some operating systems.
+ * As a result, the subprocess may inherit additional environment variable
+ * settings beyond those in the process builder's {@link #environment()}.
+ *
* <p>If there is a security manager, its
* {@link SecurityManager#checkExec checkExec}
* method is called with the first component of this object's
--- a/jdk/src/share/classes/java/lang/Runtime.java Mon Apr 18 21:44:03 2011 -0700
+++ b/jdk/src/share/classes/java/lang/Runtime.java Wed Apr 20 12:03:30 2011 +0100
@@ -544,6 +544,11 @@
* <p>If <tt>envp</tt> is <tt>null</tt>, the subprocess inherits the
* environment settings of the current process.
*
+ * <p>A minimal set of system dependent environment variables may
+ * be required to start a process on some operating systems.
+ * As a result, the subprocess may inherit additional environment variable
+ * settings beyond those in the specified environment.
+ *
* <p>{@link ProcessBuilder#start()} is now the preferred way to
* start a process with a modified environment.
*
--- a/jdk/src/windows/classes/java/lang/ProcessEnvironment.java Mon Apr 18 21:44:03 2011 -0700
+++ b/jdk/src/windows/classes/java/lang/ProcessEnvironment.java Wed Apr 20 12:03:30 2011 +0100
@@ -143,7 +143,7 @@
public void remove() { i.remove();}
};
}
- private static Map.Entry<String,String> checkedEntry (Object o) {
+ private static Map.Entry<String,String> checkedEntry(Object o) {
Map.Entry<String,String> e = (Map.Entry<String,String>) o;
nonNullString(e.getKey());
nonNullString(e.getValue());
@@ -285,7 +285,7 @@
return (Map<String,String>) theEnvironment.clone();
}
- // Only for use by Runtime.exec(...String[]envp...)
+ // Only for use by ProcessBuilder.environment(String[] envp)
static Map<String,String> emptyEnvironment(int capacity) {
return new ProcessEnvironment(capacity);
}
@@ -299,19 +299,46 @@
Collections.sort(list, entryComparator);
StringBuilder sb = new StringBuilder(size()*30);
- for (Map.Entry<String,String> e : list)
- sb.append(e.getKey())
- .append('=')
- .append(e.getValue())
- .append('\u0000');
- // Ensure double NUL termination,
- // even if environment is empty.
- if (sb.length() == 0)
+ int cmp = -1;
+
+ // Some versions of MSVCRT.DLL require SystemRoot to be set.
+ // So, we make sure that it is always set, even if not provided
+ // by the caller.
+ final String SYSTEMROOT = "SystemRoot";
+
+ for (Map.Entry<String,String> e : list) {
+ String key = e.getKey();
+ String value = e.getValue();
+ if (cmp < 0 && (cmp = nameComparator.compare(key, SYSTEMROOT)) > 0) {
+ // Not set, so add it here
+ addToEnvIfSet(sb, SYSTEMROOT);
+ }
+ addToEnv(sb, key, value);
+ }
+ if (cmp < 0) {
+ // Got to end of list and still not found
+ addToEnvIfSet(sb, SYSTEMROOT);
+ }
+ if (sb.length() == 0) {
+ // Environment was empty and SystemRoot not set in parent
sb.append('\u0000');
+ }
+ // Block is double NUL terminated
sb.append('\u0000');
return sb.toString();
}
+ // add the environment variable to the child, if it exists in parent
+ private static void addToEnvIfSet(StringBuilder sb, String name) {
+ String s = getenv(name);
+ if (s != null)
+ addToEnv(sb, name, s);
+ }
+
+ private static void addToEnv(StringBuilder sb, String name, String val) {
+ sb.append(name).append('=').append(val).append('\u0000');
+ }
+
static String toEnvironmentBlock(Map<String,String> map) {
return map == null ? null :
((ProcessEnvironment)map).toEnvironmentBlock();
--- a/jdk/test/java/lang/ProcessBuilder/Basic.java Mon Apr 18 21:44:03 2011 -0700
+++ b/jdk/test/java/lang/ProcessBuilder/Basic.java Wed Apr 20 12:03:30 2011 +0100
@@ -26,7 +26,7 @@
* @bug 4199068 4738465 4937983 4930681 4926230 4931433 4932663 4986689
* 5026830 5023243 5070673 4052517 4811767 6192449 6397034 6413313
* 6464154 6523983 6206031 4960438 6631352 6631966 6850957 6850958
- * 4947220 7018606
+ * 4947220 7018606 7034570
* @summary Basic tests for Process and Environment Variable code
* @run main/othervm/timeout=300 Basic
* @author Martin Buchholz
@@ -1440,11 +1440,12 @@
// Check for sort order of environment variables on Windows.
//----------------------------------------------------------------
try {
+ String systemRoot = "SystemRoot=" + System.getenv("SystemRoot");
// '+' < 'A' < 'Z' < '_' < 'a' < 'z' < '~'
String[]envp = {"FOO=BAR","BAZ=GORP","QUUX=",
- "+=+", "_=_", "~=~"};
+ "+=+", "_=_", "~=~", systemRoot};
String output = nativeEnv(envp);
- String expected = "+=+\nBAZ=GORP\nFOO=BAR\nQUUX=\n_=_\n~=~\n";
+ String expected = "+=+\nBAZ=GORP\nFOO=BAR\nQUUX=\n"+systemRoot+"\n_=_\n~=~\n";
// On Windows, Java must keep the environment sorted.
// Order is random on Unix, so this test does the sort.
if (! Windows.is())
@@ -1453,6 +1454,21 @@
} catch (Throwable t) { unexpected(t); }
//----------------------------------------------------------------
+ // Test Runtime.exec(...envp...)
+ // and check SystemRoot gets set automatically on Windows
+ //----------------------------------------------------------------
+ try {
+ if (Windows.is()) {
+ String systemRoot = "SystemRoot=" + System.getenv("SystemRoot");
+ String[]envp = {"FOO=BAR","BAZ=GORP","QUUX=",
+ "+=+", "_=_", "~=~"};
+ String output = nativeEnv(envp);
+ String expected = "+=+\nBAZ=GORP\nFOO=BAR\nQUUX=\n"+systemRoot+"\n_=_\n~=~\n";
+ equal(output, expected);
+ }
+ } catch (Throwable t) { unexpected(t); }
+
+ //----------------------------------------------------------------
// System.getenv() must be consistent with System.getenv(String)
//----------------------------------------------------------------
try {