You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2008/01/06 22:23:25 UTC

svn commit: r609406 - in /tomcat: connectors/trunk/util/java/org/apache/tomcat/util/http/ container/tc5.5.x/catalina/src/share/org/apache/catalina/connector/ container/tc5.5.x/webapps/docs/ current/tc5.5.x/

Author: markt
Date: Sun Jan  6 13:23:24 2008
New Revision: 609406

URL: http://svn.apache.org/viewvc?rev=609406&view=rev
Log:
Port Cookie parsing changes from TC6. I believe this patch is complete but will be double checking it after the commit.

Modified:
    tomcat/connectors/trunk/util/java/org/apache/tomcat/util/http/Cookies.java
    tomcat/connectors/trunk/util/java/org/apache/tomcat/util/http/ServerCookie.java
    tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/connector/Request.java
    tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/connector/Response.java
    tomcat/container/tc5.5.x/webapps/docs/changelog.xml
    tomcat/current/tc5.5.x/STATUS.txt

Modified: tomcat/connectors/trunk/util/java/org/apache/tomcat/util/http/Cookies.java
URL: http://svn.apache.org/viewvc/tomcat/connectors/trunk/util/java/org/apache/tomcat/util/http/Cookies.java?rev=609406&r1=609405&r2=609406&view=diff
==============================================================================
--- tomcat/connectors/trunk/util/java/org/apache/tomcat/util/http/Cookies.java (original)
+++ tomcat/connectors/trunk/util/java/org/apache/tomcat/util/http/Cookies.java Sun Jan  6 13:23:24 2008
@@ -45,6 +45,27 @@
     boolean unprocessed=true;
 
     MimeHeaders headers;
