From c3b6c426ca455ce372bd5e01a55bc20d2e0471ad Mon Sep 17 00:00:00 2001 From: Oleg Kalnichevski Date: Tue, 28 Oct 2008 14:05:58 +0000 Subject: [PATCH] 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 | 4 ++ .../http/impl/cookie/BestMatchSpec.java | 41 +++++++++++++++---- .../impl/cookie/TestCookieBestMatchSpec.java | 21 +++++++--- 3 files changed, 53 insertions(+), 13 deletions(-) diff --git a/RELEASE_NOTES.txt b/RELEASE_NOTES.txt index 3bb636426..8efa0abc2 100644 --- a/RELEASE_NOTES.txt +++ b/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 + * Fixed bug that can cause a managed connection to be returned from the pool in an inconsistent state. Contributed by Oleg Kalnichevski diff --git a/module-client/src/main/java/org/apache/http/impl/cookie/BestMatchSpec.java b/module-client/src/main/java/org/apache/http/impl/cookie/BestMatchSpec.java index 156a6014e..2e1c332e0 100644 --- a/module-client/src/main/java/org/apache/http/impl/cookie/BestMatchSpec.java +++ b/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); } diff --git a/module-client/src/test/java/org/apache/http/impl/cookie/TestCookieBestMatchSpec.java b/module-client/src/test/java/org/apache/http/impl/cookie/TestCookieBestMatchSpec.java index 41d92e952..dea666e0c 100644 --- a/module-client/src/test/java/org/apache/http/impl/cookie/TestCookieBestMatchSpec.java +++ b/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 - Header header = new BasicHeader("Set-Cookie", "name=value;path=/;domain=b.domain.com; version=1"); + // and validation is used for version 1 Set-Cookie2 headers + Header header = new BasicHeader("Set-Cookie2", "name=value;path=/;domain=b.domain.com; version=1"); List 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()); - assertTrue(cookie instanceof SetCookie2); + assertEquals("localhost", cookie.getDomain()); + 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());