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/11/04 06:49:55 UTC

mina-sshd git commit: [SSHD-576] InvertedShellWrapper would not process all output from spawned processes

Repository: mina-sshd
Updated Branches:
  refs/heads/master 84c155dc6 -> ef317acff


[SSHD-576] InvertedShellWrapper would not process all output from spawned processes


Project: http://git-wip-us.apache.org/repos/asf/mina-sshd/repo
Commit: http://git-wip-us.apache.org/repos/asf/mina-sshd/commit/ef317acf
Tree: http://git-wip-us.apache.org/repos/asf/mina-sshd/tree/ef317acf
Diff: http://git-wip-us.apache.org/repos/asf/mina-sshd/diff/ef317acf

Branch: refs/heads/master
Commit: ef317acff1d4f1233d5869925f4e2bcea96b4581
Parents: 84c155d
Author: Lyor Goldstein <lg...@vmware.com>
Authored: Wed Nov 4 07:49:42 2015 +0200
Committer: Lyor Goldstein <lg...@vmware.com>
Committed: Wed Nov 4 07:49:42 2015 +0200

----------------------------------------------------------------------
 .../sshd/server/shell/InvertedShellWrapper.java | 10 +-
 .../server/shell/InvertedShellWrapperTest.java  | 97 +++++++++++++++++++-
 2 files changed, 104 insertions(+), 3 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/ef317acf/sshd-core/src/main/java/org/apache/sshd/server/shell/InvertedShellWrapper.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/main/java/org/apache/sshd/server/shell/InvertedShellWrapper.java b/sshd-core/src/main/java/org/apache/sshd/server/shell/InvertedShellWrapper.java
index 5f167df..565ff95 100644
--- a/sshd-core/src/main/java/org/apache/sshd/server/shell/InvertedShellWrapper.java
+++ b/sshd-core/src/main/java/org/apache/sshd/server/shell/InvertedShellWrapper.java
@@ -47,7 +47,7 @@ import org.apache.sshd.server.session.ServerSession;
 public class InvertedShellWrapper extends AbstractLoggingBean implements Command, SessionAware {
 
     /**
-     * Default buffer size for the IO pumps.
+     * Default buffer size for the I/O pumps.
      */
     public static final int DEFAULT_BUFFER_SIZE = IoUtils.DEFAULT_COPY_SIZE;
 
@@ -182,10 +182,16 @@ public class InvertedShellWrapper extends AbstractLoggingBean implements Command
                 if (pumpStream(shellErr, err, buffer)) {
                     continue;
                 }
-                if (!shell.isAlive()) {
+
+                /*
+                 * Make sure we exhausted all data - the shell might be dead
+                 * but some data may still be in transit via pumping
+                 */
+                if ((!shell.isAlive()) && (in.available() <= 0) && (shellOut.available() <= 0) && (shellErr.available() <= 0)) {
                     callback.onExit(shell.exitValue());
                     return;
                 }
+
                 // Sleep a bit.  This is not very good, as it consumes CPU, but the
                 // input streams are not selectable for nio, and any other blocking
                 // method would consume at least two threads

http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/ef317acf/sshd-core/src/test/java/org/apache/sshd/server/shell/InvertedShellWrapperTest.java
----------------------------------------------------------------------
diff --git a/sshd-core/src/test/java/org/apache/sshd/server/shell/InvertedShellWrapperTest.java b/sshd-core/src/test/java/org/apache/sshd/server/shell/InvertedShellWrapperTest.java
index 53955da..e74cb08 100644
--- a/sshd-core/src/test/java/org/apache/sshd/server/shell/InvertedShellWrapperTest.java
+++ b/sshd-core/src/test/java/org/apache/sshd/server/shell/InvertedShellWrapperTest.java
@@ -158,7 +158,102 @@ public class InvertedShellWrapperTest extends BaseTestSupport {
         }
     }
 
-    private BogusInvertedShell newShell(String contentOut, String contentErr) {
+    @Test // see SSHD-576
+    public void testShellDiesBeforeAllDataExhausted() throws Exception {
+        final String IN_CONTENT = "shellInput";
+        final String OUT_CONTENT = "shellOutput";
+        final String ERR_CONTENT = "shellError";
+        try (final InputStream stdin = newDelayedInputStream(Long.SIZE, IN_CONTENT);
+             final ByteArrayOutputStream shellIn = new ByteArrayOutputStream(Byte.MAX_VALUE);
+             final InputStream shellOut = newDelayedInputStream(Byte.SIZE, OUT_CONTENT);
+             ByteArrayOutputStream stdout = new ByteArrayOutputStream(OUT_CONTENT.length() + Byte.SIZE);
+             final InputStream shellErr = newDelayedInputStream(Short.SIZE, ERR_CONTENT);
+             ByteArrayOutputStream stderr = new ByteArrayOutputStream(ERR_CONTENT.length() + Byte.SIZE)) {
+
+            InvertedShell shell = new InvertedShell() {
+                @Override
+                public void start(Map<String, String> env) throws IOException {
+                    // ignored
+                }
+
+                @Override
+                public boolean isAlive() {
+                    return false;
+                }
+
+                @Override
+                public InputStream getOutputStream() {
+                    return shellOut;
+                }
+
+                @Override
+                public OutputStream getInputStream() {
+                    return shellIn;
+                }
+
+                @Override
+                public InputStream getErrorStream() {
+                    return shellErr;
+                }
+
+                @Override
+                public int exitValue() {
+                    return -1;
+                }
+
+                @Override
+                public void destroy() {
+                    // ignored
+                }
+            };
+
+            BogusExitCallback exitCallback = new BogusExitCallback();
+            InvertedShellWrapper wrapper = new InvertedShellWrapper(shell);
+            try {
+                wrapper.setInputStream(stdin);
+                wrapper.setOutputStream(stdout);
+                wrapper.setErrorStream(stderr);
+
+                wrapper.setExitCallback(exitCallback);
+                wrapper.start(new BogusEnvironment());
+
+                wrapper.pumpStreams();
+            } finally {
+                wrapper.destroy();
+            }
+
+            assertEquals("Mismatched STDIN value", IN_CONTENT, shellIn.toString(StandardCharsets.UTF_8.name()));
+            assertEquals("Mismatched STDOUT value", OUT_CONTENT, stdout.toString(StandardCharsets.UTF_8.name()));
+            assertEquals("Mismatched STDERR value", ERR_CONTENT, stderr.toString(StandardCharsets.UTF_8.name()));
+        }
+    }
+
+    private static InputStream newDelayedInputStream(int callsCount, String data) {
+        return newDelayedInputStream(callsCount, data.getBytes(StandardCharsets.UTF_8));
+    }
+
+    private static InputStream newDelayedInputStream(final int callsCount, byte ... data) {
+        return new ByteArrayInputStream(data) {
+            private int delayCount = 0;
+
+            @Override
+            public synchronized int read() {
+                throw new UnsupportedOperationException("Unexpected single byte read call");
+            }
+
+            @Override
+            public synchronized int read(byte[] b, int off, int len) {
+                if (delayCount < callsCount) {
+                    delayCount++;
+                    return 0;
+                }
+
+                return super.read(b, off, len);
+            }
+        };
+    }
+
+    private static BogusInvertedShell newShell(String contentOut, String contentErr) {
         ByteArrayOutputStream in = new ByteArrayOutputStream(20);
         ByteArrayInputStream out = new ByteArrayInputStream(contentOut.getBytes(StandardCharsets.UTF_8));
         ByteArrayInputStream err = new ByteArrayInputStream(contentErr.getBytes(StandardCharsets.UTF_8));