[
https://issues.apache.org/jira/browse/SSHD-1295?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17602888#comment-17602888
]
Lyor Goldstein commented on SSHD-1295:
--------------------------------------
{quote}
Another shortcoming of the current approach in PR 242 is that it does create
the ClientSession first, which already sends messages, and only then decides to
close the session again. However, avoiding that would be a much more invasive
change that would have to re-define the way a ClientSession is started:
basically, don't send messages in the constructor already but have a separate
start() method that would have to be called after the session object was fully
created.
{quote}
IMO it would be even more invasive change than imagined - there is code that
+relies+ on the fact that some initial messages are sent immediately upon
construction. Delaying this until _start()_ is called, may prove quite
complex...
> Connection attempt not canceled when a connection timeout occurs
> ----------------------------------------------------------------
>
> Key: SSHD-1295
> URL: https://issues.apache.org/jira/browse/SSHD-1295
> Project: MINA SSHD
> Issue Type: Bug
> Affects Versions: 2.9.2
> Reporter: Thomas Wolf
> Priority: Major
>
> This is not a new bug; it's also present in earlier versions.
> When {{client.connect().verify(timeout)}} times out, the underlying
> asynchronous connection attempt still continues _and may still succeed_. If
> that happens, there is a connection and session that the user cannot use or
> close. (Short of calling verify().getSession() again, but that is a pattern I
> have never seen, and it also makes using a timeout rather useless.)
> This means that timeouts are inherently unreliable.
> Here's a simple test case illustrating this problem (for
> {{org.apache.sshd.client.ClientTest}}):
> {code:java}
> @Test
> public void testConnectTimeout() throws Exception {
> client.start();
> try {
> ConnectFuture future = client.connect(getCurrentTestName(),
> TEST_LOCALHOST, port);
> try {
> future.verify(1);
> fail("Timeout expected");
> } catch (InterruptedIOException | SshException e) {
> ClientSession session = null;
> try {
> session = future.verify(CONNECT_TIMEOUT).getSession();
> } catch (SshException e2) {
> assertTrue("Expected a timeout, got " + e2,
> e2.getMessage().contains("timeout"));
> }
> if (session != null) {
> assertTrue("Session should be closed", session.isClosed() ||
> session.isClosing());
> }
> }
> } finally {
> client.stop();
> }
> }
> {code}
> This test fails.
> When the {{ConnectFuture}} times out (or is canceled, or waiting is
> interrupted), the underlying {{IoConnectFuture}} must be canceled. If we do
> get an {{IoSession}} all the same, but the {{ConnectFuture}} is already
> canceled or timed out, the {{IoSession}} must be closed immediately.
> Preferrably before a {{Session}} is created at all; otherwise, that SSH
> session must be closed, too. Creating a {{ClientSession}} already starts the
> application-level protocol (sending the SSH ident, or if a proxy is used, the
> proxy connect request).
> The test also highlights another problem: there is no clear indication that
> the failure of {{verify()}} indicates a timeout. Probably there should be a
> dedicated exception for this, or at least the {{SshException}} should have a
> {{TimeoutException}} as cause.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@mina.apache.org
For additional commands, e-mail: dev-h...@mina.apache.org
