From 26169491c9c6bf5a00cca26a357c962691548ff8 Mon Sep 17 00:00:00 2001 From: Greg Wilkins Date: Thu, 16 May 2019 13:08:03 +0200 Subject: [PATCH] Issue #3655 simplifications of Cookie handling Signed-off-by: Greg Wilkins --- .../org/eclipse/jetty/http/HttpCookie.java | 227 ++++++++++ .../eclipse/jetty/http/QuotedCSVParser.java | 8 + .../eclipse/jetty/http/HttpCookieTest.java | 187 ++++++++ .../org/eclipse/jetty/server/Response.java | 406 ++---------------- .../eclipse/jetty/server/ResponseTest.java | 332 ++------------ 5 files changed, 495 insertions(+), 665 deletions(-) create mode 100644 jetty-http/src/test/java/org/eclipse/jetty/http/HttpCookieTest.java diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpCookie.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpCookie.java index 62ab47e9c55..97fe1a447cc 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpCookie.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpCookie.java @@ -18,10 +18,17 @@ package org.eclipse.jetty.http; +import java.util.List; import java.util.concurrent.TimeUnit; +import org.eclipse.jetty.util.QuotedStringTokenizer; + public class HttpCookie { + // TODO consider replacing this with java.net.HttpCookie + private static final String __COOKIE_DELIM="\",;\\ \t"; + private static final String __01Jan1970_COOKIE = DateGenerator.formatCookieDate(0).trim(); + private final String _name; private final String _value; private final String _comment; @@ -67,6 +74,29 @@ public class HttpCookie _expiration = maxAge < 0 ? -1 : System.nanoTime() + TimeUnit.SECONDS.toNanos(maxAge); } + enum State { START, NAME, VALUE, QUOTED} + + public HttpCookie(String setCookie) + { + List cookies = java.net.HttpCookie.parse(setCookie); + if (cookies.size()!=1) + throw new IllegalStateException(); + + java.net.HttpCookie cookie = cookies.get(0); + + _name = cookie.getName(); + _value = cookie.getValue(); + _domain = cookie.getDomain(); + _path = cookie.getPath(); + _maxAge = cookie.getMaxAge(); + _httpOnly = cookie.isHttpOnly(); + _secure = cookie.getSecure(); + _comment = cookie.getComment(); + _version = cookie.getVersion(); + _expiration = _maxAge < 0 ? -1 : System.nanoTime() + TimeUnit.SECONDS.toNanos(_maxAge); + } + + /** * @return the cookie name */ @@ -161,4 +191,201 @@ public class HttpCookie builder.append(";$Path=").append(getPath()); return builder.toString(); } + + + private static void quoteOnlyOrAppend(StringBuilder buf, String s, boolean quote) + { + if (quote) + QuotedStringTokenizer.quoteOnly(buf,s); + else + buf.append(s); + } + + /* ------------------------------------------------------------ */ + /** 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 + */ + private 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; + } + + public String getSetCookie(CookieCompliance compliance) + { + switch(compliance) + { + case RFC2965: + return getRFC2965SetCookie(); + case RFC6265: + return getRFC6265SetCookie(); + default: + throw new IllegalStateException(); + } + } + + public String getRFC2965SetCookie() + { + // Check arguments + if (_name == null || _name.length() == 0) + throw new IllegalArgumentException("Bad cookie name"); + + // Format value and params + StringBuilder buf = new StringBuilder(); + + // Name is checked for legality by servlet spec, but can also be passed directly so check again for quoting + boolean quote_name=isQuoteNeededForCookie(_name); + quoteOnlyOrAppend(buf,_name,quote_name); + + buf.append('='); + + // Append the value + boolean quote_value=isQuoteNeededForCookie(_value); + quoteOnlyOrAppend(buf,_value,quote_value); + + // 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 && isQuoteNeededForCookie(_path); + + // Upgrade the version if we have a comment or we need to quote value/path/domain or if they were already quoted + int version = _version; + if (version==0 && ( _comment!=null || quote_name || quote_value || quote_domain || quote_path || + QuotedStringTokenizer.isQuoted(_name) || QuotedStringTokenizer.isQuoted(_value) || + QuotedStringTokenizer.isQuoted(_path) || QuotedStringTokenizer.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) + { + buf.append(";Path="); + quoteOnlyOrAppend(buf,_path,quote_path); + } + + // Append domain + if (has_domain) + { + buf.append(";Domain="); + quoteOnlyOrAppend(buf,_domain,quote_domain); + } + + // Handle max-age and/or expires + if (_maxAge >= 0) + { + // 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 + DateGenerator.formatCookieDate(buf, System.currentTimeMillis() + 1000L * _maxAge); + + // for v1 cookies, also send max-age + if (version>=1) + { + buf.append(";Max-Age="); + buf.append(_maxAge); + } + } + + // add the other fields + if (_secure) + buf.append(";Secure"); + if (_httpOnly) + buf.append(";HttpOnly"); + if (_comment != null) + { + buf.append(";Comment="); + quoteOnlyOrAppend(buf,_comment,isQuoteNeededForCookie(_comment)); + } + return buf.toString(); + } + + public String getRFC6265SetCookie() + { + // Check arguments + if (_name == null || _name.length() == 0) + throw new IllegalArgumentException("Bad cookie name"); + + // Name is checked for legality by servlet spec, but can also be passed directly so check again for quoting + // Per RFC6265, Cookie.name follows RFC2616 Section 2.2 token rules + Syntax.requireValidRFC2616Token(_name, "RFC6265 Cookie name"); + // Ensure that Per RFC6265, Cookie.value follows syntax rules + Syntax.requireValidRFC6265CookieValue(_value); + + // Format value and params + StringBuilder buf = new StringBuilder(); + buf.append(_name).append('=').append(_value==null?"":_value); + + // Append path + if (_path!=null && _path.length()>0) + buf.append("; Path=").append(_path); + + // Append domain + if (_domain!=null && _domain.length()>0) + buf.append("; Domain=").append(_domain); + + // Handle max-age and/or expires + if (_maxAge >= 0) + { + // 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 + DateGenerator.formatCookieDate(buf, System.currentTimeMillis() + 1000L * _maxAge); + + buf.append("; Max-Age="); + buf.append(_maxAge); + } + + // add the other fields + if (_secure) + buf.append("; Secure"); + if (_httpOnly) + buf.append("; HttpOnly"); + return buf.toString(); + } + + + public static class SetCookieHttpField extends HttpField + { + final HttpCookie _cookie; + + public SetCookieHttpField(HttpCookie cookie, CookieCompliance compliance) + { + super(HttpHeader.SET_COOKIE, cookie.getSetCookie(compliance)); + this._cookie = cookie; + } + + public HttpCookie getHttpCookie() + { + return _cookie; + } + } } diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/QuotedCSVParser.java b/jetty-http/src/main/java/org/eclipse/jetty/http/QuotedCSVParser.java index 8f521db7ffb..622c3345e77 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/QuotedCSVParser.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/QuotedCSVParser.java @@ -18,6 +18,14 @@ package org.eclipse.jetty.http; + +/** + * Implements a quoted comma separated list parser + * in accordance with RFC7230. + * OWS is removed and quoted characters ignored for parsing. + * @see "https://tools.ietf.org/html/rfc7230#section-3.2.6" + * @see "https://tools.ietf.org/html/rfc7230#section-7" + */ public abstract class QuotedCSVParser { private enum State { VALUE, PARAM_NAME, PARAM_VALUE} diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/HttpCookieTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpCookieTest.java new file mode 100644 index 00000000000..3b55249ba02 --- /dev/null +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/HttpCookieTest.java @@ -0,0 +1,187 @@ +// +// ======================================================================== +// Copyright (c) 1995-2019 Mort Bay Consulting Pty. Ltd. +// ------------------------------------------------------------------------ +// All rights reserved. This program and the accompanying materials +// are made available under the terms of the Eclipse Public License v1.0 +// and Apache License v2.0 which accompanies this distribution. +// +// The Eclipse Public License is available at +// http://www.eclipse.org/legal/epl-v10.html +// +// The Apache License v2.0 is available at +// http://www.opensource.org/licenses/apache2.0.php +// +// You may elect to redistribute this code under either of these licenses. +// ======================================================================== +// + +package org.eclipse.jetty.http; + +import org.hamcrest.Matchers; +import org.junit.jupiter.api.Test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.allOf; +import static org.hamcrest.Matchers.containsString; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.fail; + +public class HttpCookieTest +{ + + @Test + public void testConstructFromSetCookie() + { + HttpCookie cookie = new HttpCookie("everything=value; Path=path; Domain=domain; Expires=Thu, 01-Jan-1970 00:00:00 GMT; Max-Age=0; Secure; HttpOnly"); + } + + @Test + public void testSetRFC2965Cookie() throws Exception + { + HttpCookie httpCookie; + + httpCookie = new HttpCookie("null", null, null, null, -1, false, false, null, -1); + assertEquals("null=",httpCookie.getRFC2965SetCookie()); + + + httpCookie = new HttpCookie("minimal", "value", null, null, -1, false, false, null, -1); + assertEquals("minimal=value",httpCookie.getRFC2965SetCookie()); + + httpCookie = new HttpCookie("everything", "something", "domain", "path", 0, true, true, "noncomment", 0); + assertEquals("everything=something;Version=1;Path=path;Domain=domain;Expires=Thu, 01-Jan-1970 00:00:00 GMT;Max-Age=0;Secure;HttpOnly;Comment=noncomment",httpCookie.getRFC2965SetCookie()); + + httpCookie = new HttpCookie("everything", "value", "domain", "path", 0, true, true, "comment", 0); + 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",httpCookie.getRFC2965SetCookie()); + + + httpCookie = new HttpCookie("ev erything", "va lue", "do main", "pa th", 1, true, true, "co mment", 1); + String setCookie=httpCookie.getRFC2965SetCookie(); + 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\"")); + + httpCookie = new HttpCookie("name", "value", null, null, -1, false, false, null, 0); + setCookie=httpCookie.getRFC2965SetCookie(); + assertEquals(-1,setCookie.indexOf("Version=")); + httpCookie = new HttpCookie("name", "v a l u e", null, null, -1, false, false, null, 0); + setCookie=httpCookie.getRFC2965SetCookie(); + + httpCookie = new HttpCookie("json","{\"services\":[\"cwa\", \"aa\"]}", null, null, -1, false, false, null, -1); + assertEquals("json=\"{\\\"services\\\":[\\\"cwa\\\", \\\"aa\\\"]}\"",httpCookie.getRFC2965SetCookie()); + + httpCookie = new HttpCookie("name", "value%=", null, null, -1, false, false, null, 0); + setCookie=httpCookie.getRFC2965SetCookie(); + assertEquals("name=value%=",setCookie); + } + + @Test + public void testSetRFC6265Cookie() throws Exception + { + // HttpCookie(String name, String value, String domain, String path, long maxAge, boolean httpOnly, boolean secure, String comment, int version) + // httpCookie = new HttpCookie("name", "value", "domain", "path", maxAge, secure, httpOnly); + + HttpCookie httpCookie; + + httpCookie = new HttpCookie("null",null,null,null,-1,false,false, null, -1); + assertEquals("null=",httpCookie.getRFC6265SetCookie()); + + httpCookie = new HttpCookie("minimal","value",null,null,-1,false,false, null, -1); + assertEquals("minimal=value",httpCookie.getRFC6265SetCookie()); + + //test cookies with same name, domain and path + httpCookie = new HttpCookie("everything","something","domain","path",0,true,true, null, -1); + assertEquals("everything=something; Path=path; Domain=domain; Expires=Thu, 01-Jan-1970 00:00:00 GMT; Max-Age=0; Secure; HttpOnly",httpCookie.getRFC6265SetCookie()); + + httpCookie = new HttpCookie("everything","value","domain","path",0,true,true, null, -1); + assertEquals("everything=value; Path=path; Domain=domain; Expires=Thu, 01-Jan-1970 00:00:00 GMT; Max-Age=0; Secure; HttpOnly",httpCookie.getRFC6265SetCookie()); + + String badNameExamples[] = { + "\"name\"", + "name\t", + "na me", + "name\u0082", + "na\tme", + "na;me", + "{name}", + "[name]", + "\"" + }; + + for (String badNameExample : badNameExamples) + { + try + { + httpCookie = new HttpCookie(badNameExample, "value", null, "/", 1, true, true, null, -1); + httpCookie.getRFC6265SetCookie(); + fail(badNameExample); + } + catch (IllegalArgumentException ex) + { + // System.err.printf("%s: %s%n", ex.getClass().getSimpleName(), ex.getMessage()); + assertThat("Testing bad name: [" + badNameExample + "]", ex.getMessage(), + allOf(containsString("RFC6265"), containsString("RFC2616"))); + } + } + + String badValueExamples[] = { + "va\tlue", + "\t", + "value\u0000", + "val\u0082ue", + "va lue", + "va;lue", + "\"value", + "value\"", + "val\\ue", + "val\"ue", + "\"" + }; + + for (String badValueExample : badValueExamples) + { + try + { + httpCookie = new HttpCookie("name", badValueExample, null, "/", 1, true, true, null, -1); + httpCookie.getRFC6265SetCookie(); + fail(); + } + catch (IllegalArgumentException ex) + { + // System.err.printf("%s: %s%n", ex.getClass().getSimpleName(), ex.getMessage()); + assertThat("Testing bad value [" + badValueExample + "]", ex.getMessage(), Matchers.containsString("RFC6265")); + } + } + + String goodNameExamples[] = { + "name", + "n.a.m.e", + "na-me", + "+name", + "na*me", + "na$me", + "#name" + }; + + for (String goodNameExample : goodNameExamples) + { + httpCookie = new HttpCookie(goodNameExample, "value", null, "/", 1, true, true, null, -1); + // should not throw an exception + } + + String goodValueExamples[] = { + "value", + "", + null, + "val=ue", + "val-ue", + "val/ue", + "v.a.l.u.e" + }; + + for (String goodValueExample : goodValueExamples) + { + httpCookie = new HttpCookie("name", goodValueExample, null, "/", 1, true, true, null, -1); + // should not throw an exception + } + } +} diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java index a64973f3f2f..86f70732f13 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Response.java @@ -40,6 +40,7 @@ import org.eclipse.jetty.http.CookieCompliance; import org.eclipse.jetty.http.DateGenerator; import org.eclipse.jetty.http.HttpContent; import org.eclipse.jetty.http.HttpCookie; +import org.eclipse.jetty.http.HttpCookie.SetCookieHttpField; import org.eclipse.jetty.http.HttpField; import org.eclipse.jetty.http.HttpFields; import org.eclipse.jetty.http.HttpGenerator; @@ -69,12 +70,8 @@ import org.eclipse.jetty.util.log.Logger; public class Response implements HttpServletResponse { private static final Logger LOG = Log.getLogger(Response.class); - private static final String __COOKIE_DELIM="\",;\\ \t"; - private static final String __01Jan1970_COOKIE = DateGenerator.formatCookieDate(0).trim(); private static final int __MIN_BUFFER_SIZE = 1; private static final HttpField __EXPIRES_01JAN1970 = new PreEncodedHttpField(HttpHeader.EXPIRES,DateGenerator.__01Jan1970); - // Cookie building buffer. Reduce garbage for cookie using applications - private static final ThreadLocal __cookieBuilder = ThreadLocal.withInitial(() -> new StringBuilder(128)); public enum OutputType { @@ -168,30 +165,13 @@ public class Response implements HttpServletResponse public void addCookie(HttpCookie cookie) { if (StringUtil.isBlank(cookie.getName())) - { throw new IllegalArgumentException("Cookie.name cannot be blank/null"); - } - - if (getHttpChannel().getHttpConfiguration().getResponseCookieCompliance()==CookieCompliance.RFC2965) - addSetRFC2965Cookie( - cookie.getName(), - cookie.getValue(), - cookie.getDomain(), - cookie.getPath(), - cookie.getMaxAge(), - cookie.getComment(), - cookie.isSecure(), - cookie.isHttpOnly(), - cookie.getVersion()); - else - addSetRFC6265Cookie( - cookie.getName(), - cookie.getValue(), - cookie.getDomain(), - cookie.getPath(), - cookie.getMaxAge(), - cookie.isSecure(), - cookie.isHttpOnly()); + + // add the set cookie + _fields.add(new SetCookieHttpField(cookie, getHttpChannel().getHttpConfiguration().getResponseCookieCompliance())); + + // Expire responses with set-cookie headers so they do not get cached. + _fields.put(__EXPIRES_01JAN1970); } /** @@ -207,84 +187,34 @@ public class Response implements HttpServletResponse if (field.getHeader() == HttpHeader.SET_COOKIE) { - String old_set_cookie = field.getValue(); - String name = cookie.getName(); - if (!old_set_cookie.startsWith(name) || old_set_cookie.length()<= name.length() || old_set_cookie.charAt(name.length())!='=') - continue; + CookieCompliance compliance = getHttpChannel().getHttpConfiguration().getResponseCookieCompliance(); - CookieCompliance responseCookieCompliance = getHttpChannel().getHttpConfiguration().getResponseCookieCompliance(); - - String expectedDomainSegment = "; Domain="; // default to RFC6265 - if (responseCookieCompliance == CookieCompliance.RFC2965) - expectedDomainSegment = ";Domain="; - - String domain = cookie.getDomain(); - //noinspection Duplicates - if (domain!=null) - { - if (responseCookieCompliance == CookieCompliance.RFC2965) - { - StringBuilder buf = new StringBuilder(); - buf.append(expectedDomainSegment); - quoteOnlyOrAppend(buf,domain,isQuoteNeededForCookie(domain)); - domain = buf.toString(); - } - else - { - domain = expectedDomainSegment+domain; - } - if (!old_set_cookie.contains(domain)) - continue; - } - else if (old_set_cookie.contains(expectedDomainSegment)) - continue; - - String expectedPathSegment = "; Path="; // default to RFC6265 - if (responseCookieCompliance == CookieCompliance.RFC2965) - expectedPathSegment = ";Path="; - String path = cookie.getPath(); - //noinspection Duplicates - if (path!=null) - { - if (responseCookieCompliance == CookieCompliance.RFC2965) - { - StringBuilder buf = new StringBuilder(); - buf.append(expectedPathSegment); - quoteOnlyOrAppend(buf,path,isQuoteNeededForCookie(path)); - path = buf.toString(); - } - else - { - path = expectedPathSegment+path; - } - if (!old_set_cookie.contains(path)) - continue; - } - else if (old_set_cookie.contains(expectedPathSegment)) - continue; - - if (responseCookieCompliance == CookieCompliance.RFC2965) - i.set(new HttpField(HttpHeader.CONTENT_ENCODING.SET_COOKIE, newRFC2965SetCookie( - cookie.getName(), - cookie.getValue(), - cookie.getDomain(), - cookie.getPath(), - cookie.getMaxAge(), - cookie.getComment(), - cookie.isSecure(), - cookie.isHttpOnly(), - cookie.getVersion()) - )); + HttpCookie oldCookie; + if (field instanceof SetCookieHttpField) + oldCookie = ((SetCookieHttpField)field).getHttpCookie(); else - i.set(new HttpField(HttpHeader.CONTENT_ENCODING.SET_COOKIE, newRFC6265SetCookie( - cookie.getName(), - cookie.getValue(), - cookie.getDomain(), - cookie.getPath(), - cookie.getMaxAge(), - cookie.isSecure(), - cookie.isHttpOnly() - ))); + oldCookie = new HttpCookie(field.getValue()); + + if (!cookie.getName().equals(oldCookie.getName())) + continue; + + if (cookie.getDomain()==null) + { + if (oldCookie.getDomain() != null) + continue; + } + else if (!cookie.getDomain().equalsIgnoreCase(oldCookie.getDomain())) + continue; + + if (cookie.getPath()==null) + { + if (oldCookie.getPath() != null) + continue; + } + else if (!cookie.getPath().equals(oldCookie.getPath())) + continue; + + i.set(new SetCookieHttpField(cookie, compliance)); return; } } @@ -296,8 +226,11 @@ public class Response implements HttpServletResponse @Override public void addCookie(Cookie cookie) { + if (StringUtil.isBlank(cookie.getName())) + throw new IllegalArgumentException("Cookie.name cannot be blank/null"); + String comment = cookie.getComment(); - boolean httpOnly = false; + boolean httpOnly = cookie.isHttpOnly(); if (comment != null) { @@ -310,264 +243,19 @@ public class Response implements HttpServletResponse comment = null; } } - - if (StringUtil.isBlank(cookie.getName())) - { - throw new IllegalArgumentException("Cookie.name cannot be blank/null"); - } - if (getHttpChannel().getHttpConfiguration().getResponseCookieCompliance()==CookieCompliance.RFC2965) - addSetRFC2965Cookie(cookie.getName(), - cookie.getValue(), - cookie.getDomain(), - cookie.getPath(), - cookie.getMaxAge(), - comment, - cookie.getSecure(), - httpOnly || cookie.isHttpOnly(), - cookie.getVersion()); - else - addSetRFC6265Cookie(cookie.getName(), - cookie.getValue(), - cookie.getDomain(), - cookie.getPath(), - cookie.getMaxAge(), - cookie.getSecure(), - httpOnly || cookie.isHttpOnly()); + addCookie(new HttpCookie( + cookie.getName(), + cookie.getValue(), + cookie.getDomain(), + cookie.getPath(), + (long) cookie.getMaxAge(), + httpOnly, + cookie.getSecure(), + comment, + cookie.getVersion())); } - - /** - * Format a set cookie value by RFC6265 - * - * @param name the name - * @param value the value - * @param domain the domain - * @param path the path - * @param maxAge the maximum age - * @param isSecure true if secure cookie - * @param isHttpOnly true if for http only - */ - public void addSetRFC6265Cookie( - final String name, - final String value, - final String domain, - final String path, - final long maxAge, - final boolean isSecure, - final boolean isHttpOnly) - { - String set_cookie = newRFC6265SetCookie(name, value, domain, path, maxAge, isSecure, isHttpOnly); - - // add the set cookie - _fields.add(HttpHeader.SET_COOKIE, set_cookie); - - // Expire responses with set-cookie headers so they do not get cached. - _fields.put(__EXPIRES_01JAN1970); - - } - - private String newRFC6265SetCookie(String name, String value, String domain, String path, long maxAge, boolean isSecure, boolean isHttpOnly) - { - // Check arguments - if (name == null || name.length() == 0) - throw new IllegalArgumentException("Bad cookie name"); - - // Name is checked for legality by servlet spec, but can also be passed directly so check again for quoting - // Per RFC6265, Cookie.name follows RFC2616 Section 2.2 token rules - Syntax.requireValidRFC2616Token(name, "RFC6265 Cookie name"); - // Ensure that Per RFC6265, Cookie.value follows syntax rules - Syntax.requireValidRFC6265CookieValue(value); - - // Format value and params - StringBuilder buf = __cookieBuilder.get(); - buf.setLength(0); - buf.append(name).append('=').append(value==null?"":value); - - // Append path - if (path!=null && path.length()>0) - buf.append("; Path=").append(path); - - // Append domain - if (domain!=null && domain.length()>0) - buf.append("; Domain=").append(domain); - - // Handle max-age and/or expires - if (maxAge >= 0) - { - // 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 - DateGenerator.formatCookieDate(buf, System.currentTimeMillis() + 1000L * maxAge); - - buf.append("; Max-Age="); - buf.append(maxAge); - } - - // add the other fields - if (isSecure) - buf.append("; Secure"); - if (isHttpOnly) - buf.append("; HttpOnly"); - return buf.toString(); - } - - /** - * Format a set cookie value - * - * @param name the name - * @param value the value - * @param domain the domain - * @param path the path - * @param maxAge the maximum age - * @param comment the comment (only present on versions > 0) - * @param isSecure true if secure cookie - * @param isHttpOnly true if for http only - * @param version version of cookie logic to use (0 == default behavior) - */ - public void addSetRFC2965Cookie( - final String name, - final String value, - final String domain, - final String path, - final long maxAge, - final String comment, - final boolean isSecure, - final boolean isHttpOnly, - int version) - { - String set_cookie = newRFC2965SetCookie(name, value, domain, path, maxAge, comment, isSecure, isHttpOnly, version); - - // add the set cookie - _fields.add(HttpHeader.SET_COOKIE, set_cookie); - - // Expire responses with set-cookie headers so they do not get cached. - _fields.put(__EXPIRES_01JAN1970); - } - - private String newRFC2965SetCookie(String name, String value, String domain, String path, long maxAge, String comment, boolean isSecure, boolean isHttpOnly, int version) - { - // Check arguments - if (name == null || name.length() == 0) - throw new IllegalArgumentException("Bad cookie name"); - - // Format value and params - StringBuilder buf = __cookieBuilder.get(); - buf.setLength(0); - - // Name is checked for legality by servlet spec, but can also be passed directly so check again for quoting - boolean quote_name=isQuoteNeededForCookie(name); - quoteOnlyOrAppend(buf,name,quote_name); - - buf.append('='); - - // Append the value - boolean quote_value=isQuoteNeededForCookie(value); - quoteOnlyOrAppend(buf,value,quote_value); - - // 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 && isQuoteNeededForCookie(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 || - QuotedStringTokenizer.isQuoted(name) || QuotedStringTokenizer.isQuoted(value) || - QuotedStringTokenizer.isQuoted(path) || QuotedStringTokenizer.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) - { - buf.append(";Path="); - quoteOnlyOrAppend(buf,path,quote_path); - } - - // Append domain - if (has_domain) - { - buf.append(";Domain="); - quoteOnlyOrAppend(buf,domain,quote_domain); - } - - // Handle max-age and/or expires - if (maxAge >= 0) - { - // 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 - DateGenerator.formatCookieDate(buf, System.currentTimeMillis() + 1000L * 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) - { - buf.append(";Comment="); - quoteOnlyOrAppend(buf,comment,isQuoteNeededForCookie(comment)); - } - return buf.toString(); - } - - - /* ------------------------------------------------------------ */ - /** 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 - */ - private 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; - } - - - private static void quoteOnlyOrAppend(StringBuilder buf, String s, boolean quote) - { - if (quote) - QuotedStringTokenizer.quoteOnly(buf,s); - else - buf.append(s); - } @Override public boolean containsHeader(String name) 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 929a935cec8..2a46b7c7be9 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 @@ -1102,6 +1102,32 @@ public class ResponseTest assertThat("HttpCookie order", actual, hasItems(expected)); } + + @Test + public void testReplaceParsedHttpCookie() + { + Response response = getResponse(); + + response.addHeader(HttpHeader.SET_COOKIE.asString(), "Foo=123456"); + response.replaceCookie(new HttpCookie("Foo","value")); + List actual = Collections.list(response.getHttpFields().getValues("Set-Cookie")); + assertThat(actual, hasItems(new String[] {"Foo=value"})); + + response.setHeader(HttpHeader.SET_COOKIE,"Foo=123456; domain=Bah; Path=/path"); + response.replaceCookie(new HttpCookie("Foo","other")); + actual = Collections.list(response.getHttpFields().getValues("Set-Cookie")); + assertThat(actual, hasItems(new String[] {"Foo=123456; domain=Bah; Path=/path", "Foo=other"})); + + response.replaceCookie(new HttpCookie("Foo","replaced", "Bah", "/path")); + actual = Collections.list(response.getHttpFields().getValues("Set-Cookie")); + assertThat(actual, hasItems(new String[] {"Foo=replaced; Path=/path; Domain=Bah", "Foo=other"})); + + response.setHeader(HttpHeader.SET_COOKIE,"Foo=123456; domain=Bah; Expires=Thu, 01-Jan-1970 00:00:00 GMT; Max-Age=0; Secure; HttpOnly; Path=/path"); + response.replaceCookie(new HttpCookie("Foo","replaced", "Bah", "/path")); + actual = Collections.list(response.getHttpFields().getValues("Set-Cookie")); + assertThat(actual, hasItems(new String[] {"Foo=replaced; Path=/path; Domain=Bah"})); + } + @Test public void testFlushAfterFullContent() throws Exception { @@ -1114,312 +1140,6 @@ public class ResponseTest // Must not throw output.flush(); } - - @Test - public void testSetRFC2965Cookie() throws Exception - { - Response response = _channel.getResponse(); - HttpFields fields = response.getHttpFields(); - - response.addSetRFC2965Cookie("null",null,null,null,-1,null,false,false,-1); - assertEquals("null=",fields.get("Set-Cookie")); - - fields.clear(); - - response.addSetRFC2965Cookie("minimal","value",null,null,-1,null,false,false,-1); - assertEquals("minimal=value",fields.get("Set-Cookie")); - - fields.clear(); - //test cookies with same name, domain and path - response.addSetRFC2965Cookie("everything","something","domain","path",0,"noncomment",true,true,0); - response.addSetRFC2965Cookie("everything","value","domain","path",0,"comment",true,true,0); - Enumeration e =fields.getValues("Set-Cookie"); - assertTrue(e.hasMoreElements()); - assertEquals("everything=something;Version=1;Path=path;Domain=domain;Expires=Thu, 01-Jan-1970 00:00:00 GMT;Max-Age=0;Secure;HttpOnly;Comment=noncomment",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.get("Expires")); - assertFalse(e.hasMoreElements()); - - //test cookies with same name, different domain - fields.clear(); - response.addSetRFC2965Cookie("everything","other","domain1","path",0,"blah",true,true,0); - response.addSetRFC2965Cookie("everything","value","domain2","path",0,"comment",true,true,0); - e =fields.getValues("Set-Cookie"); - assertTrue(e.hasMoreElements()); - 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;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 - fields.clear(); - response.addSetRFC2965Cookie("everything","other","domain1","path",0,"blah",true,true,0); - response.addSetRFC2965Cookie("everything","value","","path",0,"comment",true,true,0); - e =fields.getValues("Set-Cookie"); - assertTrue(e.hasMoreElements()); - 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;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()); - - - //test cookies with same name, different path - fields.clear(); - response.addSetRFC2965Cookie("everything","other","domain1","path1",0,"blah",true,true,0); - response.addSetRFC2965Cookie("everything","value","domain1","path2",0,"comment",true,true,0); - e =fields.getValues("Set-Cookie"); - assertTrue(e.hasMoreElements()); - 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;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 - fields.clear(); - response.addSetRFC2965Cookie("everything","other","domain1","path1",0,"blah",true,true,0); - response.addSetRFC2965Cookie("everything","value","domain1","",0,"comment",true,true,0); - e =fields.getValues("Set-Cookie"); - assertTrue(e.hasMoreElements()); - 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;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 - fields.clear(); - response.addSetRFC2965Cookie("everything","other","","",0,"blah",true,true,0); - response.addSetRFC2965Cookie("everything","value","","",0,"comment",true,true,0); - e =fields.getValues("Set-Cookie"); - assertTrue(e.hasMoreElements()); - assertEquals("everything=other;Version=1;Expires=Thu, 01-Jan-1970 00:00:00 GMT;Max-Age=0;Secure;HttpOnly;Comment=blah",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(); - response.addSetRFC2965Cookie("ev erything","va lue","do main","pa th",1,"co mment",true,true,1); - String setCookie=fields.get("Set-Cookie"); - 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(); - response.addSetRFC2965Cookie("name","value",null,null,-1,null,false,false,0); - setCookie=fields.get("Set-Cookie"); - assertEquals(-1,setCookie.indexOf("Version=")); - fields.clear(); - response.addSetRFC2965Cookie("name","v a l u e",null,null,-1,null,false,false,0); - setCookie=fields.get("Set-Cookie"); - - fields.clear(); - response.addSetRFC2965Cookie("json","{\"services\":[\"cwa\", \"aa\"]}",null,null,-1,null,false,false,-1); - assertEquals("json=\"{\\\"services\\\":[\\\"cwa\\\", \\\"aa\\\"]}\"",fields.get("Set-Cookie")); - - fields.clear(); - response.addSetRFC2965Cookie("name","value","domain",null,-1,null,false,false,-1); - response.addSetRFC2965Cookie("name","other","domain",null,-1,null,false,false,-1); - response.addSetRFC2965Cookie("name","more","domain",null,-1,null,false,false,-1); - e = fields.getValues("Set-Cookie"); - assertTrue(e.hasMoreElements()); - assertThat(e.nextElement(), Matchers.startsWith("name=value")); - assertThat(e.nextElement(), Matchers.startsWith("name=other")); - assertThat(e.nextElement(), Matchers.startsWith("name=more")); - - response.addSetRFC2965Cookie("foo","bar","domain",null,-1,null,false,false,-1); - response.addSetRFC2965Cookie("foo","bob","domain",null,-1,null,false,false,-1); - assertThat(fields.get("Set-Cookie"), Matchers.startsWith("name=value")); - - - fields.clear(); - response.addSetRFC2965Cookie("name","value%=",null,null,-1,null,false,false,0); - setCookie=fields.get("Set-Cookie"); - assertEquals("name=value%=",setCookie); - } - - @Test - public void testSetRFC6265Cookie() throws Exception - { - Response response = _channel.getResponse(); - HttpFields fields = response.getHttpFields(); - - response.addSetRFC6265Cookie("null",null,null,null,-1,false,false); - assertEquals("null=",fields.get("Set-Cookie")); - - fields.clear(); - - response.addSetRFC6265Cookie("minimal","value",null,null,-1,false,false); - assertEquals("minimal=value",fields.get("Set-Cookie")); - - fields.clear(); - //test cookies with same name, domain and path - response.addSetRFC6265Cookie("everything","something","domain","path",0,true,true); - response.addSetRFC6265Cookie("everything","value","domain","path",0,true,true); - Enumeration e =fields.getValues("Set-Cookie"); - assertTrue(e.hasMoreElements()); - assertEquals("everything=something; Path=path; Domain=domain; Expires=Thu, 01-Jan-1970 00:00:00 GMT; Max-Age=0; Secure; HttpOnly",e.nextElement()); - assertEquals("everything=value; Path=path; Domain=domain; Expires=Thu, 01-Jan-1970 00:00:00 GMT; Max-Age=0; Secure; HttpOnly",e.nextElement()); - assertFalse(e.hasMoreElements()); - assertEquals("Thu, 01 Jan 1970 00:00:00 GMT",fields.get("Expires")); - assertFalse(e.hasMoreElements()); - - //test cookies with same name, different domain - fields.clear(); - response.addSetRFC6265Cookie("everything","other","domain1","path",0,true,true); - response.addSetRFC6265Cookie("everything","value","domain2","path",0,true,true); - 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",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",e.nextElement()); - assertFalse(e.hasMoreElements()); - - //test cookies with same name, same path, one with domain, one without - fields.clear(); - response.addSetRFC6265Cookie("everything","other","domain1","path",0,true,true); - response.addSetRFC6265Cookie("everything","value","","path",0,true,true); - 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",e.nextElement()); - assertTrue(e.hasMoreElements()); - assertEquals("everything=value; Path=path; Expires=Thu, 01-Jan-1970 00:00:00 GMT; Max-Age=0; Secure; HttpOnly",e.nextElement()); - assertFalse(e.hasMoreElements()); - - - //test cookies with same name, different path - fields.clear(); - response.addSetRFC6265Cookie("everything","other","domain1","path1",0,true,true); - response.addSetRFC6265Cookie("everything","value","domain1","path2",0,true,true); - 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",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",e.nextElement()); - assertFalse(e.hasMoreElements()); - - //test cookies with same name, same domain, one with path, one without - fields.clear(); - response.addSetRFC6265Cookie("everything","other","domain1","path1",0,true,true); - response.addSetRFC6265Cookie("everything","value","domain1","",0,true,true); - 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",e.nextElement()); - assertTrue(e.hasMoreElements()); - assertEquals("everything=value; Domain=domain1; Expires=Thu, 01-Jan-1970 00:00:00 GMT; Max-Age=0; Secure; HttpOnly",e.nextElement()); - assertFalse(e.hasMoreElements()); - - //test cookies same name only, no path, no domain - fields.clear(); - response.addSetRFC6265Cookie("everything","other","","",0,true,true); - response.addSetRFC6265Cookie("everything","value","","",0,true,true); - e =fields.getValues("Set-Cookie"); - assertTrue(e.hasMoreElements()); - assertEquals("everything=other; Expires=Thu, 01-Jan-1970 00:00:00 GMT; Max-Age=0; Secure; HttpOnly",e.nextElement()); - assertEquals("everything=value; Expires=Thu, 01-Jan-1970 00:00:00 GMT; Max-Age=0; Secure; HttpOnly",e.nextElement()); - assertFalse(e.hasMoreElements()); - - String badNameExamples[] = { - "\"name\"", - "name\t", - "na me", - "name\u0082", - "na\tme", - "na;me", - "{name}", - "[name]", - "\"" - }; - - for (String badNameExample : badNameExamples) - { - fields.clear(); - try - { - response.addSetRFC6265Cookie(badNameExample, "value", null, "/", 1, true, true); - } - catch (IllegalArgumentException ex) - { - // System.err.printf("%s: %s%n", ex.getClass().getSimpleName(), ex.getMessage()); - assertThat("Testing bad name: [" + badNameExample + "]", ex.getMessage(), - allOf(containsString("RFC6265"), containsString("RFC2616"))); - } - } - - String badValueExamples[] = { - "va\tlue", - "\t", - "value\u0000", - "val\u0082ue", - "va lue", - "va;lue", - "\"value", - "value\"", - "val\\ue", - "val\"ue", - "\"" - }; - - for (String badValueExample : badValueExamples) - { - fields.clear(); - try - { - response.addSetRFC6265Cookie("name", badValueExample, null, "/", 1, true, true); - } - catch (IllegalArgumentException ex) - { - // System.err.printf("%s: %s%n", ex.getClass().getSimpleName(), ex.getMessage()); - assertThat("Testing bad value [" + badValueExample + "]", ex.getMessage(), Matchers.containsString("RFC6265")); - } - } - - String goodNameExamples[] = { - "name", - "n.a.m.e", - "na-me", - "+name", - "na*me", - "na$me", - "#name" - }; - - for (String goodNameExample : goodNameExamples) - { - fields.clear(); - response.addSetRFC6265Cookie(goodNameExample, "value", null, "/", 1, true, true); - // should not throw an exception - } - - String goodValueExamples[] = { - "value", - "", - null, - "val=ue", - "val-ue", - "val/ue", - "v.a.l.u.e" - }; - - for (String goodValueExample : goodValueExamples) - { - fields.clear(); - response.addSetRFC6265Cookie("name", goodValueExample, null, "/", 1, true, true); - // should not throw an exception - } - - fields.clear(); - - response.addSetRFC6265Cookie("name","value","domain",null,-1,false,false); - response.addSetRFC6265Cookie("name","other","domain",null,-1,false,false); - response.addSetRFC6265Cookie("name","more","domain",null,-1,false,false); - e = fields.getValues("Set-Cookie"); - assertTrue(e.hasMoreElements()); - assertThat(e.nextElement(), Matchers.startsWith("name=value")); - assertThat(e.nextElement(), Matchers.startsWith("name=other")); - assertThat(e.nextElement(), Matchers.startsWith("name=more")); - - response.addSetRFC6265Cookie("foo","bar","domain",null,-1,false,false); - response.addSetRFC6265Cookie("foo","bob","domain",null,-1,false,false); - assertThat(fields.get("Set-Cookie"), Matchers.startsWith("name=value")); - } private Response getResponse() {