From d9c97fabfbe0edf304bcb1e14e7e546287310958 Mon Sep 17 00:00:00 2001
From: Florian Schmaus <flo@geekplace.eu>
Date: Sun, 25 Jan 2015 00:05:06 +0100
Subject: [PATCH] Make connectUsingConfiguration more robust

If 'return' is not reached, then always throw a
ConnectionException.

Also make it clear that populateHostAddresses adds at least one address.
---
 .../smack/AbstractXMPPConnection.java         |  9 ++++-
 .../smack/tcp/XMPPTCPConnection.java          | 36 +++++++++----------
 2 files changed, 25 insertions(+), 20 deletions(-)

diff --git a/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java b/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java
index 0341a98e9..9870754b2 100644
--- a/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java
+++ b/smack-core/src/main/java/org/jivesoftware/smack/AbstractXMPPConnection.java
@@ -570,7 +570,11 @@ public abstract class AbstractXMPPConnection implements XMPPConnection {
 
     protected List<HostAddress> hostAddresses;
 
-    protected void populateHostAddresses() throws Exception {
+    /**
+     * Populates {@link #hostAddresses} with at least one host address.
+     *
+     */
+    protected void populateHostAddresses() {
         // N.B.: Important to use config.serviceName and not AbstractXMPPConnection.serviceName
         if (config.host != null) {
             hostAddresses = new ArrayList<HostAddress>(1);
@@ -580,6 +584,9 @@ public abstract class AbstractXMPPConnection implements XMPPConnection {
         } else {
             hostAddresses = DNSUtil.resolveXMPPDomain(config.serviceName);
         }
+        // If we reach this, then hostAddresses *must not* be empty, i.e. there is at least one host added, either the
+        // config.host one or the host representing the service name by DNSUtil
+        assert(!hostAddresses.isEmpty());
     }
 
     protected Lock getConnectionLock() {
diff --git a/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java b/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java
index 46821203d..0998ecbb2 100644
--- a/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java
+++ b/smack-tcp/src/main/java/org/jivesoftware/smack/tcp/XMPPTCPConnection.java
@@ -99,6 +99,7 @@ import java.lang.reflect.Constructor;
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.net.Socket;
+import java.net.UnknownHostException;
 import java.security.KeyManagementException;
 import java.security.KeyStore;
 import java.security.KeyStoreException;
@@ -516,25 +517,24 @@ public class XMPPTCPConnection extends AbstractXMPPConnection {
     }
 
     private void connectUsingConfiguration(XMPPTCPConnectionConfiguration config) throws SmackException, IOException {
-        try {
-            populateHostAddresses();
-        }
-        catch (Exception e) {
-            throw new SmackException(e);
-        }
-        Iterator<HostAddress> it = hostAddresses.iterator();
+        populateHostAddresses();
+
         List<HostAddress> failedAddresses = new LinkedList<HostAddress>();
         SocketFactory socketFactory = config.getSocketFactory();
         if (socketFactory == null) {
             socketFactory = SocketFactory.getDefault();
         }
-        outerloop: while (it.hasNext()) {
-            HostAddress hostAddress = it.next();
+        for (HostAddress hostAddress : hostAddresses) {
             String host = hostAddress.getFQDN();
             int port = hostAddress.getPort();
             socket = socketFactory.createSocket();
             try {
                 Iterator<InetAddress> inetAddresses = Arrays.asList(InetAddress.getAllByName(host)).iterator();
+                if (!inetAddresses.hasNext()) {
+                    // This should not happen
+                    LOGGER.warning("InetAddress.getAllByName() returned empty result array.");
+                    throw new UnknownHostException(host);
+                }
                 innerloop: while (inetAddresses.hasNext()) {
                     final InetAddress inetAddress = inetAddresses.next();
                     final String inetAddressAndPort = inetAddress + "at port " + port;
@@ -549,25 +549,21 @@ public class XMPPTCPConnection extends AbstractXMPPConnection {
                         }
                     }
                     LOGGER.finer("Established TCP connection to " + inetAddressAndPort);
-                    // We found a host to connect to, break here
+                    // We found a host to connect to, return here
                     this.host = host;
                     this.port = port;
-                    break outerloop;
+                    return;
                 }
             }
             catch (Exception e) {
                 hostAddress.setException(e);
                 failedAddresses.add(hostAddress);
-                if (!it.hasNext()) {
-                    // There are no more host addresses to try
-                    // throw an exception and report all tried
-                    // HostAddresses in the exception
-                    throw ConnectionException.from(failedAddresses);
-                }
             }
         }
-        socketClosed = false;
-        initConnection();
+        // There are no more host addresses to try
+        // throw an exception and report all tried
+        // HostAddresses in the exception
+        throw ConnectionException.from(failedAddresses);
     }
 
     /**
@@ -822,6 +818,8 @@ public class XMPPTCPConnection extends AbstractXMPPConnection {
         throwAlreadyConnectedExceptionIfAppropriate();
         // Establishes the connection, readers and writers
         connectUsingConfiguration(config);
+        socketClosed = false;
+        initConnection();
 
         // Wait with SASL auth until the SASL mechanisms have been received
         saslFeatureReceived.checkIfSuccessOrWaitOrThrow();