Move IPv4/6 selection down into BSAP.

Change-Id: I6563502da4e5ab76553ad413ea8eeb2d5afd6d29
Reviewed-on: http://gerrit.dmdirc.com/3974
Reviewed-by: Greg Holmes <greg@dmdirc.com>
Automatic-Compile: DMDirc Build Manager

src/com/dmdirc/parser/common/BaseSocketAwareParser.java

@@ -25,6 +25,7 @@ package com.dmdirc.parser.common;
 import com.dmdirc.parser.irc.IRCAuthenticator;
 
 import java.io.IOException;
+import java.net.Inet4Address;
 import java.net.Inet6Address;
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
@@ -116,97 +117,142 @@ public abstract class BaseSocketAwareParser extends BaseParser {
         // Do nothing by default
     }
 
-    private class BindingSocketFactory extends SocketFactory {
+    /**
+     * Creates and binds a new socket to the IP address specified by the parser. If the target
+     * address is an IPv6 address, the parser's {@link #getBindIPv6()} value will be used;
+     * otherwise, the standard {@link #getBindIP()} will be used.
+     *
+     * @param host The host to connect to.
+     * @param port The port to connect on.
+     * @return A new socket bound appropriately and connected.
+     */
+    @SuppressWarnings({"resource", "SocketOpenedButNotSafelyClosed"})
+    private Socket boundSocket(final InetAddress host, final int port) throws IOException {
+        final Socket socket = new Socket();
+        final String bindIp = host instanceof Inet6Address ? getBindIPv6() : getBindIP();
 
-        @Override
-        public Socket createSocket(final String host, final int port) throws IOException {
-            return createSocket(InetAddress.getByName(host), port);
+        if (bindIp != null && !bindIp.isEmpty()) {
+            try {
+                socket.bind(new InetSocketAddress(InetAddress.getByName(bindIp), 0));
+            } catch (IOException ex) {
+                // Bind failed; continue trying to connect anyway.
+                handleSocketDebug("Binding failed: " + ex.getMessage());
+            }
         }
+        setSocket(socket);
+        socket.connect(new InetSocketAddress(host, port), connectTimeout);
+        return socket;
+    }
 
-        @Override
-        @SuppressWarnings("resource")
-        public Socket createSocket(final InetAddress host, final int port) throws IOException {
-            checkPort(port, "server");
-            return getProxy() == null ? boundSocket(host, port) : proxiedSocket(host, port);
+    /**
+     * Creates a new socket via a proxy.
+     *
+     * @param host The host to connect to.
+     * @param port The port to connect on.
+     * @return A new proxy-using socket.
+     */
+    @SuppressWarnings({"resource", "SocketOpenedButNotSafelyClosed"})
+    private Socket proxiedSocket(final InetAddress host, final int port) throws IOException {
+        final URI proxy = getProxy();
+        final Proxy.Type proxyType = Proxy.Type.valueOf(proxy.getScheme().toUpperCase());
+        final String proxyHost = proxy.getHost();
+        final int proxyPort = checkPort(proxy.getPort(), "Proxy");
+
+        final Socket socket = new Socket(
+                new Proxy(proxyType, new InetSocketAddress(proxyHost, proxyPort)));
+
+        try {
+            final IRCAuthenticator ia = IRCAuthenticator.getIRCAuthenticator();
+            final URI serverUri = new URI(null, null, host.getHostName(), port,
+                    null, null, null);
+            try {
+                ia.getSemaphore().acquireUninterruptibly();
+                ia.addAuthentication(serverUri, proxy);
+                socket.connect(new InetSocketAddress(host, port), connectTimeout);
+            } finally {
+                ia.removeAuthentication(serverUri, proxy);
+                ia.getSemaphore().release();
+            }
+        } catch (URISyntaxException ex) {
+            // Won't happen.
         }
 
-        /**
-         * Creates and binds a new socket to the IP address specified by the parser. If the target
-         * address is an IPv6 address, the parser's {@link #getBindIPv6()} value will be used;
-         * otherwise, the standard {@link #getBindIP()} will be used.
-         *
-         * @param host The host to connect to.
-         * @param port The port to connect on.
-         * @return A new socket bound appropriately and connected.
-         */
-        @SuppressWarnings({"resource", "SocketOpenedButNotSafelyClosed"})
-        private Socket boundSocket(final InetAddress host, final int port) throws IOException {
-            final Socket socket = new Socket();
-            final String bindIp = host instanceof Inet6Address ? getBindIPv6() : getBindIP();
-
-            if (bindIp != null && !bindIp.isEmpty()) {
-                try {
-                    socket.bind(new InetSocketAddress(InetAddress.getByName(bindIp), 0));
-                } catch (IOException ex) {
-                    // Bind failed; continue trying to connect anyway.
-                    handleSocketDebug("Binding failed: " + ex.getMessage());
-                }
+        return socket;
+    }
+
+    /**
+     * Utility method to ensure a port is in the correct range. This stops networking classes
+     * throwing obscure exceptions.
+     *
+     * @param port The port to test.
+     * @param description Description of the port for error messages.
+     * @return The given port.
+     * @throws IOException If the port is out of range.
+     */
+    private int checkPort(final int port, final String description) throws IOException {
+        if (port > 65535 || port <= 0) {
+            throw new IOException(description + " port (" + port + ") is invalid.");
+        }
+        return port;
+    }
+
+    /**
+     * Finds an address to connect to of the specified type.
+     *
+     * @param host The host to resolve.
+     * @param type The type of address to look for
+     * @return An address of the specified type, or {@code null} if none exists.
+     * @throws IOException if there is an error.
+     */
+    private InetAddress getAddressForHost(final String host,
+            final Class<? extends InetAddress> type) throws IOException {
+        if (getProxy() != null) {
+            // If we have a proxy, let it worry about all this instead.
+            //
+            // 1) We have no idea what sort of connectivity the proxy has
+            // 2) If we do this here, then any DNS-based geo-balancing is
+            //    going to be based on our location, not the proxy.
+            return InetAddress.getByName(host);
+        }
+
+        for (InetAddress i : InetAddress.getAllByName(host)) {
+            if (type.isAssignableFrom(i.getClass())) {
+                return i;
             }
-            setSocket(socket);
-            socket.connect(new InetSocketAddress(host, port), connectTimeout);
-            return socket;
         }
 
-        /**
-         * Creates a new socket via a proxy.
-         *
-         * @param host The host to connect to.
-         * @param port The port to connect on.
-         * @return A new proxy-using socket.
-         */
-        @SuppressWarnings({"resource", "SocketOpenedButNotSafelyClosed"})
-        private Socket proxiedSocket(final InetAddress host, final int port) throws IOException {
-            final URI proxy = getProxy();
-            final Proxy.Type proxyType = Proxy.Type.valueOf(proxy.getScheme().toUpperCase());
-            final String proxyHost = proxy.getHost();
-            final int proxyPort = checkPort(proxy.getPort(), "Proxy");
-
-            final Socket socket = new Socket(
-                    new Proxy(proxyType, new InetSocketAddress(proxyHost, proxyPort)));
+        return null;
+    }
 
+    private class BindingSocketFactory extends SocketFactory {
+
+        @Override
+        public Socket createSocket(final String host, final int port) throws IOException {
+            // Attempt to connect to IPv6 addresses first.
+            IOException sixException = null;
             try {
-                final IRCAuthenticator ia = IRCAuthenticator.getIRCAuthenticator();
-                final URI serverUri = new URI(null, null, host.getHostName(), port,
-                        null, null, null);
-                try {
-                    ia.getSemaphore().acquireUninterruptibly();
-                    ia.addAuthentication(serverUri, proxy);
-                    socket.connect(new InetSocketAddress(host, port), connectTimeout);
-                } finally {
-                    ia.removeAuthentication(serverUri, proxy);
-                    ia.getSemaphore().release();
+                final InetAddress sixAddress = getAddressForHost(host, Inet6Address.class);
+                if (sixAddress != null) {
+                    return createSocket(sixAddress, port);
                 }
-            } catch (URISyntaxException ex) {
-                // Won't happen.
+            } catch (IOException ex) {
+                handleSocketDebug("Unable to use IPv6: " + ex.getMessage());
+                sixException = ex;
             }
 
-            return socket;
+            // If there isn't an IPv6 address, or it has failed, fall back to IPv4.
+            final InetAddress fourAddress = getAddressForHost(host, Inet4Address.class);
+            if (fourAddress == null) {
+                throw new IOException("No IPv4 address and IPv6 failed", sixException);
+            }
+            return createSocket(fourAddress, port);
         }
 
-        /**
-         * Utility method to ensure a port is in the correct range. This stops networking classes
-         * throwing obscure exceptions.
-         *
-         * @param port The port to test.
-         * @param description Description of the port for error messages.
-         * @return The given port.
-         * @throws IOException If the port is out of range.
-         */
-        private int checkPort(final int port, final String description) throws IOException {
-            if (port > 65535 || port <= 0) {
-                throw new IOException(description + " port (" + port + ") is invalid.");
-            }
-            return port;
+        @Override
+        @SuppressWarnings("resource")
+        public Socket createSocket(final InetAddress host, final int port) throws IOException {
+            checkPort(port, "server");
+            return getProxy() == null ? boundSocket(host, port) : proxiedSocket(host, port);
         }
 
         @Override

src/com/dmdirc/parser/irc/IRCParser.java

@@ -50,8 +50,6 @@ import com.dmdirc.parser.irc.IRCReader.ReadLine;
 import com.dmdirc.parser.irc.outputqueue.OutputQueue;
 
 import java.io.IOException;
-import java.net.Inet4Address;
-import java.net.Inet6Address;
 import java.net.InetAddress;
 import java.net.Socket;
 import java.net.URI;
@@ -731,93 +729,6 @@ public class IRCParser extends BaseSocketAwareParser implements SecureParser, En
         disconnectOnFatal = newValue;
     }
 
-    /**
-     * Find a socket to connect using, this will where possible attempt IPv6
-     * first, before falling back to IPv4.
-     *
-     * This will:
-     *   - Try using v6 first before v4.
-     *   - If there are any failures at all with v6 (including, not having a
-     *     v6 address...) then fall through to v4.
-     *   - If there is no v4 address, throw the v6 exception
-     *   - Otherwise, try v4
-     *   - If there are failures with v4, throw the exception.
-     *
-     * If we have both IPv6 and IPv4, and a target host has both IPv6 and IPv4
-     * addresses, but does not listen on the requested port on either, this
-     * will cause a double-length connection timeout.
-     *
-     * @param target Target URI to connect to.
-     * @return Socket to use in future connections.
-     * @throws IOException if there is an error.
-     */
-    private Socket findSocket(final URI target) throws IOException {
-        if (getProxy() != null) {
-            // If we have a proxy, let it worry about all this instead.
-            //
-            // 1) We have no idea what sort of connectivity the proxy has
-            // 2) If we do this here, then any DNS-based geo-balancing is
-            //    going to be based on our location, not the proxy.
-            return getSocketFactory().createSocket(target.getHost(), target.getPort());
-        }
-
-        URI target6 = null;
-        URI target4 = null;
-
-        // Get the v6 and v4 addresses if appropriate..
-        try {
-            for (InetAddress i : InetAddress.getAllByName(target.getHost())) {
-                if (target6 == null && i instanceof Inet6Address) {
-                    target6 = new URI(target.getScheme(), target.getUserInfo(), i.getHostAddress(), target.getPort(), target.getPath(), target.getQuery(), target.getFragment());
-                } else if (target4 == null && i instanceof Inet4Address) {
-                    target4 = new URI(target.getScheme(), target.getUserInfo(), i.getHostAddress(), target.getPort(), target.getPath(), target.getQuery(), target.getFragment());
-                }
-            }
-        } catch (final URISyntaxException use) { /* Won't happen. */ }
-
-        // Now try and connect.
-        // Try using v6 first before v4.
-        // If there are any failures at all with v6 (including, not having a
-        // v6 address...) then fall through to v4.
-        // If there is no v4 address, throw the v6 exception
-        // Otherwise, try v4
-        // If there are failures with v4, throw the exception.
-        //
-        // If we have both IPv6 and IPv4, and a target host has both IPv6 and
-        // IPv4 addresses, but does not listen on the requested port on either,
-        // this will cause a double-length connection timeout.
-        //
-        // In future this may want to be rewritten to perform a happy-eyeballs
-        // race rather than trying one then the other.
-        Exception v6Exception = null;
-        if (target6 != null) {
-            try {
-                return getSocketFactory().createSocket(target6.getHost(), target6.getPort());
-            } catch (final IOException ioe) {
-                if (target4 == null) {
-                    throw ioe;
-                }
-            } catch (final Exception e) {
-                v6Exception = e;
-                callDebugInfo(DEBUG_SOCKET, "Exception trying to use IPv6: " + e);
-            }
-        }
-        if (target4 != null) {
-            try {
-                return getSocketFactory().createSocket(target4.getHost(), target4.getPort());
-            } catch (final IOException e2) {
-                callDebugInfo(DEBUG_SOCKET, "Exception trying to use IPv4: " + e2);
-                throw e2;
-            }
-        } else if (v6Exception != null) {
-            throw new IOException("Error connecting to: " + target + " (IPv6 failure, with no IPv4 fallback)", v6Exception);
-        } else {
-            // We should never get here as if both target4 and target6 were null
-            // getAllByName would have thrown an exception instead.
-            throw new IOException("Error connecting to: " + target + " (General connectivity failure)");
-        }
-    }
-
     /**
      * Connect to IRC.
      *
@@ -834,7 +745,9 @@ public class IRCParser extends BaseSocketAwareParser implements SecureParser, En
         callDebugInfo(DEBUG_SOCKET, "Connecting to " + getURI().getHost() + ':' + getURI().getPort());
 
         currentSocketState = SocketState.OPENING;
-        socket = findSocket(getConnectURI(getURI()));
+
+        final URI connectUri = getConnectURI(getURI());
+        socket = getSocketFactory().createSocket(connectUri.getHost(), connectUri.getPort());
 
         rawSocket = socket;