GH-754: Don't close DefaultForwarder on bind error

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

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<SocketAddress> before = acceptor.getBoundAddresses();
-        try {
-            InetSocketAddress bindAddress = address.toInetSocketAddress();
-            acceptor.bind(bindAddress);
+        InetSocketAddress bindAddress = address.toInetSocketAddress();
+        acceptor.bind(bindAddress);
 
-            Collection<SocketAddress> 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<SocketAddress> 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<SocketAddress> 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

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)) {