Mailing-List: contact dev-help@mina.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@mina.apache.org
Date: Fri, 11 Feb 2011 01:26:57 +0000 (UTC)
From: "Chao Shi (JIRA)" <jira@apache.org>
To: dev@mina.apache.org
Message-ID: 
 <1614445732.9395.1297387617483.JavaMail.tomcat@hel.zones.apache.org>
In-Reply-To: 
 <1493749378.9391.1297387377689.JavaMail.tomcat@hel.zones.apache.org>
Subject: [jira] Updated: (SSHD-106) Wrong use of
 ProcessBuilder.environment() in ProcessShellFactory.java
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit


     [ https://issues.apache.org/jira/browse/SSHD-106?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Chao Shi updated SSHD-106:
--------------------------

          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.

  was:


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.


    Affects Version/s:     (was: 0.4.0)
              Summary: Wrong use of ProcessBuilder.environment() in ProcessShellFactory.java  (was: Wrong use of ProcessBuilder.environment())

> 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