You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hc.apache.org by ol...@apache.org on 2012/09/14 15:39:05 UTC
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...
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();
+ }
+ }
+
}