+
+    /*
+    List of Separator Characters (see isSeparator())
+    Excluding the '/' char violates the RFC, but 
+    it looks like a lot of people put '/'
+    in unquoted values: '/': ; //47 
+    '\t':9 ' ':32 '\"':34 '\'':39 '(':40 ')':41 ',':44 ':':58 ';':59 '<':60 
+    '=':61 '>':62 '?':63 '@':64 '[':91 '\\':92 ']':93 '{':123 '}':125
+    */
+    public static final char SEPARATORS[] = { '\t', ' ', '\"', '\'', '(', ')', ',', 
+        ':', ';', '<', '=', '>', '?', '@', '[', '\\', ']', '{', '}' };
+
+    protected static final boolean separators[] = new boolean[128];
+    static {
+        for (int i = 0; i < 128; i++) {
+            separators[i] = false;
+        }
+        for (int i = 0; i < SEPARATORS.length; i++) {
+            separators[SEPARATORS[i]] = true;
+        }
+    }
     
     /**
      *  Construct a new cookie collection, that will extract
@@ -182,182 +203,6 @@
         }
     }
 
-    /** Process a byte[] header - allowing fast processing of the
-     *  raw data
-     */
-    void processCookieHeader(  byte bytes[], int off, int len )
-    {
-        if( len<=0 || bytes==null ) return;
-        int end=off+len;
-        int pos=off;
-        
-        int version=0; //sticky
-        ServerCookie sc=null;
-        
-
-        while( pos<end ) {
-            byte cc;
-            // [ skip_spaces name skip_spaces "=" skip_spaces value EXTRA ; ] *
-            if( dbg>0 ) log( "Start: " + pos + " " + end );
-            
-            pos=skipSpaces(bytes, pos, end);
-            if( pos>=end )
-                return; // only spaces
-            int startName=pos;
-            if( dbg>0 ) log( "SN: " + pos );
-            
-            // Version should be the first token
-            boolean isSpecial=false;
-            if(bytes[pos]=='$') { pos++; isSpecial=true; }
-
-            pos= findDelim1( bytes, startName, end); // " =;,"
-            int endName=pos;
-            // current = "=" or " " or DELIM
-            pos= skipSpaces( bytes, endName, end ); 
-            if( dbg>0 ) log( "DELIM: " + endName + " " + (char)bytes[pos]);
-
-            if(pos >= end ) {
-                // it's a name-only cookie ( valid in RFC2109 )
-                if( ! isSpecial ) {
-                    sc=addCookie();
-                    sc.getName().setBytes( bytes, startName,
-                                           endName-startName );
-                    sc.getValue().setString("");
-                    sc.setVersion( version );
-                    if( dbg>0 ) log( "Name only, end: " + startName + " " +
-                                     endName);
-                }
-                return;
-            }
-
-            cc=bytes[pos];
-            pos++;
-            if( cc==';' || cc==',' || pos>=end ) {
-                if( ! isSpecial && startName!= endName ) {
-                    sc=addCookie();
-                    sc.getName().setBytes( bytes, startName,
-                                           endName-startName );
-                    sc.getValue().setString("");
-                    sc.setVersion( version );
-                    if( dbg>0 ) log( "Name only: " + startName + " " + endName);
-                }
-                continue;
-            }
-            
-            // we should have "=" ( tested all other alternatives )
-            int startValue=skipSpaces( bytes, pos, end);
-            int endValue=startValue;
-            
-            cc=bytes[pos];
-            if(  cc=='"' ) {
-                endValue=findDelim3( bytes, startValue+1, end, cc );
-                if (endValue == -1) {
-                    endValue = findDelim2(bytes, startValue+1, end);
-                } else startValue++;
-                pos=endValue+1; // to skip to next cookie
-             } else {
-                endValue=findDelim2( bytes, startValue, end );
-                pos=endValue+1;
-            }
-            
-            // if not $Version, etc
-            if( ! isSpecial ) {
-                sc=addCookie();
-                sc.getName().setBytes( bytes, startName, endName-startName );
-                sc.getValue().setBytes( bytes, startValue, endValue-startValue);
-                sc.setVersion( version );
-                if( dbg>0 ) {
-                    log( "New: " + sc.getName() + "X=X" + sc.getValue());
-                }
-                continue;
-            }
-            
-            // special - Path, Version, Domain, Port
-            if( dbg>0 ) log( "Special: " + startName + " " + endName);
-            // XXX TODO
-            if( equals( "$Version", bytes, startName, endName ) ) {
-                if(dbg>0 ) log( "Found version " );
-                if( bytes[startValue]=='1' && endValue==startValue+1 ) {
-                    version=1;
-                    if(dbg>0 ) log( "Found version=1" );
-                }
-                continue;
-            }
-            if( sc==null ) {
-                // Path, etc without a previous cookie
-                continue;
-            }
-            if( equals( "$Path", bytes, startName, endName ) ) {
-                sc.getPath().setBytes( bytes,
-                                       startValue,
-                                       endValue-startValue );
-            }
-            if( equals( "$Domain", bytes, startName, endName ) ) {
-                sc.getDomain().setBytes( bytes,
-                                         startValue,
-                                         endValue-startValue );
-            }
-            if( equals( "$Port", bytes, startName, endName ) ) {
-                // sc.getPort().setBytes( bytes,
-                //                        startValue,
-                //                        endValue-startValue );
-            }
-        }
-    }
-
-    // -------------------- Utils --------------------
-    public static int skipSpaces(  byte bytes[], int off, int end ) {
-        while( off < end ) {
-            byte b=bytes[off];
-            if( b!= ' ' ) return off;
-            off ++;
-        }
-        return off;
-    }
-
-    public static int findDelim1( byte bytes[], int off, int end )
-    {
-        while( off < end ) {
-            byte b=bytes[off];
-            if( b==' ' || b=='=' || b==';' || b==',' )
-                return off;
-            off++;
-        }
-        return off;
-    }
-
-    public static int findDelim2( byte bytes[], int off, int end )
-    {
-        while( off < end ) {
-            byte b=bytes[off];
-            if( b==';' || b==',' )
-                return off;
-            off++;
-        }
-        return off;
-    }
-
-    /*
-     *  search for cc but skip \cc as required by rfc2616
-     *  (according to rfc2616 cc should be ")
-     */
-    public static int findDelim3( byte bytes[], int off, int end, byte cc )
-    {
-        while( off < end ) {
-            byte b=bytes[off];
-            if (b=='\\') {
-                off++;
-                off++;
-                continue;
-            }
-            if( b==cc )
-                return off;
-            off++;
-        }
-        return -1;
-    }
-
-    
     // XXX will be refactored soon!
     public static boolean equals( String s, byte b[], int start, int end) {
         int blen = end-start;
@@ -441,42 +286,298 @@
             log.debug("Cookies: " + s);
     }
 
