Return-Path: 
 <httpcomponents-commits-return-1594-apmail-jakarta-httpcomponents-commits-archive=jakarta.apache.org@jakarta.apache.org>
Delivered-To: apmail-jakarta-httpcomponents-commits-archive@www.apache.org
Received: (qmail 57734 invoked from network); 8 Dec 2007 16:55:47 -0000
Received: from hermes.apache.org (HELO mail.apache.org) (140.211.11.2)
  by minotaur.apache.org with SMTP; 8 Dec 2007 16:55:47 -0000
Received: (qmail 31252 invoked by uid 500); 8 Dec 2007 16:55:35 -0000
Delivered-To: apmail-jakarta-httpcomponents-commits-archive@jakarta.apache.org
Received: (qmail 31232 invoked by uid 500); 8 Dec 2007 16:55:35 -0000
Mailing-List: contact httpcomponents-commits-help@jakarta.apache.org;
 run by ezmlm
Precedence: bulk
List-Help: <mailto:httpcomponents-commits-help@jakarta.apache.org>
List-Unsubscribe: 
 <mailto:httpcomponents-commits-unsubscribe@jakarta.apache.org>
List-Post: <mailto:httpcomponents-commits@jakarta.apache.org>
List-Id: <httpcomponents-commits.jakarta.apache.org>
Reply-To: httpcomponents-dev@jakarta.apache.org
Delivered-To: mailing list httpcomponents-commits@jakarta.apache.org
Received: (qmail 31223 invoked by uid 99); 8 Dec 2007 16:55:35 -0000
Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230)
    by apache.org (qpsmtpd/0.29) with ESMTP; Sat, 08 Dec 2007 08:55:35 -0800
X-ASF-Spam-Status: No, hits=-100.0 required=10.0
	tests=ALL_TRUSTED
X-Spam-Check-By: apache.org
Received: from [140.211.11.3] (HELO eris.apache.org) (140.211.11.3)
    by apache.org (qpsmtpd/0.29) with ESMTP; Sat, 08 Dec 2007 16:55:36 +0000
Received: by eris.apache.org (Postfix, from userid 65534)
	id 619C31A9832; Sat,  8 Dec 2007 08:55:14 -0800 (PST)
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Subject: svn commit: r602506 - in /jakarta/httpcomponents/httpcore/trunk:
 RELEASE_NOTES.txt
 module-main/src/main/java/org/apache/http/impl/DefaultConnectionReuseStrategy.java
 module-main/src/test/java/org/apache/http/impl/TestDefaultConnectionReuseStrategy.java
Date: Sat, 08 Dec 2007 16:55:13 -0000
To: httpcomponents-commits@jakarta.apache.org
From: rolandw@apache.org
X-Mailer: svnmailer-1.0.8
Message-Id: <20071208165514.619C31A9832@eris.apache.org>
X-Virus-Checked: Checked by ClamAV on apache.org

Author: rolandw
Date: Sat Dec  8 08:55:13 2007
New Revision: 602506

URL: http://svn.apache.org/viewvc?rev=602506&view=rev
Log:
HTTPCORE-112: patch committed with testcases

Modified:
    jakarta/httpcomponents/httpcore/trunk/RELEASE_NOTES.txt
    jakarta/httpcomponents/httpcore/trunk/module-main/src/main/java/org/apache/http/impl/DefaultConnectionReuseStrategy.java
    jakarta/httpcomponents/httpcore/trunk/module-main/src/test/java/org/apache/http/impl/TestDefaultConnectionReuseStrategy.java

Modified: jakarta/httpcomponents/httpcore/trunk/RELEASE_NOTES.txt
URL: http://svn.apache.org/viewvc/jakarta/httpcomponents/httpcore/trunk/RELEASE_NOTES.txt?rev=602506&r1=602505&r2=602506&view=diff
==============================================================================
--- jakarta/httpcomponents/httpcore/trunk/RELEASE_NOTES.txt (original)
+++ jakarta/httpcomponents/httpcore/trunk/RELEASE_NOTES.txt Sat Dec  8 08:55:13 2007
@@ -1,5 +1,8 @@
 Changes since 4.0 Alpha 6
 
+* [HTTPCORE-112] DefaultConnectionReuseStrategy interprets token sequences
+  Contributed by Roland Weber <rolandw at apache.org>
+
 * [HTTPCORE-122] new interface TokenIterator and basic implementation
   Contributed by Roland Weber <rolandw at apache.org>
 

