You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@mina.apache.org by "Chao Shi (JIRA)" <ji...@apache.org> on 2011/02/11 02:22:57 UTC

[jira] Created: (SSHD-106) Wrong use of ProcessBuilder.environment()

Wrong use of ProcessBuilder.environment()
-----------------------------------------

                 Key: SSHD-106
                 URL: https://issues.apache.org/jira/browse/SSHD-106
             Project: MINA SSHD
          Issue Type: Bug
    Affects Versions: 0.5.0, 0.4.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.


-- 
This message is automatically generated by JIRA.
-
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

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

Posted by "Chao Shi (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/SSHD-106?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12993805#comment-12993805 ] 

Chao Shi commented on SSHD-106:
-------------------------------

Oh yes, you are right. Sorry for my careless.

> 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

        

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

Posted by "Guillaume Nodet (JIRA)" <ji...@apache.org>.
    [ 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

        

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

Posted by "Chao Shi (JIRA)" <ji...@apache.org>.
     [ 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

        

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

Posted by "Guillaume Nodet (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/SSHD-106?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Guillaume Nodet resolved SSHD-106.
----------------------------------

       Resolution: Fixed
    Fix Version/s: 0.6.0

> 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
>             Fix For: 0.6.0
>
>
> 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