-    /*
-    public static void main( String args[] ) {
-        test("foo=bar; a=b");
-        test("foo=bar;a=b");
-        test("foo=bar;a=b;");
-        test("foo=bar;a=b; ");
-        test("foo=bar;a=b; ;");
-        test("foo=;a=b; ;");
-        test("foo;a=b; ;");
-        // v1 
-        test("$Version=1; foo=bar;a=b"); 
-        test("$Version=\"1\"; foo='bar'; $Path=/path; $Domain=\"localhost\"");
-        test("$Version=1;foo=bar;a=b; ; ");
-        test("$Version=1;foo=;a=b; ; ");
-        test("$Version=1;foo= ;a=b; ; ");
-        test("$Version=1;foo;a=b; ; ");
-        test("$Version=1;foo=\"bar\";a=b; ; ");
-        test("$Version=1;foo=\"bar\";$Path=/examples;a=b; ; ");
-        test("$Version=1;foo=\"bar\";$Domain=apache.org;a=b");
-        test("$Version=1;foo=\"bar\";$Domain=apache.org;a=b;$Domain=yahoo.com");
-        // rfc2965
-        test("$Version=1;foo=\"bar\";$Domain=apache.org;$Port=8080;a=b");
-
-        // wrong
-        test("$Version=1;foo=\"bar\";$Domain=apache.org;$Port=8080;a=b");
-    }
-
-    public static void test( String s ) {
-        System.out.println("Processing " + s );
-        Cookies cs=new Cookies(null);
-        cs.processCookieHeader( s.getBytes(), 0, s.length());
-        for( int i=0; i< cs.getCookieCount() ; i++ ) {
-            System.out.println("Cookie: " + cs.getCookie( i ));
+   /**
+     * Returns true if the byte is a separator character as
+     * defined in RFC2619. Since this is called often, this
+     * function should be organized with the most probable
+     * outcomes first.
+     */
+    public static final boolean isSeparator(final byte c) {
+         if (c > 0 && c < 126)
+             return separators[c];
+         else
+             return false;
+    }
+    
+    /**
+     * Returns true if the byte is a whitespace character as
+     * defined in RFC2619.
+     */
+    public static final boolean isWhiteSpace(final byte c) {
+        // This switch statement is slightly slower
+        // for my vm than the if statement.
+        // Java(TM) 2 Runtime Environment, Standard Edition (build 1.5.0_07-164)
+        /* 
+        switch (c) {
+        case ' ':;
+        case '\t':;
+        case '\n':;
+        case '\r':;
+        case '\f':;
+            return true;
+        default:;
+            return false;
+             }
+        */
+       if (c == ' ' || c == '\t' || c == '\n' || c == '\r' || c == '\f')
+           return true;
+       else
+           return false;
+    }
+
+    /**
+     * Parses a cookie header after the initial "Cookie:"
+     * [WS][$]token[WS]=[WS](token|QV)[;|,]
+     * RFC 2965
+     * JVK
+     */
+    public final void processCookieHeader(byte bytes[], int off, int len){
+        if( len<=0 || bytes==null ) return;
+        int end=off+len;
+        int pos=off;
+        int nameStart=0;
+        int nameEnd=0;
+        int valueStart=0;
+        int valueEnd=0;
+        int version = 0;
+        ServerCookie sc=null;
+        boolean isSpecial;
+        boolean isQuoted;
+
+        while (pos < end) {
+            isSpecial = false;
+            isQuoted = false;
+
+            // Skip whitespace and non-token characters (separators)
+            while (pos < end && 
+                   (isSeparator(bytes[pos]) || isWhiteSpace(bytes[pos]))) 
+                {pos++; } 
+
+            if (pos >= end)
+                return;
+
+            // Detect Special cookies
+            if (bytes[pos] == '$') {
+                isSpecial = true;
+                pos++;
+            }
+
+            // Get the cookie name. This must be a token            
+            valueEnd = valueStart = nameStart = pos; 
+            pos = nameEnd = getTokenEndPosition(bytes,pos,end);
+
+            // Skip whitespace
+            while (pos < end && isWhiteSpace(bytes[pos])) {pos++; }; 
+         
+
+            // Check for an '=' -- This could also be a name-only
+            // cookie at the end of the cookie header, so if we
+            // are past the end of the header, but we have a name
+            // skip to the name-only part.
+            if (pos < end && bytes[pos] == '=') {                
+
+                // Skip whitespace
+                do {
+                    pos++;
+                } while (pos < end && isWhiteSpace(bytes[pos])); 
+
+                if (pos >= end)
+                    return;
+
+                // Determine what type of value this is, quoted value,
+                // token, name-only with an '=', or other (bad)
+                switch (bytes[pos]) {
+                case '"':; // Quoted Value
+                    isQuoted = true;
+                    valueStart=pos + 1; // strip "
+                    // getQuotedValue returns the position before 
+                    // at the last qoute. This must be dealt with
+                    // when the bytes are copied into the cookie
+                    valueEnd=getQuotedValueEndPosition(bytes, 
+                                                       valueStart, end);
+                    // We need pos to advance
+                    pos = valueEnd; 
+                    // Handles cases where the quoted value is 
+                    // unterminated and at the end of the header, 
+                    // e.g. [myname="value]
+                    if (pos >= end)
+                        return;
+                    break;
+                case ';':
+                case ',':
+                    // Name-only cookie with an '=' after the name token
+                    // This may not be RFC compliant
+                    valueStart = valueEnd = -1;
+                    // The position is OK (On a delimiter)
+                    break;
+                default:;
+                    if (!isSeparator(bytes[pos])) {
+                        // Token
+                        valueStart=pos;
+                        // getToken returns the position at the delimeter
+                        // or other non-token character
+                        valueEnd=getTokenEndPosition(bytes, valueStart, end);
+                        // We need pos to advance
+                        pos = valueEnd;
+                    } else  {
+                        // INVALID COOKIE, advance to next delimiter
+                        // The starting character of the cookie value was
+                        // not valid.
+                        log("Invalid cookie. Value not a token or quoted value");
+                        while (pos < end && bytes[pos] != ';' && 
+                               bytes[pos] != ',') 
+                            {pos++; };
+                        pos++;
+                        // Make sure no special avpairs can be attributed to 
+                        // the previous cookie by setting the current cookie
+                        // to null
+                        sc = null;
+                        continue;                        
+                    }
+                }
+            } else {
+                // Name only cookie
+                valueStart = valueEnd = -1;
+                pos = nameEnd;
+
+            }
+          
+            // We should have an avpair or name-only cookie at this
+            // point. Perform some basic checks to make sure we are
+            // in a good state.
+  
+            // Skip whitespace
+            while (pos < end && isWhiteSpace(bytes[pos])) {pos++; }; 
+
+
+            // Make sure that after the cookie we have a separator. This
+            // is only important if this is not the last cookie pair
+            while (pos < end && bytes[pos] != ';' && bytes[pos] != ',') { 
+                pos++;
+            }
+                 
+            pos++;
+
+            /*
+            if (nameEnd <= nameStart || valueEnd < valueStart ) {
+                // Something is wrong, but this may be a case
+                // of having two ';' characters in a row.
+                // log("Cookie name/value does not conform to RFC 2965");
+                // Advance to next delimiter (ignoring everything else)
+                while (pos < end && bytes[pos] != ';' && bytes[pos] != ',') 
+                    { pos++; };
+                pos++;
+                // Make sure no special cookies can be attributed to 
+                // the previous cookie by setting the current cookie
+                // to null
+                sc = null;
+                continue;
+            }
+            */
+
+            // All checks passed. Add the cookie, start with the 
+            // special avpairs first
+            if (isSpecial) {
+                isSpecial = false;
+                // $Version must be the first avpair in the cookie header
+                // (sc must be null)
+                if (equals( "Version", bytes, nameStart, nameEnd) && 
+                    sc == null) {
+                    // Set version
+                    if( bytes[valueStart] =='1' && valueEnd == (valueStart+1)) {
+                        version=1;
+                    } else {
+                        // unknown version (Versioning is not very strict)
+                    }
+                    continue;
+                } 
+                
+                // We need an active cookie for Path/Port/etc.
+                if (sc == null) {
+                    continue;
+                }
+
+                // Domain is more common, so it goes first
+                if (equals( "Domain", bytes, nameStart, nameEnd)) {
+                    sc.getDomain().setBytes( bytes,
+                                           valueStart,
+                                           valueEnd-valueStart);
+                    continue;
+                } 
+
+                if (equals( "Path", bytes, nameStart, nameEnd)) {
+                    sc.getPath().setBytes( bytes,
+                                           valueStart,
+                                           valueEnd-valueStart);
+                    continue;
+                } 
+
+
+                if (equals( "Port", bytes, nameStart, nameEnd)) {
+                    // sc.getPort is not currently implemented.
+                    // sc.getPort().setBytes( bytes,
+                    //                        valueStart,
+                    //                        valueEnd-valueStart );
+                    continue;
+                } 
+
+                // Unknown cookie, complain
+                log("Unknown Special Cookie");
+
+            } else { // Normal Cookie
+                sc = addCookie();
+                sc.setVersion( version );
+                sc.getName().setBytes( bytes, nameStart,
+                                       nameEnd-nameStart);
+                
+                if (valueStart != -1) { // Normal AVPair
+                    sc.getValue().setBytes( bytes, valueStart,
+                            valueEnd-valueStart);
+                    if (isQuoted) {
+                        // We know this is a byte value so this is safe
+                        ServerCookie.unescapeDoubleQuotes(
+                                sc.getValue().getByteChunk());
+                    }                    
+                } else {
+                    // Name Only
+                    sc.getValue().setString(""); 
+                }
+                continue;
+            }
         }
-            
     }
