Author: olegk Date: Fri Apr 28 08:13:27 2006 New Revision: 397917 URL: http://svn.apache.org/viewcvs?rev=397917&view=rev Log: Throw a MalformedCookieException and discard all cookies in the Set-Cookie2 if a single invalid header element is encountered.
(1) This behavior is consistent with that of other cookie specs (2) Generally it is a very bad idea to silently swallow exceptions Fundamentally the problem is caused by a design flaw in the existing cookie API. Namely, the tokanization of header elements should not be a concern of a cookie spec. Cookie specs should be dealing with one cookie at a time. We will provide a proper solution to this problem in HttpComponents Modified: jakarta/commons/proper/httpclient/branches/COOKIE_2_BRANCH/src/java/org/apache/commons/httpclient/HttpMethodBase.java jakarta/commons/proper/httpclient/branches/COOKIE_2_BRANCH/src/java/org/apache/commons/httpclient/cookie/RFC2965Spec.java jakarta/commons/proper/httpclient/branches/COOKIE_2_BRANCH/src/test/org/apache/commons/httpclient/cookie/TestCookieRFC2965Spec.java Modified: jakarta/commons/proper/httpclient/branches/COOKIE_2_BRANCH/src/java/org/apache/commons/httpclient/HttpMethodBase.java URL: http://svn.apache.org/viewcvs/jakarta/commons/proper/httpclient/branches/COOKIE_2_BRANCH/src/java/org/apache/commons/httpclient/HttpMethodBase.java?rev=397917&r1=397916&r2=397917&view=diff ============================================================================== --- jakarta/commons/proper/httpclient/branches/COOKIE_2_BRANCH/src/java/org/apache/commons/httpclient/HttpMethodBase.java (original) +++ jakarta/commons/proper/httpclient/branches/COOKIE_2_BRANCH/src/java/org/apache/commons/httpclient/HttpMethodBase.java Fri Apr 28 08:13:27 2006 @@ -45,7 +45,6 @@ import org.apache.commons.httpclient.protocol.Protocol; import org.apache.commons.httpclient.util.EncodingUtil; import org.apache.commons.httpclient.util.ExceptionUtil; -import org.apache.commons.httpclient.util.ParameterFormatter; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; Modified: jakarta/commons/proper/httpclient/branches/COOKIE_2_BRANCH/src/java/org/apache/commons/httpclient/cookie/RFC2965Spec.java URL: http://svn.apache.org/viewcvs/jakarta/commons/proper/httpclient/branches/COOKIE_2_BRANCH/src/java/org/apache/commons/httpclient/cookie/RFC2965Spec.java?rev=397917&r1=397916&r2=397917&view=diff ============================================================================== --- jakarta/commons/proper/httpclient/branches/COOKIE_2_BRANCH/src/java/org/apache/commons/httpclient/cookie/RFC2965Spec.java (original) +++ jakarta/commons/proper/httpclient/branches/COOKIE_2_BRANCH/src/java/org/apache/commons/httpclient/cookie/RFC2965Spec.java Fri Apr 28 08:13:27 2006 @@ -29,14 +29,21 @@ package org.apache.commons.httpclient.cookie; -import org.apache.commons.httpclient.NameValuePair; +import java.util.ArrayList; +import java.util.Date; +import java.util.HashMap; +import java.util.Iterator; +import java.util.LinkedList; +import java.util.List; +import java.util.Map; +import java.util.StringTokenizer; + import org.apache.commons.httpclient.Cookie; -import org.apache.commons.httpclient.HeaderElement; import org.apache.commons.httpclient.Header; +import org.apache.commons.httpclient.HeaderElement; +import org.apache.commons.httpclient.NameValuePair; import org.apache.commons.httpclient.util.ParameterFormatter; -import java.util.*; - /** * <p>RFC 2965 specific cookie management functions.</p> * @@ -250,26 +257,19 @@ null, false, new int[] {port}); - - // cycle through the parameters - NameValuePair[] parameters = headerelement.getParameters(); - // could be null. In case only a header element and no parameters. - if (parameters != null) { - for (int j = 0; j < parameters.length; j++) { - parseAttribute(parameters[j], cookie); - } + } catch (IllegalArgumentException ex) { + throw new MalformedCookieException(ex.getMessage()); + } + NameValuePair[] parameters = headerelement.getParameters(); + // could be null. In case only a header element and no parameters. + if (parameters != null) { + for (int j = 0; j < parameters.length; j++) { + parseAttribute(parameters[j], cookie); } - cookies.add(cookie); - } catch (Exception e) { - //TODO(jain): when do we consider the header malformed and stop processing it? - // throw this cookie, continue processing other cookies in header - if (LOG.isDebugEnabled()) - LOG.debug("Error occured while parsing cookie: \"" + - headerelement + "\". " + e); } + cookies.add(cookie); + // cycle through the parameters } - if (cookies.isEmpty()) - return null; return (Cookie[]) cookies.toArray(new Cookie[cookies.size()]); } Modified: jakarta/commons/proper/httpclient/branches/COOKIE_2_BRANCH/src/test/org/apache/commons/httpclient/cookie/TestCookieRFC2965Spec.java URL: http://svn.apache.org/viewcvs/jakarta/commons/proper/httpclient/branches/COOKIE_2_BRANCH/src/test/org/apache/commons/httpclient/cookie/TestCookieRFC2965Spec.java?rev=397917&r1=397916&r2=397917&view=diff ============================================================================== --- jakarta/commons/proper/httpclient/branches/COOKIE_2_BRANCH/src/test/org/apache/commons/httpclient/cookie/TestCookieRFC2965Spec.java (original) +++ jakarta/commons/proper/httpclient/branches/COOKIE_2_BRANCH/src/test/org/apache/commons/httpclient/cookie/TestCookieRFC2965Spec.java Fri Apr 28 08:13:27 2006 @@ -93,53 +93,56 @@ */ public void testParsePath() throws Exception { CookieSpec cookiespec = new RFC2965Spec(); - // path cannot be null - Header header = new Header("Set-Cookie2", "name=value;Path=;Version=1"); + Header header = new Header("Set-Cookie2", "name=value;Path=/;Version=1;Path="); Cookie[] parsed = cookiespec.parse("www.domain.com", 80, "/", false, header); - assertNull(parsed); - - //path cannot be blank - header = new Header("Set-Cookie2", "name=value;Path=\" \";Version=1"); - parsed = cookiespec.parse("www.domain.com", 80, "/", false, header); - assertNull(parsed); - - // valid path - header = new Header("Set-Cookie2", "name=value;Path=/;Version=1;Path="); - parsed = cookiespec.parse("www.domain.com", 80, "/", false, header); assertNotNull(parsed); assertEquals(1, parsed.length); // only the first occurence of path attribute is considered, others ignored Cookie2 cookie = (Cookie2) parsed[0]; assertEquals("/", cookie.getPath()); assertTrue(cookie.isPathAttributeSpecified()); + } + public void testParsePathDefault() throws Exception { + CookieSpec cookiespec = new RFC2965Spec(); // Path is OPTIONAL, defaults to the request path - header = new Header("Set-Cookie2", "name=value;Version=1"); - parsed = cookiespec.parse("www.domain.com", 80, "/path" /* request path */, false, header); + Header header = new Header("Set-Cookie2", "name=value;Version=1"); + Cookie[] parsed = cookiespec.parse("www.domain.com", 80, "/path" /* request path */, false, header); assertNotNull(parsed); assertEquals(1, parsed.length); - cookie = (Cookie2) parsed[0]; + Cookie2 cookie = (Cookie2) parsed[0]; assertEquals("/path", cookie.getPath()); assertFalse(cookie.isPathAttributeSpecified()); } + public void testParseNullPath() throws Exception { + CookieSpec cookiespec = new RFC2965Spec(); + Header header = new Header("Set-Cookie2", "name=value;Path=;Version=1"); + try { + cookiespec.parse("www.domain.com", 80, "/", false, header); + fail("MalformedCookieException should have been thrown"); + } catch (MalformedCookieException ex) { + // expected + } + } + + public void testParseBlankPath() throws Exception { + CookieSpec cookiespec = new RFC2965Spec(); + Header header = new Header("Set-Cookie2", "name=value;Path=\" \";Version=1"); + try { + cookiespec.parse("www.domain.com", 80, "/", false, header); + fail("MalformedCookieException should have been thrown"); + } catch (MalformedCookieException ex) { + // expected + } + } /** * Test parsing cookie <tt>"Domain"</tt> attribute. */ public void testParseDomain() throws Exception { CookieSpec cookiespec = new RFC2965Spec(); - // domain cannot be null - Header header = new Header("Set-Cookie2", "name=value;Domain=;Version=1"); + Header header = new Header("Set-Cookie2", "name=value;Domain=.domain.com;Version=1;Domain="); Cookie[] parsed = cookiespec.parse("www.domain.com", 80, "/", false, header); - assertNull(parsed); - - // domain cannot be blank - header = new Header("Set-Cookie2", "name=value;Domain=\" \";Version=1"); - parsed = cookiespec.parse("www.domain.com", 80, "/", false, header); - assertNull(parsed); - - header = new Header("Set-Cookie2", "name=value;Domain=.domain.com;Version=1;Domain="); - parsed = cookiespec.parse("www.domain.com", 80, "/", false, header); assertNotNull(parsed); assertEquals(1, parsed.length); // only the first occurence of domain attribute is considered, others ignored @@ -154,22 +157,71 @@ assertEquals(1, parsed.length); cookie = (Cookie2) parsed[0]; assertEquals(".domain.com", cookie.getDomain()); + } + public void testParseDomainDefault() throws Exception { + CookieSpec cookiespec = new RFC2965Spec(); // Domain is OPTIONAL, defaults to the request host - header = new Header("Set-Cookie2", "name=value;Version=1"); - parsed = cookiespec.parse("www.domain.com" /* request host */, 80, "/", false, header); + Header header = new Header("Set-Cookie2", "name=value;Version=1"); + Cookie[] parsed = cookiespec.parse("www.domain.com" /* request host */, 80, "/", false, header); assertNotNull(parsed); assertEquals(1, parsed.length); - cookie = (Cookie2) parsed[0]; + Cookie2 cookie = (Cookie2) parsed[0]; assertEquals("www.domain.com", cookie.getDomain()); assertFalse(cookie.isDomainAttributeSpecified()); } + public void testParseNullDomain() throws Exception { + CookieSpec cookiespec = new RFC2965Spec(); + // domain cannot be null + Header header = new Header("Set-Cookie2", "name=value;Domain=;Version=1"); + try { + cookiespec.parse("www.domain.com", 80, "/", false, header); + fail("MalformedCookieException should have been thrown"); + } catch (MalformedCookieException ex) { + // expected + } + } + + public void testParseBlankDomain() throws Exception { + CookieSpec cookiespec = new RFC2965Spec(); + Header header = new Header("Set-Cookie2", "name=value;Domain=\" \";Version=1"); + try { + cookiespec.parse("www.domain.com", 80, "/", false, header); + fail("MalformedCookieException should have been thrown"); + } catch (MalformedCookieException ex) { + // expected + } + } + /** * Test parsing cookie <tt>"Port"</tt> attribute. */ public void testParsePort() throws Exception { CookieSpec cookiespec = new RFC2965Spec(); + Header header = new Header("Set-Cookie2", "name=value;Port=\"80,800,8000\";Version=1;Port=nonsense"); + Cookie[] parsed = cookiespec.parse("www.domain.com", 80, "/", false, header); + assertNotNull(parsed); + assertEquals(1, parsed.length); + // only the first occurence of port attribute is considered, others ignored + Cookie2 cookie = (Cookie2) parsed[0]; + assertEquals(new int[] {80,800,8000}, cookie.getPorts()); + assertTrue(cookie.isPortAttributeSpecified()); + } + + public void testParsePortDefault() throws Exception { + CookieSpec cookiespec = new RFC2965Spec(); + // Port is OPTIONAL, cookie can be accepted from any port + Header header = new Header("Set-Cookie2", "name=value;Version=1"); + Cookie[] parsed = cookiespec.parse("www.domain.com", 80, "/", false, header); + assertNotNull(parsed); + assertEquals(1, parsed.length); + Cookie2 cookie = (Cookie2) parsed[0]; + assertFalse(cookie.isPortAttributeSpecified()); + } + + public void testParseNullPort() throws Exception { + CookieSpec cookiespec = new RFC2965Spec(); // null port defaults to request port Header header = new Header("Set-Cookie2", "name=value;Port=;Version=1"); Cookie[] parsed = cookiespec.parse("www.domain.com", 80 /* request port */, "/", false, header); @@ -178,43 +230,40 @@ Cookie2 cookie = (Cookie2) parsed[0]; assertEquals(new int[] {80}, cookie.getPorts()); assertTrue(cookie.isPortAttributeSpecified() && cookie.isPortAttributeBlank()); + } + public void testParseBlankPort() throws Exception { + CookieSpec cookiespec = new RFC2965Spec(); // blank port defaults to request port - header = new Header("Set-Cookie2", "name=value;Port=\" \";Version=1"); - parsed = cookiespec.parse("www.domain.com", 80 /* request port */, "/", false, header); + Header header = new Header("Set-Cookie2", "name=value;Port=\" \";Version=1"); + Cookie[] parsed = cookiespec.parse("www.domain.com", 80 /* request port */, "/", false, header); assertNotNull(parsed); assertEquals(1, parsed.length); - cookie = (Cookie2) parsed[0]; + Cookie2 cookie = (Cookie2) parsed[0]; assertEquals(new int[] {80}, cookie.getPorts()); assertTrue(cookie.isPortAttributeSpecified() && cookie.isPortAttributeBlank()); + } - // invalid port - header = new Header("Set-Cookie2", "name=value;Port=nonsense;Version=1"); - parsed = cookiespec.parse("www.domain.com", 80, "/", false, header); - assertNull(parsed); - - // all ports must be > 0 - header = new Header("Set-Cookie2", "name=value;Port=\"80,-800,8000\";Version=1"); - parsed = cookiespec.parse("www.domain.com", 80, "/", false, header); - assertNull(parsed); - - // valid ports - header = new Header("Set-Cookie2", "name=value;Port=\"80,800,8000\";Version=1;Port=nonsense"); - parsed = cookiespec.parse("www.domain.com", 80, "/", false, header); - assertNotNull(parsed); - assertEquals(1, parsed.length); - // only the first occurence of port attribute is considered, others ignored - cookie = (Cookie2) parsed[0]; - assertEquals(new int[] {80,800,8000}, cookie.getPorts()); - assertTrue(cookie.isPortAttributeSpecified()); + public void testParseInvalidPort() throws Exception { + CookieSpec cookiespec = new RFC2965Spec(); + Header header = new Header("Set-Cookie2", "name=value;Port=nonsense;Version=1"); + try { + cookiespec.parse("www.domain.com", 80, "/", false, header); + fail("MalformedCookieException should have been thrown"); + } catch (MalformedCookieException ex) { + // expected + } + } - // Port is OPTIONAL, cookie can be accepted from any port - header = new Header("Set-Cookie2", "name=value;Version=1"); - parsed = cookiespec.parse("www.domain.com", 80, "/", false, header); - assertNotNull(parsed); - assertEquals(1, parsed.length); - cookie = (Cookie2) parsed[0]; - assertFalse(cookie.isPortAttributeSpecified()); + public void testParseNegativePort() throws Exception { + CookieSpec cookiespec = new RFC2965Spec(); + Header header = new Header("Set-Cookie2", "name=value;Port=\"80,-800,8000\";Version=1"); + try { + cookiespec.parse("www.domain.com", 80, "/", false, header); + fail("MalformedCookieException should have been thrown"); + } catch (MalformedCookieException ex) { + // expected + } } /** @@ -236,19 +285,8 @@ */ public void testParseVersion() throws Exception { CookieSpec cookiespec = new RFC2965Spec(); - // version cannot ne null - Header header = new Header("Set-Cookie2", "name=value;Version=;"); + Header header = new Header("Set-Cookie2", "name=value;Version=1;"); Cookie[] parsed = cookiespec.parse("www.domain.com", 80, "/", false, header); - assertNull(parsed); - - // version must be > 0 - header = new Header("Set-Cookie2", "name=value;Version=-1;"); - parsed = cookiespec.parse("www.domain.com", 80, "/", false, header); - assertNull(parsed); - - // valid version - header = new Header("Set-Cookie2", "name=value;Version=1;"); - parsed = cookiespec.parse("www.domain.com", 80, "/", false, header); assertNotNull(parsed); assertEquals(1, parsed.length); Cookie2 cookie = (Cookie2) parsed[0]; @@ -256,39 +294,75 @@ assertTrue(cookie.isVersionAttributeSpecified()); } + public void testParseNullVersion() throws Exception { + CookieSpec cookiespec = new RFC2965Spec(); + // version cannot ne null + Header header = new Header("Set-Cookie2", "name=value;Version=;"); + try { + cookiespec.parse("www.domain.com", 80, "/", false, header); + fail("MalformedCookieException should have been thrown"); + } catch (MalformedCookieException ex) { + // expected + } + } + + public void testParseNegativeVersion() throws Exception { + CookieSpec cookiespec = new RFC2965Spec(); + Header header = new Header("Set-Cookie2", "name=value;Version=-1;"); + try { + cookiespec.parse("www.domain.com", 80, "/", false, header); + fail("MalformedCookieException should have been thrown"); + } catch (MalformedCookieException ex) { + // expected + } + } /** * test parsing cookie <tt>"Max-age"</tt> attribute. */ public void testParseMaxage() throws Exception { CookieSpec cookiespec = new RFC2965Spec(); - // max-age cannot be null - Header header = new Header("Set-Cookie2", "name=value;Max-age=;Version=1"); + Header header = new Header("Set-Cookie2", "name=value;Max-age=3600;Version=1;Max-age=nonsense"); Cookie[] parsed = cookiespec.parse("www.domain.com", 80, "/", false, header); - assertNull(parsed); - - // max-age must be > 0 - header = new Header("Set-Cookie2", "name=value;Max-age=-3600;Version=1;"); - parsed = cookiespec.parse("www.domain.com", 80, "/", false, header); - assertNull(parsed); - - // valid max-age - header = new Header("Set-Cookie2", "name=value;Max-age=3600;Version=1;Max-age=nonsense"); - parsed = cookiespec.parse("www.domain.com", 80, "/", false, header); assertNotNull(parsed); assertEquals(1, parsed.length); // only the first occurence of max-age attribute is considered, others ignored Cookie2 cookie = (Cookie2) parsed[0]; assertFalse(cookie.isExpired()); + } + public void testParseMaxageDefault() throws Exception { + CookieSpec cookiespec = new RFC2965Spec(); // Max-age is OPTIONAL, defaults to session cookie - header = new Header("Set-Cookie2", "name=value;Version=1"); - parsed = cookiespec.parse("www.domain.com", 80, "/", false, header); + Header header = new Header("Set-Cookie2", "name=value;Version=1"); + Cookie[] parsed = cookiespec.parse("www.domain.com", 80, "/", false, header); assertNotNull(parsed); assertEquals(1, parsed.length); - cookie = (Cookie2) parsed[0]; + Cookie2 cookie = (Cookie2) parsed[0]; assertFalse(cookie.isPersistent()); } + public void testParseNullMaxage() throws Exception { + CookieSpec cookiespec = new RFC2965Spec(); + Header header = new Header("Set-Cookie2", "name=value;Max-age=;Version=1"); + try { + cookiespec.parse("www.domain.com", 80, "/", false, header); + fail("MalformedCookieException should have been thrown"); + } catch (MalformedCookieException ex) { + // expected + } + } + + public void testParseNegativeMaxage() throws Exception { + CookieSpec cookiespec = new RFC2965Spec(); + Header header = new Header("Set-Cookie2", "name=value;Max-age=-3600;Version=1;"); + try { + cookiespec.parse("www.domain.com", 80, "/", false, header); + fail("MalformedCookieException should have been thrown"); + } catch (MalformedCookieException ex) { + // expected + } + } + /** * test parsing <tt>"Discard"</tt> attribute. */ @@ -740,11 +814,11 @@ failNotEquals(null, intarrayToString(expected), intarrayToString(actual)); } - static private void failNotEquals(String message, Object expected, Object actual) { + public static void failNotEquals(String message, Object expected, Object actual) { fail(format(message, expected, actual)); } - static private String format(String message, Object expected, Object actual) { + public static String format(String message, Object expected, Object actual) { String formatted= ""; if (message != null) formatted = message + " "; --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]