Mailing-List: contact httpcomponents-commits-help@jakarta.apache.org;
 run by ezmlm
Precedence: bulk
Reply-To: httpcomponents-dev@jakarta.apache.org
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r490691 - in
 /jakarta/httpcomponents/httpclient/trunk/src:
 java/org/apache/http/conn/ java/org/apache/http/conn/impl/
 java/org/apache/http/conn/ssl/ test/org/apache/http/conn/ssl/
Date: Thu, 28 Dec 2006 10:04:57 -0000
To: httpcomponents-commits@jakarta.apache.org
From: rolandw@apache.org
Message-Id: <20061228100457.DC6C71A981A@eris.apache.org>

Author: rolandw
Date: Thu Dec 28 02:04:56 2006
New Revision: 490691

URL: http://svn.apache.org/viewvc?view=rev&rev=490691
Log:
HTTPCLIENT-475, take 1 with suggested modifications

Modified:
    jakarta/httpcomponents/httpclient/trunk/src/java/org/apache/http/conn/PlainSocketFactory.java
    jakarta/httpcomponents/httpclient/trunk/src/java/org/apache/http/conn/SocketFactory.java
    jakarta/httpcomponents/httpclient/trunk/src/java/org/apache/http/conn/impl/DefaultHttpHostConnection.java
    jakarta/httpcomponents/httpclient/trunk/src/java/org/apache/http/conn/ssl/SSLSocketFactory.java
    jakarta/httpcomponents/httpclient/trunk/src/test/org/apache/http/conn/ssl/TestSSLSocketFactory.java

