commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jsde...@apache.org
Subject cvs commit: jakarta-commons/httpclient/src/test/org/apache/commons/httpclient TestCookie.java
Date Wed, 27 Nov 2002 01:32:34 GMT
jsdever     2002/11/26 17:32:34

  Modified:    httpclient/src/java/org/apache/commons/httpclient
                        Cookie.java
               httpclient/src/test/org/apache/commons/httpclient
                        TestCookie.java
  Log:
  Fixes for Cookie.java
  
  Fixes bugs:
  http://issues.apache.org/bugzilla/show_bug.cgi?id=6513
  http://issues.apache.org/bugzilla/show_bug.cgi?id=11223
  
  Whats Changed:
  1) Constructors throw IllegalArguementException if name parameter is
  null or blank. Current implementation of the cookie class accepts blank
  cookie name, which is disallowed if I interpret RFC-2109 right. Actually
  as far as I can see blank cookie values also violate the RFC-2109.
  Please let me know if you agree or disagree
  
  2) Default (parameterless) constructor added. The constructor assigns
  'noname' per default for a lack of a better idea on my part. Please let
  me know if you see a problem with this kind of naming convention
  
  3) Method Cookie.match() logs a warning message if a cookie has invalid state
  (domain and/or path not specified) and returns false.  IllegalArguementException
  is thrown if any of the input parameters is null.
  
  4) Path matching logic is somewhat improved as the current
  implementation is a bit flaky. Currently /foo and /foobar paths would
  match which is not supposed to be the case, unless I have missed something
  in the RFC
  
  5) All methods should react appropriately to null parameter input
  throwing an exception if null parameter is disallowed
  
  Contributed by: Oleg Kalnichevski
  
  Revision  Changes    Path
  1.25      +74 -22    jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/Cookie.java
  
  Index: Cookie.java
  ===================================================================
  RCS file: /home/cvs/jakarta-commons/httpclient/src/java/org/apache/commons/httpclient/Cookie.java,v
  retrieving revision 1.24
  retrieving revision 1.25
  diff -u -r1.24 -r1.25
  --- Cookie.java	21 Oct 2002 14:39:22 -0000	1.24
  +++ Cookie.java	27 Nov 2002 01:32:34 -0000	1.25
  @@ -91,6 +91,7 @@
    * @author Sean C. Sullivan
    * @author <a href="mailto:JEvans@Cyveillance.com">John Evans</a>
    * @author Marc A. Saegesser
  + * @author <a href="mailto:oleg@ural.ru">Oleg Kalnichevski</a>
    * @version $Revision$ $Date$
    */
   
  @@ -99,6 +100,16 @@
       // ----------------------------------------------------------- Constructors
   
       /**
  +     * Create a cookie. Default constructor 
  +     * 
  +     * The new cookie is assigned 
  +     */
  +
  +    public Cookie() {
  +        this(null, "noname", null, null, null, false);
  +    }
  +
  +    /**
        * Create a cookie.
        *
        * @param name    the cookie name
  @@ -124,6 +135,12 @@
       public Cookie(String domain, String name, String value, String path, Date expires,
boolean secure) {
           super(name, value);
           log.trace("enter Cookie(String, String, String, String, Date, boolean)");
  +        if (name == null) {
  +            throw new IllegalArgumentException("Cookie name may not be null");
  +        }
  +        if (name.equals("")) {
  +            throw new IllegalArgumentException("Cookie name may not be blank");
  +        }
           this.setPath(path);
           this.setDomain(domain);
           this.setExpiryDate(expires);
  @@ -448,7 +465,7 @@
        * @param secure <tt>true</tt> if the request is using the HTTPS protocol
        * @param date the time at which the request is submitted
        */
  -    public boolean matches(String domain, int port, String path, boolean secure, Date now)
{
  +    public boolean matches(String domain, int port, String path, boolean secure, Date date)
{
           log.trace("enter Cookie.matches(Strinng, int, String, boolean, Date");
           // FIXME: RFC2109 doesn't consider ports when filtering/matching
           //        cookies. Quoting from Section 2 - Terminology:
  @@ -464,18 +481,32 @@
           //
           //        The current implementation doesn't support RFC2965,
           //        and ignores ports when matching cookies.
  +        if (domain == null) {
  +            throw new IllegalArgumentException("domain parameter may not be null");
  +        }
  +        if (path == null) {
  +            throw new IllegalArgumentException("path parameter may not be null");
  +        }
  +        if (date == null) {
  +            throw new IllegalArgumentException("date parameter may not be null");
  +        }
  +        if (getDomain() == null) {
  +            log.warn("Invalid cookie state: domain not specified");
  +            return false;
  +        }
  +        if (getPath() == null) {
  +            log.warn("Invalid cookie state: path not specified");
  +            return false;
  +        }
  +        
           domain = domain.toLowerCase();
   
  -        // FIXME: Is path.startsWith(cookie.getPath()) enough?
  -        //        Or do we need to check that we are comparing
  -        //        at a path-element break?
  -        //        E.g.., if "/foo" is the cookie's path,
  -        //        should /foobar see the cookie? Probably not.
           return (
  -                (getExpiryDate() == null || getExpiryDate().after(now)) && // only
add the cookie if it hasn't yet expired
  -                (getDomain() != null && domainMatch(domain, getDomain())) &&//
and the domain pattern matches
  -                ((getPath() != null) && (path.startsWith(getPath()))) &&
  // and the path is null or matching
  -                (getSecure() ? secure : true)                              // and if the
secure flag is set, only if the request is actually secure
  +                (getExpiryDate() == null || 
  +                 getExpiryDate().after(date)) &&      // only add the cookie if
it hasn't yet expired
  +                (domainMatch(domain, getDomain())) && // and the domain pattern
matches
  +                (pathMatch(path, getPath())) &&       // and the path is null or
matching
  +                (getSecure() ? secure : true)         // and if the secure flag is set,
only if the request is actually secure
                  );
       }
   
  @@ -631,14 +662,14 @@
               return 0;
           } else if (c1.getPath() == null) {
               // null is assumed to be "/"
  -            if (c2.getPath().equals("/")) {
  +            if (c2.getPath().equals(PATH_DELIM)) {
                   return 0;
               } else {
                   return -1;
               }
           } else if (c2.getPath() == null) {
               // null is assumed to be "/"
  -            if (c1.getPath().equals("/")) {
  +            if (c1.getPath().equals(PATH_DELIM)) {
                   return 0;
               } else {
                   return 1;
  @@ -750,18 +781,21 @@
           if(path == null){
               throw new IllegalArgumentException("path may not be null.");
           }
  +        if(setCookie == null){
  +            throw new IllegalArgumentException("set-cookie header may not be null.");
  +        }
   
           /* Build the default path.  Per RFC 2109/4.3.1 this is the 
            * request path up to, but not including, the right-most / charater.
            */
           if(path.length() == 0){
               log.debug("Cookie.parse():  Fixing up empty request path.");
  -            path = "/";
  +            path = PATH_DELIM;
           }
           String defaultPath = null;
  -        int lastSlashIndex = path.lastIndexOf("/");
  +        int lastSlashIndex = path.lastIndexOf(PATH_DELIM);
           if(lastSlashIndex == 0){
  -            defaultPath = "/";
  +            defaultPath = PATH_DELIM;
           }else if(lastSlashIndex > 0){
               defaultPath = path.substring(0, lastSlashIndex);
           }else{
  @@ -985,12 +1019,12 @@
               // set path if not otherwise specified
               if(null == cookie.getPath()) {
                   if(null != path) {
  -                    if(!path.endsWith("/")) {
  -                        int x = path.lastIndexOf("/");
  +                    if(!path.endsWith(PATH_DELIM)) {
  +                        int x = path.lastIndexOf(PATH_DELIM);
                           if(0 < x) {
                               cookie.setPath(path.substring(0,x));
                           } else {
  -                            cookie.setPath("/");
  +                            cookie.setPath(PATH_DELIM);
                           }
                       } else {
                           cookie.setPath(path);
  @@ -1040,6 +1074,20 @@
       }
   
       /**
  +     * Performs a path-match slightly smarter than a straight-forward startsWith check.
  +     */
  +    private static boolean pathMatch(final String path, final String topmostPath)
  +    {
  +        boolean match = path.startsWith( topmostPath );
  +        if (match) {
  +            if (!topmostPath.endsWith(PATH_DELIM)) {
  +                match = (path.charAt(topmostPath.length()) == PATH_DELIM_CHAR);
  +            }
  +        }
  +        return match;
  +    }
  +
  +    /**
        * Adds the given cookie into the given list in descending path order.  That is, 
        * more specific path to least specific paths.  This may not be the fastest
        * algorythm, but it'll work OK for the small number of cookies we're 
  @@ -1108,6 +1156,10 @@
      private static final RuleBasedCollator stringCollator =
           (RuleBasedCollator)RuleBasedCollator.getInstance(
                                                   new Locale("en", "US", ""));
  +
  +   /** Path delimiter */
  +   private static final String PATH_DELIM = "/";
  +   private static final char   PATH_DELIM_CHAR = PATH_DELIM.charAt(0);
   
      /** Log object for this class */
      private static final Log log = LogFactory.getLog(Cookie.class);
  
  
  
  1.12      +29 -10    jakarta-commons/httpclient/src/test/org/apache/commons/httpclient/TestCookie.java
  
  Index: TestCookie.java
  ===================================================================
  RCS file: /home/cvs/jakarta-commons/httpclient/src/test/org/apache/commons/httpclient/TestCookie.java,v
  retrieving revision 1.11
  retrieving revision 1.12
  diff -u -r1.11 -r1.12
  --- TestCookie.java	21 Oct 2002 14:39:22 -0000	1.11
  +++ TestCookie.java	27 Nov 2002 01:32:34 -0000	1.12
  @@ -92,6 +92,7 @@
       private static final String OLD_EXPIRY = "Expires=Thu, 01-Jan-1970 00:00:10 GMT";
       private static final String SEP = ";";
       private static final String ROOT_PATH = "/";
  +    private static final int DEFAULT_PORT = 80;
   
       private String[] testName = { "custno", "name", "name" };
       private String[] testValue = { "12345", "John", "Doe, John" };
  @@ -222,7 +223,7 @@
           assertTrue("Secure",!parsed[0].getSecure());
           assertEquals("Version",0,parsed[0].getVersion());
       }
  -
  +/*
       public void testParseNoName() throws Exception {
           Header setCookie = new Header("Set-Cookie","=cookie-value");
           Cookie[] parsed = Cookie.parse("127.0.0.1","/",setCookie);
  @@ -238,7 +239,7 @@
           assertTrue("Secure",!parsed[0].getSecure());
           assertEquals("Version",0,parsed[0].getVersion());
       }
  -
  +*/
       public void testParseNoValue() throws Exception {
           Header setCookie = new Header("Set-Cookie","cookie-name=");
           Cookie[] parsed = Cookie.parse("127.0.0.1","/",setCookie);
  @@ -508,6 +509,18 @@
           }
       }
       
  +    public void testParseWithPathMismatch2() {
  +        Header setCookie = new Header("Set-Cookie","cookie-name=cookie-value; path=/foobar");
  +        try {
  +            Cookie[] parsed = Cookie.parse("127.0.0.1","/foo",false,setCookie);
  +            fail("HttpException should have been thrown.");
  +        } catch (HttpException e) {
  +            // expected
  +        } catch (Exception e){
  +            fail("Should have thrown HttpException.");
  +        }
  +    }
  +    
       public void testComparator() throws Exception {
           Header setCookie = null;
           Cookie[] parsed = null;
  @@ -578,7 +591,7 @@
           Cookie[] parsed = Cookie.parse(DOMAIN_NAME, ROOT_PATH, true, setCookie);
   
           try{
  -            Header header = Cookie.createCookieHeader(null, ROOT_PATH, false, parsed);
  +            Header header = Cookie.createCookieHeader(null, DEFAULT_PORT, ROOT_PATH, false,
parsed);
               fail("IllegalArgumentException should have been thrown.");
           }catch(IllegalArgumentException e){
               // Expected
  @@ -595,7 +608,7 @@
           Cookie[] parsed = Cookie.parse(DOMAIN_NAME, ROOT_PATH, false, setCookie);
   
           try{
  -            Header header = Cookie.createCookieHeader(DOMAIN_NAME, null, false, parsed);
  +            Header header = Cookie.createCookieHeader(DOMAIN_NAME, DEFAULT_PORT, null,
false, parsed);
               fail("IllegalArgumentException should have been thrown.");
           }catch(IllegalArgumentException e){
               // Expected
  @@ -612,7 +625,7 @@
           cookies[0] = new Cookie(null, "name0", "value0");
           cookies[1] = new Cookie(null, "name1", "value1", null, null, false);
   
  -        Header header = Cookie.createCookieHeader(DOMAIN_NAME, ROOT_PATH, false, cookies);
  +        Header header = Cookie.createCookieHeader(DOMAIN_NAME, DEFAULT_PORT, ROOT_PATH,
false, cookies);
           assertEquals("createCookieHeader added cookies with null domains or paths", null,
header);
       }
   
  @@ -625,7 +638,7 @@
           Cookie[] parsed = Cookie.parse(DOMAIN_NAME, ROOT_PATH, true, setCookie);
   
           try{
  -            Header header = Cookie.createCookieHeader(null, null, false, parsed);
  +            Header header = Cookie.createCookieHeader(null, DEFAULT_PORT, null, false,
parsed);
               fail("IllegalArgumentException should have been thrown.");
           }catch(IllegalArgumentException e){
               // Expected
  @@ -665,6 +678,12 @@
       private void checkDate(String date) throws Exception {
           Header setCookie = new Header("Set-Cookie", "custno=12345;Expires='"+date+"'");
           Cookie.parse("localhost","/",setCookie);
  +    }
  +    
  +    
  +    public void testDefaultConsttuctor() throws Exception {
  +        Cookie dummy = new Cookie();
  +        assertEquals( dummy.toExternalForm(), "noname=null" );
       }
   }
   
  
  
  

--
To unsubscribe, e-mail:   <mailto:commons-dev-unsubscribe@jakarta.apache.org>
For additional commands, e-mail: <mailto:commons-dev-help@jakarta.apache.org>


Mime
View raw message