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 2020/02/03 17:54:46 UTC

[mina-sshd] branch master updated (5efd3bc -> 385f637)

This is an automated email from the ASF dual-hosted git repository.

lgoldstein pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/mina-sshd.git.


    from 5efd3bc  Fixed version 2.4.0 changes sub-header
     new 0afb936  Fixed typo in 'IoSession#shu(t)downOutputStream' method
     new a3bca7b  [SSHD-964] Allow sending SSH_MSG_CHANNEL_EOF while channel is being closed gracefully
     new 385f637  [SSHD-964] Do not shutdown the TCPIP channel output stream when receiving SSH_MSG_CHANNEL_EOF

The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 CHANGES.md                                         |  2 +
 .../java/org/apache/sshd/common/io/IoSession.java  |  2 +-
 .../common/util/closeable/AbstractCloseable.java   | 16 +++++--
 .../sshd/client/channel/AbstractClientChannel.java |  6 ++-
 .../sshd/common/channel/AbstractChannel.java       | 50 ++++++++++++++--------
 .../common/channel/ChannelAsyncOutputStream.java   |  2 +-
 .../common/forward/DefaultForwardingFilter.java    | 14 ++----
 .../org/apache/sshd/common/forward/SocksProxy.java |  4 +-
 .../sshd/common/forward/TcpipClientChannel.java    |  6 ---
 .../apache/sshd/common/io/nio2/Nio2Session.java    |  2 +-
 .../common/session/helpers/AbstractSession.java    |  4 +-
 .../session/helpers/AbstractSessionTest.java       |  2 +-
 .../apache/sshd/common/io/mina/MinaSession.java    |  2 +-
 .../java/org/apache/sshd/netty/NettyIoSession.java |  2 +-
 14 files changed, 65 insertions(+), 49 deletions(-)


[mina-sshd] 02/03: [SSHD-964] Allow sending SSH_MSG_CHANNEL_EOF while channel is being closed gracefully

Posted by lg...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

lgoldstein pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mina-sshd.git

commit a3bca7b22eaddb54b51c9b1b5ca264dac10b7903
Author: Lyor Goldstein <lg...@apache.org>
AuthorDate: Wed Jan 22 18:59:26 2020 +0200

    [SSHD-964] Allow sending SSH_MSG_CHANNEL_EOF while channel is being closed gracefully
---
 CHANGES.md                                         |  2 +
 .../common/util/closeable/AbstractCloseable.java   | 16 +++++--
 .../sshd/client/channel/AbstractClientChannel.java |  6 ++-
 .../sshd/common/channel/AbstractChannel.java       | 50 ++++++++++++++--------
 .../common/channel/ChannelAsyncOutputStream.java   |  2 +-
 .../common/forward/DefaultForwardingFilter.java    | 14 ++----
 .../org/apache/sshd/common/forward/SocksProxy.java |  4 +-
 .../common/session/helpers/AbstractSession.java    |  4 +-
 8 files changed, 60 insertions(+), 38 deletions(-)

diff --git a/CHANGES.md b/CHANGES.md
index 5c5e872..dd584ff 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -11,3 +11,5 @@
 ## Minor code helpers
 
 ## Behavioral changes and enhancements
