You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by fh...@apache.org on 2007/11/14 18:36:50 UTC
svn commit: r594968 - in /tomcat/tc6.0.x/trunk: ./
java/org/apache/catalina/connector/ java/org/apache/tomcat/util/http/
webapps/docs/
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: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org