hc-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ol...@apache.org
Subject svn commit: r1384777 - in /httpcomponents/httpcore/trunk: ./ httpcore-nio/src/test/java/org/apache/http/nio/integration/ httpcore/src/main/java/org/apache/http/impl/ httpcore/src/test/java/org/apache/http/impl/ httpcore/src/test/java/org/apache/http/in...
Date Fri, 14 Sep 2012 13:39:05 GMT
Author: olegk
Date: Fri Sep 14 13:39:04 2012
New Revision: 1384777

URL: http://svn.apache.org/viewvc?rev=1384777&view=rev
Log:
HTTPCORE-310: Fixed regression in DefaultConnectionReuseStrategy causing it to incorrectly
flag connections as non-reusable after a 204, 205 or 304 response

Modified:
    httpcomponents/httpcore/trunk/RELEASE_NOTES.txt
    httpcomponents/httpcore/trunk/httpcore-nio/src/test/java/org/apache/http/nio/integration/TestHttpAsyncHandlers.java
    httpcomponents/httpcore/trunk/httpcore/src/main/java/org/apache/http/impl/DefaultConnectionReuseStrategy.java
    httpcomponents/httpcore/trunk/httpcore/src/test/java/org/apache/http/impl/TestDefaultConnectionReuseStrategy.java
    httpcomponents/httpcore/trunk/httpcore/src/test/java/org/apache/http/integration/TestSyncHttp.java

Modified: httpcomponents/httpcore/trunk/RELEASE_NOTES.txt
URL: http://svn.apache.org/viewvc/httpcomponents/httpcore/trunk/RELEASE_NOTES.txt?rev=1384777&r1=1384776&r2=1384777&view=diff
==============================================================================
--- httpcomponents/httpcore/trunk/RELEASE_NOTES.txt (original)
+++ httpcomponents/httpcore/trunk/RELEASE_NOTES.txt Fri Sep 14 13:39:04 2012
@@ -1,7 +1,11 @@
 Changes since 4.2.1
 
-* [HTTPCORE-309] Fixed regression causing HttpAsyncRequestExecutor to handle 204, 205, 304

-  responses incorrectly by returning a message with an enclosed content body.
+* [HTTPCORE-310] Fixed regression in DefaultConnectionReuseStrategy causing it to incorrectly

+  flag connections as non-reusable after a 204, 205 or 304 response. 
+  Contributed by Oleg Kalnichevski <olegk at apache.org>
+
+* [HTTPCORE-309] Fixed regression in HttpAsyncRequestExecutor causing it to handle 204, 205

+  and 304 responses incorrectly by returning a message with an enclosed content body.
   Contributed by Oleg Kalnichevski <olegk at apache.org>
 
 * [HTTPCORE-306] I/O reactor TCP_NODELAY parameter now defaults to true.
@@ -14,7 +18,6 @@ Changes since 4.2.1
 * [HTTPCORE-303] ContentType made Serializable. 
   Contributed by Oleg Kalnichevski <olegk at apache.org>
 
-
 Release 4.2.1
 -------------------
 

Modified: httpcomponents/httpcore/trunk/httpcore-nio/src/test/java/org/apache/http/nio/integration/TestHttpAsyncHandlers.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpcore/trunk/httpcore-nio/src/test/java/org/apache/http/nio/integration/TestHttpAsyncHandlers.java?rev=1384777&r1=1384776&r2=1384777&view=diff
==============================================================================
--- httpcomponents/httpcore/trunk/httpcore-nio/src/test/java/org/apache/http/nio/integration/TestHttpAsyncHandlers.java
(original)
+++ httpcomponents/httpcore/trunk/httpcore-nio/src/test/java/org/apache/http/nio/integration/TestHttpAsyncHandlers.java
Fri Sep 14 13:39:04 2012
@@ -74,6 +74,7 @@ import org.apache.http.params.CoreProtoc
 import org.apache.http.params.HttpParams;
 import org.apache.http.protocol.HTTP;
 import org.apache.http.protocol.HttpContext;
+import org.apache.http.protocol.HttpRequestHandler;
 import org.apache.http.protocol.ImmutableHttpProcessor;
 import org.apache.http.protocol.RequestConnControl;
 import org.apache.http.protocol.RequestExpectContinue;
