Index: httpcore/src/test/org/apache/http/impl/TestDefaultConnectionReuseStrategy.java =================================================================== --- httpcore/src/test/org/apache/http/impl/TestDefaultConnectionReuseStrategy.java (revision 429015) +++ httpcore/src/test/org/apache/http/impl/TestDefaultConnectionReuseStrategy.java (working copy) @@ -29,6 +29,7 @@ package org.apache.http.impl; import org.apache.http.ConnectionReuseStrategy; +import org.apache.http.HttpConnection; import org.apache.http.HttpResponse; import org.apache.http.HttpVersion; import org.apache.http.StatusLine; @@ -42,6 +43,14 @@ public class TestDefaultConnectionReuseStrategy extends TestCase { + /** A mock connection that is open and not stale. */ + private HttpConnection mockConnection; + + /** The reuse strategy to be tested. */ + private ConnectionReuseStrategy reuseStrategy; + + + public TestDefaultConnectionReuseStrategy(String testName) { super(testName); } @@ -52,16 +61,39 @@ return new TestSuite(TestDefaultConnectionReuseStrategy.class); } + public void setUp() { + // open and not stale is required for most of the tests here + mockConnection = new MockConnection(true, false); + reuseStrategy = new DefaultConnectionReuseStrategy(); + } + + public void tearDown() { + mockConnection = null; + } + + // ------------------------------------------------------------------- Main public static void main(String args[]) { String[] testCaseName = { TestDefaultConnectionReuseStrategy.class.getName() }; junit.textui.TestRunner.main(testCaseName); } + public void testIllegalConnectionArg() throws Exception { + + HttpResponse response = + createResponse(HttpVersion.HTTP_1_1, 200, "OK", false, -1); + + try { + reuseStrategy.keepAlive(null, response); + fail("IllegalArgumentException should have been thrown"); + } catch (IllegalArgumentException ex) { + // expected + } + } + public void testIllegalResponseArg() throws Exception { - ConnectionReuseStrategy s = new DefaultConnectionReuseStrategy(); try { - s.keepAlive(null); + reuseStrategy.keepAlive(mockConnection, null); fail("IllegalArgumentException should have been thrown"); } catch (IllegalArgumentException ex) { // expected @@ -69,124 +101,199 @@ } public void testNoContentLengthResponseHttp1_0() throws Exception { - BasicHttpEntity entity = new BasicHttpEntity(); - entity.setChunked(false); - entity.setContentLength(-1); - StatusLine statusline = new BasicStatusLine(HttpVersion.HTTP_1_0, 200, "OK"); - HttpResponse response = new BasicHttpResponse(statusline); - response.setEntity(entity); + HttpResponse response = + createResponse(HttpVersion.HTTP_1_0, 200, "OK", false, -1); - ConnectionReuseStrategy s = new DefaultConnectionReuseStrategy(); - assertFalse(s.keepAlive(response)); + assertFalse(reuseStrategy.keepAlive(mockConnection, response)); } public void testNoContentLengthResponseHttp1_1() throws Exception { - BasicHttpEntity entity = new BasicHttpEntity(); - entity.setChunked(false); - entity.setContentLength(-1); - StatusLine statusline = new BasicStatusLine(HttpVersion.HTTP_1_1, 200, "OK"); - HttpResponse response = new BasicHttpResponse(statusline); - response.setEntity(entity); + HttpResponse response = + createResponse(HttpVersion.HTTP_1_1, 200, "OK", false, -1); - ConnectionReuseStrategy s = new DefaultConnectionReuseStrategy(); - assertFalse(s.keepAlive(response)); + assertFalse(reuseStrategy.keepAlive(mockConnection, response)); } public void testChunkedContent() throws Exception { - BasicHttpEntity entity = new BasicHttpEntity(); - entity.setChunked(true); - entity.setContentLength(-1); - StatusLine statusline = new BasicStatusLine(HttpVersion.HTTP_1_1, 200, "OK"); - HttpResponse response = new BasicHttpResponse(statusline); - response.setEntity(entity); + HttpResponse response = + createResponse(HttpVersion.HTTP_1_1, 200, "OK", true, -1); - ConnectionReuseStrategy s = new DefaultConnectionReuseStrategy(); - assertTrue(s.keepAlive(response)); + assertTrue(reuseStrategy.keepAlive(mockConnection, response)); } + 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); + assertFalse("closed connection should not be kept alive", + reuseStrategy.keepAlive(mockonn, response)); + } + + 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); + assertTrue("stale connection should not be detected", + reuseStrategy.keepAlive(mockonn, response)); + } + public void testIgnoreInvalidKeepAlive() throws Exception { - BasicHttpEntity entity = new BasicHttpEntity(); - entity.setChunked(false); - entity.setContentLength(-1); - StatusLine statusline = new BasicStatusLine(HttpVersion.HTTP_1_0, 200, "OK"); - HttpResponse response = new BasicHttpResponse(statusline); + HttpResponse response = + createResponse(HttpVersion.HTTP_1_0, 200, "OK", false, -1); response.addHeader("Connection", "keep-alive"); - response.setEntity(entity); - ConnectionReuseStrategy s = new DefaultConnectionReuseStrategy(); - assertFalse(s.keepAlive(response)); + assertFalse(reuseStrategy.keepAlive(mockConnection, response)); } public void testExplicitClose() throws Exception { - BasicHttpEntity entity = new BasicHttpEntity(); - entity.setChunked(true); - entity.setContentLength(-1); // Use HTTP 1.1 - StatusLine statusline = new BasicStatusLine(HttpVersion.HTTP_1_1, 200, "OK"); - HttpResponse response = new BasicHttpResponse(statusline); + HttpResponse response = + createResponse(HttpVersion.HTTP_1_1, 200, "OK", true, -1); response.addHeader("Connection", "close"); - response.setEntity(entity); - ConnectionReuseStrategy s = new DefaultConnectionReuseStrategy(); - assertFalse(s.keepAlive(response)); + assertFalse(reuseStrategy.keepAlive(mockConnection, response)); } public void testExplicitKeepAlive() throws Exception { - BasicHttpEntity entity = new BasicHttpEntity(); - entity.setChunked(false); - entity.setContentLength(10); // Use HTTP 1.0 - StatusLine statusline = new BasicStatusLine(HttpVersion.HTTP_1_0, 200, "OK"); - HttpResponse response = new BasicHttpResponse(statusline); + HttpResponse response = + createResponse(HttpVersion.HTTP_1_0, 200, "OK", false, 10); response.addHeader("Connection", "keep-alive"); - response.setEntity(entity); - ConnectionReuseStrategy s = new DefaultConnectionReuseStrategy(); - assertTrue(s.keepAlive(response)); + assertTrue(reuseStrategy.keepAlive(mockConnection, response)); } public void testHTTP10Default() throws Exception { - StatusLine statusline = new BasicStatusLine(HttpVersion.HTTP_1_0, 200, "OK"); - HttpResponse response = new BasicHttpResponse(statusline); + HttpResponse response = + createResponse(HttpVersion.HTTP_1_0, 200, "OK"); - ConnectionReuseStrategy s = new DefaultConnectionReuseStrategy(); - assertFalse(s.keepAlive(response)); + assertFalse(reuseStrategy.keepAlive(mockConnection, response)); } public void testHTTP11Default() throws Exception { - StatusLine statusline = new BasicStatusLine(HttpVersion.HTTP_1_1, 200, "OK"); - HttpResponse response = new BasicHttpResponse(statusline); - - ConnectionReuseStrategy s = new DefaultConnectionReuseStrategy(); - assertTrue(s.keepAlive(response)); + HttpResponse response = + createResponse(HttpVersion.HTTP_1_1, 200, "OK"); + assertTrue(reuseStrategy.keepAlive(mockConnection, response)); } public void testFutureHTTP() throws Exception { - StatusLine statusline = new BasicStatusLine(new HttpVersion(3, 45), 200, "OK"); - HttpResponse response = new BasicHttpResponse(statusline); + HttpResponse response = + createResponse(new HttpVersion(3, 45), 200, "OK"); - ConnectionReuseStrategy s = new DefaultConnectionReuseStrategy(); - assertTrue(s.keepAlive(response)); + assertTrue(reuseStrategy.keepAlive(mockConnection, response)); } public void testBrokenConnectionDirective1() throws Exception { // Use HTTP 1.0 - StatusLine statusline = new BasicStatusLine(HttpVersion.HTTP_1_0, 200, "OK"); - HttpResponse response = new BasicHttpResponse(statusline); + HttpResponse response = + createResponse(HttpVersion.HTTP_1_0, 200, "OK"); response.addHeader("Connection", "keep--alive"); - ConnectionReuseStrategy s = new DefaultConnectionReuseStrategy(); - assertFalse(s.keepAlive(response)); + assertFalse(reuseStrategy.keepAlive(mockConnection, response)); } public void testBrokenConnectionDirective2() throws Exception { // Use HTTP 1.0 - StatusLine statusline = new BasicStatusLine(HttpVersion.HTTP_1_0, 200, "OK"); - HttpResponse response = new BasicHttpResponse(statusline); + HttpResponse response = + createResponse(HttpVersion.HTTP_1_0, 200, "OK"); response.addHeader("Connection", null); - ConnectionReuseStrategy s = new DefaultConnectionReuseStrategy(); - assertFalse(s.keepAlive(response)); + assertFalse(reuseStrategy.keepAlive(mockConnection, response)); } -} + + /** + * 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 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"); + } + } // class MockConnection + +} // class TestDefaultConnectionReuseStrategy + Index: httpcore/src/java/org/apache/http/impl/DefaultConnectionReuseStrategy.java =================================================================== --- httpcore/src/java/org/apache/http/impl/DefaultConnectionReuseStrategy.java (revision 429015) +++ httpcore/src/java/org/apache/http/impl/DefaultConnectionReuseStrategy.java (working copy) @@ -30,6 +30,7 @@ package org.apache.http.impl; import org.apache.http.ConnectionReuseStrategy; +import org.apache.http.HttpConnection; import org.apache.http.Header; import org.apache.http.HttpEntity; import org.apache.http.HttpResponse; @@ -50,11 +51,21 @@ public DefaultConnectionReuseStrategy() { super(); } - - public boolean keepAlive(final HttpResponse response) { + + // see interface ConnectionReuseStrategy + public boolean keepAlive(final HttpConnection connection, + final HttpResponse response) { + if (connection == null) { + throw new IllegalArgumentException("connection may not be null"); + } if (response == null) { throw new IllegalArgumentException("HTTP response may not be null"); } + + if (!connection.isOpen()) + return false; + // do NOT check for stale connection, that is an expensive operation + HttpEntity entity = response.getEntity(); HttpVersion ver = response.getStatusLine().getHttpVersion(); if (entity != null) { Index: httpcore/src/java/org/apache/http/protocol/HttpService.java =================================================================== --- httpcore/src/java/org/apache/http/protocol/HttpService.java (revision 429015) +++ httpcore/src/java/org/apache/http/protocol/HttpService.java (working copy) @@ -162,7 +162,7 @@ closeConnection(); return; } - if (!this.connStrategy.keepAlive(response)) { + if (!this.connStrategy.keepAlive(conn, response)) { closeConnection(); } else { logMessage("Connection kept alive"); Index: httpcore/src/java/org/apache/http/ConnectionReuseStrategy.java =================================================================== --- httpcore/src/java/org/apache/http/ConnectionReuseStrategy.java (revision 429015) +++ httpcore/src/java/org/apache/http/ConnectionReuseStrategy.java (working copy) @@ -35,6 +35,8 @@ * * @author Oleg Kalnichevski * + * + * * @version $Revision$ * * @since 4.0 @@ -42,15 +44,25 @@ public interface ConnectionReuseStrategy { /** - * Tells if the connection used to receive a response should be closed or - * kept open. Http processors should close the physical connection if this - * method returns false. They should do their best to keep it open if this - * method returns true. - * - * @param response The response whose connection is concerned. - * @return true if the connection is to be kept open, false if it is to be - * closed. + * Decides whether a connection can be kept open after a request. + * If this method returns false, the caller MUST + * close the connection to correctly implement the HTTP protocol. + * If it returns true, the caller SHOULD attempt to + * keep the connection open for reuse with another request. + *
+ * If the connection is already closed, false is returned. + * The stale connection check MUST NOT be triggered by a + * connection reuse strategy. + * + * @param connection + * The connection for which to decide about reuse. + * @param response + * The last response received over that connection. + * + * @return true if the connection is allowed to be reused, or + * false if it MUST NOT be reused */ - boolean keepAlive(HttpResponse response); + public boolean keepAlive(HttpConnection connection, + HttpResponse response); } Index: httpcore/src/examples/org/apache/http/examples/ElementalHttpGet.java =================================================================== --- httpcore/src/examples/org/apache/http/examples/ElementalHttpGet.java (revision 429015) +++ httpcore/src/examples/org/apache/http/examples/ElementalHttpGet.java (working copy) @@ -99,7 +99,7 @@ System.out.println("<< Response: " + response.getStatusLine()); System.out.println(EntityUtils.toString(response.getEntity())); System.out.println("=============="); - if (!connStrategy.keepAlive(response)) { + if (!connStrategy.keepAlive(conn, response)) { conn.close(); } else { System.out.println("Connection kept alive..."); Index: httpcore/src/examples/org/apache/http/examples/ElementalHttpPost.java =================================================================== --- httpcore/src/examples/org/apache/http/examples/ElementalHttpPost.java (revision 429015) +++ httpcore/src/examples/org/apache/http/examples/ElementalHttpPost.java (working copy) @@ -112,7 +112,7 @@ System.out.println("<< Response: " + response.getStatusLine()); System.out.println(EntityUtils.toString(response.getEntity())); System.out.println("=============="); - if (!connStrategy.keepAlive(response)) { + if (!connStrategy.keepAlive(conn, response)) { conn.close(); } else { System.out.println("Connection kept alive..."); Index: httpasync/src/java/org/apache/http/async/impl/SimpleHttpDispatcher.java =================================================================== --- httpasync/src/java/org/apache/http/async/impl/SimpleHttpDispatcher.java (revision 429035) +++ httpasync/src/java/org/apache/http/async/impl/SimpleHttpDispatcher.java (working copy) @@ -296,7 +296,7 @@ try { if (!abort && (response != null) && (reuse_strategy != null) && - reuse_strategy.keepAlive(response)) { + reuse_strategy.keepAlive(client_connection, response)) { // consume rest of the entity, if there is one HttpEntity entity = response.getEntity();