Author: fhanik Date: Wed Nov 14 09:36:47 2007 New Revision: 594968 URL: http://svn.apache.org/viewvc?rev=594968&view=rev Log: Cookie parsing patch
Modified: tomcat/tc6.0.x/trunk/STATUS tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/Request.java tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/Response.java tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/http/Cookies.java tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/http/ServerCookie.java tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml Modified: tomcat/tc6.0.x/trunk/STATUS URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS?rev=594968&r1=594967&r2=594968&view=diff ============================================================================== --- tomcat/tc6.0.x/trunk/STATUS (original) +++ tomcat/tc6.0.x/trunk/STATUS Wed Nov 14 09:36:47 2007 @@ -42,11 +42,6 @@ +1: markt, remm -1: -* Fix Tomcat's cookie parsing/writing, changelog updated with changes - http://people.apache.org/~fhanik/patches/cookie_change.patch.5 - +1: fhanik, jfclere, remm - -1: - * Add tests for the cookie parsing. http://people.apache.org/~jfclere/patches/test_cookies.patch +1: jfclere Modified: tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/Request.java URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/Request.java?rev=594968&r1=594967&r2=594968&view=diff ============================================================================== --- tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/Request.java (original) +++ tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/Request.java Wed Nov 14 09:36:47 2007 @@ -2350,6 +2350,22 @@ cookie.setSecure(true); } } + + 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. @@ -2369,14 +2385,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/tc6.0.x/trunk/java/org/apache/catalina/connector/Response.java URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/Response.java?rev=594968&r1=594967&r2=594968&view=diff ============================================================================== --- tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/Response.java (original) +++ tomcat/tc6.0.x/trunk/java/org/apache/catalina/connector/Response.java Wed Nov 14 09:36:47 2007 @@ -955,9 +955,9 @@ if (isCommitted()) 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(){ @@ -975,12 +975,13 @@ cookie.getPath(), cookie.getDomain(), cookie.getComment(), 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/tc6.0.x/trunk/java/org/apache/tomcat/util/http/Cookies.java URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/http/Cookies.java?rev=594968&r1=594967&r2=594968&view=diff ============================================================================== --- tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/http/Cookies.java (original) +++ tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/http/Cookies.java Wed Nov 14 09:36:47 2007 @@ -487,7 +487,7 @@ if (equals( "Version", bytes, nameStart, nameEnd) && sc == null) { // Set version - if( bytes[valueStart] =='1' && valueEnd == valueStart) { + if( bytes[valueStart] =='1' && valueEnd == (valueStart+1)) { version=1; } else { // unknown version (Versioning is not very strict) Modified: tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/http/ServerCookie.java URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/http/ServerCookie.java?rev=594968&r1=594967&r2=594968&view=diff ============================================================================== --- tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/http/ServerCookie.java (original) +++ tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/http/ServerCookie.java Wed Nov 14 09:36:47 2007 @@ -153,20 +153,34 @@ 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(); 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; @@ -226,7 +240,7 @@ DateTool.formatOldCookie(new Date(10000)); // TODO RFC2965 fields also need to be passed - public static void appendCookieValue( StringBuffer buf, + public static void appendCookieValue( StringBuffer headerBuf, int version, String name, String value, @@ -236,6 +250,7 @@ int maxAge, boolean isSecure ) { + StringBuffer buf = new StringBuffer(); // Servlet implementation checks name buf.append( name ); buf.append("="); @@ -293,39 +308,54 @@ buf.append ("; Secure"); } - + headerBuf.append(buf); } /** * @deprecated - Not used */ - public static void maybeQuote (int version, StringBuffer buf, - String value) { + @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)=='\"'); + } + /** * 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 (version == 0 && isToken(value) || version == 1 && isToken2(value)) { - buf.append(value); - } else { + public static void maybeQuote2 (int version, StringBuffer buf, String value) { + 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(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); } } @@ -334,19 +364,25 @@ * 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(); - for (int i = 0; i < s.length(); i++) { + for (int i = beginIndex; i < endIndex; i++) { char c = s.charAt(i); - if (c == '"') + 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); Modified: tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml?rev=594968&r1=594967&r2=594968&view=diff ============================================================================== --- tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml (original) +++ tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml Wed Nov 14 09:36:47 2007 @@ -32,6 +32,22 @@ </properties> <body> +<section name="Tomcat 6.0.16 (remm)"> + <subsection name="General"> + <changelog> + <update> + Cookie handling/parsing changes! + The following behavior has been changed with regards to Tomcat's cookie handling + 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 tspecials(ver0), tspecials2(ver1) characters<br/> + c) Escape character '\\' is allowed and respected as a escape character, will be unescaped during parsing + </update> + <fix> + Cookie parsing of $Version regression from 6.0.15 has been fixed + </fix> + </changelog> + </subsection> +</section> <section name="Tomcat 6.0.15 (remm)"> <subsection name="General"> <changelog> --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]