Push some logic into BaseSocketAwareParser.

Make IRCParser extend the BSAP, and use that for creating new
sockets that respect bind IP/proxy settings.

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

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

@@ -22,10 +22,16 @@
 
 package com.dmdirc.parser.common;
 
+import com.dmdirc.parser.irc.IRCAuthenticator;
+
 import java.io.IOException;
+import java.net.Inet6Address;
 import java.net.InetAddress;
+import java.net.InetSocketAddress;
+import java.net.Proxy;
 import java.net.Socket;
 import java.net.URI;
+import java.net.URISyntaxException;
 
 import javax.net.SocketFactory;
 
@@ -41,6 +47,9 @@ public abstract class BaseSocketAwareParser extends BaseParser {
     /** The local port that this parser's *most recently created* socket bound to. */
     private int localPort = -1;
 
+    /** The connection timeout, in milliseconds. */
+    private int connectTimeout = 5000;
+
     /**
      * Creates a new base parser for the specified URI.
      *
@@ -62,17 +71,21 @@ public abstract class BaseSocketAwareParser extends BaseParser {
     }
 
     /**
-     * Convenience method to record the local port of the specified socket.
+     * Gets the current connection timeout.
      *
-     * @param socket The socket whose port is being recorded
-     * @return The socket reference passed in, for convenience
+     * @return The connection timeout, in milliseconds.
      */
-    private Socket handleSocket(final Socket socket) {
-        // Store the socket as it might not have been bound yet
-        this.socket = socket;
-        this.localPort = socket.getLocalPort();
+    public int getConnectTimeout() {
+        return connectTimeout;
+    }
 
-        return socket;
+    /**
+     * Sets the connection timeout.
+     *
+     * @param connectTimeout The connection timeout, in milliseconds.
+     */
+    public void setConnectTimeout(final int connectTimeout) {
+        this.connectTimeout = connectTimeout;
     }
 
     /**
@@ -81,41 +94,132 @@ public abstract class BaseSocketAwareParser extends BaseParser {
      * @return An appropriately configured socket factory
      */
     protected SocketFactory getSocketFactory() {
-        return new SocketFactory() {
-
-            @Override
-            public Socket createSocket(final String host, final int port) throws IOException {
-                if (getBindIP() == null) {
-                    return handleSocket(new Socket(host, port));
-                } else {
-                    return handleSocket(new Socket(host, port, InetAddress.getByName(getBindIP()), 0));
+        return new BindingSocketFactory();
+    }
+
+    /**
+     * Stores a reference to a recently created socket.
+     *
+     * @param socket The newly created socket.
+     */
+    private void setSocket(final Socket socket) {
+        this.socket = socket;
+        this.localPort = socket.getLocalPort();
+    }
+
+    /**
+     * Allows subclasses to handle socket-related debug messages.
+     *
+     * @param message The debug message.
+     */
+    protected void handleSocketDebug(final String message) {
+        // Do nothing by default
+    }
+
+    private class BindingSocketFactory extends SocketFactory {
+
+        @Override
+        public Socket createSocket(final String host, final int port) throws IOException {
+            return createSocket(InetAddress.getByName(host), 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);
+        }
+
+        /**
+         * 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());
                 }
             }
+            setSocket(socket);
+            socket.connect(new InetSocketAddress(host, port), connectTimeout);
+            return socket;
+        }
 
-            @Override
-            public Socket createSocket(final InetAddress host, final int port) throws IOException {
-                if (getBindIP() == null) {
-                    return handleSocket(new Socket(host, port));
-                } else {
-                    return handleSocket(new Socket(host, port, InetAddress.getByName(getBindIP()), 0));
+        /**
+         * 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.
             }
 
-            @Override
-            public Socket createSocket(final String host, final int port,
-                    final InetAddress localHost, final int localPort) throws IOException {
-                return handleSocket(new Socket(host, port,
-                        getBindIP() == null ? localHost : InetAddress.getByName(getBindIP()), localPort));
-            }
+            return socket;
+        }
 
-            @Override
-            public Socket createSocket(final InetAddress address,
-                    final int port, final InetAddress localAddress,
-                    final int localPort) throws IOException {
-                return handleSocket(new Socket(address, port,
-                        getBindIP() == null ? localAddress : InetAddress.getByName(getBindIP()), localPort));
+        /**
+         * 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
+        public Socket createSocket(final String host, final int port,
+                final InetAddress localHost, final int localPort) throws IOException {
+            return createSocket(host, port);
+        }
+
+        @Override
+        public Socket createSocket(final InetAddress address, final int port,
+                final InetAddress localAddress, final int localPort) throws IOException {
+            return createSocket(address, port);
+        }
 
-        };
     }
 }

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

@@ -21,7 +21,7 @@
  */
 package com.dmdirc.parser.irc;
 
-import com.dmdirc.parser.common.BaseParser;
+import com.dmdirc.parser.common.BaseSocketAwareParser;
 import com.dmdirc.parser.common.ChannelJoinRequest;
 import com.dmdirc.parser.common.ChildImplementations;
 import com.dmdirc.parser.common.CompositionState;
@@ -53,8 +53,6 @@ import java.io.IOException;
 import java.net.Inet4Address;
 import java.net.Inet6Address;
 import java.net.InetAddress;
-import java.net.InetSocketAddress;
-import java.net.Proxy;
 import java.net.Socket;
 import java.net.URI;
 import java.net.URISyntaxException;
@@ -92,7 +90,7 @@ import javax.net.ssl.X509TrustManager;
     IRCChannelInfo.class,
     IRCClientInfo.class
 })
-public class IRCParser extends BaseParser implements SecureParser, EncodingParser {
+public class IRCParser extends BaseSocketAwareParser implements SecureParser, EncodingParser {
 
     /** Max length an outgoing line should be (NOT including \r\n). */
     public static final int MAX_LINELENGTH = 510;
@@ -164,8 +162,6 @@ public class IRCParser extends BaseParser implements SecureParser, EncodingParse
     boolean post005;
     /** Has the thread started execution yet, (Prevents run() being called multiple times). */
     boolean hasBegan;
-    /** Connect timeout. */
-    private int connectTimeout = 5000;
     /** Manager used to handle prefix modes. */
     public final PrefixModeManager prefixModes = new PrefixModeManager();
     /** Manager used to handle user modes (owxis etc). */
@@ -480,24 +476,6 @@ public class IRCParser extends BaseParser implements SecureParser, EncodingParse
         addLastLine = newValue;
     }
 
-    /**
-     * Get the current Value of connectTimeout.
-     *
-     * @return The value of getConnectTimeout.
-     */
-    public int getConnectTimeout() {
-        return connectTimeout;
-    }
-
-    /**
-     * Set the Value of connectTimeout.
-     *
-     * @param newValue new value for value of getConnectTimeout.
-     */
-    public void setConnectTimeout(final int newValue) {
-        connectTimeout = newValue;
-    }
-
     /**
      * Get the current socket State.
      *
@@ -753,85 +731,6 @@ public class IRCParser extends BaseParser implements SecureParser, EncodingParse
         disconnectOnFatal = newValue;
     }
 
-    /**
-     * Create a new Socket object for the given target, using the given proxy
-     * if appropriate.
-     *
-     * @param target Target URI to connect to.
-     * @param proxy Proxy URI to use
-     * @return Socket, with IP Binding or Proxy as appropriate,
-     * @throws IOException If there was an issue creating the socket.
-     */
-    private Socket newSocket(final URI target, final URI proxy) throws IOException {
-        if (target.getPort() > 65535 || target.getPort() <= 0) {
-            throw new IOException("Server port (" + target.getPort() + ") is invalid.");
-        }
-
-        final Socket mySocket;
-        if (proxy == null) {
-            callDebugInfo(DEBUG_SOCKET, "Not using Proxy");
-            mySocket = new Socket();
-
-            final InetSocketAddress sockAddr = new InetSocketAddress(target.getHost(), target.getPort());
-
-            if (sockAddr.getAddress() instanceof Inet6Address) {
-                if (getBindIPv6() != null && !getBindIPv6().isEmpty()) {
-                    callDebugInfo(DEBUG_SOCKET, "Binding to IPv6: " + getBindIPv6());
-                    try {
-                        mySocket.bind(new InetSocketAddress(InetAddress.getByName(getBindIPv6()), 0));
-                    } catch (IOException e) {
-                        callDebugInfo(DEBUG_SOCKET, "Binding failed: " + e.getMessage());
-                    }
-                }
-            } else {
-                if (getBindIP() != null && !getBindIP().isEmpty()) {
-                    callDebugInfo(DEBUG_SOCKET, "Binding to IPv4: " + getBindIP());
-                    try {
-                        mySocket.bind(new InetSocketAddress(InetAddress.getByName(getBindIP()), 0));
-                    } catch (IOException e) {
-                        callDebugInfo(DEBUG_SOCKET, "Binding failed: " + e.getMessage());
-                    }
-                }
-            }
-
-            mySocket.connect(sockAddr, connectTimeout);
-        } else {
-            callDebugInfo(DEBUG_SOCKET, "Using Proxy");
-
-            if (getBindIP() != null && !getBindIP().isEmpty() ||
-                    getBindIPv6() != null && !getBindIPv6().isEmpty()) {
-                callDebugInfo(DEBUG_SOCKET, "IP Binding is not possible when using a proxy.");
-            }
-
-            final String proxyHost = proxy.getHost();
-            final int proxyPort = proxy.getPort();
-
-            if (proxyPort > 65535 || proxyPort <= 0) {
-                throw new IOException("Proxy port (" + proxyPort + ") is invalid.");
-            }
-
-            final Proxy.Type proxyType = Proxy.Type.valueOf(proxy.getScheme().toUpperCase());
-            mySocket = new Socket(new Proxy(proxyType, new InetSocketAddress(proxyHost, proxyPort)));
-
-            final IRCAuthenticator ia = IRCAuthenticator.getIRCAuthenticator();
-
-            try {
-                try {
-                    ia.getSemaphore().acquire();
-                } catch (InterruptedException ex) {
-                }
-
-                ia.addAuthentication(target, proxy);
-                mySocket.connect(new InetSocketAddress(target.getHost(), target.getPort()), connectTimeout);
-            } finally {
-                ia.removeAuthentication(target, proxy);
-                ia.getSemaphore().release();
-            }
-        }
-
-        return mySocket;
-    }
-
     /**
      * Find a socket to connect using, this will where possible attempt IPv6
      * first, before falling back to IPv4.
@@ -849,18 +748,17 @@ public class IRCParser extends BaseParser implements SecureParser, EncodingParse
      * will cause a double-length connection timeout.
      *
      * @param target Target URI to connect to.
-     * @param proxy Proxy URI to use
      * @return Socket to use in future connections.
      * @throws IOException if there is an error.
      */
-    private Socket findSocket(final URI target, final URI proxy) throws IOException {
-        if (proxy != null) {
+    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 newSocket(target, proxy);
+            return getSocketFactory().createSocket(target.getHost(), target.getPort());
         }
 
         URI target6 = null;
@@ -894,7 +792,7 @@ public class IRCParser extends BaseParser implements SecureParser, EncodingParse
         Exception v6Exception = null;
         if (target6 != null) {
             try {
-                return newSocket(target6, null);
+                return getSocketFactory().createSocket(target6.getHost(), target6.getPort());
             } catch (final IOException ioe) {
                 if (target4 == null) {
                     throw ioe;
@@ -906,7 +804,7 @@ public class IRCParser extends BaseParser implements SecureParser, EncodingParse
         }
         if (target4 != null) {
             try {
-                return newSocket(target4, null);
+                return getSocketFactory().createSocket(target4.getHost(), target4.getPort());
             } catch (final IOException e2) {
                 callDebugInfo(DEBUG_SOCKET, "Exception trying to use IPv4: " + e2);
                 throw e2;
@@ -936,7 +834,7 @@ public class IRCParser extends BaseParser implements SecureParser, EncodingParse
         callDebugInfo(DEBUG_SOCKET, "Connecting to " + getURI().getHost() + ':' + getURI().getPort());
 
         currentSocketState = SocketState.OPENING;
-        socket = findSocket(getConnectURI(getURI()), getProxy());
+        socket = findSocket(getConnectURI(getURI()));
 
         rawSocket = socket;
 
@@ -1063,15 +961,6 @@ public class IRCParser extends BaseParser implements SecureParser, EncodingParse
         callDebugInfo(DEBUG_INFO, "End Thread Execution");
     }
 
-    @Override
-    public int getLocalPort() {
-        if (currentSocketState == SocketState.OPENING || currentSocketState == SocketState.OPEN) {
-            return socket.getLocalPort();
-        } else {
-            return 0;
-        }
-    }
-
     /** Close socket on destroy. */
     @Override
     protected void finalize() throws Throwable {
@@ -2340,4 +2229,10 @@ public class IRCParser extends BaseParser implements SecureParser, EncodingParse
     public void setCompositionState(final String host, final CompositionState state) {
         // Do nothing
     }
+
+    @Override
+    protected void handleSocketDebug(final String message) {
+        super.handleSocketDebug(message);
+        callDebugInfo(DEBUG_SOCKET, message);
+    }
 }