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 2017/05/09 20:03:57 UTC
[28/35] httpcomponents-core git commit: Merged HTTPCORE-257 from trunk
Merged HTTPCORE-257 from trunk
git-svn-id: https://svn.apache.org/repos/asf/httpcomponents/httpcore/branches/4.1.x@1215569 13f79535-47bb-0310-9956-ffa450edef68
Project: http://git-wip-us.apache.org/repos/asf/httpcomponents-core/repo
Commit: http://git-wip-us.apache.org/repos/asf/httpcomponents-core/commit/a92bd33a
Tree: http://git-wip-us.apache.org/repos/asf/httpcomponents-core/tree/a92bd33a
Diff: http://git-wip-us.apache.org/repos/asf/httpcomponents-core/diff/a92bd33a
Branch: refs/heads/4.1.x
Commit: a92bd33a59d3e208438cb99becc71a257a0c5f14
Parents: ab58c97
Author: Oleg Kalnichevski <ol...@apache.org>
Authored: Sat Dec 17 20:59:41 2011 +0000
Committer: Oleg Kalnichevski <ol...@apache.org>
Committed: Sat Dec 17 20:59:41 2011 +0000
----------------------------------------------------------------------
RELEASE_NOTES.txt | 8 +-
.../impl/DefaultConnectionReuseStrategy.java | 35 +--
.../entity/StrictContentLengthStrategy.java | 3 +
.../TestDefaultConnectionReuseStrategy.java | 237 +++++--------------
.../entity/TestStrictContentLengthStrategy.java | 11 +
5 files changed, 91 insertions(+), 203 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/httpcomponents-core/blob/a92bd33a/RELEASE_NOTES.txt
----------------------------------------------------------------------
diff --git a/RELEASE_NOTES.txt b/RELEASE_NOTES.txt
index cf2dadf..8a3e3b9 100644
--- a/RELEASE_NOTES.txt
+++ b/RELEASE_NOTES.txt
@@ -1,6 +1,10 @@
Changes since 4.1.3
-------------------
+* [HTTPCORE-257] Fixed incorrect results produced by DefaultConnectionReuseStrategy when handling
+ response messages whose content entity has been decoded or modified by a protocol interceptor.
+ Contributed by Oleg Kalnichevski <olegk at apache.org>
+
* [HTTPCORE-283] Workaround for a bug causing termination of the I/O reactor in case of exception
thrown by NHttpServiceHandler#requestReceived or NHttpClientHandler#responseReceived
methods. A more comprehensive fix for the bug applied to the (4.2 branch).
@@ -47,10 +51,6 @@ recommendations in javadocs.
truncation when message content is chunk coded and the connection is closed on the opposite end.
Contributed by Oleg Kalnichevski <olegk at apache.org>
-* [HTTPCORE-257] Fixed incorrect results produced by DefaultConnectionReuseStrategy when handling
- response messages whose content entity has been decoded or modified by a protocol interceptor.
- Contributed by Oleg Kalnichevski <olegk at apache.org>
-
Release 4.1.1
-------------------
http://git-wip-us.apache.org/repos/asf/httpcomponents-core/blob/a92bd33a/httpcore/src/main/java/org/apache/http/impl/DefaultConnectionReuseStrategy.java
----------------------------------------------------------------------
diff --git a/httpcore/src/main/java/org/apache/http/impl/DefaultConnectionReuseStrategy.java b/httpcore/src/main/java/org/apache/http/impl/DefaultConnectionReuseStrategy.java
index 47828f6..865da5b 100644
--- a/httpcore/src/main/java/org/apache/http/impl/DefaultConnectionReuseStrategy.java
+++ b/httpcore/src/main/java/org/apache/http/impl/DefaultConnectionReuseStrategy.java
@@ -28,16 +28,14 @@
package org.apache.http.impl;
import org.apache.http.ConnectionReuseStrategy;
-import org.apache.http.HttpConnection;
+import org.apache.http.Header;
import org.apache.http.HeaderIterator;
-import org.apache.http.HttpEntity;
import org.apache.http.HttpResponse;
import org.apache.http.HttpVersion;
import org.apache.http.ParseException;
import org.apache.http.ProtocolVersion;
import org.apache.http.protocol.HTTP;
import org.apache.http.protocol.HttpContext;
-import org.apache.http.protocol.ExecutionContext;
import org.apache.http.TokenIterator;
import org.apache.http.message.BasicTokenIterator;
@@ -77,25 +75,28 @@ public class DefaultConnectionReuseStrategy implements ConnectionReuseStrategy {
("HTTP context may not be null.");
}
- HttpConnection conn = (HttpConnection)
- context.getAttribute(ExecutionContext.HTTP_CONNECTION);
-
- if (conn != null && !conn.isOpen())
- return false;
- // do NOT check for stale connection, that is an expensive operation
-
// Check for a self-terminating entity. If the end of the entity will
// be indicated by closing the connection, there is no keep-alive.
- HttpEntity entity = response.getEntity();
ProtocolVersion ver = response.getStatusLine().getProtocolVersion();
- if (entity != null) {
- if (entity.getContentLength() < 0) {
- if (!entity.isChunked() ||
- ver.lessEquals(HttpVersion.HTTP_1_0)) {
- // if the content length is not known and is not chunk
- // encoded, the connection cannot be reused
+ Header teh = response.getFirstHeader(HTTP.TRANSFER_ENCODING);
+ if (teh != null) {
+ if (!HTTP.CHUNK_CODING.equalsIgnoreCase(teh.getValue())) {
+ 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) {
return false;
}
+ } catch (NumberFormatException ex) {
+ return false;
}
}
http://git-wip-us.apache.org/repos/asf/httpcomponents-core/blob/a92bd33a/httpcore/src/main/java/org/apache/http/impl/entity/StrictContentLengthStrategy.java
----------------------------------------------------------------------
diff --git a/httpcore/src/main/java/org/apache/http/impl/entity/StrictContentLengthStrategy.java b/httpcore/src/main/java/org/apache/http/impl/entity/StrictContentLengthStrategy.java
index 18f772e..8575d3d 100644
--- a/httpcore/src/main/java/org/apache/http/impl/entity/StrictContentLengthStrategy.java
+++ b/httpcore/src/main/java/org/apache/http/impl/entity/StrictContentLengthStrategy.java
@@ -79,6 +79,9 @@ public class StrictContentLengthStrategy implements ContentLengthStrategy {
String s = contentLengthHeader.getValue();
try {
long len = Long.parseLong(s);
+ if (len < 0) {
+ throw new ProtocolException("Negative content length: " + s);
+ }
return len;
} catch (NumberFormatException e) {
throw new ProtocolException("Invalid content length: " + s);
http://git-wip-us.apache.org/repos/asf/httpcomponents-core/blob/a92bd33a/httpcore/src/test/java/org/apache/http/impl/TestDefaultConnectionReuseStrategy.java
----------------------------------------------------------------------
diff --git a/httpcore/src/test/java/org/apache/http/impl/TestDefaultConnectionReuseStrategy.java b/httpcore/src/test/java/org/apache/http/impl/TestDefaultConnectionReuseStrategy.java
index a52ff5f..3cc3ba5 100644
--- a/httpcore/src/test/java/org/apache/http/impl/TestDefaultConnectionReuseStrategy.java
+++ b/httpcore/src/test/java/org/apache/http/impl/TestDefaultConnectionReuseStrategy.java
@@ -30,23 +30,14 @@ package org.apache.http.impl;
import junit.framework.TestCase;
import org.apache.http.ConnectionReuseStrategy;
-import org.apache.http.HttpConnection;
-import org.apache.http.HttpConnectionMetrics;
import org.apache.http.HttpResponse;
import org.apache.http.HttpVersion;
-import org.apache.http.StatusLine;
-import org.apache.http.entity.BasicHttpEntity;
import org.apache.http.message.BasicHttpResponse;
-import org.apache.http.message.BasicStatusLine;
import org.apache.http.protocol.BasicHttpContext;
-import org.apache.http.protocol.ExecutionContext;
import org.apache.http.protocol.HttpContext;
public class TestDefaultConnectionReuseStrategy extends TestCase {
- /** A mock connection that is open and not stale. */
- private HttpConnection mockConnection;
-
/** HTTP context. */
private HttpContext context;
@@ -60,15 +51,8 @@ public class TestDefaultConnectionReuseStrategy extends TestCase {
}
public void setUp() {
- // open and not stale is required for most of the tests here
- mockConnection = new MockConnection(true, false);
reuseStrategy = new DefaultConnectionReuseStrategy();
context = new BasicHttpContext(null);
- context.setAttribute(ExecutionContext.HTTP_CONNECTION, mockConnection);
- }
-
- public void tearDown() {
- mockConnection = null;
}
// ------------------------------------------------------- TestCase Methods
@@ -86,66 +70,36 @@ public class TestDefaultConnectionReuseStrategy extends TestCase {
}
public void testIllegalContextArg() throws Exception {
- HttpResponse response =
- createResponse(HttpVersion.HTTP_1_1, 200, "OK", false, -1);
+ HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1, 200, "OK");
try {
reuseStrategy.keepAlive(response, null);
fail("IllegalArgumentException should have been thrown");
} catch (IllegalArgumentException ex) {
// expected
}
+
}
public void testNoContentLengthResponseHttp1_0() throws Exception {
- HttpResponse response =
- createResponse(HttpVersion.HTTP_1_0, 200, "OK", false, -1);
+ HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_0, 200, "OK");
assertFalse(reuseStrategy.keepAlive(response, context));
}
public void testNoContentLengthResponseHttp1_1() throws Exception {
- HttpResponse response =
- createResponse(HttpVersion.HTTP_1_1, 200, "OK", false, -1);
+ HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1, 200, "OK");
assertFalse(reuseStrategy.keepAlive(response, context));
}
public void testChunkedContent() throws Exception {
- HttpResponse response =
- createResponse(HttpVersion.HTTP_1_1, 200, "OK", true, -1);
-
+ HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1, 200, "OK");
+ response.addHeader("Transfer-Encoding", "chunked");
assertTrue(reuseStrategy.keepAlive(response, context));
}
- public void testClosedConnection() throws Exception {
-
- // based on testChunkedContent which is known to return true
- // the difference is in the mock connection passed here
- HttpResponse response =
- createResponse(HttpVersion.HTTP_1_1, 200, "OK", true, -1);
-
- HttpConnection mockonn = new MockConnection(false, false);
- context.setAttribute(ExecutionContext.HTTP_CONNECTION, mockonn);
- assertFalse("closed connection should not be kept alive",
- reuseStrategy.keepAlive(response, context));
- }
-
- public void testStaleConnection() throws Exception {
-
- // based on testChunkedContent which is known to return true
- // the difference is in the mock connection passed here
- HttpResponse response =
- createResponse(HttpVersion.HTTP_1_1, 200, "OK", true, -1);
-
- HttpConnection mockonn = new MockConnection(true, true);
- context.setAttribute(ExecutionContext.HTTP_CONNECTION, mockonn);
- assertTrue("stale connection should not be detected",
- reuseStrategy.keepAlive(response, context));
- }
-
public void testIgnoreInvalidKeepAlive() throws Exception {
- HttpResponse response =
- createResponse(HttpVersion.HTTP_1_0, 200, "OK", false, -1);
+ HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_0, 200, "OK");
response.addHeader("Connection", "keep-alive");
assertFalse(reuseStrategy.keepAlive(response, context));
@@ -153,8 +107,8 @@ public class TestDefaultConnectionReuseStrategy extends TestCase {
public void testExplicitClose() throws Exception {
// Use HTTP 1.1
- HttpResponse response =
- createResponse(HttpVersion.HTTP_1_1, 200, "OK", true, -1);
+ HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1, 200, "OK");
+ response.addHeader("Transfer-Encoding", "chunked");
response.addHeader("Connection", "close");
assertFalse(reuseStrategy.keepAlive(response, context));
@@ -162,37 +116,36 @@ public class TestDefaultConnectionReuseStrategy extends TestCase {
public void testExplicitKeepAlive() throws Exception {
// Use HTTP 1.0
- HttpResponse response =
- createResponse(HttpVersion.HTTP_1_0, 200, "OK", false, 10);
+ HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_0, 200, "OK");
+ response.addHeader("Content-Length", "10");
response.addHeader("Connection", "keep-alive");
assertTrue(reuseStrategy.keepAlive(response, context));
}
public void testHTTP10Default() throws Exception {
- HttpResponse response =
- createResponse(HttpVersion.HTTP_1_0, 200, "OK");
-
+ HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_0, 200, "OK");
+ response.addHeader("Content-Length", "10");
assertFalse(reuseStrategy.keepAlive(response, context));
}
public void testHTTP11Default() throws Exception {
- HttpResponse response =
- createResponse(HttpVersion.HTTP_1_1, 200, "OK");
+ HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1, 200, "OK");
+ response.addHeader("Content-Length", "10");
assertTrue(reuseStrategy.keepAlive(response, context));
}
public void testFutureHTTP() throws Exception {
- HttpResponse response =
- createResponse(new HttpVersion(3, 45), 200, "OK");
+ HttpResponse response = new BasicHttpResponse(new HttpVersion(3, 45), 200, "OK");
+ response.addHeader("Content-Length", "10");
assertTrue(reuseStrategy.keepAlive(response, context));
}
public void testBrokenConnectionDirective1() throws Exception {
// Use HTTP 1.0
- HttpResponse response =
- createResponse(HttpVersion.HTTP_1_0, 200, "OK");
+ HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_0, 200, "OK");
+ response.addHeader("Content-Length", "10");
response.addHeader("Connection", "keep--alive");
assertFalse(reuseStrategy.keepAlive(response, context));
@@ -200,8 +153,8 @@ public class TestDefaultConnectionReuseStrategy extends TestCase {
public void testBrokenConnectionDirective2() throws Exception {
// Use HTTP 1.0
- HttpResponse response =
- createResponse(HttpVersion.HTTP_1_0, 200, "OK");
+ HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_0, 200, "OK");
+ response.addHeader("Content-Length", "10");
response.addHeader("Connection", null);
assertFalse(reuseStrategy.keepAlive(response, context));
@@ -209,8 +162,8 @@ public class TestDefaultConnectionReuseStrategy extends TestCase {
public void testConnectionTokens1() throws Exception {
// Use HTTP 1.1
- HttpResponse response =
- createResponse(HttpVersion.HTTP_1_1, 200, "OK", true, -1);
+ HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1, 200, "OK");
+ response.addHeader("Transfer-Encoding", "chunked");
response.addHeader("Connection", "yadda, cLOSe, dumdy");
assertFalse(reuseStrategy.keepAlive(response, context));
@@ -218,8 +171,8 @@ public class TestDefaultConnectionReuseStrategy extends TestCase {
public void testConnectionTokens2() throws Exception {
// Use HTTP 1.1
- HttpResponse response =
- createResponse(HttpVersion.HTTP_1_1, 200, "OK", true, -1);
+ HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1, 200, "OK");
+ response.addHeader("Transfer-Encoding", "chunked");
response.addHeader("Connection", "yadda, kEEP-alive, dumdy");
assertTrue(reuseStrategy.keepAlive(response, context));
@@ -227,8 +180,8 @@ public class TestDefaultConnectionReuseStrategy extends TestCase {
public void testConnectionTokens3() throws Exception {
// Use HTTP 1.1
- HttpResponse response =
- createResponse(HttpVersion.HTTP_1_1, 200, "OK", true, -1);
+ HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1, 200, "OK");
+ response.addHeader("Transfer-Encoding", "chunked");
response.addHeader("Connection", "yadda, keep-alive, close, dumdy");
assertFalse(reuseStrategy.keepAlive(response, context));
@@ -236,22 +189,20 @@ public class TestDefaultConnectionReuseStrategy extends TestCase {
public void testConnectionTokens4() throws Exception {
// Use HTTP 1.1
- HttpResponse response =
- createResponse(HttpVersion.HTTP_1_1, 200, "OK", true, -1);
+ HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1, 200, "OK");
+ response.addHeader("Transfer-Encoding", "chunked");
response.addHeader("Connection", "yadda, close, dumdy");
response.addHeader("Proxy-Connection", "keep-alive");
-
// Connection takes precedence over Proxy-Connection
assertFalse(reuseStrategy.keepAlive(response, context));
}
public void testConnectionTokens5() throws Exception {
// Use HTTP 1.1
- HttpResponse response =
- createResponse(HttpVersion.HTTP_1_1, 200, "OK", true, -1);
+ HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1, 200, "OK");
+ response.addHeader("Transfer-Encoding", "chunked");
response.addHeader("Connection", "yadda, dumdy");
response.addHeader("Proxy-Connection", "close");
-
// Connection takes precedence over Proxy-Connection,
// even if it doesn't contain a recognized token.
// Default for HTTP/1.1 is to keep alive.
@@ -260,11 +211,10 @@ public class TestDefaultConnectionReuseStrategy extends TestCase {
public void testConnectionTokens6() throws Exception {
// Use HTTP 1.1
- HttpResponse response =
- createResponse(HttpVersion.HTTP_1_1, 200, "OK", true, -1);
+ HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1, 200, "OK");
+ response.addHeader("Transfer-Encoding", "chunked");
response.addHeader("Connection", "");
response.addHeader("Proxy-Connection", "close");
-
// Connection takes precedence over Proxy-Connection,
// even if it is empty. Default for HTTP/1.1 is to keep alive.
assertTrue(reuseStrategy.keepAlive(response, context));
@@ -272,110 +222,33 @@ public class TestDefaultConnectionReuseStrategy extends TestCase {
public void testConnectionTokensInvalid() throws Exception {
// Use HTTP 1.1
- HttpResponse response =
- createResponse(HttpVersion.HTTP_1_1, 200, "OK", true, -1);
+ HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1, 200, "OK");
+ response.addHeader("Transfer-Encoding", "chunked");
response.addHeader("Connection", "keep-alive=true");
-
assertFalse(reuseStrategy.keepAlive(response, context));
}
+ public void testMultipleContentLength() throws Exception {
+ // Use HTTP 1.1
+ HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1, 200, "OK");
+ response.addHeader("Content-Length", "10");
+ response.addHeader("Content-Length", "11");
+ assertFalse(reuseStrategy.keepAlive(response, context));
+ }
- /**
- * Creates a response without an entity.
- *
- * @param version the HTTP version
- * @param status the status code
- * @param message the status message
- *
- * @return a response with the argument attributes, but no headers
- */
- private final static HttpResponse createResponse(HttpVersion version,
- int status,
- String message) {
-
- StatusLine statusline = new BasicStatusLine(version, status, message);
- HttpResponse response = new BasicHttpResponse(statusline);
-
- return response;
-
- } // createResponse/empty
-
-
- /**
- * Creates a response with an entity.
- *
- * @param version the HTTP version
- * @param status the status code
- * @param message the status message
- * @param chunked whether the entity should indicate chunked encoding
- * @param length the content length to be indicated by the entity
- *
- * @return a response with the argument attributes, but no headers
- */
- private final static HttpResponse createResponse(HttpVersion version,
- int status,
- String message,
- boolean chunked,
- int length) {
-
- BasicHttpEntity entity = new BasicHttpEntity();
- entity.setChunked(chunked);
- entity.setContentLength(length);
- HttpResponse response = createResponse(version, status, message);
- response.setEntity(entity);
-
- return response;
-
- } // createResponse/entity
-
-
- /**
- * A mock connection.
- * This is neither client nor server connection, since the default
- * strategy is agnostic. It does not allow modification of it's state,
- * since the strategy is supposed to decide about keep-alive, but not
- * to modify the connection's state.
- */
- private final static class MockConnection implements HttpConnection {
-
- private boolean iAmOpen;
- private boolean iAmStale;
-
- public MockConnection(boolean open, boolean stale) {
- iAmOpen = open;
- iAmStale = stale;
- }
-
- public final boolean isOpen() {
- return iAmOpen;
- }
-
- public void setSocketTimeout(int timeout) {
- }
-
- public int getSocketTimeout() {
- return -1;
- }
-
- public final boolean isStale() {
- return iAmStale;
- }
-
- public final void close() {
- throw new UnsupportedOperationException
- ("connection state must not be modified");
- }
-
- public final void shutdown() {
- throw new UnsupportedOperationException
- ("connection state must not be modified");
- }
-
- public HttpConnectionMetrics getMetrics() {
- return null;
- }
+ public void testInvalidContentLength() throws Exception {
+ // Use HTTP 1.1
+ HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1, 200, "OK");
+ response.addHeader("Content-Length", "crap");
+ assertFalse(reuseStrategy.keepAlive(response, context));
+ }
- } // class MockConnection
+ public void testInvalidNegativeContentLength() throws Exception {
+ // Use HTTP 1.1
+ HttpResponse response = new BasicHttpResponse(HttpVersion.HTTP_1_1, 200, "OK");
+ response.addHeader("Content-Length", "-10");
+ assertFalse(reuseStrategy.keepAlive(response, context));
+ }
-} // class TestDefaultConnectionReuseStrategy
+}
http://git-wip-us.apache.org/repos/asf/httpcomponents-core/blob/a92bd33a/httpcore/src/test/java/org/apache/http/impl/entity/TestStrictContentLengthStrategy.java
----------------------------------------------------------------------
diff --git a/httpcore/src/test/java/org/apache/http/impl/entity/TestStrictContentLengthStrategy.java b/httpcore/src/test/java/org/apache/http/impl/entity/TestStrictContentLengthStrategy.java
index a853ee4..7868e76 100644
--- a/httpcore/src/test/java/org/apache/http/impl/entity/TestStrictContentLengthStrategy.java
+++ b/httpcore/src/test/java/org/apache/http/impl/entity/TestStrictContentLengthStrategy.java
@@ -97,7 +97,18 @@ public class TestStrictContentLengthStrategy extends TestCase {
ContentLengthStrategy lenStrategy = new StrictContentLengthStrategy();
HttpMessage message = new HttpMessageMockup();
message.addHeader("Content-Length", "whatever");
+ try {
+ lenStrategy.determineLength(message);
+ fail("ProtocolException should have been thrown");
+ } catch (ProtocolException ex) {
+ // expected
+ }
+ }
+ public void testEntityWithNegativeContentLength() throws Exception {
+ ContentLengthStrategy lenStrategy = new StrictContentLengthStrategy();
+ HttpMessage message = new HttpMessageMockup();
+ message.addHeader("Content-Length", "-10");
try {
lenStrategy.determineLength(message);
fail("ProtocolException should have been thrown");