Fixed parsing and validation of RFC2109 compliant Set-Cookie headers by the Best-Match cookie spec

git-svn-id: https://svn.apache.org/repos/asf/httpcomponents/httpclient/trunk@708579 13f79535-47bb-0310-9956-ffa450edef68

RELEASE_NOTES.txt

@@ -1,6 +1,10 @@
 Changes since 4.0 beta 1
 -------------------
 
+* Fixed parsing and validation of RFC2109 compliant Set-Cookie headers 
+  by the Best-Match cookie spec.     
+  Contributed by Oleg Kalnichevski <olegk at apache.org>
+
 * Fixed bug that can cause a managed connection to be returned from the
   pool in an inconsistent state.     
   Contributed by Oleg Kalnichevski <olegk at apache.org>

module-client/src/main/java/org/apache/http/impl/cookie/BestMatchSpec.java

@@ -39,6 +39,8 @@ import org.apache.http.cookie.Cookie;
 import org.apache.http.cookie.CookieOrigin;
 import org.apache.http.cookie.CookieSpec;
 import org.apache.http.cookie.MalformedCookieException;
+import org.apache.http.cookie.SM;
+import org.apache.http.cookie.SetCookie2;
 
 /**
  * 'Meta' cookie specification that selects a cookie policy depending
@@ -54,6 +56,7 @@ public class BestMatchSpec implements CookieSpec {
     private final boolean oneHeader;
     
     private RFC2965Spec strict;
+    private RFC2109Spec obsoleteStrict;
     private BrowserCompatSpec compat;
     private NetscapeDraftSpec netscape;
 
@@ -74,6 +77,13 @@ public class BestMatchSpec implements CookieSpec {
         return strict;
     }
 
+    private RFC2109Spec getObsoleteStrict() {
+        if (this.obsoleteStrict == null) {
+             this.obsoleteStrict = new RFC2109Spec(this.datepatterns, this.oneHeader);
+        }
+        return obsoleteStrict;
+    }
+
     private BrowserCompatSpec getCompat() {
         if (this.compat == null) {
             this.compat = new BrowserCompatSpec(this.datepatterns);
@@ -111,13 +121,14 @@ public class BestMatchSpec implements CookieSpec {
             if (helem.getParameterByName("expires") != null) {
                netscape = true;
             }
-        }
-        if (netscape) {
-            
         }
         // Do we have a cookie with a version attribute?
         if (versioned) {
-            return getStrict().parse(helems, origin);
+            if (SM.SET_COOKIE2.equals(header.getName())) {
+                return getStrict().parse(helems, origin);
+            } else {
+                return getObsoleteStrict().parse(helems, origin);
+            }
         } else if (netscape) {
             // Need to parse the header again,
             // because Netscape draft cannot handle
@@ -138,7 +149,11 @@ public class BestMatchSpec implements CookieSpec {
             throw new IllegalArgumentException("Cookie origin may not be null");
         }
         if (cookie.getVersion() > 0) {
-            getStrict().validate(cookie, origin);
+            if (cookie instanceof SetCookie2) {
+                getStrict().validate(cookie, origin);
+            } else {
+                getObsoleteStrict().validate(cookie, origin);
+            }
         } else {
             getCompat().validate(cookie, origin);
         }
@@ -152,7 +167,11 @@ public class BestMatchSpec implements CookieSpec {
             throw new IllegalArgumentException("Cookie origin may not be null");
         }
         if (cookie.getVersion() > 0) {
-            return getStrict().match(cookie, origin);
+            if (cookie instanceof SetCookie2) {
+                return getStrict().match(cookie, origin);
+            } else {
+                return getObsoleteStrict().match(cookie, origin);
+            }
         } else {
             return getCompat().match(cookie, origin);
         }
@@ -163,13 +182,21 @@ public class BestMatchSpec implements CookieSpec {
             throw new IllegalArgumentException("List of cookie may not be null");
         }
         int version = Integer.MAX_VALUE;
+        boolean isSetCookie2 = true; 
         for (Cookie cookie: cookies) {
+            if (!(cookie instanceof SetCookie2)) {
+                isSetCookie2 = false;
+            }
             if (cookie.getVersion() < version) {
                 version = cookie.getVersion();
             }
         }
         if (version > 0) {
-            return getStrict().formatCookies(cookies);
+            if (isSetCookie2) {
+                return getStrict().formatCookies(cookies);
+            } else {
+                return getObsoleteStrict().formatCookies(cookies);
+            }
         } else {
             return getCompat().formatCookies(cookies);
         }

module-client/src/test/java/org/apache/http/impl/cookie/TestCookieBestMatchSpec.java

@@ -106,15 +106,24 @@ public class TestCookieBestMatchSpec extends TestCase {
         CookieOrigin origin = new CookieOrigin("a.b.domain.com", 80, "/", false);
 
         // Make sure the strict (RFC2965) cookie parsing
-        // and validation is used for version 1 cookies
+        // and validation is used for version 1 Set-Cookie2 headers
-        Header header = new BasicHeader("Set-Cookie", "name=value;path=/;domain=b.domain.com; version=1");
+        Header header = new BasicHeader("Set-Cookie2", "name=value;path=/;domain=b.domain.com; version=1");
 
         List<Cookie> cookies = cookiespec.parse(header, origin);
         for (int i = 0; i < cookies.size(); i++) {
             cookiespec.validate(cookies.get(i), origin);
         }
 
-        header = new BasicHeader("Set-Cookie", "name=value;path=/;domain=domain.com; version=1");
+        // Make sure the strict (RFC2109) cookie parsing
+        // and validation is used for version 1 Set-Cookie headers
+        header = new BasicHeader("Set-Cookie", "name=value;path=/;domain=.b.domain.com; version=1");
+
+        cookies = cookiespec.parse(header, origin);
+        for (int i = 0; i < cookies.size(); i++) {
+            cookiespec.validate(cookies.get(i), origin);
+        }
+
+        header = new BasicHeader("Set-Cookie2", "name=value;path=/;domain=domain.com; version=1");
         try {
             cookies = cookiespec.parse(header, origin);
             cookiespec.validate(cookies.get(0), origin);
@@ -134,8 +143,8 @@ public class TestCookieBestMatchSpec extends TestCase {
         for (int i = 0; i < cookies.size(); i++) {
             Cookie cookie = cookies.get(i);
             cookiespec.validate(cookie, origin);
-            assertEquals("localhost.local", cookie.getDomain());
+            assertEquals("localhost", cookie.getDomain());
-            assertTrue(cookie instanceof SetCookie2);
+            assertFalse(cookie instanceof SetCookie2);
         }
     }
 
@@ -175,7 +184,7 @@ public class TestCookieBestMatchSpec extends TestCase {
 
         // Make sure the strict (RFC2965) cookie matching
         // is used for version 1 cookies
-        BasicClientCookie cookie = new BasicClientCookie("name", "value");
+        BasicClientCookie2 cookie = new BasicClientCookie2("name", "value");
         cookie.setVersion(1);
         cookie.setDomain(".domain.com");
         cookie.setAttribute(ClientCookie.DOMAIN_ATTR, cookie.getDomain());