diff --git a/CHANGES.md b/CHANGES.md index ddcc779fb..aaa50964f 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -40,6 +40,7 @@ * [GH-727](https://github.com/apache/mina-sshd/issues/727) Supply default port 22 for proxy jump hosts for which there is no `HostConfigEntry` * [GH-733](https://github.com/apache/mina-sshd/issues/733) Fix `SftpRemotePathChannel.transferTo()` (avoid NPE) * [GH-751](https://github.com/apache/mina-sshd/issues/751) Fix SFTP v3 "long name" if SFTP server uses an `SftpFileSystem` to another server +* [GH-754](https://github.com/apache/mina-sshd/issues/754) `DefaultFowarder` must not be closed after a bind error * [GH-767](https://github.com/apache/mina-sshd/issues/767) Remove dependency on net.i2p.crypto in `SkED25519PublicKey` * [GH-771](https://github.com/apache/mina-sshd/issues/771) Remove dependency on net.i2p.crypto in `EdDSAPuttyKeyDecoder` * [GH-774](https://github.com/apache/mina-sshd/issues/774) Fix `WritePendingException` in SFTP file copy diff --git a/sshd-core/src/main/java/org/apache/sshd/common/forward/DefaultForwarder.java b/sshd-core/src/main/java/org/apache/sshd/common/forward/DefaultForwarder.java index e2d5b6e11..195e911e8 100644 --- a/sshd-core/src/main/java/org/apache/sshd/common/forward/DefaultForwarder.java +++ b/sshd-core/src/main/java/org/apache/sshd/common/forward/DefaultForwarder.java @@ -932,31 +932,23 @@ protected InetSocketAddress doBind(SshdSocketAddress address, IoAcceptor accepto throws IOException { // TODO find a better way to determine the resulting bind address - what if multi-threaded calls... Collection before = acceptor.getBoundAddresses(); - try { - InetSocketAddress bindAddress = address.toInetSocketAddress(); - acceptor.bind(bindAddress); + InetSocketAddress bindAddress = address.toInetSocketAddress(); + acceptor.bind(bindAddress); - Collection after = acceptor.getBoundAddresses(); - if (GenericUtils.size(after) > 0) { - after.removeAll(before); - } - if (GenericUtils.isEmpty(after)) { - throw new IOException("Error binding to " + address + "[" + bindAddress + "]: no local addresses bound"); - } - - if (after.size() > 1) { - throw new IOException("Multiple local addresses have been bound for " + address + "[" + bindAddress + "]"); - } + Collection after = acceptor.getBoundAddresses(); + if (GenericUtils.size(after) > 0) { + after.removeAll(before); + } + if (GenericUtils.isEmpty(after)) { + throw new IOException("Error binding to " + address + "[" + bindAddress + "]: no local addresses bound"); + } - InetSocketAddress boundAddress = (InetSocketAddress) GenericUtils.head(after); - return boundAddress; - } catch (IOException bindErr) { - Collection after = acceptor.getBoundAddresses(); - if (GenericUtils.isEmpty(after)) { - close(); - } - throw bindErr; + if (after.size() > 1) { + throw new IOException("Multiple local addresses have been bound for " + address + "[" + bindAddress + "]"); } + + InetSocketAddress boundAddress = (InetSocketAddress) GenericUtils.head(after); + return boundAddress; } @Override diff --git a/sshd-core/src/test/java/org/apache/sshd/common/forward/PortForwardingTest.java b/sshd-core/src/test/java/org/apache/sshd/common/forward/PortForwardingTest.java index 44c84de8f..699433a51 100644 --- a/sshd-core/src/test/java/org/apache/sshd/common/forward/PortForwardingTest.java +++ b/sshd-core/src/test/java/org/apache/sshd/common/forward/PortForwardingTest.java @@ -71,6 +71,7 @@ import org.apache.sshd.common.session.ConnectionService; import org.apache.sshd.common.util.GenericUtils; import org.apache.sshd.common.util.MapEntryUtils.NavigableMapBuilder; +import org.apache.sshd.common.util.OsUtils; import org.apache.sshd.common.util.ProxyUtils; import org.apache.sshd.common.util.buffer.ByteArrayBuffer; import org.apache.sshd.common.util.io.IoUtils; @@ -699,6 +700,27 @@ void localForwardingNativeReuse() throws Exception { } } + @Test // GH-754 : forwarder should _not_ be closed after bind error + void localForwardingNativeError() throws Exception { + Assumptions.assumeFalse(OsUtils.isWin32(), "Privileged port can be bound on Windows"); + try (ClientSession session = createNativeSession(null)) { + // Use a privileged port to provoke an exception + SshdSocketAddress local = new SshdSocketAddress(TEST_LOCALHOST, 22); + SshdSocketAddress remote = new SshdSocketAddress(TEST_LOCALHOST, echoPort); + try { + SshdSocketAddress bound = session.startLocalPortForwarding(local, remote); + // If we get here, we have a problem + session.stopLocalPortForwarding(bound); + fail("Expected an exception (privileged port)"); + } catch (IOException e) { + local = new SshdSocketAddress("", 0); + SshdSocketAddress bound = session.startLocalPortForwarding(local, remote); + assertNotNull(bound); + session.stopLocalPortForwarding(bound); + } + } + } + @Test void localForwardingNativeBigPayload() throws Exception { try (ClientSession session = createNativeSession(null)) {