[SSHD-106] Wrong use of ProcessBuilder.environment() in ProcessShellFactory.java

• Type: Bug
• Status: Resolved
• Priority: Major
• Resolution: Fixed
• Affects Version/s: 0.5.0
• Fix Version/s: 0.6.0
• Environment: Android 1.6 (Dalvik)
• Assignee: Unassigned
• Reporter: Chao Shi
• Created: 11/Feb/11 01:22
• Updated: 05/Apr/11 11:27
• Resolved: 05/Apr/11 11:27

Description

It seems ProcessBuilder.environment() behaves differently on android and sun's JDK. The latest javadoc says;

The behavior of the returned map is system-dependent. A system may not allow modifications to environment variables or may forbid certain variable names or values. For this reason, attempts to modify the map may fail with UnsupportedOperationException or IllegalArgumentException if the modification is not permitted by the operating system.

http://download.oracle.com/javase/6/docs/api/java/lang/ProcessBuilder.html#environment()

I'm porting sshd to android platform. i got UnsupportedOperationException if I modify the return value of ProcessBuilder.environment(). Instead, it's OK to make a copy of it.

Here's my patch:

Index: sshd-core/src/main/java/org/apache/sshd/server/shell/ProcessShellFactory.java
===================================================================
— sshd-core/src/main/java/org/apache/sshd/server/shell/ProcessShellFactory.java (revision 1069206)
+++ sshd-core/src/main/java/org/apache/sshd/server/shell/ProcessShellFactory.java (working copy)
@@ -24,6 +24,7 @@
import java.io.InputStream;
import java.io.OutputStream;
import java.util.EnumSet;
+import java.util.HashMap;
import java.util.Map;

import org.apache.sshd.common.Factory;
@@ -94,10 +95,12 @@
}
}
ProcessBuilder builder = new ProcessBuilder(cmds);
+ Map<String, String> mergedEnv = new HashMap<String, String>();
+ mergedEnv.putAll(env);
if (env != null)

{ - builder.environment().putAll(env); + mergedEnv.putAll(builder.environment()); }

• LOG.info("Starting shell with command: '{}' and env: {}", builder.command(), builder.environment());
  + LOG.info("Starting shell with command: '{}' and env: {}", builder.command(), mergedEnv);
  process = builder.start();
  out = new TtyFilterInputStream(process.getInputStream());
  err = new TtyFilterInputStream(process.getErrorStream());

It would work for both 0.5.0 and trunk.