Modified: jakarta/httpcomponents/httpclient/trunk/src/java/org/apache/http/conn/PlainSocketFactory.java
URL: http://svn.apache.org/viewvc/jakarta/httpcomponents/httpclient/trunk/src/java/org/apache/http/conn/PlainSocketFactory.java?view=diff&rev=490691&r1=490690&r2=490691
==============================================================================
--- jakarta/httpcomponents/httpclient/trunk/src/java/org/apache/http/conn/PlainSocketFactory.java (original)
+++ jakarta/httpcomponents/httpclient/trunk/src/java/org/apache/http/conn/PlainSocketFactory.java Thu Dec 28 02:04:56 2006
@@ -33,6 +33,7 @@
 
 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.HttpParams;
 
 /**
- * The default class for creating protocol sockets.  This class just uses the
- * {@link java.net.Socket socket} constructors.
+ * The default class for creating sockets.
+ * 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.
-     * @return a DefaultProtocolSocketFactory
+     * Gets the singleton instance of this class.
+     * @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.
-     * This socket factory does not support connect timeout as it requires Java 1.4
-     * functionality.
-     *  
-     * @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");
+
+    // non-javadoc, see interface org.apache.http.conn.SocketFactory
+    public Socket createSocket() {
+        return new Socket();
+    }
+
+    // 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.");
         }
-        int timeout = HttpConnectionParams.getConnectionTimeout(params);
-        if (timeout != 0) {
-            throw new IllegalStateException("Connection timeout is not supported in old IO mode");
+        if (params == null) {
+            throw new IllegalArgumentException("Parameters may not be null.");
         }
-        if (localAddress != null) {
-            return new Socket(host, port, localAddress, localPort);
-        } else {
-            return new Socket(host, port);
+
+        // 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();

Modified: jakarta/httpcomponents/httpclient/trunk/src/java/org/apache/http/conn/SocketFactory.java
URL: http://svn.apache.org/viewvc/jakarta/httpcomponents/httpclient/trunk/src/java/org/apache/http/conn/SocketFactory.java?view=diff&rev=490691&r1=490690&r2=490691
==============================================================================
--- jakarta/httpcomponents/httpclient/trunk/src/java/org/apache/http/conn/SocketFactory.java (original)
+++ jakarta/httpcomponents/httpclient/trunk/src/java/org/apache/http/conn/SocketFactory.java Thu Dec 28 02:04:56 2006
@@ -39,43 +39,60 @@
 import org.apache.http.params.HttpParams;
 
 /**
- * A factory for creating Sockets.
- * 
- * <p>Both {@link java.lang.Object#equals(java.lang.Object) Object.equals()} and 
- * {@link java.lang.Object#hashCode() Object.hashCode()} should be overridden appropriately.  
- * Protocol socket factories are used to uniquely identify <code>Protocol</code>s and 
- * <code>HostConfiguration</code>s, and <code>equals()</code> and <code>hashCode()</code> are 
- * required for the correct operation of some connection managers.</p>
- * 
- * @see Scheme
+ * A factory for creating and connecting sockets.
+ * The factory encapsulates the logic for establishing a socket connection.
+ * <br/>
+ * Both {@link java.lang.Object#equals(java.lang.Object) Object.equals()}
+ * and {@link java.lang.Object#hashCode() Object.hashCode()}
+ * must be overridden for the correct operation of some connection managers.
  * 
+ * @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
+     * {@link #connectSocket connectSocket}.
+     *
+     * @return  a 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 HttpParams Http parameters}
+     * @throws IOException if an I/O error occurs while creating the socket
+     */
+    Socket createSocket()
+        throws IOException
+        ;
+
+
+    /**
+     * Connects a socket to the given host.
      * 
-     * @return Socket a new socket
+     * @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
      * 
-     * @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
+     * @return  the connected socket. The returned object may be different
+     *          from the <code>sock</code> argument if this factory supports
+     *          a layered protocol.
      * 
-     * @since 3.0
+     * @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 createSocket(
+    Socket connectSocket(
+        Socket sock,
         String host, 
         int port, 
         InetAddress localAddress, 

Modified: jakarta/httpcomponents/httpclient/trunk/src/java/org/apache/http/conn/impl/DefaultHttpHostConnection.java
URL: http://svn.apache.org/viewvc/jakarta/httpcomponents/httpclient/trunk/src/java/org/apache/http/conn/impl/DefaultHttpHostConnection.java?view=diff&rev=490691&r1=490690&r2=490691
==============================================================================
--- jakarta/httpcomponents/httpclient/trunk/src/java/org/apache/http/conn/impl/DefaultHttpHostConnection.java (original)
+++ jakarta/httpcomponents/httpclient/trunk/src/java/org/apache/http/conn/impl/DefaultHttpHostConnection.java Thu Dec 28 02:04:56 2006
@@ -156,8 +156,8 @@
             scheme = Scheme.getScheme(target.getSchemeName());
         }
         socketFactory = scheme.getSocketFactory();
-        Socket socket = socketFactory.createSocket(
-                hostname, port, 
+        Socket socket = socketFactory.connectSocket(
+                null, hostname, port, 
                 this.hostconf.getLocalAddress(), 0, params);
 
         // Bind connection to the socket

Modified: jakarta/httpcomponents/httpclient/trunk/src/java/org/apache/http/conn/ssl/SSLSocketFactory.java
URL: http://svn.apache.org/viewvc/jakarta/httpcomponents/httpclient/trunk/src/java/org/apache/http/conn/ssl/SSLSocketFactory.java?view=diff&rev=490691&r1=490690&r2=490691
==============================================================================
--- jakarta/httpcomponents/httpclient/trunk/src/java/org/apache/http/conn/ssl/SSLSocketFactory.java (original)
+++ jakarta/httpcomponents/httpclient/trunk/src/java/org/apache/http/conn/ssl/SSLSocketFactory.java Thu Dec 28 02:04:56 2006
@@ -56,6 +56,8 @@
 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 @@
         return tmfactory.getTrustManagers();
     }
 
-    /**
-     * Attempts to get a new socket connection to the given host within the given time limit.
-     *  
-     * @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
-     * determined
-     * @throws ConnectTimeoutException if socket cannot be connected within the
-     *  given time limit
-     * 
-     * @since 3.0
-     */
-    public Socket createSocket(
+
+    // non-javadoc, see interface org.apache.http.conn.SocketFactory
+    public Socket createSocket()
+        throws IOException {
+
+        // the cast makes sure that the factory is working as expected
+        return (SSLSocket) this.socketfactory.createSocket();
+    }
+
+
+    // non-javadoc, see interface org.apache.http.conn.SocketFactory
+    public Socket connectSocket(
+        final Socket sock,
         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) {
-            sslSocket.bind(new InetSocketAddress(localAddress, localPort));
+
+        // resolve the target hostname first
+        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);
-        hostnameVerifier.verify(host, sslSocket);
-        // verifyHostName() didn't blowup - good!
-        return sslSocket;
+        sslock.connect(target, timeout);
+
+        try {
+            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;
     }
 
     /**

Modified: jakarta/httpcomponents/httpclient/trunk/src/test/org/apache/http/conn/ssl/TestSSLSocketFactory.java
URL: http://svn.apache.org/viewvc/jakarta/httpcomponents/httpclient/trunk/src/test/org/apache/http/conn/ssl/TestSSLSocketFactory.java?view=diff&rev=490691&r1=490690&r2=490691
==============================================================================
--- jakarta/httpcomponents/httpclient/trunk/src/test/org/apache/http/conn/ssl/TestSSLSocketFactory.java (original)
+++ jakarta/httpcomponents/httpclient/trunk/src/test/org/apache/http/conn/ssl/TestSSLSocketFactory.java Thu Dec 28 02:04:56 2006
@@ -151,7 +151,8 @@
             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