HTTPCLIENT-475, take 1 with suggested modifications

git-svn-id: https://svn.apache.org/repos/asf/jakarta/httpcomponents/httpclient/trunk@490691 13f79535-47bb-0310-9956-ffa450edef68

src/java/org/apache/http/conn/PlainSocketFactory.java

@@ -33,6 +33,7 @@ package org.apache.http.conn;
 
 import java.io.IOException;
 import java.net.InetAddress;
+import java.net.InetSocketAddress;
 import java.net.Socket;
 import java.net.UnknownHostException;
 
@@ -40,85 +41,96 @@ import org.apache.http.params.HttpConnectionParams;
 import org.apache.http.params.HttpParams;
 
 /**
- * The default class for creating protocol sockets.  This class just uses the
+ * The default class for creating sockets.
- * {@link java.net.Socket socket} constructors.
+ * This class just uses the {@link java.net.Socket socket} API
+ * in Java 1.4 or greater.
  * 
+ * @author <a href="mailto:http-async@dubioso.net">Roland Weber</a>
  * @author Michael Becke
- * 
- * @since 2.0
  */
-public class PlainSocketFactory implements SocketFactory {
+public final class PlainSocketFactory implements SocketFactory {
 
     /**
      * The factory singleton.
      */
-    private static final PlainSocketFactory DEFAULT_FACTORY = new PlainSocketFactory();
+    private static final
-    
+        PlainSocketFactory DEFAULT_FACTORY = new PlainSocketFactory();
+
     /**
-     * Gets an singleton instance of the DefaultProtocolSocketFactory.
+     * Gets the singleton instance of this class.
-     * @return a DefaultProtocolSocketFactory
+     * @return the one and only plain socket factory
      */
-    public static PlainSocketFactory getSocketFactory() {
+    public static final PlainSocketFactory getSocketFactory() {
         return DEFAULT_FACTORY;
     }
-    
+
     /**
-     * Constructor for DefaultProtocolSocketFactory.
+     * Restricted default constructor.
      */
     private PlainSocketFactory() {
         super();
     }
 
-    /**
+
-     * Attempts to get a new socket connection to using old (pre Java 1.4) IO mode.
+    // non-javadoc, see interface org.apache.http.conn.SocketFactory
-     * This socket factory does not support connect timeout as it requires Java 1.4
+    public Socket createSocket() {
-     * functionality.
+        return new Socket();
-     *  
-     * @param host the host name/IP
-     * @param port the port on the host
-     * @param localAddress the local host name/IP to bind the socket to
-     * @param localPort the port on the local machine
-     * @param params {@link HttpConnectionParams Http connection parameters}
-     * 
-     * @return Socket a new socket
-     * 
-     * @throws IOException if an I/O error occurs while creating the socket
-     * @throws UnknownHostException if the IP address of the host cannot be
-     * @throws IllegalStateException if connection timeout is set
-     * determined
-     * 
-     * @since 3.0
-     */
-    public Socket createSocket(
-        final String host,
-        final int port,
-        final InetAddress localAddress,
-        final int localPort,
-        final HttpParams params
-    ) throws IOException, UnknownHostException {
-        if (params == null) {
-            throw new IllegalArgumentException("Parameters may not be null");
-        }
-        int timeout = HttpConnectionParams.getConnectionTimeout(params);
-        if (timeout != 0) {
-            throw new IllegalStateException("Connection timeout is not supported in old IO mode");
-        }
-        if (localAddress != null) {
-            return new Socket(host, port, localAddress, localPort);
-        } else {
-            return new Socket(host, port);
-        }
     }
 
+    // non-javadoc, see interface org.apache.http.conn.SocketFactory
+    public Socket connectSocket(Socket sock, String host, int port, 
+                                InetAddress localAddress, int localPort,
+                                HttpParams params)
+        throws IOException {
+
+        if (host == null) {
+            throw new IllegalArgumentException("Target host may not be null.");
+        }
+        if (params == null) {
+            throw new IllegalArgumentException("Parameters may not be null.");
+        }
+
+        // resolve the target hostname first
+        final InetSocketAddress target = new InetSocketAddress(host, port);
+
+        if (sock == null)
+            sock = createSocket();
+
+        if ((localAddress != null) || (localPort > 0)) {
+
+            // we need to bind explicitly
+            if (localPort < 0)
+                localPort = 0; // indicates "any"
+
+            InetSocketAddress isa =
+                new InetSocketAddress(localAddress, localPort);
+            sock.bind(isa);
+        }
+
+        int timeout = HttpConnectionParams.getConnectionTimeout(params);
+        sock.connect(target, timeout);
+
+        return sock;
+
+    } // connectSocket
+
+
     /**
-     * All instances of DefaultProtocolSocketFactory are the same.
+     * Compares this factory with an object.
+     * There is only one instance of this class.
+     *
+     * @param obj       the object to compare with
+     *
+     * @return  iff the argument is this object
      */
     public boolean equals(Object obj) {
-        return ((obj != null) && obj.getClass().equals(PlainSocketFactory.class));
+        return (obj == this);
     }
 
     /**
-     * All instances of DefaultProtocolSocketFactory have the same hash code.
+     * Obtains a hash code for this object.
+     * All instances of this class have the same hash code.
+     * There is only one instance of this class.
      */
     public int hashCode() {
         return PlainSocketFactory.class.hashCode();

src/java/org/apache/http/conn/SocketFactory.java

@@ -39,43 +39,60 @@ import java.net.UnknownHostException;
 import org.apache.http.params.HttpParams;
 
 /**
- * A factory for creating Sockets.
+ * A factory for creating and connecting sockets.
- * 
+ * The factory encapsulates the logic for establishing a socket connection.
- * <p>Both {@link java.lang.Object#equals(java.lang.Object) Object.equals()} and 
+ * <br/>
- * {@link java.lang.Object#hashCode() Object.hashCode()} should be overridden appropriately.  
+ * Both {@link java.lang.Object#equals(java.lang.Object) Object.equals()}
- * Protocol socket factories are used to uniquely identify <code>Protocol</code>s and 
+ * and {@link java.lang.Object#hashCode() Object.hashCode()}
- * <code>HostConfiguration</code>s, and <code>equals()</code> and <code>hashCode()</code> are 
+ * must be overridden for the correct operation of some connection managers.
- * required for the correct operation of some connection managers.</p>
- * 
- * @see Scheme
  * 
+ * @author <a href="mailto:http-async@dubioso.net">Roland Weber</a>
  * @author Michael Becke
  * @author <a href="mailto:mbowler@GargoyleSoftware.com">Mike Bowler</a>
- * 
- * @since 2.0
  */
 public interface SocketFactory {
 
     /**
-     * Gets a new socket connection to the given host.
+     * Creates a new, unconnected socket.
-     * 
+     * The socket should subsequently be passed to
-     * @param host the host name/IP
+     * {@link #connectSocket connectSocket}.
-     * @param port the port on the host
+     *
-     * @param localAddress the local host name/IP to bind the socket to
+     * @return  a new socket
-     * @param localPort the port on the local machine
-     * @param params {@link HttpParams Http parameters}
-     * 
-     * @return Socket a new socket
      * 
      * @throws IOException if an I/O error occurs while creating the socket
-     * @throws UnknownHostException if the IP address of the host cannot be
-     * determined
-     * @throws ConnectTimeoutException if socket cannot be connected within the
-     *  given time limit
-     * 
-     * @since 3.0
      */
-    Socket createSocket(
+    Socket createSocket()
+        throws IOException
+        ;
+
+
+    /**
+     * Connects a socket to the given host.
+     * 
+     * @param sock      the socket to connect, as obtained from
+     *                  {@link #createSocket createSocket}.
+     *                  <code>null</code> indicates that a new socket
+     *                  should be created and connected.
+     * @param host      the host to connect to
+     * @param port      the port to connect to on the host
+     * @param localAddress the local address to bind the socket to, or
+     *                  <code>null</code> for any
+     * @param localPort the port on the local machine,
+     *                  0 or a negative number for any
+     * @param params    additional {@link HttpParams parameters} for connecting
+     * 
+     * @return  the connected socket. The returned object may be different
+     *          from the <code>sock</code> argument if this factory supports
+     *          a layered protocol.
+     * 
+     * @throws IOException if an I/O error occurs
+     * @throws UnknownHostException if the IP address of the target host
+     *          can not be determined
+     * @throws ConnectTimeoutException if the socket cannot be connected
+     *          within the time limit defined in the <code>params</code>
+     */
+    Socket connectSocket(
+        Socket sock,
         String host, 
         int port, 
         InetAddress localAddress, 

src/java/org/apache/http/conn/impl/DefaultHttpHostConnection.java

@@ -156,8 +156,8 @@ public class DefaultHttpHostConnection
             scheme = Scheme.getScheme(target.getSchemeName());
         }
         socketFactory = scheme.getSocketFactory();
-        Socket socket = socketFactory.createSocket(
+        Socket socket = socketFactory.connectSocket(
-                hostname, port, 
+                null, hostname, port, 
                 this.hostconf.getLocalAddress(), 0, params);
 
         // Bind connection to the socket

src/java/org/apache/http/conn/ssl/SSLSocketFactory.java

@@ -56,6 +56,8 @@ import java.security.SecureRandom;
 import java.security.UnrecoverableKeyException;
 
 /**
+ * Secure socket factory based on {@link javax.net.ssl JSSE}
+ *.
  * <p>
  * SSLProtocolSocketFactory can be used to validate the identity of the HTTPS 
  * server against a list of trusted certificates and to authenticate to the HTTPS 
@@ -230,44 +232,63 @@ public class SSLSocketFactory implements SecureSocketFactory {
         return tmfactory.getTrustManagers();
     }
 
-    /**
+
-     * Attempts to get a new socket connection to the given host within the given time limit.
+    // non-javadoc, see interface org.apache.http.conn.SocketFactory
-     *  
+    public Socket createSocket()
-     * @param host the host name/IP
+        throws IOException {
-     * @param port the port on the host
+
-     * @param localAddress the local host name/IP to bind the socket to
+        // the cast makes sure that the factory is working as expected
-     * @param localPort the port on the local machine
+        return (SSLSocket) this.socketfactory.createSocket();
-     * @param params {@link HttpConnectionParams Http connection parameters}
+    }
-     * 
+
-     * @return Socket a new socket
+
-     * 
+    // non-javadoc, see interface org.apache.http.conn.SocketFactory
-     * @throws IOException if an I/O error occurs while creating the socket
+    public Socket connectSocket(
-     * @throws UnknownHostException if the IP address of the host cannot be
+        final Socket sock,
-     * determined
-     * @throws ConnectTimeoutException if socket cannot be connected within the
-     *  given time limit
-     * 
-     * @since 3.0
-     */
-    public Socket createSocket(
         final String host,
         final int port,
         final InetAddress localAddress,
-        final int localPort,
+        int localPort,
         final HttpParams params
-    ) throws IOException, UnknownHostException, ConnectTimeoutException {
+    ) throws IOException {
+
+        if (host == null) {
+            throw new IllegalArgumentException("Target host may not be null.");
+        }
         if (params == null) {
-            throw new IllegalArgumentException("Parameters may not be null");
+            throw new IllegalArgumentException("Parameters may not be null.");
         }
-        SSLSocket sslSocket = (SSLSocket) this.socketfactory.createSocket();
+
-        if (localAddress != null) {
+        // resolve the target hostname first
-            sslSocket.bind(new InetSocketAddress(localAddress, localPort));
+        final InetSocketAddress target = new InetSocketAddress(host, port);
+
+        SSLSocket sslock = (SSLSocket)
+            ((sock != null) ? sock : createSocket());
+
+        if ((localAddress != null) || (localPort > 0)) {
+
+            // we need to bind explicitly
+            if (localPort < 0)
+                localPort = 0; // indicates "any"
+
+            InetSocketAddress isa =
+                new InetSocketAddress(localAddress, localPort);
+            sslock.bind(isa);
         }
+
         int timeout = HttpConnectionParams.getConnectionTimeout(params);
-        sslSocket.connect(new InetSocketAddress(host, port), timeout);
+        sslock.connect(target, timeout);
-        hostnameVerifier.verify(host, sslSocket);
+
-        // verifyHostName() didn't blowup - good!
+        try {
-        return sslSocket;
+            hostnameVerifier.verify(host, sslock);
+            // verifyHostName() didn't blowup - good!
+        } catch (IOException iox) {
+            // close the socket before re-throwing the exception
+            try { sslock.close(); } catch (Exception x) { /*ignore*/ }
+            throw iox;
+        }
+
+        return sslock;
     }
 
     /**

src/test/org/apache/http/conn/ssl/TestSSLSocketFactory.java

@@ -151,7 +151,8 @@ public class TestSSLSocketFactory extends TestCase
             IOException[] e = new IOException[1];
             boolean[] success = new boolean[1];
             listen(serverSocket, e, success);
-            Socket s = ssf.createSocket("localhost", port, null, 0, params);
+            Socket s = ssf.connectSocket(null, "localhost", port,
+                                         null, 0, params);
             exerciseSocket(s, e, success);
 
             // Test 2 - createSocket( Socket ), where we upgrade a plain socket