JBossWeb SVN: r348 - in trunk: java/org/apache/tomcat/util/http and 1 other directories.
by jbossweb-commits@lists.jboss.org
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">