diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpFields.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpFields.java index 98e3e5937ca..0df296425e6 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpFields.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpFields.java @@ -18,6 +18,9 @@ package org.eclipse.jetty.http; +import static org.eclipse.jetty.util.QuotedStringTokenizer.isQuoted; +import static org.eclipse.jetty.util.QuotedStringTokenizer.quoteOnly; + import java.io.IOException; import java.nio.ByteBuffer; import java.text.SimpleDateFormat; @@ -54,23 +57,27 @@ import org.eclipse.jetty.util.log.Logger; /** * HTTP Fields. A collection of HTTP header and or Trailer fields. * - *

This class is not synchronized as it is expected that modifications will only be performed by a + *

This class is not synchronised as it is expected that modifications will only be performed by a * single thread. + * + *

The cookie handling provided by this class is guided by the Servlet specification and RFC6265. * */ public class HttpFields implements Iterable { private static final Logger LOG = Log.getLogger(HttpFields.class); - public static final String __COOKIE_DELIM="\"\\\n\r\t\f\b%+ ;="; public static final TimeZone __GMT = TimeZone.getTimeZone("GMT"); public static final DateCache __dateCache = new DateCache("EEE, dd MMM yyyy HH:mm:ss 'GMT'", Locale.US); + public static final String __COOKIE_DELIM_PATH="\"\\\t%+ :;,@?=()<>{}[]"; + public static final String __COOKIE_DELIM=__COOKIE_DELIM_PATH+"/"; + static { __GMT.setID("GMT"); __dateCache.setTimeZone(__GMT); } - + public final static String __separators = ", \t"; private static final String[] DAYS = @@ -808,69 +815,87 @@ public class HttpFields implements Iterable final boolean isHttpOnly, int version) { - String delim=__COOKIE_DELIM; - // Check arguments if (name == null || name.length() == 0) throw new IllegalArgumentException("Bad cookie name"); // Format value and params StringBuilder buf = new StringBuilder(128); - String name_value_params; - QuotedStringTokenizer.quoteIfNeeded(buf, name, delim); - buf.append('='); - String start=buf.toString(); - boolean hasDomain = false; - boolean hasPath = false; - if (value != null && value.length() > 0) - QuotedStringTokenizer.quoteIfNeeded(buf, value, delim); + // Name is checked by servlet spec, but can also be passed directly so check again + boolean quote_name=isQuoteNeededForCookie(name); + quoteOnlyOrAppend(buf,name,quote_name); + + buf.append('='); + + // Remember name= part to look for other matching set-cookie + String name_equals=buf.toString(); + // Append the value + boolean quote_value=isQuoteNeededForCookie(value); + quoteOnlyOrAppend(buf,value,quote_value); - if (path != null && path.length() > 0) + // Look for domain and path fields and check if they need to be quoted + boolean has_domain = domain!=null && domain.length()>0; + boolean quote_domain = has_domain && isQuoteNeededForCookie(domain); + boolean has_path = path!=null && path.length()>0; + boolean quote_path = has_path && isQuoteNeededForCookiePath(path); + + // Upgrade the version if we have a comment or we need to quote value/path/domain or if they were already quoted + if (version==0 && ( comment!=null || quote_name || quote_value || quote_domain || quote_path || isQuoted(name) || isQuoted(value) || isQuoted(path) || isQuoted(domain))) + version=1; + + // Append version + if (version==1) + buf.append (";Version=1"); + else if (version>1) + buf.append (";Version=").append(version); + + // Append path + if (has_path) { - hasPath = true; buf.append(";Path="); - if (path.trim().startsWith("\"")) - buf.append(path); - else - QuotedStringTokenizer.quoteIfNeeded(buf,path,delim); + quoteOnlyOrAppend(buf,path,quote_path); } - if (domain != null && domain.length() > 0) + + // Append domain + if (has_domain) { - hasDomain = true; buf.append(";Domain="); - QuotedStringTokenizer.quoteIfNeeded(buf,domain.toLowerCase(Locale.ENGLISH),delim); + quoteOnlyOrAppend(buf,domain,quote_domain); } + // Handle max-age and/or expires if (maxAge >= 0) { - // Always add the expires param as some browsers still don't handle max-age + // Always use expires + // This is required as some browser (M$ this means you!) don't handle max-age even with v1 cookies buf.append(";Expires="); if (maxAge == 0) buf.append(__01Jan1970_COOKIE); else formatCookieDate(buf, System.currentTimeMillis() + 1000L * maxAge); - - buf.append(";Max-Age="); - buf.append(maxAge); + + // for v1 cookies, also send max-age + if (version>=1) + { + buf.append(";Max-Age="); + buf.append(maxAge); + } } + // add the other fields if (isSecure) buf.append(";Secure"); if (isHttpOnly) buf.append(";HttpOnly"); - - if (comment != null && comment.length() > 0) + if (comment != null) { buf.append(";Comment="); - QuotedStringTokenizer.quoteIfNeeded(buf, comment, delim); + quoteOnlyOrAppend(buf,comment,isQuoteNeededForCookie(comment)); } - name_value_params = buf.toString(); - - // remove existing set-cookie of same name - + // remove any existing set-cookie fields of same name Iterator i=_fields.iterator(); while (i.hasNext()) { @@ -878,26 +903,26 @@ public class HttpFields implements Iterable if (field.getHeader()==HttpHeader.SET_COOKIE) { String val = (field.getValue() == null ? null : field.getValue().toString()); - if (val!=null && val.startsWith(start)) + if (val!=null && val.startsWith(name_equals)) { //existing cookie has same name, does it also match domain and path? - if (((!hasDomain && !val.contains("Domain")) || (hasDomain && val.contains("Domain="+domain))) && - ((!hasPath && !val.contains("Path")) || (hasPath && val.contains("Path="+path)))) + if (((!has_domain && !val.contains("Domain")) || (has_domain && val.contains(domain))) && + ((!has_path && !val.contains("Path")) || (has_path && val.contains(path)))) { i.remove(); } } - } } - add(HttpHeader.SET_COOKIE.toString(), name_value_params); + // add the set cookie + add(HttpHeader.SET_COOKIE.toString(), buf.toString()); // Expire responses with set-cookie headers so they do not get cached. put(HttpHeader.EXPIRES.toString(), __01Jan1970); } - public void putTo(ByteBuffer bufferInFillMode) throws IOException + public void putTo(ByteBuffer bufferInFillMode) { for (HttpField field : _fields) { @@ -1095,19 +1120,20 @@ public class HttpFields implements Iterable } } - List vl = LazyList.getList(list, false); - if (vl.size() < 2) return vl; + List vl = LazyList.getList(list, false); + if (vl.size() < 2) + return vl; - List ql = LazyList.getList(qual, false); + List ql = LazyList.getList(qual, false); // sort list with swaps Float last = __zero; for (int i = vl.size(); i-- > 0;) { - Float q = (Float) ql.get(i); + Float q = ql.get(i); if (last.compareTo(q) > 0) { - Object tmp = vl.get(i); + String tmp = vl.get(i); vl.set(i, vl.get(i + 1)); vl.set(i + 1, tmp); ql.set(i, ql.get(i + 1)); @@ -1123,4 +1149,66 @@ public class HttpFields implements Iterable } + + /* ------------------------------------------------------------ */ + /** Does a cookie value need to be quoted? + * @param s value string + * @return true if quoted; + * @throws IllegalArgumentException If there a control characters in the string + */ + public static boolean isQuoteNeededForCookie(String s) + { + if (s==null || s.length()==0) + return true; + + if (QuotedStringTokenizer.isQuoted(s)) + return false; + + for (int i=0;i=0) + return true; + + if (c<0x20 || c>=0x7f) + throw new IllegalArgumentException("Illegal character in cookie value"); + } + + return false; + } + + /* ------------------------------------------------------------ */ + /** Does a cookie path need to be quoted? + * @param s value string + * @return true if quoted; + * @throws IllegalArgumentException If there a control characters in the string + */ + public static boolean isQuoteNeededForCookiePath(String s) + { + if (s==null || s.length()==0) + return true; + + if (QuotedStringTokenizer.isQuoted(s)) + return false; + + for (int i=0;i=0) + return true; + + if (c<0x20 || c>=0x7f) + throw new IllegalArgumentException("Illegal character in cookie value"); + } + + return false; + } + + private static void quoteOnlyOrAppend(StringBuilder buf, String s, boolean quote) + { + if (quote) + QuotedStringTokenizer.quoteOnly(buf,s); + else + buf.append(s); + } } diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpFieldsTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpFieldsTest.java index 3f8a91dd623..d1674aa7430 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpFieldsTest.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpFieldsTest.java @@ -279,10 +279,10 @@ public class HttpFieldsTest //test cookies with same name, domain and path, only 1 allowed fields.addSetCookie("everything","wrong","domain","path",0,"to be replaced",true,true,0); fields.addSetCookie("everything","value","domain","path",0,"comment",true,true,0); - assertEquals("everything=value;Path=path;Domain=domain;Expires=Thu, 01-Jan-1970 00:00:00 GMT;Max-Age=0;Secure;HttpOnly;Comment=comment",fields.getStringField("Set-Cookie")); + assertEquals("everything=value;Version=1;Path=path;Domain=domain;Expires=Thu, 01-Jan-1970 00:00:00 GMT;Max-Age=0;Secure;HttpOnly;Comment=comment",fields.getStringField("Set-Cookie")); Enumeration e =fields.getValues("Set-Cookie"); assertTrue(e.hasMoreElements()); - assertEquals("everything=value;Path=path;Domain=domain;Expires=Thu, 01-Jan-1970 00:00:00 GMT;Max-Age=0;Secure;HttpOnly;Comment=comment",e.nextElement()); + assertEquals("everything=value;Version=1;Path=path;Domain=domain;Expires=Thu, 01-Jan-1970 00:00:00 GMT;Max-Age=0;Secure;HttpOnly;Comment=comment",e.nextElement()); assertFalse(e.hasMoreElements()); assertEquals("Thu, 01 Jan 1970 00:00:00 GMT",fields.getStringField("Expires")); assertFalse(e.hasMoreElements()); @@ -293,9 +293,9 @@ public class HttpFieldsTest fields.addSetCookie("everything","value","domain2","path",0,"comment",true,true,0); e =fields.getValues("Set-Cookie"); assertTrue(e.hasMoreElements()); - assertEquals("everything=other;Path=path;Domain=domain1;Expires=Thu, 01-Jan-1970 00:00:00 GMT;Max-Age=0;Secure;HttpOnly;Comment=blah",e.nextElement()); + assertEquals("everything=other;Version=1;Path=path;Domain=domain1;Expires=Thu, 01-Jan-1970 00:00:00 GMT;Max-Age=0;Secure;HttpOnly;Comment=blah",e.nextElement()); assertTrue(e.hasMoreElements()); - assertEquals("everything=value;Path=path;Domain=domain2;Expires=Thu, 01-Jan-1970 00:00:00 GMT;Max-Age=0;Secure;HttpOnly;Comment=comment",e.nextElement()); + assertEquals("everything=value;Version=1;Path=path;Domain=domain2;Expires=Thu, 01-Jan-1970 00:00:00 GMT;Max-Age=0;Secure;HttpOnly;Comment=comment",e.nextElement()); assertFalse(e.hasMoreElements()); //test cookies with same name, same path, one with domain, one without @@ -304,9 +304,9 @@ public class HttpFieldsTest fields.addSetCookie("everything","value","","path",0,"comment",true,true,0); e =fields.getValues("Set-Cookie"); assertTrue(e.hasMoreElements()); - assertEquals("everything=other;Path=path;Domain=domain1;Expires=Thu, 01-Jan-1970 00:00:00 GMT;Max-Age=0;Secure;HttpOnly;Comment=blah",e.nextElement()); + assertEquals("everything=other;Version=1;Path=path;Domain=domain1;Expires=Thu, 01-Jan-1970 00:00:00 GMT;Max-Age=0;Secure;HttpOnly;Comment=blah",e.nextElement()); assertTrue(e.hasMoreElements()); - assertEquals("everything=value;Path=path;Expires=Thu, 01-Jan-1970 00:00:00 GMT;Max-Age=0;Secure;HttpOnly;Comment=comment",e.nextElement()); + assertEquals("everything=value;Version=1;Path=path;Expires=Thu, 01-Jan-1970 00:00:00 GMT;Max-Age=0;Secure;HttpOnly;Comment=comment",e.nextElement()); assertFalse(e.hasMoreElements()); @@ -316,9 +316,9 @@ public class HttpFieldsTest fields.addSetCookie("everything","value","domain1","path2",0,"comment",true,true,0); e =fields.getValues("Set-Cookie"); assertTrue(e.hasMoreElements()); - assertEquals("everything=other;Path=path1;Domain=domain1;Expires=Thu, 01-Jan-1970 00:00:00 GMT;Max-Age=0;Secure;HttpOnly;Comment=blah",e.nextElement()); + assertEquals("everything=other;Version=1;Path=path1;Domain=domain1;Expires=Thu, 01-Jan-1970 00:00:00 GMT;Max-Age=0;Secure;HttpOnly;Comment=blah",e.nextElement()); assertTrue(e.hasMoreElements()); - assertEquals("everything=value;Path=path2;Domain=domain1;Expires=Thu, 01-Jan-1970 00:00:00 GMT;Max-Age=0;Secure;HttpOnly;Comment=comment",e.nextElement()); + assertEquals("everything=value;Version=1;Path=path2;Domain=domain1;Expires=Thu, 01-Jan-1970 00:00:00 GMT;Max-Age=0;Secure;HttpOnly;Comment=comment",e.nextElement()); assertFalse(e.hasMoreElements()); //test cookies with same name, same domain, one with path, one without @@ -327,9 +327,9 @@ public class HttpFieldsTest fields.addSetCookie("everything","value","domain1","",0,"comment",true,true,0); e =fields.getValues("Set-Cookie"); assertTrue(e.hasMoreElements()); - assertEquals("everything=other;Path=path1;Domain=domain1;Expires=Thu, 01-Jan-1970 00:00:00 GMT;Max-Age=0;Secure;HttpOnly;Comment=blah",e.nextElement()); + assertEquals("everything=other;Version=1;Path=path1;Domain=domain1;Expires=Thu, 01-Jan-1970 00:00:00 GMT;Max-Age=0;Secure;HttpOnly;Comment=blah",e.nextElement()); assertTrue(e.hasMoreElements()); - assertEquals("everything=value;Domain=domain1;Expires=Thu, 01-Jan-1970 00:00:00 GMT;Max-Age=0;Secure;HttpOnly;Comment=comment",e.nextElement()); + assertEquals("everything=value;Version=1;Domain=domain1;Expires=Thu, 01-Jan-1970 00:00:00 GMT;Max-Age=0;Secure;HttpOnly;Comment=comment",e.nextElement()); assertFalse(e.hasMoreElements()); //test cookies same name only, no path, no domain @@ -338,13 +338,13 @@ public class HttpFieldsTest fields.addSetCookie("everything","value","","",0,"comment",true,true,0); e =fields.getValues("Set-Cookie"); assertTrue(e.hasMoreElements()); - assertEquals("everything=value;Expires=Thu, 01-Jan-1970 00:00:00 GMT;Max-Age=0;Secure;HttpOnly;Comment=comment",e.nextElement()); + assertEquals("everything=value;Version=1;Expires=Thu, 01-Jan-1970 00:00:00 GMT;Max-Age=0;Secure;HttpOnly;Comment=comment",e.nextElement()); assertFalse(e.hasMoreElements()); fields.clear(); - fields.addSetCookie("ev erything","va lue","do main","pa th",1,"co mment",true,true,2); + fields.addSetCookie("ev erything","va lue","do main","pa th",1,"co mment",true,true,1); String setCookie=fields.getStringField("Set-Cookie"); - assertThat(setCookie,Matchers.startsWith("\"ev erything\"=\"va lue\";Path=\"pa th\";Domain=\"do main\";Expires=")); + assertThat(setCookie,Matchers.startsWith("\"ev erything\"=\"va lue\";Version=1;Path=\"pa th\";Domain=\"do main\";Expires=")); assertThat(setCookie,Matchers.endsWith(" GMT;Max-Age=1;Secure;HttpOnly;Comment=\"co mment\"")); fields.clear(); @@ -376,7 +376,7 @@ public class HttpFieldsTest fields=new HttpFields(); fields.addSetCookie("name","value==",null,null,-1,null,false,false,0); setCookie=fields.getStringField("Set-Cookie"); - assertEquals("name=\"value==\"",setCookie); + assertEquals("name=\"value==\";Version=1",setCookie); } diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/ResponseTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/ResponseTest.java index 84967af2422..245fb5cb607 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/ResponseTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/ResponseTest.java @@ -601,7 +601,7 @@ public class ResponseTest String set = response.getHttpFields().getStringField("Set-Cookie"); - assertEquals("name=value;Path=/path;Domain=domain;Secure;HttpOnly;Comment=comment", set); + assertEquals("name=value;Version=1;Path=/path;Domain=domain;Secure;HttpOnly;Comment=comment", set); } @@ -630,7 +630,7 @@ public class ResponseTest assertNotNull(set); ArrayList list = Collections.list(set); assertEquals(2, list.size()); - assertTrue(list.contains("name=value;Path=/path;Domain=domain;Secure;HttpOnly;Comment=comment")); + assertTrue(list.contains("name=value;Version=1;Path=/path;Domain=domain;Secure;HttpOnly;Comment=comment")); assertTrue(list.contains("name2=value2;Path=/path;Domain=domain")); //get rid of the cookies diff --git a/jetty-spdy/spdy-http-server/src/test/java/org/eclipse/jetty/spdy/server/http/ServerHTTPSPDYTest.java b/jetty-spdy/spdy-http-server/src/test/java/org/eclipse/jetty/spdy/server/http/ServerHTTPSPDYTest.java index de6c4d72eb0..98519952a3b 100644 --- a/jetty-spdy/spdy-http-server/src/test/java/org/eclipse/jetty/spdy/server/http/ServerHTTPSPDYTest.java +++ b/jetty-spdy/spdy-http-server/src/test/java/org/eclipse/jetty/spdy/server/http/ServerHTTPSPDYTest.java @@ -189,9 +189,9 @@ public class ServerHTTPSPDYTest extends AbstractHTTPSPDYTest assertThat("response code is 200 OK", replyHeaders.get(HTTPSPDYHeader.STATUS.name(version)).value() .contains("200"), is(true)); assertThat(replyInfo.getHeaders().get("Set-Cookie").values()[0], is(cookie1 + "=\"" + cookie1Value + - "\"")); + "\";Version=1")); assertThat(replyInfo.getHeaders().get("Set-Cookie").values()[1], is(cookie2 + "=\"" + cookie2Value + - "\"")); + "\";Version=1")); replyLatch.countDown(); } }); diff --git a/jetty-util/src/main/java/org/eclipse/jetty/util/QuotedStringTokenizer.java b/jetty-util/src/main/java/org/eclipse/jetty/util/QuotedStringTokenizer.java index 561c674eabe..0931f328271 100644 --- a/jetty-util/src/main/java/org/eclipse/jetty/util/QuotedStringTokenizer.java +++ b/jetty-util/src/main/java/org/eclipse/jetty/util/QuotedStringTokenizer.java @@ -332,6 +332,32 @@ public class QuotedStringTokenizer escapes['\r'] = 'r'; } + /* ------------------------------------------------------------ */ + /** Quote a string into an Appendable. + * Only quotes and backslash are escaped. + * @param buffer The Appendable + * @param input The String to quote. + */ + public static void quoteOnly(Appendable buffer, String input) + { + try + { + buffer.append('"'); + for (int i = 0; i < input.length(); ++i) + { + char c = input.charAt(i); + if (c == '"' || c == '\\') + buffer.append('\\'); + buffer.append(c); + } + buffer.append('"'); + } + catch (IOException x) + { + throw new RuntimeException(x); + } + } + /* ------------------------------------------------------------ */ /** Quote a string into an Appendable. * The characters ", \, \n, \r, \t, \f and \b are escaped @@ -377,38 +403,6 @@ public class QuotedStringTokenizer } } - /* ------------------------------------------------------------ */ - /** Quote a string into a StringBuffer only if needed. - * Quotes are forced if any delim characters are present. - * - * @param buf The StringBuffer - * @param s The String to quote. - * @param delim String of characters that must be quoted. - * @return true if quoted; - */ - public static boolean quoteIfNeeded(Appendable buf, String s,String delim) - { - for (int i=0;i=0) - { - quote(buf,s); - return true; - } - } - - try - { - buf.append(s); - return false; - } - catch(IOException e) - { - throw new RuntimeException(e); - } - } - /* ------------------------------------------------------------ */ public static String unquoteOnly(String s) @@ -565,6 +559,12 @@ public class QuotedStringTokenizer (c == '/') || (c == '"') || (c == 'u')); } + /* ------------------------------------------------------------ */ + public static boolean isQuoted(String s) + { + return s!=null && s.length()>0 && s.charAt(0)=='"' && s.charAt(s.length()-1)=='"'; + } + /* ------------------------------------------------------------ */ /** * @return handle double quotes if true diff --git a/jetty-util/src/test/java/org/eclipse/jetty/util/QuotedStringTokenizerTest.java b/jetty-util/src/test/java/org/eclipse/jetty/util/QuotedStringTokenizerTest.java index a4290e4af40..692f61b896f 100644 --- a/jetty-util/src/test/java/org/eclipse/jetty/util/QuotedStringTokenizerTest.java +++ b/jetty-util/src/test/java/org/eclipse/jetty/util/QuotedStringTokenizerTest.java @@ -112,14 +112,6 @@ public class QuotedStringTokenizerTest QuotedStringTokenizer.quote(buf,"abcefg\""); assertEquals("\"abcefg\\\"\"",buf.toString()); - buf.setLength(0); - QuotedStringTokenizer.quoteIfNeeded(buf,"abc \n efg","\"\\\n\r\t\f\b%+ ;="); - assertEquals("\"abc \\n efg\"",buf.toString()); - - buf.setLength(0); - QuotedStringTokenizer.quoteIfNeeded(buf,"abcefg","\"\\\n\r\t\f\b%+ ;="); - assertEquals("abcefg",buf.toString()); - } /*