removed pointless socket check; test coverage improved

Signed-off-by: Martin Volf <[email protected]>
hierynomus · Jan 26, 2024 · 5ff91e2 · 5ff91e2
1 parent 0ea7932
commit 5ff91e2
Show file tree

Hide file tree

Showing 2 changed files with 37 additions and 14 deletions.
diff --git a/src/main/java/net/schmizz/sshj/SocketClient.java b/src/main/java/net/schmizz/sshj/SocketClient.java
@@ -79,10 +79,8 @@ public void connect(String hostname, int port, InetAddress localAddr, int localP
             this.hostname = hostname;
             this.port = port;
             socket = socketFactory.createSocket();
-            if (! socket.isConnected()) {
-                socket.bind(new InetSocketAddress(localAddr, localPort));
-                socket.connect(makeInetSocketAddress(hostname, port), connectTimeout);
-            }
+            socket.bind(new InetSocketAddress(localAddr, localPort));
+            socket.connect(makeInetSocketAddress(hostname, port), connectTimeout);
             onConnect();
         }
     }
@@ -118,10 +116,8 @@ public void connect(InetAddress host, int port, InetAddress localAddr, int local
             throws IOException {
         this.port = port;
         socket = socketFactory.createSocket();
-        if (! socket.isConnected()) {
-            socket.bind(new InetSocketAddress(localAddr, localPort));
-            socket.connect(new InetSocketAddress(host, port), connectTimeout);
-        }
+        socket.bind(new InetSocketAddress(localAddr, localPort));
+        socket.connect(new InetSocketAddress(host, port), connectTimeout);
         onConnect();
     }
 

diff --git a/src/test/java/net/schmizz/sshj/ConnectedSocketTest.java b/src/test/java/net/schmizz/sshj/ConnectedSocketTest.java
@@ -20,11 +20,17 @@
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import org.apache.sshd.server.SshServer;
 
 import java.io.IOException;
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.net.Socket;
+import java.util.stream.Stream;
+
 import javax.net.SocketFactory;
 
 import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
@@ -39,13 +45,34 @@ public void setupClient() throws IOException {
         SSHClient defaultClient = fixture.setupDefaultClient();
     }
 
-    @Test
-    public void connectsIfUnconnected() {
-        assertDoesNotThrow(() -> fixture.connectClient(fixture.getClient()));
+    private static interface Connector {
+        void connect(SshServerExtension fx) throws IOException;
+    }
+
+    private static void connectViaHostname(SshServerExtension fx) throws IOException {
+        SshServer server = fx.getServer();
+        fx.getClient().connect(server.getHost(), server.getPort());
+    }
+
+    private static void connectViaAddr(SshServerExtension fx) throws IOException {
+        SshServer server = fx.getServer();
+        InetAddress addr = InetAddress.getByName(server.getHost());
+        fx.getClient().connect(addr, server.getPort());
+    }
+
+    private static Stream<Connector> connectMethods() {
+        return Stream.of(fx -> connectViaHostname(fx), fx -> connectViaAddr(fx));
+    }
+
+    @ParameterizedTest
+    @MethodSource("connectMethods")
+    public void connectsIfUnconnected(Connector connector) {
+        assertDoesNotThrow(() -> connector.connect(fixture));
     }
 
-    @Test
-    public void handlesConnected() throws IOException {
+    @ParameterizedTest
+    @MethodSource("connectMethods")
+    public void handlesConnected(Connector connector) throws IOException {
         Socket socket = SocketFactory.getDefault().createSocket();
         SocketFactory factory = new SocketFactory() {
                 @Override
@@ -73,6 +100,6 @@ public Socket createSocket(String host, int port,
             };
         socket.connect(new InetSocketAddress("localhost", fixture.getServer().getPort()));
         fixture.getClient().setSocketFactory(factory);
-        assertDoesNotThrow(() -> fixture.connectClient(fixture.getClient()));
+        assertDoesNotThrow(() -> connector.connect(fixture));
     }
 }