Modified: jakarta/httpcomponents/httpcore/trunk/module-main/src/main/java/org/apache/http/impl/DefaultConnectionReuseStrategy.java
URL: http://svn.apache.org/viewvc/jakarta/httpcomponents/httpcore/trunk/module-main/src/main/java/org/apache/http/impl/DefaultConnectionReuseStrategy.java?rev=602506&r1=602505&r2=602506&view=diff
==============================================================================
--- jakarta/httpcomponents/httpcore/trunk/module-main/src/main/java/org/apache/http/impl/DefaultConnectionReuseStrategy.java (original)
+++ jakarta/httpcomponents/httpcore/trunk/module-main/src/main/java/org/apache/http/impl/DefaultConnectionReuseStrategy.java Sat Dec  8 08:55:13 2007
@@ -34,36 +34,46 @@
 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;
+
 
 /**
  * Default implementation of a strategy deciding about connection re-use.
  *
  * @author <a href="mailto:oleg at ural.ru">Oleg Kalnichevski</a>
+ * @author <a href="mailto:rolandw at apache.org">Roland Weber</a>
  *
  * @version $Revision$
  * 
  * @since 4.0
  */
-public class DefaultConnectionReuseStrategy implements ConnectionReuseStrategy {
+public class DefaultConnectionReuseStrategy
+    implements ConnectionReuseStrategy {
 
     public DefaultConnectionReuseStrategy() {
         super();
     }
 
     // see interface ConnectionReuseStrategy
-    public boolean keepAlive(final HttpResponse response, final HttpContext context) {
+    public boolean keepAlive(final HttpResponse response,
+                             final HttpContext context) {
         if (response == null) {
-            throw new IllegalArgumentException("HTTP response may not be null");
+            throw new IllegalArgumentException
+                ("HTTP response may not be null.");
         }
         if (context == null) {
-            throw new IllegalArgumentException("HTTP context may not be null");
+            throw new IllegalArgumentException
+                ("HTTP context may not be null.");
         }
         
         HttpConnection conn = (HttpConnection)
@@ -73,34 +83,90 @@
             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 (!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
                     return false;
                 }
             }
         }
-        // Check for 'Connection' directive
-        Header connheader = response.getFirstHeader(HTTP.CONN_DIRECTIVE);
-        if (connheader == null) {
-            connheader = response.getFirstHeader("Proxy-Connection");
-        }
-        if (connheader != null) {
-            String conndirective = connheader.getValue(); 
-            if (HTTP.CONN_CLOSE.equalsIgnoreCase(conndirective)) {
+
+        // Check for the "Connection" header. If that is absent, check for
+        // the "Proxy-Connection" header. The latter is an unspecified and
+        // broken but unfortunately common extension of HTTP.
+        HeaderIterator hit = response.headerIterator(HTTP.CONN_DIRECTIVE);
+        if (!hit.hasNext())
+            hit = response.headerIterator("Proxy-Connection");
+
+        // Experimental usage of the "Connection" header in HTTP/1.0 is
+        // documented in RFC 2068, section 19.7.1. A token "keep-alive" is
+        // used to indicate that the connection should be persistent.
+        // Note that the final specification of HTTP/1.1 in RFC 2616 does not
+        // include this information. Neither is the "Connection" header
+        // mentioned in RFC 1945, which informally describes HTTP/1.0.
+        //
+        // RFC 2616 specifies "close" as the only connection token with a
+        // specific meaning: it disables persistent connections.
+        //
+        // The "Proxy-Connection" header is not formally specified anywhere,
+        // but is commonly used to carry one token, "close" or "keep-alive".
+        // The "Connection" header, on the other hand, is defined as a
+        // sequence of tokens, where each token is a header name, and the
+        // token "close" has the above-mentioned additional meaning.
+        //
+        // To get through this mess, we treat the "Proxy-Connection" header
+        // in exactly the same way as the "Connection" header, but only if
+        // the latter is missing. We scan the sequence of tokens for both
+        // "close" and "keep-alive". As "close" is specified by RFC 2068,
+        // it takes precedence and indicates a non-persistent connection.
+        // If there is no "close" but a "keep-alive", we take the hint.
+
+        if (hit.hasNext()) {
+            try {
+                TokenIterator ti = createTokenIterator(hit);
+                boolean keepalive = false;
+                while (ti.hasNext()) {
+                    final String token = ti.nextToken();
+                    if (HTTP.CONN_CLOSE.equalsIgnoreCase(token)) {
+                        return false;
+                    } else if (HTTP.CONN_KEEP_ALIVE.equalsIgnoreCase(token)) {
+                        // continue the loop, there may be a "close" afterwards
+                        keepalive = true;
+                    }
+                }
+                if (keepalive)
+                    return true;
+                // neither "close" nor "keep-alive", use default policy
+
+            } catch (ParseException px) {
+                // invalid connection header means no persistent connection
+                // we don't have logging in HttpCore, so the exception is lost
                 return false;
-            } else if (HTTP.CONN_KEEP_ALIVE.equalsIgnoreCase(conndirective)) {
-                return true;
-            } else {
-                // log unknown directive
             }
         }
-        // Resorting to protocol version default close connection policy
+
+        // default since HTTP/1.1 is persistent, before it was non-persistent
         return !ver.lessEquals(HttpVersion.HTTP_1_0);
     }
-            
+
+
+    /**
+     * Creates a token iterator from a header iterator.
+     * This method can be overridden to replace the implementation of
+     * the token iterator.
+     *
+     * @param hit       the header iterator
+     *
+     * @return  the token iterator
+     */
+    protected TokenIterator createTokenIterator(HeaderIterator hit) {
+        return new BasicTokenIterator(hit);
+    }
 }

Modified: jakarta/httpcomponents/httpcore/trunk/module-main/src/test/java/org/apache/http/impl/TestDefaultConnectionReuseStrategy.java
URL: http://svn.apache.org/viewvc/jakarta/httpcomponents/httpcore/trunk/module-main/src/test/java/org/apache/http/impl/TestDefaultConnectionReuseStrategy.java?rev=602506&r1=602505&r2=602506&view=diff
==============================================================================
--- jakarta/httpcomponents/httpcore/trunk/module-main/src/test/java/org/apache/http/impl/TestDefaultConnectionReuseStrategy.java (original)
+++ jakarta/httpcomponents/httpcore/trunk/module-main/src/test/java/org/apache/http/impl/TestDefaultConnectionReuseStrategy.java Sat Dec  8 08:55:13 2007
@@ -223,6 +223,78 @@
         assertFalse(reuseStrategy.keepAlive(response, context));
     }
 
+    public void testConnectionTokens1() throws Exception {
+        // Use HTTP 1.1
+        HttpResponse response =
+            createResponse(HttpVersion.HTTP_1_1, 200, "OK", true, -1);
+        response.addHeader("Connection", "yadda, cLOSe, dumdy");
+
+        assertFalse(reuseStrategy.keepAlive(response, context));
+    }
+
+    public void testConnectionTokens2() throws Exception {
+        // Use HTTP 1.1
+        HttpResponse response =
+            createResponse(HttpVersion.HTTP_1_1, 200, "OK", true, -1);
+        response.addHeader("Connection", "yadda, kEEP-alive, dumdy");
+
+        assertTrue(reuseStrategy.keepAlive(response, context));
+    }
+
+    public void testConnectionTokens3() throws Exception {
+        // Use HTTP 1.1
+        HttpResponse response =
+            createResponse(HttpVersion.HTTP_1_1, 200, "OK", true, -1);
+        response.addHeader("Connection", "yadda, keep-alive, close, dumdy");
+
+        assertFalse(reuseStrategy.keepAlive(response, context));
+    }
+
+    public void testConnectionTokens4() throws Exception {
+        // Use HTTP 1.1
+        HttpResponse response =
+            createResponse(HttpVersion.HTTP_1_1, 200, "OK", true, -1);
+        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);
+        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.
+        assertTrue(reuseStrategy.keepAlive(response, context));
+    }
+
+    public void testConnectionTokens6() throws Exception {
+        // Use HTTP 1.1
+        HttpResponse response =
+            createResponse(HttpVersion.HTTP_1_1, 200, "OK", true, -1);
+        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));
+    }
+
+    public void testConnectionTokensInvalid() throws Exception {
+        // Use HTTP 1.1
+        HttpResponse response =
+            createResponse(HttpVersion.HTTP_1_1, 200, "OK", true, -1);
+        response.addHeader("Connection", "keep-alive=true");
+
+        assertFalse(reuseStrategy.keepAlive(response, context));
+    }
+
 
     /**
      * Creates a response without an entity.