[
https://issues.apache.org/jira/browse/SSHD-106?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12993367#comment-12993367
]
Guillaume Nodet commented on SSHD-106:
--------------------------------------
I think the patch is wrong because the env map is overriden by the
builder.environment(), and the mergedEnv isn't used at all.
Maybe a better idea would be simply to catch the exception:
{code}
ProcessBuilder builder = new ProcessBuilder(cmds);
if (env != null) {
try {
builer.environment().putAll(env);
} catch (Exception e) {
LOG.info("Could not set environment for command", e);
}
}
LOG.info("Starting shell with command: '{}' and env: {}", builder.command(),
builder.environment());
{code}
> Wrong use of ProcessBuilder.environment() in ProcessShellFactory.java
> ---------------------------------------------------------------------
>
> Key: SSHD-106
> URL: https://issues.apache.org/jira/browse/SSHD-106
> Project: MINA SSHD
> Issue Type: Bug
> Affects Versions: 0.5.0
> Environment: Android 1.6 (Dalvik)
> Reporter: Chao Shi
>
> 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.
--
This message is automatically generated by JIRA.
-
For more information on JIRA, see: http://www.atlassian.com/software/jira
