You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by ol...@apache.org on 2006/04/28 17:13:28 UTC

svn commit: r397917 - in /jakarta/commons/proper/httpclient/branches/COOKIE_2_BRANCH/src: java/org/apache/commons/httpclient/ java/org/apache/commons/httpclient/cookie/ test/org/apache/commons/httpclient/cookie/

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: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org