+
+* [SSHD-964](https://issues.apache.org/jira/browse/SSHD-964) - Send SSH_MSG_CHANNEL_EOF when tunnel channel being closed.
diff --git a/sshd-common/src/main/java/org/apache/sshd/common/util/closeable/AbstractCloseable.java b/sshd-common/src/main/java/org/apache/sshd/common/util/closeable/AbstractCloseable.java
index e503431..46eae38 100644
--- a/sshd-common/src/main/java/org/apache/sshd/common/util/closeable/AbstractCloseable.java
+++ b/sshd-common/src/main/java/org/apache/sshd/common/util/closeable/AbstractCloseable.java
@@ -26,14 +26,22 @@ import org.apache.sshd.common.future.SshFuture;
 import org.apache.sshd.common.future.SshFutureListener;
 
 /**
- * Provides some default implementations
+ * Provides some default implementations for managing channel/connection open/close state
  *
  * @author <a href="mailto:dev@mina.apache.org">Apache MINA SSHD Project</a>
  */
 public abstract class AbstractCloseable extends IoBaseCloseable {
 
     public enum State {
-        Opened, Graceful, Immediate, Closed
+        /** Connection is open */
+        Opened,
+        /** Connection is being closed gracefully */
+        Graceful,
+        /** Connection is being terminated immediately */
+        Immediate,
+        /** Connection is closed */
+        Closed,
+        /* end */;
     }
 
     /**
@@ -90,7 +98,7 @@ public abstract class AbstractCloseable extends IoBaseCloseable {
                 }
             } else {
                 if (debugEnabled) {
-                    log.debug("close({})[Immediately] state already {}", this, state.get());
+                    log.debug("close({})[Immediately] state already {}", this, state);
                 }
             }
         } else {
@@ -119,7 +127,7 @@ public abstract class AbstractCloseable extends IoBaseCloseable {
                 }
             } else {
                 if (debugEnabled) {
-                    log.debug("close({})[Graceful] state already {}", this, state.get());
+                    log.debug("close({})[Graceful] state already {}", this, state);
                 }
             }
         }
diff --git a/sshd-core/src/main/java/org/apache/sshd/client/channel/AbstractClientChannel.java b/sshd-core/src/main/java/org/apache/sshd/client/channel/AbstractClientChannel.java
index 93832c6..3a51dda 100644
--- a/sshd-core/src/main/java/org/apache/sshd/client/channel/AbstractClientChannel.java
+++ b/sshd-core/src/main/java/org/apache/sshd/client/channel/AbstractClientChannel.java
@@ -318,7 +318,7 @@ public abstract class AbstractClientChannel extends AbstractChannel implements C
     @Override
     public synchronized OpenFuture open() throws IOException {
         if (isClosing()) {
-            throw new SshException("Session has been closed");
+            throw new SshException("Session has been closed: " + state);
         }
 
         openFuture = new DefaultOpenFuture(this.toString(), futureLock);
@@ -399,6 +399,10 @@ public abstract class AbstractClientChannel extends AbstractChannel implements C
     protected void doWriteData(byte[] data, int off, long len) throws IOException {
         // If we're already closing, ignore incoming data
         if (isClosing()) {
+            if (log.isDebugEnabled()) {
+                log.debug("doWriteData({}) ignored (len={}) channel state={}", this, len, state);
+            }
+
             return;
         }
         ValidateUtils.checkTrue(
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/channel/AbstractChannel.java b/sshd-core/src/main/java/org/apache/sshd/common/channel/AbstractChannel.java
index a5b788e..d8ebe09 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/channel/AbstractChannel.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/channel/AbstractChannel.java
@@ -780,19 +780,19 @@ public abstract class AbstractChannel
 
     @Override
     public IoWriteFuture writePacket(Buffer buffer) throws IOException {
-        Session s = getSession();
         if (!isClosing()) {
+            Session s = getSession();
             return s.writePacket(buffer);
-        } else {
-            if (log.isDebugEnabled()) {
-                log.debug("writePacket({}) Discarding output packet because channel is being closed", this);
-            }
-            return new AbstractIoWriteFuture(s.toString(), null) {
-                {
-                    setValue(new EOFException("Channel is being closed"));
-                }
-            };
         }
+
+        if (log.isDebugEnabled()) {
+            log.debug("writePacket({}) Discarding output packet because channel state={}", this, state);
+        }
+        return new AbstractIoWriteFuture(toString(), null) {
+            {
+                setValue(new EOFException("Channel is being closed"));
+            }
+        };
     }
 
     @Override
@@ -921,29 +921,43 @@ public abstract class AbstractChannel
 
     protected abstract void doWriteExtendedData(byte[] data, int off, long len) throws IOException;
 
-    protected void sendEof() throws IOException {
-        if (isClosing()) {
+    /**
+     * Sends {@code SSH_MSG_CHANNEL_EOF} provided not already sent
+     * and current channel state allows it.
+     *
+     * @return The {@link IoWriteFuture} of the sent packet - {@code null}
+     * if message not sent due to channel state (or already sent)
+     * @throws IOException If failed to send the packet
+     */
+    protected IoWriteFuture sendEof() throws IOException {
+        State channelState = state.get();
+        // OK to send EOF if channel is open or being closed gracefully
+        if ((channelState != State.Opened) && (channelState != State.Graceful)) {
             if (log.isDebugEnabled()) {
-                log.debug("sendEof({}) already closing or closed", this);
+                log.debug("sendEof({}) already closing or closed - state={}", this, state);
             }
-            return;
+            return null;
         }
 
         if (eofSent.getAndSet(true)) {
             if (log.isDebugEnabled()) {
-                log.debug("sendEof({}) already sent", this);
+                log.debug("sendEof({}) already sent (state={})", this, channelState);
             }
-            return;
+            return null;
         }
 
         if (log.isDebugEnabled()) {
-            log.debug("sendEof({}) SSH_MSG_CHANNEL_EOF", this);
+            log.debug("sendEof({}) SSH_MSG_CHANNEL_EOF (state={})", this, channelState);
         }
 
         Session s = getSession();
         Buffer buffer = s.createBuffer(SshConstants.SSH_MSG_CHANNEL_EOF, Short.SIZE);
         buffer.putInt(getRecipient());
-        writePacket(buffer);
+        /*
+         * The default "writePacket" does not send packets if state
+         * is not open so we need to bypass it.
+         */
+        return s.writePacket(buffer);
     }
 
     public boolean isEofSent() {
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/channel/ChannelAsyncOutputStream.java b/sshd-core/src/main/java/org/apache/sshd/common/channel/ChannelAsyncOutputStream.java
index b0e715e..e2f1ebd 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/channel/ChannelAsyncOutputStream.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/channel/ChannelAsyncOutputStream.java
@@ -60,7 +60,7 @@ public class ChannelAsyncOutputStream extends AbstractCloseable implements IoOut
     @Override
     public synchronized IoWriteFuture writePacket(Buffer buffer) throws IOException {
         if (isClosing()) {
-            throw new EOFException("Closed");
+            throw new EOFException("Closing: " + state);
         }
 
         IoWriteFutureImpl future = new IoWriteFutureImpl(packetWriteId, buffer);
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/forward/DefaultForwardingFilter.java b/sshd-core/src/main/java/org/apache/sshd/common/forward/DefaultForwardingFilter.java
index 8836ece..7673920 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/forward/DefaultForwardingFilter.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/forward/DefaultForwardingFilter.java
@@ -192,11 +192,8 @@ public class DefaultForwardingFilter
         ValidateUtils.checkTrue(local.getPort() >= 0, "Invalid local port: %s", local);
         Objects.requireNonNull(remote, "Remote address is null");
 
-        if (isClosed()) {
-            throw new IllegalStateException("TcpipForwarder is closed");
-        }
-        if (isClosing()) {
-            throw new IllegalStateException("TcpipForwarder is closing");
+        if (isClosed() || isClosing()) {
+            throw new IllegalStateException("TcpipForwarder is closed or closing: " + state);
         }
 
         InetSocketAddress bound = null;
@@ -453,11 +450,8 @@ public class DefaultForwardingFilter
         Objects.requireNonNull(local, "Local address is null");
         ValidateUtils.checkTrue(local.getPort() >= 0, "Invalid local port: %s", local);
 
-        if (isClosed()) {
-            throw new IllegalStateException("TcpipForwarder is closed");
-        }
-        if (isClosing()) {
-            throw new IllegalStateException("TcpipForwarder is closing");
+        if (isClosed() || isClosing()) {
+            throw new IllegalStateException("TcpipForwarder is closed or closing: " + state);
         }
 
         SocksProxy proxy = null;
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/forward/SocksProxy.java b/sshd-core/src/main/java/org/apache/sshd/common/forward/SocksProxy.java
index c59cef2..77bb724 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/forward/SocksProxy.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/forward/SocksProxy.java
@@ -52,7 +52,7 @@ public class SocksProxy extends AbstractCloseable implements IoHandler {
     @Override
     public void sessionCreated(IoSession session) throws Exception {
         if (isClosing()) {
-            throw new SshException("SocksProxy is closing or closed");
+            throw new SshException("SocksProxy is closing or closed: " + state);
         }
     }
 
@@ -65,7 +65,7 @@ public class SocksProxy extends AbstractCloseable implements IoHandler {
     }
 
     @Override
-    public void messageReceived(final IoSession session, org.apache.sshd.common.util.Readable message) throws Exception {
+    public void messageReceived(IoSession session, org.apache.sshd.common.util.Readable message) throws Exception {
         Buffer buffer = new ByteArrayBuffer(message.available() + Long.SIZE, false);
         buffer.putBuffer(message);
         Proxy proxy = proxies.get(session);
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java b/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java
index 54d9929..4468276 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java
@@ -1942,7 +1942,7 @@ public abstract class AbstractSession extends SessionHelper {
         }
 
         return ValidateUtils.checkNotNull(
-            kexFutureHolder.get(), "No current KEX future on state=%s", kexState.get());
+            kexFutureHolder.get(), "No current KEX future on state=%s", kexState);
     }
 
     /**
@@ -1970,7 +1970,7 @@ public abstract class AbstractSession extends SessionHelper {
     protected KeyExchangeFuture requestNewKeysExchange() throws IOException, GeneralSecurityException {
         if (!kexState.compareAndSet(KexState.DONE, KexState.INIT)) {
             if (log.isDebugEnabled()) {
-                log.debug("requestNewKeysExchange({}) KEX state not DONE: {}", this, kexState.get());
+                log.debug("requestNewKeysExchange({}) KEX state not DONE: {}", this, kexState);
             }
 
             return null;


[mina-sshd] 03/03: [SSHD-964] Do not shutdown the TCPIP channel output stream when receiving SSH_MSG_CHANNEL_EOF

Posted by lg...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

lgoldstein pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mina-sshd.git

commit 385f63731e74d0c73b6624170bfd1ad385ebffc6
Author: Lyor Goldstein <lg...@apache.org>
AuthorDate: Sat Jan 25 18:54:44 2020 +0200

    [SSHD-964] Do not shutdown the TCPIP channel output stream when receiving SSH_MSG_CHANNEL_EOF
---
 .../java/org/apache/sshd/common/forward/TcpipClientChannel.java     | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/sshd-core/src/main/java/org/apache/sshd/common/forward/TcpipClientChannel.java b/sshd-core/src/main/java/org/apache/sshd/common/forward/TcpipClientChannel.java
index fc0643e..bc33a24 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/forward/TcpipClientChannel.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/forward/TcpipClientChannel.java
@@ -220,10 +220,4 @@ public class TcpipClientChannel extends AbstractClientChannel implements Forward
     public SshdSocketAddress getTunnelExit() {
         return tunnelExit;
     }
-
-    @Override
-    public void handleEof() throws IOException {
-        super.handleEof();
-        serverSession.shutdownOutputStream();
-    }
 }


[mina-sshd] 01/03: Fixed typo in 'IoSession#shu(t)downOutputStream' method

Posted by lg...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

lgoldstein pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mina-sshd.git

commit 0afb936cc445abf26de1caa7305d53e1b4592dd1
Author: Lyor Goldstein <lg...@apache.org>
AuthorDate: Wed Jan 22 18:39:07 2020 +0200

    Fixed typo in 'IoSession#shu(t)downOutputStream' method
---
 sshd-common/src/main/java/org/apache/sshd/common/io/IoSession.java      | 2 +-
 .../main/java/org/apache/sshd/common/forward/TcpipClientChannel.java    | 2 +-
 sshd-core/src/main/java/org/apache/sshd/common/io/nio2/Nio2Session.java | 2 +-
 .../org/apache/sshd/common/session/helpers/AbstractSessionTest.java     | 2 +-
 sshd-mina/src/main/java/org/apache/sshd/common/io/mina/MinaSession.java | 2 +-
 sshd-netty/src/main/java/org/apache/sshd/netty/NettyIoSession.java      | 2 +-
 6 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/sshd-common/src/main/java/org/apache/sshd/common/io/IoSession.java b/sshd-common/src/main/java/org/apache/sshd/common/io/IoSession.java
index 9e6dcfc..7d02968 100644
--- a/sshd-common/src/main/java/org/apache/sshd/common/io/IoSession.java
+++ b/sshd-common/src/main/java/org/apache/sshd/common/io/IoSession.java
@@ -90,5 +90,5 @@ public interface IoSession extends ConnectionEndpointsIndicator, PacketWriter, C
      * Handle received EOF.
      * @throws IOException If failed to shutdown the stream
      */
-    void shudownOutputStream() throws IOException;
+    void shutdownOutputStream() throws IOException;
 }
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/forward/TcpipClientChannel.java b/sshd-core/src/main/java/org/apache/sshd/common/forward/TcpipClientChannel.java
index ac1d6ab..fc0643e 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/forward/TcpipClientChannel.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/forward/TcpipClientChannel.java
@@ -224,6 +224,6 @@ public class TcpipClientChannel extends AbstractClientChannel implements Forward
     @Override
     public void handleEof() throws IOException {
         super.handleEof();
-        serverSession.shudownOutputStream();
+        serverSession.shutdownOutputStream();
     }
 }
diff --git a/sshd-core/src/main/java/org/apache/sshd/common/io/nio2/Nio2Session.java b/sshd-core/src/main/java/org/apache/sshd/common/io/nio2/Nio2Session.java
index a0e1a68..9d52d55 100644
--- a/sshd-core/src/main/java/org/apache/sshd/common/io/nio2/Nio2Session.java
+++ b/sshd-core/src/main/java/org/apache/sshd/common/io/nio2/Nio2Session.java
@@ -300,7 +300,7 @@ public class Nio2Session extends AbstractCloseable implements IoSession {
     }
 
     @Override
-    public void shudownOutputStream() throws IOException {
+    public void shutdownOutputStream() throws IOException {
         AsynchronousSocketChannel socket = getSocket();
         if (socket.isOpen()) {
             if (log.isDebugEnabled()) {
diff --git a/sshd-core/src/test/java/org/apache/sshd/common/session/helpers/AbstractSessionTest.java b/sshd-core/src/test/java/org/apache/sshd/common/session/helpers/AbstractSessionTest.java
index 2e06fdc..567024d 100644
--- a/sshd-core/src/test/java/org/apache/sshd/common/session/helpers/AbstractSessionTest.java
+++ b/sshd-core/src/test/java/org/apache/sshd/common/session/helpers/AbstractSessionTest.java
@@ -414,7 +414,7 @@ public class AbstractSessionTest extends BaseTestSupport {
         }
 
         @Override
-        public void shudownOutputStream() {
+        public void shutdownOutputStream() {
           // do nothing
         }
     }
diff --git a/sshd-mina/src/main/java/org/apache/sshd/common/io/mina/MinaSession.java b/sshd-mina/src/main/java/org/apache/sshd/common/io/mina/MinaSession.java
index 0751747..6ed0134 100644
--- a/sshd-mina/src/main/java/org/apache/sshd/common/io/mina/MinaSession.java
+++ b/sshd-mina/src/main/java/org/apache/sshd/common/io/mina/MinaSession.java
@@ -209,7 +209,7 @@ public class MinaSession extends AbstractInnerCloseable implements IoSession {
     }
 
     @Override   // see SSHD-902
-    public void shudownOutputStream() throws IOException {
+    public void shutdownOutputStream() throws IOException {
         boolean debugEnabled = log.isDebugEnabled();
         if (!(session instanceof NioSession)) {
             if (debugEnabled) {
diff --git a/sshd-netty/src/main/java/org/apache/sshd/netty/NettyIoSession.java b/sshd-netty/src/main/java/org/apache/sshd/netty/NettyIoSession.java
index 521de9f..2186e67 100644
--- a/sshd-netty/src/main/java/org/apache/sshd/netty/NettyIoSession.java
+++ b/sshd-netty/src/main/java/org/apache/sshd/netty/NettyIoSession.java
@@ -166,7 +166,7 @@ public class NettyIoSession extends AbstractCloseable implements IoSession {
     }
 
     @Override   // see SSHD-902
-    public void shudownOutputStream() throws IOException {
+    public void shutdownOutputStream() throws IOException {
         Channel ch = context.channel();
         boolean debugEnabled = log.isDebugEnabled();
         if (!(ch instanceof AbstractNioChannel)) {