[JENKINS-59094] Jenkins connection failure with tunneling (#345)

* [JENKINS-59094] Jenkins connection failure with tunneling Adjust the logic for parsing the tunneling host:port to match what it used to bebefore the latest release. Co-Authored-By: Matt Sicker <boards@gmail.com>
jenkinsci · Sep 6, 2019 · 69618d1 · 69618d1
1 parent 51360cb
commit 69618d1
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 17 deletions.
diff --git a/src/main/java/org/jenkinsci/remoting/engine/HostPort.java b/src/main/java/org/jenkinsci/remoting/engine/HostPort.java
@@ -1,28 +1,42 @@
 package org.jenkinsci.remoting.engine;
 
 class HostPort {
+
+    private static final int PORT_MIN = 0;
+    private static final int PORT_MAX = (1 << 16) - 1;
+
     private String host;
     private int port;
 
     public HostPort(String value) {
-        splitHostPort(value);
+        splitHostPort(value, null, 0);
+    }
+
+    public HostPort(String value, String defaultHost, int defaultPort) {
+        splitHostPort(value, defaultHost, defaultPort);
     }
 
-    private void splitHostPort(String value) {
+    private void splitHostPort(String value, String defaultHost, int defaultPort) {
         String hostPortValue = value.trim();
         if (hostPortValue.charAt(0) == '[') {
             extractIPv6(hostPortValue);
             return;
         }
         int portSeparator = hostPortValue.lastIndexOf(':');
-        if (portSeparator <= 0) {
+        if (portSeparator < 0) {
             throw new IllegalArgumentException("Invalid HOST:PORT value: " + value);
         }
-        host = hostPortValue.substring(0, portSeparator).trim();
-        if (host.isEmpty()) {
-            throw new IllegalArgumentException("Invalid HOST:PORT value: " + value);
+        String hostValue = hostPortValue.substring(0, portSeparator).trim();
+        host = hostValue.length() > 0 ? hostValue : defaultHost;
+        String portString = hostPortValue.substring(portSeparator + 1).trim();
+        if (portString.length() > 0) {
+            port = Integer.parseInt(portString);
+            if (port <= PORT_MIN || port > PORT_MAX) {
+                throw new IllegalArgumentException("Port " + value + " out of valid range [" + PORT_MIN + ", " + PORT_MAX + ")");
+            }
+        } else {
+            port = defaultPort;
         }
-        port = Integer.parseInt(hostPortValue.substring(portSeparator + 1).trim());
     }
 
     private void extractIPv6(String hostPortValue) {
@@ -45,4 +59,5 @@ public String getHost() {
     public int getPort() {
         return port;
     }
+
 }
diff --git a/src/main/java/org/jenkinsci/remoting/engine/JnlpAgentEndpointResolver.java b/src/main/java/org/jenkinsci/remoting/engine/JnlpAgentEndpointResolver.java
@@ -325,7 +325,7 @@ public int compare(String o1, String o2) {
                     }
                 });
                 if (tunnel != null) {
-                    HostPort hostPort = new HostPort(tunnel);
+                    HostPort hostPort = new HostPort(tunnel, host, port);
                     host = hostPort.getHost();
                     port = hostPort.getPort();
                 }

diff --git a/src/test/java/org/jenkinsci/remoting/engine/HostPortTest.java b/src/test/java/org/jenkinsci/remoting/engine/HostPortTest.java
@@ -72,28 +72,58 @@ public void testInvalidPort() {
     }
 
     @Test(expected = IllegalArgumentException.class)
-    public void testNoPort() {
+    public void testNoSeparator() {
         new HostPort("hostname");
     }
 
-    @Test(expected = IllegalArgumentException.class)
+    @Test
     public void testNoHost() {
-        new HostPort(":5555");
+        HostPort hostPort = new HostPort(":5555", "hostname", -1);
+        assertThat(hostPort.getHost(), is("hostname"));
+        assertThat(hostPort.getPort(), is(5555));
     }
 
-    @Test(expected = IllegalArgumentException.class)
+    @Test
     public void testEmptyHost() {
-        new HostPort("    :5555");
+        HostPort hostPort = new HostPort("    :5555", "hostname", -1);
+        assertThat(hostPort.getHost(), is("hostname"));
+        assertThat(hostPort.getPort(), is(5555));
     }
 
-    @Test(expected = IllegalArgumentException.class)
+    @Test
     public void testEmptyPort() {
-        new HostPort("hostname:   ");
+        HostPort hostPort = new HostPort("hostname:   ", null, 7777);
+        assertThat(hostPort.getHost(), is("hostname"));
+        assertThat(hostPort.getPort(), is(7777));
     }
 
-    @Test(expected = IllegalArgumentException.class)
+    @Test
     public void testSeparatorNoPort() {
-        new HostPort("hostname:");
+        HostPort hostPort = new HostPort("hostname:", null, 7777);
+        assertThat(hostPort.getHost(), is("hostname"));
+        assertThat(hostPort.getPort(), is( 7777));
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testNegativePort() {
+        new HostPort("hostname:-4");
+    }
+
+    @Test(expected = IllegalArgumentException.class)
+    public void testPortTooHigh() {
+        new HostPort("hostname:100000");
+    }
+
+    @Test
+    public void testOnlySeparator() {
+        HostPort hostPort = new HostPort(":", "hostname", 7777);
+        assertThat(hostPort.getHost(), is("hostname"));
+        assertThat(hostPort.getPort(), is( 7777));
+    }
+
+    @Test(expected = NumberFormatException.class)
+    public void testPortNotANumber() {
+        new HostPort("hostname:notAPort");
     }
 
 }