-    */
 
+    /**
+     * Given the starting position of a token, this gets the end of the
+     * token, with no separator characters in between.
+     * JVK
+     */
+    public static final int getTokenEndPosition(byte bytes[], int off, int end){
+        int pos = off;
+        while (pos < end && !isSeparator(bytes[pos])) {pos++; };
+        
+        if (pos > end)
+            return end;
+        return pos;
+    }
+
+    /** 
+     * Given a starting position after an initial quote chracter, this gets
+     * the position of the end quote. This escapes anything after a '\' char
+     * JVK RFC 2616
+     */
+    public static final int getQuotedValueEndPosition(byte bytes[], int off, int end){
+        int pos = off;
+        while (pos < end) {
+            if (bytes[pos] == '"') {
+                return pos;                
+            } else if (bytes[pos] == '\\' && pos < (end - 1)) {
+                pos+=2;
+            } else {
+                pos++;
+            }
+        }
+        // Error, we have reached the end of the header w/o a end quote
+        return end;
+    }
 }

Modified: tomcat/connectors/trunk/util/java/org/apache/tomcat/util/http/ServerCookie.java
URL: http://svn.apache.org/viewvc/tomcat/connectors/trunk/util/java/org/apache/tomcat/util/http/ServerCookie.java?rev=609406&r1=609405&r2=609406&view=diff
==============================================================================
--- tomcat/connectors/trunk/util/java/org/apache/tomcat/util/http/ServerCookie.java (original)
+++ tomcat/connectors/trunk/util/java/org/apache/tomcat/util/http/ServerCookie.java Sun Jan  6 13:23:24 2008
@@ -21,13 +21,14 @@
 import java.text.FieldPosition;
 import java.util.Date;
 
