[FIXED JENKINS-37539] - Prevent NPE in Engine#connect() when host or …

…port are null or empty (#101) Bug diagnosis: * https://scan8.coverity.com/reports.htm#v14462/p10499/fileInstanceId=12836372&defectInstanceId=4427367&mergedDefectId=152195 * https://scan8.coverity.com/reports.htm#v14462/p10499/fileInstanceId=12836372&defectInstanceId=4427365&mergedDefectId=152194
jenkinsci · Aug 26, 2016 · 98107a2 · 98107a2
1 parent b968750
commit 98107a2
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 6 deletions.
diff --git a/src/main/java/hudson/remoting/Engine.java b/src/main/java/hudson/remoting/Engine.java
@@ -23,6 +23,7 @@
  */
 package hudson.remoting;
 
+import edu.umd.cs.findbugs.annotations.Nullable;
 import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import hudson.remoting.Channel.Mode;
 import java.io.FileInputStream;
@@ -45,6 +46,7 @@
 import java.util.Set;
 import java.util.logging.Level;
 import javax.annotation.CheckForNull;
+import javax.annotation.Nonnull;
 import javax.annotation.concurrent.NotThreadSafe;
 import javax.net.ssl.SSLContext;
 import javax.net.ssl.SSLSocketFactory;
@@ -356,10 +358,13 @@ private void onConnectionRejected(String greeting) throws InterruptedException {
 
     /**
      * Connects to TCP slave host:port, with a few retries.
+     * @param host Host to be used. May be {@code null} if the data comes from the {@link #tunnel} definition.
+     * @param port Port to be used. May be {@code null} if the data comes from the {@link #tunnel} definition.
+     * @throws IOException Connection failure or invalid parameter specification
      */
     @SuppressFBWarnings(value = "VA_FORMAT_STRING_USES_NEWLINE",
-            justification = "Unsafe endline symbol is a pert of the protocol. Unsafe to fix it. See TODO below")
-    private Socket connect(String host, String port) throws IOException, InterruptedException {
+            justification = "Unsafe endline symbol is a part of the protocol. Unsafe to fix it. See TODO below")
+    private Socket connect(@CheckForNull String host, @CheckForNull String port) throws IOException, InterruptedException {
 
         if(tunnel!=null) {
             String[] tokens = tunnel.split(":",3);
@@ -369,6 +374,21 @@ private Socket connect(String host, String port) throws IOException, Interrupted
             if(tokens[1].length()>0)    port = tokens[1];
         }
 
+        // Verify settings
+        if (host == null || host.isEmpty()) {
+            throw new IOException("Illegal parameter: Empty destination host");
+        }
+        final int portValue;
+        if (port == null || port.isEmpty()) {
+            throw new IOException("Illegal parameter: Empty destination port");
+        }
+        try {
+            portValue = Integer.parseInt(port);
+        } catch (NumberFormatException ex) {
+            throw new IOException("Illegal parameter: Destination port value '" + port + "' is not integer");
+        }
+
+
         String msg = "Connecting to " + host + ':' + port;
         events.status(msg);
         int retry = 1;
@@ -377,10 +397,10 @@ private Socket connect(String host, String port) throws IOException, Interrupted
             InetSocketAddress targetAddress = null;
             try {
                 Socket s = null;
-                targetAddress = Util.getResolvedHttpProxyAddress(host, Integer.parseInt(port));
+                targetAddress = Util.getResolvedHttpProxyAddress(host, portValue);
 
                 if(targetAddress == null) {
-                    targetAddress = new InetSocketAddress(host, Integer.parseInt(port));
+                    targetAddress = new InetSocketAddress(host, portValue);
                 } else {
                     isHttpProxy = true;
                 }

diff --git a/src/main/java/hudson/remoting/Util.java b/src/main/java/hudson/remoting/Util.java
@@ -125,7 +125,7 @@ static String indent(String s) {
      *
      * Warning: this method won't match shortened representation of IPV6 address
      */
-    static boolean inNoProxyEnvVar(String host) {
+    static boolean inNoProxyEnvVar(@Nonnull  String host) {
         String noProxy = System.getenv("no_proxy");
         if (noProxy != null) {
             noProxy = noProxy.trim()
@@ -218,7 +218,7 @@ static URLConnection openURLConnection(URL url) throws IOException {
         return openURLConnection(url, null, null, null);
     }
 
-    static InetSocketAddress getResolvedHttpProxyAddress(String host, int port) throws IOException {
+    static InetSocketAddress getResolvedHttpProxyAddress(@Nonnull String host, int port) throws IOException {
         InetSocketAddress targetAddress = null;
         Iterator<Proxy> proxies = ProxySelector.getDefault().select(URI.create(String.format("http://%s:%d", host, port))).iterator();
         while (targetAddress == null && proxies.hasNext()) {