You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mina.apache.org by lg...@apache.org on 2015/07/07 14:57:54 UTC
mina-sshd git commit: [SSHD-532] Environment variables not set
Repository: mina-sshd
Updated Branches:
refs/heads/master 4818061d8 -> 5cd057a72
[SSHD-532] Environment variables not set
Project: http://git-wip-us.apache.org/repos/asf/mina-sshd/repo
Commit: http://git-wip-us.apache.org/repos/asf/mina-sshd/commit/5cd057a7
Tree: http://git-wip-us.apache.org/repos/asf/mina-sshd/tree/5cd057a7
Diff: http://git-wip-us.apache.org/repos/asf/mina-sshd/diff/5cd057a7
Branch: refs/heads/master
Commit: 5cd057a72e0ff551cb1ff5c1b1a877664495184c
Parents: 4818061
Author: Lyor Goldstein <lg...@vmware.com>
Authored: Tue Jul 7 15:57:44 2015 +0300
Committer: Lyor Goldstein <lg...@vmware.com>
Committed: Tue Jul 7 15:57:44 2015 +0300
----------------------------------------------------------------------
.../apache/sshd/client/channel/ChannelExec.java | 7 +-
.../channel/PtyCapableChannelSession.java | 93 +++++++++++----
.../server/channel/AbstractServerChannel.java | 3 +-
.../sshd/server/channel/ChannelSession.java | 16 ++-
.../sshd/server/shell/ProcessShellFactory.java | 14 ++-
.../java/org/apache/sshd/server/ServerTest.java | 115 ++++++++++++++++++-
.../org/apache/sshd/util/EchoShellFactory.java | 5 +
7 files changed, 212 insertions(+), 41 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/5cd057a7/sshd-core/src/main/java/org/apache/sshd/client/channel/ChannelExec.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/client/channel/ChannelExec.java b/sshd-core/src/main/java/org/apache/sshd/client/channel/ChannelExec.java
index 125cdbb..1671962 100644
--- a/sshd-core/src/main/java/org/apache/sshd/client/channel/ChannelExec.java
+++ b/sshd-core/src/main/java/org/apache/sshd/client/channel/ChannelExec.java
@@ -21,6 +21,8 @@ package org.apache.sshd.client.channel;
import java.io.IOException;
import org.apache.sshd.common.SshConstants;
+import org.apache.sshd.common.util.GenericUtils;
+import org.apache.sshd.common.util.ValidateUtils;
import org.apache.sshd.common.util.buffer.Buffer;
/**
@@ -34,10 +36,7 @@ public class ChannelExec extends PtyCapableChannelSession {
public ChannelExec(String command) {
super(false);
- if (command == null) {
- throw new IllegalArgumentException("command must not be null");
- }
- this.command = command;
+ this.command = ValidateUtils.checkNotNullAndNotEmpty(command, "Command may not be null/empty", GenericUtils.EMPTY_OBJECT_ARRAY);
}
@Override
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/5cd057a7/sshd-core/src/main/java/org/apache/sshd/client/channel/PtyCapableChannelSession.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/client/channel/PtyCapableChannelSession.java b/sshd-core/src/main/java/org/apache/sshd/client/channel/PtyCapableChannelSession.java
index 2c2b50b..8c9b3a4 100644
--- a/sshd-core/src/main/java/org/apache/sshd/client/channel/PtyCapableChannelSession.java
+++ b/sshd-core/src/main/java/org/apache/sshd/client/channel/PtyCapableChannelSession.java
@@ -19,52 +19,97 @@
package org.apache.sshd.client.channel;
import java.io.IOException;
-import java.util.HashMap;
+import java.util.Collections;
+import java.util.EnumMap;
import java.util.LinkedHashMap;
import java.util.Map;
+import java.util.concurrent.TimeUnit;
+import org.apache.sshd.client.SshClient;
+import org.apache.sshd.client.session.ClientSession;
import org.apache.sshd.common.SshConstants;
import org.apache.sshd.common.channel.PtyMode;
import org.apache.sshd.common.channel.SttySupport;
+import org.apache.sshd.common.util.GenericUtils;
import org.apache.sshd.common.util.OsUtils;
import org.apache.sshd.common.util.buffer.Buffer;
import org.apache.sshd.common.util.buffer.ByteArrayBuffer;
/**
- * TODO Add javadoc
+ * <P>Serves as the base channel session for executing remote commands - including
+ * a full shell. <B>Note:</B> all the configuration changes via the various
+ * {@code setXXX} methods must be made <U>before</U> the channel is actually
+ * open. If they are invoked afterwards then they have no effect (silently
+ * ignored).</P>
+ * <P>A typical code snippet would be:</P>
+ * <CODE><PRE>
+ * client = SshClient.setUpDefaultClient();
+ * client.start();
*
+ * try(ClientSession s = client.connect(getCurrentTestName(), "localhost", port).verify(7L, TimeUnit.SECONDS).getSession()) {
+ * s.addPasswordIdentity(getCurrentTestName());
+ * s.auth().verify(5L, TimeUnit.SECONDS);
+ *
+ * try(ChannelExec shell = s.createExecChannel("my super duper command")) {
+ * shell.setEnv("var1", "val1");
+ * shell.setEnv("var2", "val2");
+ * ...etc...
+ * shell.setPtyType(...);
+ * shell.setPtyLines(...);
+ * ...etc...
+ *
+ * shell.open().verify(5L, TimeUnit.SECONDS);
+ * shell.waitFor(ClientChannel.CLOSED, TimeUnit.SECONDS.toMillis(17L)); // can use zero for infinite wait
+ *
+ * Integer status = shell.getExitStatus();
+ * if (status.intValue() != 0) {
+ * ...error...
+ * }
+ * }
+ * } finally {
+ * client.stop();
+ * }
+ * </PRE></CODE>
* @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
*/
public class PtyCapableChannelSession extends ChannelSession {
+ public static final int DEFAULT_COLUMNS_COUNT = 80;
+ public static final int DEFAULT_ROWS_COUNT = 24;
+ public static final int DEFAULT_WIDTH = 640;
+ public static final int DEFAULT_HEIGHT = 480;
+ public static final Map<PtyMode, Integer> DEFAULT_PTY_MODES =
+ Collections.unmodifiableMap(new EnumMap<PtyMode, Integer>(PtyMode.class) {
+ private static final long serialVersionUID = 1L; // we're not serializing it
+
+ {
+ put(PtyMode.ISIG, Integer.valueOf(1));
+ put(PtyMode.ICANON, Integer.valueOf(1));
+ put(PtyMode.ECHO, Integer.valueOf(1));
+ put(PtyMode.ECHOE, Integer.valueOf(1));
+ put(PtyMode.ECHOK, Integer.valueOf(1));
+ put(PtyMode.ECHONL, Integer.valueOf(0));
+ put(PtyMode.NOFLSH, Integer.valueOf(0));
+ }
+ });
+
private boolean agentForwarding;
private boolean usePty;
private String ptyType;
- private int ptyColumns;
- private int ptyLines;
- private int ptyWidth;
- private int ptyHeight;
- private Map<PtyMode, Integer> ptyModes;
- private Map<String, String> env = new LinkedHashMap<String, String>();
+ private int ptyColumns = DEFAULT_COLUMNS_COUNT;
+ private int ptyLines = DEFAULT_ROWS_COUNT;
+ private int ptyWidth = DEFAULT_WIDTH;
+ private int ptyHeight = DEFAULT_HEIGHT;
+ private Map<PtyMode, Integer> ptyModes = new EnumMap<PtyMode, Integer>(PtyMode.class);
+ private final Map<String, String> env = new LinkedHashMap<String, String>();
public PtyCapableChannelSession(boolean usePty) {
this.usePty = usePty;
ptyType = System.getenv("TERM");
- if (ptyType == null) {
+ if (GenericUtils.isEmpty(ptyType)) {
ptyType = "dummy";
}
- ptyColumns = 80;
- ptyLines = 24;
- ptyWidth = 640;
- ptyHeight = 480;
- // Set up default pty modes
- ptyModes = new HashMap<PtyMode, Integer>();
- ptyModes.put(PtyMode.ISIG, Integer.valueOf(1));
- ptyModes.put(PtyMode.ICANON, Integer.valueOf(1));
- ptyModes.put(PtyMode.ECHO, Integer.valueOf(1));
- ptyModes.put(PtyMode.ECHOE, Integer.valueOf(1));
- ptyModes.put(PtyMode.ECHOK, Integer.valueOf(1));
- ptyModes.put(PtyMode.ECHONL, Integer.valueOf(0));
- ptyModes.put(PtyMode.NOFLSH, Integer.valueOf(0));
+
+ ptyModes.putAll(DEFAULT_PTY_MODES);
}
public void setupSensibleDefaultPty() {
@@ -142,7 +187,7 @@ public class PtyCapableChannelSession extends ChannelSession {
}
public void setPtyModes(Map<PtyMode, Integer> ptyModes) {
- this.ptyModes = ptyModes;
+ this.ptyModes = (ptyModes == null) ? Collections.<PtyMode, Integer>emptyMap() : ptyModes;
}
public void setEnv(String key, String value) {
@@ -192,7 +237,7 @@ public class PtyCapableChannelSession extends ChannelSession {
writePacket(buffer);
}
- if (!env.isEmpty()) {
+ if (GenericUtils.size(env) > 0) {
log.debug("Send SSH_MSG_CHANNEL_REQUEST env: {}", env);
for (Map.Entry<String, String> entry : env.entrySet()) {
buffer = session.createBuffer(SshConstants.SSH_MSG_CHANNEL_REQUEST);
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/5cd057a7/sshd-core/src/main/java/org/apache/sshd/server/channel/AbstractServerChannel.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/server/channel/AbstractServerChannel.java b/sshd-core/src/main/java/org/apache/sshd/server/channel/AbstractServerChannel.java
index 8fc39ec..218e76a 100644
--- a/sshd-core/src/main/java/org/apache/sshd/server/channel/AbstractServerChannel.java
+++ b/sshd-core/src/main/java/org/apache/sshd/server/channel/AbstractServerChannel.java
@@ -73,10 +73,11 @@ public abstract class AbstractServerChannel extends AbstractChannel {
if (log.isDebugEnabled()) {
log.debug("Send SSH_MSG_CHANNEL_REQUEST exit-status on channel {}", Integer.valueOf(id));
}
+
Buffer buffer = session.createBuffer(SshConstants.SSH_MSG_CHANNEL_REQUEST);
buffer.putInt(recipient);
buffer.putString("exit-status");
- buffer.putByte((byte) 0);
+ buffer.putBoolean(false); // want-reply - must be FALSE - see https://tools.ietf.org/html/rfc4254 section 6.10
buffer.putInt(v);
writePacket(buffer);
notifyStateChanged();
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/5cd057a7/sshd-core/src/main/java/org/apache/sshd/server/channel/ChannelSession.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/server/channel/ChannelSession.java b/sshd-core/src/main/java/org/apache/sshd/server/channel/ChannelSession.java
index 04de401..691c67e 100644
--- a/sshd-core/src/main/java/org/apache/sshd/server/channel/ChannelSession.java
+++ b/sshd-core/src/main/java/org/apache/sshd/server/channel/ChannelSession.java
@@ -586,19 +586,23 @@ public class ChannelSession extends AbstractServerChannel {
doWriteData(buffer.array(), buffer.rpos(), buffer.available());
}
command.setExitCallback(new ExitCallback() {
- @SuppressWarnings("synthetic-access")
@Override
public void onExit(int exitValue) {
+ onExit(exitValue, "");
+ }
+
+ @Override
+ @SuppressWarnings("synthetic-access")
+ public void onExit(int exitValue, String exitMessage) {
try {
closeShell(exitValue);
+ if (log.isDebugEnabled()) {
+ log.debug("onExit(" + exitValue + ")[" + exitMessage + ") shell closed");
+ }
} catch (IOException e) {
- log.info("Error closing shell", e);
+ log.warn("onExit(" + exitValue + ")[" + exitMessage + ") Error closing shell", e);
}
}
- @Override
- public void onExit(int exitValue, String exitMessage) {
- onExit(exitValue);
- }
});
}
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/5cd057a7/sshd-core/src/main/java/org/apache/sshd/server/shell/ProcessShellFactory.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/server/shell/ProcessShellFactory.java b/sshd-core/src/main/java/org/apache/sshd/server/shell/ProcessShellFactory.java
index eb7334f..be54fea 100644
--- a/sshd-core/src/main/java/org/apache/sshd/server/shell/ProcessShellFactory.java
+++ b/sshd-core/src/main/java/org/apache/sshd/server/shell/ProcessShellFactory.java
@@ -123,13 +123,15 @@ public class ProcessShellFactory extends AbstractLoggingBean implements Factory<
}
}
ProcessBuilder builder = new ProcessBuilder(cmds);
- if (env != null) {
+ if (GenericUtils.size(env) > 0) {
try {
- builder.environment().putAll(env);
+ Map<String,String> procEnv = builder.environment();
+ procEnv.putAll(env);
} catch (Exception e) {
- log.info("Could not set environment for command", e);
+ log.warn("Could not set environment for command=" + GenericUtils.join(cmds, ' '), e);
}
}
+
log.info("Starting shell with command: '{}' and env: {}", builder.command(), builder.environment());
process = builder.start();
out = new TtyFilterInputStream(process.getInputStream());
@@ -174,7 +176,11 @@ public class ProcessShellFactory extends AbstractLoggingBean implements Factory<
@Override
public void destroy() {
if (process != null) {
- process.destroy();
+ try {
+ process.destroy();
+ } finally {
+ process = null;
+ }
}
}
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/5cd057a7/sshd-core/src/test/java/org/apache/sshd/server/ServerTest.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/test/java/org/apache/sshd/server/ServerTest.java b/sshd-core/src/test/java/org/apache/sshd/server/ServerTest.java
index 43af998..c77645a 100644
--- a/sshd-core/src/test/java/org/apache/sshd/server/ServerTest.java
+++ b/sshd-core/src/test/java/org/apache/sshd/server/ServerTest.java
@@ -24,6 +24,7 @@ import java.io.InputStream;
import java.io.OutputStream;
import java.io.PipedInputStream;
import java.io.PipedOutputStream;
+import java.io.StreamCorruptedException;
import java.net.SocketAddress;
import java.util.Arrays;
import java.util.Map;
@@ -32,11 +33,13 @@ import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.TimeoutException;
import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.atomic.AtomicReference;
import org.apache.sshd.client.SessionFactory;
import org.apache.sshd.client.SshClient;
import org.apache.sshd.client.channel.ChannelExec;
import org.apache.sshd.client.channel.ChannelShell;
+import org.apache.sshd.client.channel.ClientChannel;
import org.apache.sshd.client.future.AuthFuture;
import org.apache.sshd.client.session.ClientConnectionServiceFactory;
import org.apache.sshd.client.session.ClientSession;
@@ -52,6 +55,8 @@ import org.apache.sshd.common.session.AbstractConnectionService;
import org.apache.sshd.common.session.AbstractSession;
import org.apache.sshd.common.session.Session;
import org.apache.sshd.common.session.SessionListener;
+import org.apache.sshd.common.util.GenericUtils;
+import org.apache.sshd.common.util.ValidateUtils;
import org.apache.sshd.deprecated.ClientUserAuthServiceOld;
import org.apache.sshd.server.command.ScpCommandFactory;
import org.apache.sshd.server.subsystem.sftp.SftpSubsystemFactory;
@@ -79,6 +84,10 @@ public class ServerTest extends BaseTestSupport {
private SshClient client;
private int port;
+ public ServerTest() {
+ super();
+ }
+
@Before
public void setUp() throws Exception {
sshd = SshServer.setUpDefaultServer();
@@ -264,8 +273,8 @@ public class ServerTest extends BaseTestSupport {
client = SshClient.setUpDefaultClient();
client.start();
- try(ClientSession s = client.connect("test", "localhost", port).verify(7L, TimeUnit.SECONDS).getSession()) {
- s.addPasswordIdentity("test");
+ try(ClientSession s = client.connect(getCurrentTestName(), "localhost", port).verify(7L, TimeUnit.SECONDS).getSession()) {
+ s.addPasswordIdentity(getCurrentTestName());
s.auth().verify(5L, TimeUnit.SECONDS);
try(ChannelExec shell = s.createExecChannel("normal");
@@ -444,15 +453,117 @@ public class ServerTest extends BaseTestSupport {
fail("No success to authenticate");
}
+ @Test
+ public void testEnvironmentVariablesPropagationToServer() throws Exception {
+ final AtomicReference<Environment> envHolder = new AtomicReference<Environment>(null);
+ sshd.setCommandFactory(new CommandFactory() {
+ @Override
+ public Command createCommand(final String command) {
+ ValidateUtils.checkTrue(String.CASE_INSENSITIVE_ORDER.compare(command, getCurrentTestName()) == 0, "Unexpected command: %s", command);
+
+ return new Command() {
+ private ExitCallback cb;
+
+ @Override
+ public void setOutputStream(OutputStream out) {
+ // ignored
+ }
+
+ @Override
+ public void setInputStream(InputStream in) {
+ // ignored
+ }
+
+ @Override
+ public void setExitCallback(ExitCallback callback) {
+ cb = callback;
+ }
+
+ @Override
+ public void setErrorStream(OutputStream err) {
+ // ignored
+ }
+
+ @Override
+ public void destroy() {
+ // ignored
+ }
+
+ @Override
+ public void start(Environment env) throws IOException {
+ if (envHolder.getAndSet(env) != null) {
+ throw new StreamCorruptedException("Multiple starts for command=" + command);
+ }
+
+ cb.onExit(0, command);
+ }
+ };
+ }
+ });
+
+
+ @SuppressWarnings("synthetic-access")
+ Map<String,String> expected = new TreeMap<String,String>(String.CASE_INSENSITIVE_ORDER) {
+ private static final long serialVersionUID = 1L; // we're not serializing it
+
+ {
+ put("test", getCurrentTestName());
+ put("port", Integer.toString(port));
+ put("user", System.getProperty("user.name"));
+ }
+ };
+
+ client = SshClient.setUpDefaultClient();
+ client.start();
+ try(ClientSession s = client.connect(getCurrentTestName(), "localhost", port).verify(7L, TimeUnit.SECONDS).getSession()) {
+ s.addPasswordIdentity(getCurrentTestName());
+ s.auth().verify(5L, TimeUnit.SECONDS);
+
+ try(ChannelExec shell = s.createExecChannel(getCurrentTestName())) {
+ for (Map.Entry<String,String> ee : expected.entrySet()) {
+ shell.setEnv(ee.getKey(), ee.getValue());
+ }
+ shell.open().verify(5L, TimeUnit.SECONDS);
+ shell.waitFor(ClientChannel.CLOSED, TimeUnit.SECONDS.toMillis(17L));
+
+ Integer status = shell.getExitStatus();
+ assertNotNull("No exit status", status);
+ assertEquals("Bad exit status", 0, status.intValue());
+ }
+
+ Environment cmdEnv = envHolder.get();
+ assertNotNull("No environment set", cmdEnv);
+
+ Map<String,String> vars = cmdEnv.getEnv();
+ assertTrue("Mismatched vars count", GenericUtils.size(vars) >= GenericUtils.size(expected));
+ for (Map.Entry<String,String> ee : expected.entrySet()) {
+ String key = ee.getKey(), expValue = ee.getValue(), actValue = vars.get(key);
+ assertEquals("Mismatched value for " + key, expValue, actValue);
+ }
+ } finally {
+ client.stop();
+ }
+
+ }
+
public static class TestEchoShellFactory extends EchoShellFactory {
+ public TestEchoShellFactory() {
+ super();
+ }
+
@Override
public Command create() {
return new TestEchoShell();
}
+
public static class TestEchoShell extends EchoShell {
public static CountDownLatch latch = new CountDownLatch(1);
+ public TestEchoShell() {
+ super();
+ }
+
@Override
public void destroy() {
if (latch != null) {
http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/5cd057a7/sshd-core/src/test/java/org/apache/sshd/util/EchoShellFactory.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/test/java/org/apache/sshd/util/EchoShellFactory.java b/sshd-core/src/test/java/org/apache/sshd/util/EchoShellFactory.java
index 74bf954..da466da 100644
--- a/sshd-core/src/test/java/org/apache/sshd/util/EchoShellFactory.java
+++ b/sshd-core/src/test/java/org/apache/sshd/util/EchoShellFactory.java
@@ -51,6 +51,10 @@ public class EchoShellFactory implements Factory<Command> {
private Environment environment;
private Thread thread;
+ public EchoShell() {
+ super();
+ }
+
public InputStream getIn() {
return in;
}
@@ -91,6 +95,7 @@ public class EchoShellFactory implements Factory<Command> {
public void start(Environment env) throws IOException {
environment = env;
thread = new Thread(this, "EchoShell");
+ thread.setDaemon(true);
thread.start();
}