Author: remy.maucherat(a)jboss.com Date: 2007-11-16 12:16:52 -0500 (Fri, 16 Nov 2007) New Revision: 348 Modified: trunk/java/org/apache/catalina/connector/Request.java trunk/java/org/apache/catalina/connector/Response.java trunk/java/org/apache/tomcat/util/http/ServerCookie.java trunk/webapps/docs/changelog.xml Log: - Port additional cookie fixes. Modified: trunk/java/org/apache/catalina/connector/Request.java =================================================================== --- trunk/java/org/apache/catalina/connector/Request.java 2007-11-16 14:51:34 UTC (rev 347) +++ trunk/java/org/apache/catalina/connector/Request.java 2007-11-16 17:16:52 UTC (rev 348) @@ -2385,6 +2385,25 @@ } } + 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. */ @@ -2403,14 +2422,19 @@ 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: trunk/java/org/apache/catalina/connector/Response.java =================================================================== --- trunk/java/org/apache/catalina/connector/Response.java 2007-11-16 14:51:34 UTC (rev 347) +++ trunk/java/org/apache/catalina/connector/Response.java 2007-11-16 17:16:52 UTC (rev 348) @@ -963,9 +963,9 @@ if (isCommitted()) return; - cookies.add(cookie); - final StringBuffer sb = new StringBuffer(); + // web application code can receive a IllegalArgumentException + // from the appendCookieValue invocation if (SecurityUtil.isPackageProtectionEnabled()) { AccessController.doPrivileged(new PrivilegedAction() { public Object run(){ @@ -983,12 +983,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: trunk/java/org/apache/tomcat/util/http/ServerCookie.java =================================================================== --- trunk/java/org/apache/tomcat/util/http/ServerCookie.java 2007-11-16 14:51:34 UTC (rev 347) +++ trunk/java/org/apache/tomcat/util/http/ServerCookie.java 2007-11-16 17:16:52 UTC (rev 348) @@ -145,20 +145,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; @@ -218,7 +232,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, @@ -228,6 +242,7 @@ int maxAge, boolean isSecure ) { + StringBuffer buf = new StringBuffer(); // Servlet implementation checks name buf.append( name ); buf.append("="); @@ -285,39 +300,27 @@ 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 { - if (alreadyQuoted(value)) - buf.append(value); - else { - buf.append('"'); - buf.append(escapeDoubleQuotes(value)); - buf.append('"'); - } + buf.append('"'); + buf.append(escapeDoubleQuotes(value,0,value.length())); + buf.append('"'); } } + public static boolean alreadyQuoted (String value) { - if (value.startsWith("\"") && value.endsWith("\"")) { - int len = value.length(); - for (int i = 1; i < len-1; i++) { - char c = value.charAt(i); - // Make sure there aren't unescaped controls in value. - if (c < 0x20 || c >= 0x7f) - return false; // We will escape the " and controls. - } - return true; - } - return false; + if (value==null || value.length()==0) return false; + return (value.charAt(0)=='\"' && value.charAt(value.length()-1)=='\"'); } /** @@ -326,19 +329,25 @@ * @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)) { + 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('"'); + } 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); - } else { - if (alreadyQuoted(value)) - buf.append(value); - else { - buf.append('"'); - buf.append(escapeDoubleQuotes(value)); - buf.append('"'); - } } } @@ -347,19 +356,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); @@ -395,3 +410,4 @@ bc.setEnd(dest); } } + Modified: trunk/webapps/docs/changelog.xml =================================================================== --- trunk/webapps/docs/changelog.xml 2007-11-16 14:51:34 UTC (rev 347) +++ trunk/webapps/docs/changelog.xml 2007-11-16 17:16:52 UTC (rev 348) @@ -79,6 +79,9 @@ Remove legacy org.apache.jk AJP connector and utility components, replaced by the org.apache.coyote.ajp connector. (remm) </update> + <fix> + Additional cookie fixes. (jfclere) + </fix> </changelog> </subsection> <subsection name="Jasper">