+import org.apache.tomcat.util.buf.ByteChunk;
 import org.apache.tomcat.util.buf.DateTool;
 import org.apache.tomcat.util.buf.MessageBytes;
 
 
 /**
  *  Server-side cookie representation.
- *   Allows recycling and uses MessageBytes as low-level
+ *  Allows recycling and uses MessageBytes as low-level
  *  representation ( and thus the byte-> char conversion can be delayed
  *  until we know the charset ).
  *
@@ -37,38 +38,43 @@
 public class ServerCookie implements Serializable {
     
     
-    private static org.apache.commons.logging.Log log=
-        org.apache.commons.logging.LogFactory.getLog(ServerCookie.class );
+    private static org.apache.commons.logging.Log log =
+        org.apache.commons.logging.LogFactory.getLog(ServerCookie.class);
     
+    // Version 0 (Netscape) attributes
     private MessageBytes name=MessageBytes.newInstance();
     private MessageBytes value=MessageBytes.newInstance();
+    // Expires - Not stored explicitly. Generated from Max-Age (see V1)
+    private MessageBytes path=MessageBytes.newInstance();
+    private MessageBytes domain=MessageBytes.newInstance();
+    private boolean secure;
+
+    // Version 1 (RFC2109) attributes
+    private MessageBytes comment=MessageBytes.newInstance();
+    private int maxAge = -1;
+    private int version = 0;
+
+    // Note: Servlet Spec =< 2.5 only refers to Netscape and RFC2109,
+    // not RFC2965
+
+    // Version 1 (RFC2965) attributes
+    // TODO Add support for CommentURL
+    // Discard - implied by maxAge <0
+    // TODO Add support for Port
 
-    private MessageBytes comment=MessageBytes.newInstance(); //;Comment=VALUE
-    private MessageBytes domain=MessageBytes.newInstance();  //;Domain=VALUE ...
-
-    private int maxAge = -1;        // ;Max-Age=VALUE
-                                // ;Discard ... implied by maxAge < 0
-    // RFC2109: maxAge=0 will end a session
-    private MessageBytes path=MessageBytes.newInstance();        // ;Path=VALUE
-    private boolean secure;        // ;Secure
-    private int version = 0;        // ;Version=1
-
-    //XXX CommentURL, Port -> use notes ?
-    
     public ServerCookie() {
-
     }
 
     public void recycle() {
         path.recycle();
-            name.recycle();
-            value.recycle();
-            comment.recycle();
-            maxAge=-1;
-            path.recycle();
+        name.recycle();
+        value.recycle();
+        comment.recycle();
+        maxAge=-1;
+        path.recycle();
         domain.recycle();
-            version=0;
-            secure=false;
+        version=0;
+        secure=false;
     }
 
     public MessageBytes getComment() {
@@ -87,7 +93,6 @@
         return maxAge;
     }
 
-
     public MessageBytes getPath() {
         return path;
     }
@@ -112,7 +117,6 @@
         return version;
     }
 
-
     public void setVersion(int v) {
         version = v;
     }
@@ -120,17 +124,18 @@
 
     // -------------------- utils --------------------
 
+    public static void log(String s ) {
+        if (log.isDebugEnabled())
+            log.debug("ServerCookie: " + s);
+    }
+
     public String toString() {
         return "Cookie " + getName() + "=" + getValue() + " ; "
             + getVersion() + " " + getPath() + " " + getDomain();
     }
     
-    // Note -- disabled for now to allow full Netscape compatibility
-    // from RFC 2068, token special case characters
-    //
-    // private static final String tspecials = "()<>@,;:\\\"/[]?={} \t";
     private static final String tspecials = ",; ";
-    private static final String tspecials2 = ",; \"";
+    private static final String tspecials2 = "()<>@,;:\\\"/[]?={} \t";
 
     /*
      * Tests a string and returns true if the string counts as a
@@ -149,12 +154,27 @@
         for (int i = 0; i < len; i++) {
             char c = value.charAt(i);
 
-            if (c < 0x20 || c >= 0x7f || tspecials.indexOf(c) != -1)
+            if (tspecials.indexOf(c) != -1)
                 return false;
         }
         return true;
     }
 
+    public static boolean containsCTL(String value, int version) {
+        if( value==null) return false;
+        int len = value.length();
+        for (int i = 0; i < len; i++) {
+            char c = value.charAt(i);
+            if (c < 0x20 || c >= 0x7f) {
+                if (c == 0x09)
+                    continue; //allow horizontal tabs
+                return true;
+            }
+        }
+        return false;
+    }
+    
+        
     public static boolean isToken2(String value) {
         if( value==null) return true;
         int len = value.length();
@@ -162,23 +182,27 @@
         for (int i = 0; i < len; i++) {
             char c = value.charAt(i);
 
-            if (c < 0x20 || c >= 0x7f || tspecials2.indexOf(c) != -1)
+            if (tspecials2.indexOf(c) != -1)
                 return false;
         }
         return true;
     }
 
 
+    /**
+     * @deprecated - Not used
+     */
     public static boolean checkName( String name ) {
         if (!isToken(name)
-                || name.equalsIgnoreCase("Comment")        // rfc2019
-                || name.equalsIgnoreCase("Discard")        // 2019++
-                || name.equalsIgnoreCase("Domain")
-                || name.equalsIgnoreCase("Expires")        // (old cookies)
-                || name.equalsIgnoreCase("Max-Age")        // rfc2019
-                || name.equalsIgnoreCase("Path")
-                || name.equalsIgnoreCase("Secure")
-                || name.equalsIgnoreCase("Version")
+                || name.equalsIgnoreCase("Comment")     // rfc2019
+                || name.equalsIgnoreCase("Discard")     // rfc2965
+                || name.equalsIgnoreCase("Domain")      // rfc2019
+                || name.equalsIgnoreCase("Expires")     // Netscape
+                || name.equalsIgnoreCase("Max-Age")     // rfc2019
+                || name.equalsIgnoreCase("Path")        // rfc2019
+                || name.equalsIgnoreCase("Secure")      // rfc2019
+                || name.equalsIgnoreCase("Version")     // rfc2019
+                // TODO remaining RFC2965 attributes
             ) {
             return false;
         }
@@ -188,8 +212,8 @@
     // -------------------- Cookie parsing tools
 
     
-    /** Return the header name to set the cookie, based on cookie
-     *  version
+    /**
+     * Return the header name to set the cookie, based on cookie version.
      */
     public String getCookieHeaderName() {
         return getCookieHeaderName(version);
@@ -199,14 +223,16 @@
      *  version
      */
     public static String getCookieHeaderName(int version) {
-        if( dbg>0 ) log( (version==1) ? "Set-Cookie2" : "Set-Cookie");
+        // TODO Re-enable logging when RFC2965 is implemented
+        // log( (version==1) ? "Set-Cookie2" : "Set-Cookie");
         if (version == 1) {
+            // XXX RFC2965 not referenced in Servlet Spec
+            // Set-Cookie2 is not supported by Netscape 4, 6, IE 3, 5
+            // Set-Cookie2 is supported by Lynx and Opera
+            // Need to check on later IE and FF releases but for now...
             // RFC2109
             return "Set-Cookie";
-            // XXX RFC2965 is not standard yet, and Set-Cookie2
-            // is not supported by Netscape 4, 6, IE 3, 5 .
-            // It is supported by Lynx, and there is hope 
-            //            return "Set-Cookie2";
+            // return "Set-Cookie2";
         } else {
             // Old Netscape
             return "Set-Cookie";
@@ -216,7 +242,8 @@
     private static final String ancientDate =
         DateTool.formatOldCookie(new Date(10000));
 
-    public static void appendCookieValue( StringBuffer buf,
+    // TODO RFC2965 fields also need to be passed
+    public static void appendCookieValue( StringBuffer headerBuf,
                                           int version,
                                           String name,
                                           String value,
@@ -226,13 +253,15 @@
                                           int maxAge,
                                           boolean isSecure )
     {
-        // this part is the same for all cookies
+        StringBuffer buf = new StringBuffer();
+        // Servlet implementation checks name
         buf.append( name );
         buf.append("=");
+        // Servlet implementation does not check anything else
+        
         maybeQuote2(version, buf, value);
 
-        // XXX Netscape cookie: "; "
-         // add version 1 specific information
+        // Add version 1 specific information
         if (version == 1) {
             // Version=1 ... required
             buf.append ("; Version=1");
@@ -240,26 +269,23 @@
             // Comment=comment
             if ( comment!=null ) {
                 buf.append ("; Comment=");
-                maybeQuote (version, buf, comment);
+                maybeQuote2(version, buf, comment);
             }
         }
         
-        // add domain information, if present
-
+        // Add domain information, if present
         if (domain!=null) {
             buf.append("; Domain=");
-            maybeQuote (version, buf, domain);
+            maybeQuote2(version, buf, domain);
         }
 
-        // Max-Age=secs/Discard ... or use old "Expires" format
+        // Max-Age=secs ... or use old "Expires" format
+        // TODO RFC2965 Discard
         if (maxAge >= 0) {
             if (version == 0) {
-                // XXX XXX XXX We need to send both, for
-                // interoperatibility (long word )
+                // Wdy, DD-Mon-YY HH:MM:SS GMT ( Expires Netscape format )
                 buf.append ("; Expires=");
-                // Wdy, DD-Mon-YY HH:MM:SS GMT ( Expires netscape format )
-                // To expire we need to set the time back in future
-                // ( pfrieden@dChain.com )
+                // To expire immediately we need to set the time in past
                 if (maxAge == 0)
                     buf.append( ancientDate );
                 else
@@ -277,7 +303,7 @@
         // Path=path
         if (path!=null) {
             buf.append ("; Path=");
-            maybeQuote (version, buf, path);
+            maybeQuote2(version, buf, path);
         }
 
         // Secure
@@ -285,69 +311,117 @@
           buf.append ("; Secure");
         }
         
-        
+        headerBuf.append(buf);
     }
 
-    public static void maybeQuote (int version, StringBuffer buf,
-            String value) {
+    /**
+     * @deprecated - Not used
+     */
+    @Deprecated
+    public static void maybeQuote(int version, StringBuffer buf, String value) {
         // special case - a \n or \r  shouldn't happen in any case
         if (isToken(value)) {
             buf.append(value);
         } else {
             buf.append('"');
-            buf.append(escapeDoubleQuotes(value));
+            buf.append(escapeDoubleQuotes(value,0,value.length()));
             buf.append('"');
         }
     }
 
+
+    public static boolean alreadyQuoted (String value) {
+        if (value==null || value.length()==0) return false;
+        return (value.charAt(0)=='\"' && value.charAt(value.length()-1)=='\"');
+    }
     
-    public static void maybeQuote2 (int version, StringBuffer buf,
+        
+    /**
+     * Quotes values using rules that vary depending on Cookie version.
+     * @param version
+     * @param buf
+     * @param value
+     */
+    public static void maybeQuote2(int version, StringBuffer buf,
             String value) {
-        // special case - a \n or \r  shouldn't happen in any case
-        if (isToken2(value)) {
-            buf.append(value);
-        } else {
+        if (value==null || value.length()==0) {
+            buf.append("\"\"");
+        } else if (containsCTL(value,version)) 
+            throw new IllegalArgumentException("Control character in cookie value, consider BASE64 encoding your value");
+        else if (alreadyQuoted(value)) {
+            buf.append('"');
+            buf.append(escapeDoubleQuotes(value,1,value.length()-1));            buf.append('"');
             buf.append('"');
-            buf.append(escapeDoubleQuotes(value));
+        } else if (version==0 && !isToken(value)) {
             buf.append('"');
+            buf.append(escapeDoubleQuotes(value,0,value.length()));
+            buf.append('"');
+        } else if (version==1 && !isToken2(value)) {
+            buf.append('"');
+            buf.append(escapeDoubleQuotes(value,0,value.length()));
+            buf.append('"');
+        } else {
+            buf.append(value);
         }
     }
     
-    
-    // log
-    static final int dbg=1;
-    public static void log(String s ) {
-        if (log.isDebugEnabled())
-            log.debug("ServerCookie: " + s);
-    }
-
-
     /**
      * Escapes any double quotes in the given string.
      *
      * @param s the input string
-     *
+     * @param beginIndex start index inclusive
+     * @param endIndex exclusive
      * @return The (possibly) escaped string
      */
-    private static String escapeDoubleQuotes(String s) {
+    private static String escapeDoubleQuotes(String s, int beginIndex,
+            int endIndex) {
 
         if (s == null || s.length() == 0 || s.indexOf('"') == -1) {
             return s;
         }
 
         StringBuffer b = new StringBuffer();
-        char p = s.charAt(0);
-        for (int i = 0; i < s.length(); i++) {
+        for (int i = beginIndex; i < endIndex; i++) {
             char c = s.charAt(i);
-            if (c == '"' && p != '\\')
+            if (c == '\\' ) {
+                b.append(c);
+                //ignore the character after an escape, just append it
+                if (++i>=endIndex) throw new IllegalArgumentException("Invalid escape character in cookie value.");
+                b.append(s.charAt(i));
+            } else if (c == '"')
                 b.append('\\').append('"');
             else
                 b.append(c);
-            p = c;
         }
 
         return b.toString();
     }
 
+    /**
+     * Unescapes any double quotes in the given cookie value.
+     *
+     * @param bc The cookie value to modify
+     */
+    public static void unescapeDoubleQuotes(ByteChunk bc) {
+
+        if (bc == null || bc.getLength() == 0 || bc.indexOf('"', 0) == -1) {
+            return;
+        }
+
+        int src = bc.getStart();
+        int end = bc.getEnd();
+        int dest = src;
+        byte[] buffer = bc.getBuffer();
+        
+        while (src < end) {
+            if (buffer[src] == '\\' && src < end && buffer[src+1]  == '"') {
+                src++;
+            }
+            buffer[dest] = buffer[src];
+            dest ++;
+            src ++;
+        }
+        bc.setEnd(dest);
+    }
 }
 

Modified: tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/connector/Request.java
URL: http://svn.apache.org/viewvc/tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/connector/Request.java?rev=609406&r1=609405&r2=609406&view=diff
==============================================================================
--- tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/connector/Request.java (original)
+++ tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/connector/Request.java Sun Jan  6 13:23:24 2008
@@ -2271,6 +2271,22 @@
         }
     }
 
+    protected String unescape(String s) {
+        if (s==null) return null;
+        if (s.indexOf('\\') == -1) return s;
+        StringBuffer buf = new StringBuffer();
+        for (int i=0; i<s.length(); i++) {
+            char c = s.charAt(i);
+            if (c!='\\') buf.append(c);
+            else {
+                if (++i >= s.length()) throw new IllegalArgumentException();//invalid escape, hence invalid cookie
+                c = s.charAt(i);
+                buf.append(c);
+            }
+        }
+        return buf.toString();
+    }
+    
     /**
      * Parse cookies.
      */
@@ -2289,14 +2305,18 @@
         for (int i = 0; i < count; i++) {
             ServerCookie scookie = serverCookies.getCookie(i);
             try {
-                Cookie cookie = new Cookie(scookie.getName().toString(),
-                                           scookie.getValue().toString());
-                cookie.setPath(scookie.getPath().toString());
-                cookie.setVersion(scookie.getVersion());
+                /*
+                we must unescape the '\\' escape character
+                */
+                Cookie cookie = new Cookie(scookie.getName().toString(),null);
+                int version = scookie.getVersion();
+                cookie.setVersion(version);
+                cookie.setValue(unescape(scookie.getValue().toString()));
+                cookie.setPath(unescape(scookie.getPath().toString()));
                 String domain = scookie.getDomain().toString();
-                if (domain != null) {
-                    cookie.setDomain(scookie.getDomain().toString());
-                }
+                if (domain!=null) cookie.setDomain(unescape(domain));//avoid NPE
+                String comment = scookie.getComment().toString();
+                cookie.setComment(version==1?unescape(comment):null);
                 cookies[idx++] = cookie;
             } catch(IllegalArgumentException e) {
                 // Ignore bad cookie

Modified: tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/connector/Response.java
URL: http://svn.apache.org/viewvc/tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/connector/Response.java?rev=609406&r1=609405&r2=609406&view=diff
==============================================================================
--- tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/connector/Response.java (original)
+++ tomcat/container/tc5.5.x/catalina/src/share/org/apache/catalina/connector/Response.java Sun Jan  6 13:23:24 2008
@@ -937,9 +937,9 @@
         if (included)
             return;
 
-        cookies.add(cookie);
-
         final StringBuffer sb = new StringBuffer();
+        //web application code can receive a IllegalArgumentException 
+        //from the appendCookieValue invokation
         if (SecurityUtil.isPackageProtectionEnabled()) {
             AccessController.doPrivileged(new PrivilegedAction() {
                 public Object run(){
@@ -958,11 +958,13 @@
                      cookie.getMaxAge(), cookie.getSecure());
         }
 
+        // if we reached here, no exception, cookie is valid
         // the header name is Set-Cookie for both "old" and v.1 ( RFC2109 )
         // RFC2965 is not supported by browsers and the Servlet spec
         // asks for 2109.
         addHeader("Set-Cookie", sb.toString());
 
+        cookies.add(cookie);
     }
 
 

Modified: tomcat/container/tc5.5.x/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/container/tc5.5.x/webapps/docs/changelog.xml?rev=609406&r1=609405&r2=609406&view=diff
==============================================================================
--- tomcat/container/tc5.5.x/webapps/docs/changelog.xml (original)
+++ tomcat/container/tc5.5.x/webapps/docs/changelog.xml Sun Jan  6 13:23:24 2008
@@ -70,6 +70,18 @@
         Fix bug in CGI Servlet that caused it to fail when a CGI resource was
         included in another resource. (markt)
       </fix>
+      <fix>
+        Cookie handling/parsing changes!
+        The following behavior has been changed with regards to Tomcat's cookie
+        handling:<br/>
+        a) Cookies containing control characters, except 0x09(HT), are rejected
+        using an InvalidArgumentException.<br/>
+        b) If cookies are not quoted, they will be quoted if they contain
+        <code>tspecials(ver0)</code> or <code>tspecials2(ver1)</code>
+        characters.<br/>
+        c) Escape character '\\' is allowed and respected as a escape character,
+        and will be unescaped during parsing.
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Jasper" >

Modified: tomcat/current/tc5.5.x/STATUS.txt
URL: http://svn.apache.org/viewvc/tomcat/current/tc5.5.x/STATUS.txt?rev=609406&r1=609405&r2=609406&view=diff
==============================================================================
--- tomcat/current/tc5.5.x/STATUS.txt (original)
+++ tomcat/current/tc5.5.x/STATUS.txt Sun Jan  6 13:23:24 2008
@@ -53,17 +53,6 @@
   +1: markt, pero, fhanik
   -1:
 
-* Update TC5 with the new Cookie handling code from TC6
-  http://svn.apache.org/viewvc?view=rev&revision=553218
-  http://svn.apache.org/viewvc?view=rev&revision=553410
-  http://svn.apache.org/viewvc?view=rev&revision=553700
-  http://svn.apache.org/viewvc?view=rev&revision=557467
-  http://svn.apache.org/viewvc?view=rev&revision=580754
-  http://svn.apache.org/viewvc?view=rev&revision=586818
-  http://svn.apache.org/viewvc?view=rev&revision=594968
-  +1: markt, pero, fhanik
-  -1:
-
 * Fix http://issues.apache.org/bugzilla/show_bug.cgi?id=43887
   Include exception in log message
   http://svn.apache.org/viewvc?rev=597738&view=rev



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org