@@ -787,4 +788,42 @@ public class TestHttpAsyncHandlers exten
         }
     }
 
+    @Test
+    public void testResponseNoContent() throws Exception {
+        UriHttpAsyncRequestHandlerMapper registry = new UriHttpAsyncRequestHandlerMapper();
+        registry.register("*", new BasicAsyncRequestHandler(new HttpRequestHandler() {
+            
+            public void handle(
+                    final HttpRequest request, 
+                    final HttpResponse response, 
+                    final HttpContext context) throws HttpException, IOException {
+                response.setStatusCode(HttpStatus.SC_NO_CONTENT);
+            }
+        
+        }));
+        InetSocketAddress address = start(registry, null);
+
+        this.connpool.setDefaultMaxPerRoute(3);
+        this.connpool.setMaxTotal(3);
+
+        HttpHost target = new HttpHost("localhost", address.getPort());
+
+        Queue<Future<HttpResponse>> queue = new ConcurrentLinkedQueue<Future<HttpResponse>>();
+        for (int i = 0; i < 30; i++) {
+            BasicHttpRequest request = new BasicHttpRequest("GET", "/");
+            Future<HttpResponse> future = this.executor.execute(
+                    new BasicAsyncRequestProducer(target, request),
+                    new BasicAsyncResponseConsumer(),
+                    this.connpool);
+            queue.add(future);
+        }
+
+        while (!queue.isEmpty()) {
+            Future<HttpResponse> future = queue.remove();
+            HttpResponse response = future.get();
+            Assert.assertNotNull(response);
+            Assert.assertNull(response.getEntity());
+        }
+    }
+
 }

Modified: httpcomponents/httpcore/trunk/httpcore/src/main/java/org/apache/http/impl/DefaultConnectionReuseStrategy.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpcore/trunk/httpcore/src/main/java/org/apache/http/impl/DefaultConnectionReuseStrategy.java?rev=1384777&r1=1384776&r2=1384777&view=diff
==============================================================================
--- httpcomponents/httpcore/trunk/httpcore/src/main/java/org/apache/http/impl/DefaultConnectionReuseStrategy.java
(original)
+++ httpcomponents/httpcore/trunk/httpcore/src/main/java/org/apache/http/impl/DefaultConnectionReuseStrategy.java
Fri Sep 14 13:39:04 2012
@@ -31,6 +31,7 @@ import org.apache.http.ConnectionReuseSt
 import org.apache.http.Header;
 import org.apache.http.HeaderIterator;
 import org.apache.http.HttpResponse;
+import org.apache.http.HttpStatus;
 import org.apache.http.HttpVersion;
 import org.apache.http.ParseException;
 import org.apache.http.ProtocolVersion;
@@ -83,19 +84,22 @@ public class DefaultConnectionReuseStrat
                 return false;
             }
         } else {
-            Header[] clhs = response.getHeaders(HTTP.CONTENT_LEN);
-            // Do not reuse if not properly content-length delimited
-            if (clhs == null || clhs.length != 1) {
-                return false;
-            }
-            Header clh = clhs[0];
-            try {
-                int contentLen = Integer.parseInt(clh.getValue());
-                if (contentLen < 0) {
+            if (canResponseHaveBody(response)) {
+                Header[] clhs = response.getHeaders(HTTP.CONTENT_LEN);
+                // Do not reuse if not properly content-length delimited
+                if (clhs.length == 1) {
+                    Header clh = clhs[0];
+                    try {
+                        int contentLen = Integer.parseInt(clh.getValue());
+                        if (contentLen < 0) {
+                            return false;
+                        }
+                    } catch (NumberFormatException ex) {
+                        return false;
+                    }
+                } else {
                     return false;
                 }
-            } catch (NumberFormatException ex) {
-                return false;
             }
         }
 
@@ -170,4 +174,13 @@ public class DefaultConnectionReuseStrat
     protected TokenIterator createTokenIterator(HeaderIterator hit) {
         return new BasicTokenIterator(hit);
     }
+
+    private boolean canResponseHaveBody(final HttpResponse response) {
+        int status = response.getStatusLine().getStatusCode();
+        return status >= HttpStatus.SC_OK
+            && status != HttpStatus.SC_NO_CONTENT
+            && status != HttpStatus.SC_NOT_MODIFIED
+            && status != HttpStatus.SC_RESET_CONTENT;
+    }
+
 }

Modified: httpcomponents/httpcore/trunk/httpcore/src/test/java/org/apache/http/impl/TestDefaultConnectionReuseStrategy.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpcore/trunk/httpcore/src/test/java/org/apache/http/impl/TestDefaultConnectionReuseStrategy.java?rev=1384777&r1=1384776&r2=1384777&view=diff
==============================================================================
--- httpcomponents/httpcore/trunk/httpcore/src/test/java/org/apache/http/impl/TestDefaultConnectionReuseStrategy.java
(original)
+++ httpcomponents/httpcore/trunk/httpcore/src/test/java/org/apache/http/impl/TestDefaultConnectionReuseStrategy.java
Fri Sep 14 13:39:04 2012
@@ -29,6 +29,7 @@ package org.apache.http.impl;
 
 import org.apache.http.ConnectionReuseStrategy;
 import org.apache.http.HttpResponse;
+import org.apache.http.HttpStatus;
 import org.apache.http.HttpVersion;
 import org.apache.http.message.BasicHttpResponse;
 import org.apache.http.protocol.BasicHttpContext;
@@ -243,5 +244,21 @@ public class TestDefaultConnectionReuseS
         Assert.assertFalse(reuseStrategy.keepAlive(response, context));
     }
 
