[SSHD-734] When ClientSessionImpl construction fails, AbstractSessionIoHandler#exceptionCaught may throw NPE
Project: http://git-wip-us.apache.org/repos/asf/mina-sshd/repo Commit: http://git-wip-us.apache.org/repos/asf/mina-sshd/commit/a1cb02b4 Tree: http://git-wip-us.apache.org/repos/asf/mina-sshd/tree/a1cb02b4 Diff: http://git-wip-us.apache.org/repos/asf/mina-sshd/diff/a1cb02b4 Branch: refs/heads/master Commit: a1cb02b41355c176693ead60c0bce80eb858eb5b Parents: 6ecd949 Author: Guillaume Nodet <gno...@apache.org> Authored: Tue Mar 28 11:16:25 2017 +0200 Committer: Guillaume Nodet <gno...@apache.org> Committed: Tue Mar 28 11:59:28 2017 +0200 ---------------------------------------------------------------------- .../common/session/helpers/AbstractSession.java | 2 + .../helpers/AbstractSessionIoHandler.java | 3 +- .../apache/sshd/client/ClientDeadlockTest.java | 86 ++++++++++++++++++++ 3 files changed, 89 insertions(+), 2 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/a1cb02b4/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSession.java ---------------------------------------------------------------------- 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 18c84bc..49ce76b 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 @@ -255,6 +255,8 @@ public abstract class AbstractSession extends AbstractKexFactoryManager implemen this.ioSession = ioSession; this.decoderBuffer = new SessionWorkBuffer(this); + attachSession(ioSession, this); + Factory<Random> factory = ValidateUtils.checkNotNull(factoryManager.getRandomFactory(), "No random factory for %s", ioSession); random = ValidateUtils.checkNotNull(factory.create(), "No randomizer instance for %s", ioSession); http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/a1cb02b4/sshd-core/src/main/java/org/apache/sshd/common/session/helpers/AbstractSessionIoHandler.java ---------------------------------------------------------------------- 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 34f4465..1610030 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,9 +37,8 @@ public abstract class AbstractSessionIoHandler extends AbstractLoggingBean imple @Override public void sessionCreated(IoSession ioSession) throws Exception { - AbstractSession session = ValidateUtils.checkNotNull( + ValidateUtils.checkNotNull( createSession(ioSession), "No session created for %s", ioSession); - AbstractSession.attachSession(ioSession, session); } @Override http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/a1cb02b4/sshd-core/src/test/java/org/apache/sshd/client/ClientDeadlockTest.java ---------------------------------------------------------------------- diff --git a/sshd-core/src/test/java/org/apache/sshd/client/ClientDeadlockTest.java b/sshd-core/src/test/java/org/apache/sshd/client/ClientDeadlockTest.java new file mode 100644 index 0000000..bff2420 --- /dev/null +++ b/sshd-core/src/test/java/org/apache/sshd/client/ClientDeadlockTest.java @@ -0,0 +1,86 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.sshd.client; + +import java.io.IOException; +import java.util.EnumSet; +import java.util.concurrent.TimeUnit; + +import org.apache.sshd.client.future.ConnectFuture; +import org.apache.sshd.client.session.ClientSession; +import org.apache.sshd.common.io.IoSession; +import org.apache.sshd.server.SshServer; +import org.apache.sshd.server.session.ServerSessionImpl; +import org.apache.sshd.server.session.SessionFactory; +import org.apache.sshd.util.test.BaseTestSupport; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; + +/** + * TODO Add javadoc + * + * @author <a href="mailto:d...@mina.apache.org">Apache MINA SSHD Project</a> + */ +public class ClientDeadlockTest extends BaseTestSupport { + + private SshServer sshd; + private SshClient client; + private int port; + + public ClientDeadlockTest() { + super(); + } + + @Before + public void setUp() throws Exception { + sshd = setupTestServer(); + sshd.setSessionFactory(new SessionFactory(sshd) { + @Override + protected ServerSessionImpl doCreateSession(IoSession ioSession) throws Exception { + throw new IOException("Closing"); + } + }); + sshd.start(); + port = sshd.getPort(); + + client = setupTestClient(); + } + + @After + public void tearDown() throws Exception { + if (sshd != null) { + sshd.stop(true); + } + if (client != null) { + client.stop(); + } + } + + @Test + public void testSimpleClient() throws Exception { + client.start(); + + ConnectFuture future = client.connect(getCurrentTestName(), TEST_LOCALHOST, port); + ClientSession session = future.verify(5, TimeUnit.SECONDS).getSession(); + session.waitFor(EnumSet.of(ClientSession.ClientSessionEvent.CLOSED), TimeUnit.SECONDS.toMillis(3L)); + assertTrue(session.isClosed()); + } + +}