Uploaded image for project: 'MINA SSHD'
  1. MINA SSHD
  2. SSHD-106

Wrong use of ProcessBuilder.environment() in ProcessShellFactory.java

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: 0.5.0
    • Fix Version/s: 0.6.0
    • Labels:
      None
    • Environment:
      Android 1.6 (Dalvik)

      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.

        Attachments

          Activity

            People

            • Assignee:
              Unassigned
              Reporter:
              stepinto Chao Shi
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved: