hc-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ol...@apache.org
Subject svn commit: r677240 - in /httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http: client/ conn/ssl/ impl/client/ impl/conn/tsccm/ impl/cookie/
Date Wed, 16 Jul 2008 11:25:48 GMT
Author: olegk
Date: Wed Jul 16 04:25:47 2008
New Revision: 677240

URL: http://svn.apache.org/viewvc?rev=677240&view=rev
Log:
* Added handling of exceptions to the HttpClient#execute methods that take a ResponseHandler
as a parameter

* Modified AbstractVerifier to use log instead of printStackTrace()

* Modified BasicResponseHandler to always throw an exception when status >= 300, even if
entity == null

* Fixed concurrency problem in ConnectionPoolByRoute. Before this change, the RouteSpecificPool
could be removed between the time that the waiting thread was notified and it actually woke
up. The code was modified to not remove the thread from the RSP until it wakes up, so pool.isUnused()
returns false when a thread is actually still waiting.

* Reordered date patterns in DateUtils. Someone claimed this enhanced performance.

* Small Javadoc fix in NetscapeDraftSpec

* Modified RFC2109Spec#formatCookies() to not modify the passed in list (it could be unmodifiable)

Contributed by Bob Lee <crazybob at crazybob.org>
Reviewed by Oleg Kalnichevski

Modified:
    httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/client/ResponseHandler.java
    httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/conn/ssl/AbstractVerifier.java
    httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/client/AbstractHttpClient.java
    httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/client/BasicResponseHandler.java
    httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/ConnPoolByRoute.java
    httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/RouteSpecificPool.java
    httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/cookie/DateUtils.java
    httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/cookie/NetscapeDraftSpec.java
    httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/cookie/RFC2109Spec.java

Modified: httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/client/ResponseHandler.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/client/ResponseHandler.java?rev=677240&r1=677239&r2=677240&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/client/ResponseHandler.java
(original)
+++ httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/client/ResponseHandler.java
Wed Jul 16 04:25:47 2008
@@ -32,7 +32,6 @@
 
 import java.io.IOException;
 
-import org.apache.http.HttpEntity;
 import org.apache.http.HttpResponse;
 
 /**
@@ -49,11 +48,6 @@
      * Processes an {@link HttpResponse} and returns some value
      * corresponding to that response.
      * 
-     * Implementations should make an effort to ensure that the
-     * response {@link HttpEntity} is fully consumed before
-     * returning from this method or document that users
-     * must consume the entity.
-     * 
      * @param response The response to process
      * @return A value determined by the response
      * 

Modified: httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/conn/ssl/AbstractVerifier.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/conn/ssl/AbstractVerifier.java?rev=677240&r1=677239&r2=677240&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/conn/ssl/AbstractVerifier.java
(original)
+++ httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/conn/ssl/AbstractVerifier.java
Wed Jul 16 04:25:47 2008
@@ -45,6 +45,8 @@
 import java.util.List;
 import java.util.Locale;
 import java.util.StringTokenizer;
+import java.util.logging.Logger;
+import java.util.logging.Level;
 
 import javax.net.ssl.SSLException;
 import javax.net.ssl.SSLSession;
@@ -299,8 +301,8 @@
             c = cert.getSubjectAlternativeNames();
         }
         catch(CertificateParsingException cpe) {
-            // Should probably log.debug() this?
-            cpe.printStackTrace();
+            Logger.getLogger(AbstractVerifier.class.getName())
+                    .log(Level.FINE, "Error parsing certificate.", cpe);
         }
         if(c != null) {
             for (List<?> aC : c) {

Modified: httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/client/AbstractHttpClient.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/client/AbstractHttpClient.java?rev=677240&r1=677239&r2=677240&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/client/AbstractHttpClient.java
(original)
+++ httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/client/AbstractHttpClient.java
Wed Jul 16 04:25:47 2008
@@ -33,7 +33,10 @@
 
 import java.io.IOException;
 import java.net.URI;
+import java.lang.reflect.UndeclaredThrowableException;
 
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
 import org.apache.http.ConnectionReuseStrategy;
 import org.apache.http.HttpException;
 import org.apache.http.HttpHost;
@@ -41,6 +44,7 @@
 import org.apache.http.HttpRequestInterceptor;
 import org.apache.http.HttpResponse;
 import org.apache.http.HttpResponseInterceptor;
+import org.apache.http.HttpEntity;
 import org.apache.http.auth.AuthSchemeRegistry;
 import org.apache.http.client.AuthenticationHandler;
 import org.apache.http.client.ClientProtocolException;
@@ -78,6 +82,8 @@
  */
 public abstract class AbstractHttpClient implements HttpClient {
 
+    private final Log log = LogFactory.getLog(getClass());
+
     /** The parameters. */
     private HttpParams defaultParams;
 
@@ -479,22 +485,24 @@
                 ("Request must not be null.");
         }
 
+        return execute(determineTarget(request), request, context);
+    }
+
+    private HttpHost determineTarget(HttpUriRequest request) {
         // A null target may be acceptable if there is a default target.
         // Otherwise, the null target is detected in the director.
         HttpHost target = null;
-        
+
         URI requestURI = request.getURI();
         if (requestURI.isAbsolute()) {
             target = new HttpHost(
-                    requestURI.getHost(), 
-                    requestURI.getPort(), 
+                    requestURI.getHost(),
+                    requestURI.getPort(),
                     requestURI.getScheme());
         }
-        
-        return execute(target, request, context);
+        return target;
     }
 
-
     // non-javadoc, see interface HttpClient
     public final HttpResponse execute(HttpHost target, HttpRequest request)
         throws IOException, ClientProtocolException {
@@ -619,12 +627,7 @@
             final HttpUriRequest request, 
             final ResponseHandler<? extends T> responseHandler) 
                 throws IOException, ClientProtocolException {
-        if (responseHandler == null) {
-            throw new IllegalArgumentException
-                ("Response handler must not be null.");
-        }
-        HttpResponse response = execute(request);
-        return responseHandler.handleResponse(response);
+        return execute(request, responseHandler, null);
     }
 
 
@@ -634,12 +637,8 @@
             final ResponseHandler<? extends T> responseHandler, 
             final HttpContext context)
                 throws IOException, ClientProtocolException {
-        if (responseHandler == null) {
-            throw new IllegalArgumentException
-                ("Response handler must not be null.");
-        }
-        HttpResponse response = execute(request, context);
-        return responseHandler.handleResponse(response);
+        HttpHost target = determineTarget(request);
+        return execute(target, request, responseHandler, context);
     }
 
 
@@ -649,12 +648,7 @@
             final HttpRequest request,
             final ResponseHandler<? extends T> responseHandler) 
                 throws IOException, ClientProtocolException {
-        if (responseHandler == null) {
-            throw new IllegalArgumentException
-                ("Response handler must not be null.");
-        }
-        HttpResponse response = execute(target, request);
-        return responseHandler.handleResponse(response);
+        return execute(target, request, responseHandler, null);
     }
 
 
@@ -669,8 +663,48 @@
             throw new IllegalArgumentException
                 ("Response handler must not be null.");
         }
+
         HttpResponse response = execute(target, request, context);
-        return responseHandler.handleResponse(response);
+
+        T result;
+        try {
+            result = responseHandler.handleResponse(response);
+        } catch (Throwable t) {
+            HttpEntity entity = response.getEntity();
+            if (entity != null) {
+                try {
+                    entity.consumeContent();
+                } catch (Throwable t2) {
+                    // Log this exception. The original exception is more
+                    // important and will be thrown to the caller.
+                    this.log.warn("Error consuming content after an exception.", t2);
+                }
+            }
+
+            if (t instanceof Error) {
+                throw (Error) t;
+            }
+
+            if (t instanceof RuntimeException) {
+                throw (RuntimeException) t;
+            }
+
+            if (t instanceof IOException) {
+                throw (IOException) t;
+            }
+            
+            throw new UndeclaredThrowableException(t);
+        }
+
+        // Handling the response was successful. Ensure that the content has
+        // been fully consumed.
+        HttpEntity entity = response.getEntity();
+        if (entity != null) {
+            // Let this exception go to the caller.
+            entity.consumeContent();
+        }
+
+        return result;
     }
 
 

Modified: httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/client/BasicResponseHandler.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/client/BasicResponseHandler.java?rev=677240&r1=677239&r2=677240&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/client/BasicResponseHandler.java
(original)
+++ httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/client/BasicResponseHandler.java
Wed Jul 16 04:25:47 2008
@@ -36,7 +36,6 @@
 import org.apache.http.HttpEntity;
 import org.apache.http.HttpResponse;
 import org.apache.http.StatusLine;
-import org.apache.http.client.ClientProtocolException;
 import org.apache.http.client.ResponseHandler;
 import org.apache.http.client.HttpResponseException;
 import org.apache.http.util.EntityUtils;
@@ -47,7 +46,8 @@
  * body is consumed and an {@link HttpResponseException} is thrown.
  * 
  * If this is used with
- * {@link HttpClient#execute(org.apache.http.client.methods.HttpUriRequest, ClientResponseHandler),
+ * {@link org.apache.http.client.HttpClient#execute(
+ *  org.apache.http.client.methods.HttpUriRequest, ResponseHandler),
  * HttpClient may handle redirects (3xx responses) internally.
  * 
  * @author <a href="mailto:oleg at ural.ru">Oleg Kalnichevski</a>
@@ -64,20 +64,16 @@
      * response was unsuccessful (>= 300 status code), throws an
      * {@link HttpResponseException}.
      */
-    public String handleResponse(
-            final HttpResponse response) throws ClientProtocolException, IOException {
-        HttpEntity entity = response.getEntity();
-        if (entity != null) {
-            StatusLine statusLine = response.getStatusLine();
-            if (statusLine.getStatusCode() >= 300) {
-                entity.consumeContent();
-                throw new HttpResponseException(statusLine.getStatusCode(), statusLine.getReasonPhrase());
-            } else {
-                return EntityUtils.toString(entity);
-            }
-        } else {
-            return null;
+    public String handleResponse(final HttpResponse response)
+            throws HttpResponseException, IOException {
+        StatusLine statusLine = response.getStatusLine();
+        if (statusLine.getStatusCode() >= 300) {
+            throw new HttpResponseException(statusLine.getStatusCode(),
+                    statusLine.getReasonPhrase());
         }
+
+        HttpEntity entity = response.getEntity();
+        return entity == null ? null : EntityUtils.toString(entity);
     }
 
 }

Modified: httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/ConnPoolByRoute.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/ConnPoolByRoute.java?rev=677240&r1=677239&r2=677240&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/ConnPoolByRoute.java
(original)
+++ httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/ConnPoolByRoute.java
Wed Jul 16 04:25:47 2008
@@ -343,14 +343,8 @@
                         // connection pool and should now have a connection
                         // waiting for us, or else we're shutting down.
                         // Just continue in the loop, both cases are checked.
-                        if (!success) {
-                            // Either we timed out, experienced a
-                            // "spurious wakeup", or were interrupted by
-                            // an external thread. Regardless, we need to 
-                            // cleanup for ourselves in the wait queue.
-                            rospl.removeThread(waitingThread);
-                            waitingThreads.remove(waitingThread);
-                        }
+                        rospl.removeThread(waitingThread);
+                        waitingThreads.remove(waitingThread);
                     }
 
                     // check for spurious wakeup vs. timeout
@@ -622,16 +616,12 @@
                     log.debug("Notifying thread waiting on pool" +
                             " [" + rospl.getRoute() + "]");
                 }
-                waitingThread = rospl.dequeueThread();
-                waitingThreads.remove(waitingThread);
-
+                waitingThread = rospl.nextThread();
             } else if (!waitingThreads.isEmpty()) {
                 if (log.isDebugEnabled()) {
                     log.debug("Notifying thread waiting on any pool");
                 }
                 waitingThread = waitingThreads.remove();
-                waitingThread.getPool().removeThread(waitingThread);
-
             } else if (log.isDebugEnabled()) {
                 log.debug("Notifying no-one, there are no waiting threads");
             }

Modified: httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/RouteSpecificPool.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/RouteSpecificPool.java?rev=677240&r1=677239&r2=677240&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/RouteSpecificPool.java
(original)
+++ httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/conn/tsccm/RouteSpecificPool.java
Wed Jul 16 04:25:47 2008
@@ -276,12 +276,12 @@
 
 
     /**
-     * Obtains and removes a waiting thread.
+     * Returns the next thread in the queue.
      *
      * @return  a waiting thread, or <code>null</code> if there is none
      */
-    public WaitingThread dequeueThread() {
-        return this.waitingThreads.poll();
+    public WaitingThread nextThread() {
+        return this.waitingThreads.peek();
     }
 
 

Modified: httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/cookie/DateUtils.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/cookie/DateUtils.java?rev=677240&r1=677239&r2=677240&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/cookie/DateUtils.java
(original)
+++ httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/cookie/DateUtils.java
Wed Jul 16 04:25:47 2008
@@ -67,11 +67,12 @@
      */
     public static final String PATTERN_ASCTIME = "EEE MMM d HH:mm:ss yyyy";
 
-    private static final String[] DEFAULT_PATTERNS = new String[] { 
-        PATTERN_ASCTIME, 
-        PATTERN_RFC1036, 
-        PATTERN_RFC1123 };
-    
+    private static final String[] DEFAULT_PATTERNS = new String[] {
+    	PATTERN_RFC1036,
+    	PATTERN_RFC1123,
+        PATTERN_ASCTIME
+    };
+
     private static final Date DEFAULT_TWO_DIGIT_YEAR_START;
     
     public static final TimeZone GMT = TimeZone.getTimeZone("GMT");

Modified: httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/cookie/NetscapeDraftSpec.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/cookie/NetscapeDraftSpec.java?rev=677240&r1=677239&r2=677240&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/cookie/NetscapeDraftSpec.java
(original)
+++ httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/cookie/NetscapeDraftSpec.java
Wed Jul 16 04:25:47 2008
@@ -109,8 +109,9 @@
       * character may be present in unquoted cookie value or unquoted 
       * parameter value.</p>
       * 
-      * @link http://wp.netscape.com/newsref/std/cookie_spec.html
-      * 
+      * @see <a href="http://wp.netscape.com/newsref/std/cookie_spec.html">
+      *  The Cookie Spec.</a>
+      *
       * @param header the <tt>Set-Cookie</tt> received from the server
       * @return an array of <tt>Cookie</tt>s parsed from the Set-Cookie value
       * @throws MalformedCookieException if an exception occurs during parsing

Modified: httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/cookie/RFC2109Spec.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/cookie/RFC2109Spec.java?rev=677240&r1=677239&r2=677240&view=diff
==============================================================================
--- httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/cookie/RFC2109Spec.java
(original)
+++ httpcomponents/httpclient/trunk/module-client/src/main/java/org/apache/http/impl/cookie/RFC2109Spec.java
Wed Jul 16 04:25:47 2008
@@ -128,14 +128,18 @@
         super.validate(cookie, origin);
     }
 
-    public List<Header> formatCookies(final List<Cookie> cookies) {
+    public List<Header> formatCookies(List<Cookie> cookies) {
         if (cookies == null) {
             throw new IllegalArgumentException("List of cookies may not be null");
         }
         if (cookies.isEmpty()) {
             throw new IllegalArgumentException("List of cookies may not be empty");
         }
-        Collections.sort(cookies, PATH_COMPARATOR);
+        if (cookies.size() > 1) {
+            // Create a mutable copy and sort the copy.
+            cookies = new ArrayList<Cookie>(cookies);
+            Collections.sort(cookies, PATH_COMPARATOR);
+        }
         if (this.oneHeader) {
             return doFormatOneHeader(cookies);
         } else {



Mime
View raw message