+    @Test
+    public void testNoContentResponse() throws Exception {
+        // Use HTTP 1.1
+        HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1, 
+                HttpStatus.SC_NO_CONTENT, "No Content");
+        Assert.assertTrue(reuseStrategy.keepAlive(response, context));
+    }
+
+    @Test
+    public void testNoContentResponseHttp10() throws Exception {
+        // Use HTTP 1.0
+        HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_0, 
+                HttpStatus.SC_NO_CONTENT, "No Content");
+        Assert.assertFalse(reuseStrategy.keepAlive(response, context));
+    }
+
 }
 

Modified: httpcomponents/httpcore/trunk/httpcore/src/test/java/org/apache/http/integration/TestSyncHttp.java
URL: http://svn.apache.org/viewvc/httpcomponents/httpcore/trunk/httpcore/src/test/java/org/apache/http/integration/TestSyncHttp.java?rev=1384777&r1=1384776&r2=1384777&view=diff
==============================================================================
--- httpcomponents/httpcore/trunk/httpcore/src/test/java/org/apache/http/integration/TestSyncHttp.java
(original)
+++ httpcomponents/httpcore/trunk/httpcore/src/test/java/org/apache/http/integration/TestSyncHttp.java
Fri Sep 14 13:39:04 2012
@@ -934,4 +934,53 @@ public class TestSyncHttp {
         }
     }
 
+    @Test
+    public void testNoContentResponse() throws Exception {
+
+        int reqNo = 20;
+
+        // Initialize the server-side request handler
+        this.server.registerHandler("*", new HttpRequestHandler() {
+
+            public void handle(
+                    final HttpRequest request,
+                    final HttpResponse response,
+                    final HttpContext context) throws HttpException, IOException {
+                response.setStatusCode(HttpStatus.SC_NO_CONTENT);
+            }
+
+        });
+
+        this.server.start();
+
+        DefaultHttpClientConnection conn = new DefaultHttpClientConnection();
+        HttpHost host = new HttpHost("localhost", this.server.getPort());
+
+        try {
+            for (int r = 0; r < reqNo; r++) {
+                if (!conn.isOpen()) {
+                    Socket socket = new Socket(host.getHostName(), host.getPort());
+                    conn.bind(socket, this.client.getParams());
+                }
+
+                BasicHttpRequest get = new BasicHttpRequest("GET", "/?" + r);
+                HttpResponse response = this.client.execute(get, host, conn);
+                Assert.assertNull(response.getEntity());
+                if (!this.client.keepAlive(response)) {
+                    conn.close();
+                    Assert.fail("Connection expected to be re-usable");
+                }
+            }
+
+            //Verify the connection metrics
+            HttpConnectionMetrics cm = conn.getMetrics();
+            Assert.assertEquals(reqNo, cm.getRequestCount());
+            Assert.assertEquals(reqNo, cm.getResponseCount());
+
+        } finally {
+            conn.close();
+            this.server.shutdown();
+        }
+    }
+
 }



Mime
View raw message