This is an automated email from the ASF dual-hosted git repository.
twolf pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mina-sshd.git
The following commit(s) were added to refs/heads/master by this push:
new f58dfdea1 Refactor session creation
f58dfdea1 is described below
commit f58dfdea15025733c4f9f8b57c9811ccd1092dff
Author: Thomas Wolf <[email protected]>
AuthorDate: Sun Feb 16 10:44:30 2025 +0100
Refactor session creation
Previously a ClientSessionImpl or ServerSessionImpl would send messages
from the constructor. They would also trigger the "session created"
session listener callback from within the constructor.
This made subclassing these session classes tricky, fragile, and
difficult.
Give sessions an explicit start() method and move the initial message
sending from the constructors there. Also remove triggering the "session
created" listener event.
Instead change the session factory to trigger the "session created"
event once the session object has been completely created. The session
factory then calls the start() method.
That way, the session listener gets the "session created" event at a
well-defined moment even when someone has subclassed ClientSessionImpl
or ServerSessionImpl: the session object has been fully initialized,
but no messages have yet been sent or received.
This makes this "session created" event much more useful, and gives the
user a last chance to modify the session configuration before messaging
starts.
---
.../org/apache/sshd/client/session/ClientSessionImpl.java | 5 +++--
.../sshd/common/session/helpers/AbstractSession.java | 8 ++++++++
.../common/session/helpers/AbstractSessionIoHandler.java | 14 +++++++++++++-
.../org/apache/sshd/server/session/ServerSessionImpl.java | 5 ++++-
.../sshd/common/session/helpers/AbstractSessionTest.java | 11 +++++------
5 files changed, 33 insertions(+), 10 deletions(-)
diff --git
a/sshd-core/src/main/java/org/apache/sshd/client/session/ClientSessionImpl.java
b/sshd-core/src/main/java/org/apache/sshd/client/session/ClientSessionImpl.java
index 19e1b680f..e0ed8a437 100644
---
a/sshd-core/src/main/java/org/apache/sshd/client/session/ClientSessionImpl.java
+++
b/sshd-core/src/main/java/org/apache/sshd/client/session/ClientSessionImpl.java
@@ -81,9 +81,10 @@ public class ClientSessionImpl extends AbstractClientSession
{
// manipulate it before the connection has even been established. For
instance, to
// set the authPassword.
getCurrentServices().initialize(client.getServiceFactories());
+ }
- signalSessionCreated(ioSession);
-
+ @Override
+ public void start() throws Exception {
/*
* Must be called regardless of whether the client identification is
sent or not immediately in order to allow
* opening any underlying proxy protocol - e.g., SOCKS or HTTP CONNECT
- otherwise the server's identification
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 a4b52adf2..029808107 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
@@ -346,6 +346,14 @@ public abstract class AbstractSession extends
SessionHelper {
}
}
+ /**
+ * Starts the SSH protocol. Invoked by the framework after the session
object was fully created, and after
+ * {@link
SessionListener#sessionCreated(org.apache.sshd.common.session.Session)} has
been invoked.
+ *
+ * @throws Exception on errors
+ */
+ protected abstract void start() throws Exception;
+
/**
* Creates a new {@link KeyExchangeMessageHandler} instance managing
packet sending for this session.
* <p>
diff --git
a/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSessionIoHandler.java
b/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSessionIoHandler.java
index 136eb2cf5..feed5db66 100644
---
a/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSessionIoHandler.java
+++
b/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSessionIoHandler.java
@@ -37,7 +37,19 @@ public abstract class AbstractSessionIoHandler extends
AbstractLoggingBean imple
@Override
public void sessionCreated(IoSession ioSession) throws Exception {
- ValidateUtils.checkNotNull(createSession(ioSession), "No session
created for %s", ioSession);
+ AbstractSession sshSession =
ValidateUtils.checkNotNull(createSession(ioSession), "No session created for
%s",
+ ioSession);
+ // Now that the session was created, emit the "created" event and
start it.
+ sshSession.signalSessionCreated(ioSession);
+ // TODO (3.0) Starting the session here is still a bit early.
+ // The IoSession is registered with the IoConnector later, which may
still fail and close the session
+ // right away. We'd need am IoHandler.sessionOpened() event that would
get emitted after "created"
+ // and after the IoSession is really fully operational. Or call *this*
sessionCreated() method here
+ // only then. (Would break the current IoConnectionTest.)
+ //
+ // Note that there's also a "session established" event sent to the
session listener before the session
+ // is fully initialized. (From AbstractSession().)
+ sshSession.start();
}
@Override
diff --git
a/sshd-core/src/main/java/org/apache/sshd/server/session/ServerSessionImpl.java
b/sshd-core/src/main/java/org/apache/sshd/server/session/ServerSessionImpl.java
index ea99c9bda..cbabc3585 100644
---
a/sshd-core/src/main/java/org/apache/sshd/server/session/ServerSessionImpl.java
+++
b/sshd-core/src/main/java/org/apache/sshd/server/session/ServerSessionImpl.java
@@ -33,10 +33,13 @@ import org.apache.sshd.server.ServerFactoryManager;
* @author <a href="mailto:[email protected]";>Apache MINA SSHD Project</a>
*/
public class ServerSessionImpl extends AbstractServerSession {
+
public ServerSessionImpl(ServerFactoryManager server, IoSession ioSession)
throws Exception {
super(server, ioSession);
- signalSessionCreated(ioSession);
+ }
+ @Override
+ public void start() throws Exception {
String headerConfig =
CoreModuleProperties.SERVER_EXTRA_IDENTIFICATION_LINES.getOrNull(this);
String[] headers = GenericUtils.split(headerConfig,
CoreModuleProperties.SERVER_EXTRA_IDENT_LINES_SEPARATOR);
// We intentionally create a modifiable array so as to allow users to
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 39f461acc..d4b2df0c5 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
@@ -58,12 +58,6 @@ import org.junit.jupiter.api.Tag;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.TestMethodOrder;
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertNull;
-import static org.junit.jupiter.api.Assertions.assertThrows;
-import static org.junit.jupiter.api.Assertions.assertTrue;
-import static org.junit.jupiter.api.Assertions.fail;
-
/**
* Test basic stuff on AbstractSession.
*
@@ -459,6 +453,11 @@ public class AbstractSessionTest extends BaseTestSupport {
initialKexDone = true;
}
+ @Override
+ public void start() throws Exception {
+ // Nothing to do for this test
+ }
+
@Override
protected void handleMessage(Buffer buffer) throws Exception {
// ignored
