From 5d267963a3016d4810d10aa272f512247c0f3b60 Mon Sep 17 00:00:00 2001 From: Joakim Erdfelt Date: Wed, 15 May 2019 13:55:20 -0500 Subject: [PATCH] Issue #3655 - Cookie generation now complies with RFC6265 spaces Signed-off-by: Joakim Erdfelt --- .../jetty/security/ConstraintTest.java | 62 +++++++++---------- .../eclipse/jetty/server/CookieCutter.java | 24 ++++--- .../org/eclipse/jetty/server/Response.java | 43 ++++++++----- .../jetty/server/CookieCutterTest.java | 48 +++++++++++--- .../eclipse/jetty/server/ResponseTest.java | 55 ++++++++-------- 5 files changed, 142 insertions(+), 90 deletions(-) diff --git a/jetty-security/src/test/java/org/eclipse/jetty/security/ConstraintTest.java b/jetty-security/src/test/java/org/eclipse/jetty/security/ConstraintTest.java index 51573bb354f..481e1ce87ca 100644 --- a/jetty-security/src/test/java/org/eclipse/jetty/security/ConstraintTest.java +++ b/jetty-security/src/test/java/org/eclipse/jetty/security/ConstraintTest.java @@ -833,7 +833,7 @@ public class ConstraintTest assertThat(response, containsString("Expires")); assertThat(response, containsString("URI=/ctx/testLoginPage")); - String session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf(";Path=/ctx")); + String session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf("; Path=/ctx")); response = _connector.getResponse("POST /ctx/j_security_check HTTP/1.0\r\n" + "Cookie: JSESSIONID=" + session + "\r\n" + @@ -853,7 +853,7 @@ public class ConstraintTest assertThat(response, containsString("Location")); assertThat(response, containsString("Location")); assertThat(response, containsString("/ctx/auth/info")); - session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf(";Path=/ctx")); + session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf("; Path=/ctx")); response = _connector.getResponse("GET /ctx/auth/info HTTP/1.0\r\n" + "Cookie: JSESSIONID=" + session + "\r\n" + @@ -888,7 +888,7 @@ public class ConstraintTest assertThat(response, containsString(" 302 Found")); assertThat(response, containsString("/ctx/testLoginPage")); assertThat(response, containsString("JSESSIONID=")); - String session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf(";Path=/ctx")); + String session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf("; Path=/ctx")); response = _connector.getResponse("GET /ctx/testLoginPage HTTP/1.0\r\n"+ "Cookie: JSESSIONID=" + session + "\r\n" + @@ -917,7 +917,7 @@ public class ConstraintTest assertThat(response, containsString("/ctx/auth/info")); assertThat(response, containsString("JSESSIONID=")); assertThat(response, not(containsString("JSESSIONID=" + session))); - session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf(";Path=/ctx")); + session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf("; Path=/ctx")); response = _connector.getResponse("GET /ctx/auth/info HTTP/1.0\r\n" + "Cookie: JSESSIONID=" + session + "\r\n" + @@ -954,7 +954,7 @@ public class ConstraintTest "test_parameter=test_value\r\n"); assertThat(response, containsString(" 302 Found")); assertThat(response, containsString("/ctx/testLoginPage")); - String session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf(";Path=/ctx")); + String session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf("; Path=/ctx")); response = _connector.getResponse("GET /ctx/testLoginPage HTTP/1.0\r\n"+ "Cookie: JSESSIONID=" + session + "\r\n" + @@ -980,7 +980,7 @@ public class ConstraintTest assertThat(response, startsWith("HTTP/1.1 302 ")); assertThat(response, containsString("Location")); assertThat(response, containsString("/ctx/auth/info")); - session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf(";Path=/ctx")); + session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf("; Path=/ctx")); // sneak in other request response = _connector.getResponse("GET /ctx/auth/other HTTP/1.0\r\n" + @@ -1043,7 +1043,7 @@ public class ConstraintTest assertThat(response, startsWith("HTTP/1.1 302 ")); assertThat(response, containsString("Location")); assertThat(response, containsString("/ctx/auth/info")); - session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf(";Path=/ctx")); + session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf("; Path=/ctx")); response = _connector.getResponse("GET /ctx/auth/info;jsessionid="+session+";other HTTP/1.0\r\n" + "\r\n"); @@ -1079,7 +1079,7 @@ public class ConstraintTest //login response = _connector.getResponse("GET /ctx/prog?action=login HTTP/1.0\r\n\r\n"); assertThat(response, startsWith("HTTP/1.1 200 OK")); - String session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf(";Path=/ctx")); + String session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf("; Path=/ctx")); response = _connector.getResponse("GET /ctx/prog?x=y HTTP/1.0\r\n" + "Cookie: JSESSIONID=" + session + "\r\n" + "\r\n"); @@ -1093,7 +1093,7 @@ public class ConstraintTest assertThat(response, startsWith("HTTP/1.1 500 Server Error")); if (response.contains("JSESSIONID")) { - session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf(";Path=/ctx")); + session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf("; Path=/ctx")); response = _connector.getResponse("GET /ctx/prog?x=y HTTP/1.0\r\n" + "Cookie: JSESSIONID=" + session + "\r\n" + "\r\n"); @@ -1110,7 +1110,7 @@ public class ConstraintTest assertThat(response, startsWith("HTTP/1.1 500 Server Error")); response = _connector.getResponse("GET /ctx/prog?action=login HTTP/1.0\r\n\r\n"); assertThat(response, startsWith("HTTP/1.1 200 OK")); - session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf(";Path=/ctx")); + session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf("; Path=/ctx")); response = _connector.getResponse("GET /ctx/prog?x=y HTTP/1.0\r\n" + "Cookie: JSESSIONID=" + session + "\r\n" + "\r\n"); @@ -1122,7 +1122,7 @@ public class ConstraintTest _server.start(); response = _connector.getResponse("GET /ctx/prog?action=loginlogin HTTP/1.0\r\n\r\n"); assertThat(response, startsWith("HTTP/1.1 500 Server Error")); - session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf(";Path=/ctx")); + session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf("; Path=/ctx")); response = _connector.getResponse("GET /ctx/prog?x=y HTTP/1.0\r\n" + "Cookie: JSESSIONID=" + session + "\r\n" + "\r\n"); @@ -1134,7 +1134,7 @@ public class ConstraintTest _server.start(); response = _connector.getResponse("GET /ctx/prog?action=loginlogout HTTP/1.0\r\n\r\n"); assertThat(response, startsWith("HTTP/1.1 200 OK")); - session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf(";Path=/ctx")); + session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf("; Path=/ctx")); response = _connector.getResponse("GET /ctx/prog?x=y HTTP/1.0\r\n" + "Cookie: JSESSIONID=" + session + "\r\n" + "\r\n"); @@ -1146,7 +1146,7 @@ public class ConstraintTest _server.start(); response = _connector.getResponse("GET /ctx/prog?action=loginlogoutlogin HTTP/1.0\r\n\r\n"); assertThat(response, startsWith("HTTP/1.1 200 OK")); - session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf(";Path=/ctx")); + session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf("; Path=/ctx")); response = _connector.getResponse("GET /ctx/prog?x=y HTTP/1.0\r\n" + "Cookie: JSESSIONID=" + session + "\r\n" + "\r\n"); @@ -1162,7 +1162,7 @@ public class ConstraintTest assertThat(response, containsString(" 302 Found")); assertThat(response, containsString("/ctx/testLoginPage")); assertThat(response, containsString("JSESSIONID=")); - session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf(";Path=/ctx")); + session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf("; Path=/ctx")); response = _connector.getResponse("GET /ctx/testLoginPage HTTP/1.0\r\n"+ "Cookie: JSESSIONID=" + session + "\r\n" + @@ -1180,7 +1180,7 @@ public class ConstraintTest assertThat(response, containsString("/ctx/auth/info")); assertThat(response, containsString("JSESSIONID=")); assertThat(response, not(containsString("JSESSIONID=" + session))); - session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf(";Path=/ctx")); + session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf("; Path=/ctx")); response = _connector.getResponse("GET /ctx/prog?action=constraintlogin HTTP/1.0\r\n" + "Cookie: JSESSIONID=" + session + "\r\n" + "\r\n"); @@ -1193,7 +1193,7 @@ public class ConstraintTest assertThat(response, containsString(" 302 Found")); assertThat(response, containsString("/ctx/testLoginPage")); assertThat(response, containsString("JSESSIONID=")); - session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf(";Path=/ctx")); + session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf("; Path=/ctx")); response = _connector.getResponse("GET /ctx/testLoginPage HTTP/1.0\r\n"+ "Cookie: JSESSIONID=" + session + "\r\n" + @@ -1211,7 +1211,7 @@ public class ConstraintTest assertThat(response, containsString("/ctx/auth/info")); assertThat(response, containsString("JSESSIONID=")); assertThat(response, not(containsString("JSESSIONID=" + session))); - session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf(";Path=/ctx")); + session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf("; Path=/ctx")); response = _connector.getResponse("GET /ctx/prog?action=logout HTTP/1.0\r\n" + "Cookie: JSESSIONID=" + session + "\r\n" + "\r\n"); @@ -1307,7 +1307,7 @@ public class ConstraintTest assertThat(response, containsString("Expires")); assertThat(response, containsString("URI=/ctx/testLoginPage")); - String session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf(";Path=/ctx")); + String session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf("; Path=/ctx")); response = _connector.getResponse("POST /ctx/j_security_check HTTP/1.0\r\n" + "Cookie: JSESSIONID=" + session + "\r\n" + @@ -1327,7 +1327,7 @@ public class ConstraintTest assertThat(response, startsWith("HTTP/1.1 302 ")); assertThat(response, containsString("Location")); assertThat(response, containsString("/ctx/auth/info")); - session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf(";Path=/ctx")); + session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf("; Path=/ctx")); response = _connector.getResponse("GET /ctx/auth/info HTTP/1.0\r\n" + "Cookie: JSESSIONID=" + session + "\r\n" + @@ -1347,7 +1347,7 @@ public class ConstraintTest response = _connector.getResponse("GET /ctx/auth/info HTTP/1.0\r\n\r\n"); // assertThat(response,startsWith("HTTP/1.1 302 ")); // assertThat(response,containsString("testLoginPage")); - session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf(";Path=/ctx")); + session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf("; Path=/ctx")); response = _connector.getResponse("POST /ctx/j_security_check HTTP/1.0\r\n" + "Cookie: JSESSIONID=" + session + "\r\n" + @@ -1358,7 +1358,7 @@ public class ConstraintTest assertThat(response, startsWith("HTTP/1.1 302 ")); assertThat(response, containsString("Location")); assertThat(response, containsString("/ctx/auth/info")); - session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf(";Path=/ctx")); + session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf("; Path=/ctx")); response = _connector.getResponse("GET /ctx/auth/info HTTP/1.0\r\n" + "Cookie: JSESSIONID=" + session + "\r\n" + @@ -1377,7 +1377,7 @@ public class ConstraintTest response = _connector.getResponse("GET /ctx/auth/info HTTP/1.0\r\n\r\n"); // assertThat(response,startsWith("HTTP/1.1 302 ")); // assertThat(response,containsString("testLoginPage")); - session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf(";Path=/ctx")); + session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf("; Path=/ctx")); response = _connector.getResponse("POST /ctx/j_security_check HTTP/1.0\r\n" + "Cookie: JSESSIONID=" + session + "\r\n" + @@ -1388,7 +1388,7 @@ public class ConstraintTest assertThat(response, startsWith("HTTP/1.1 302 ")); assertThat(response, containsString("Location")); assertThat(response, containsString("/ctx/auth/info")); - session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf(";Path=/ctx")); + session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf("; Path=/ctx")); response = _connector.getResponse("GET /ctx/auth/info HTTP/1.0\r\n" + "Cookie: JSESSIONID=" + session + "\r\n" + @@ -1419,7 +1419,7 @@ public class ConstraintTest assertThat(response, containsString(" 302 Found")); assertThat(response, containsString("http://wibble.com:8888/ctx/testLoginPage")); - String session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf(";Path=/ctx")); + String session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf("; Path=/ctx")); response = _connector.getResponse("POST /ctx/j_security_check HTTP/1.0\r\n" + "Cookie: JSESSIONID=" + session + "\r\n" + @@ -1438,7 +1438,7 @@ public class ConstraintTest assertThat(response, startsWith("HTTP/1.1 302 ")); assertThat(response, containsString("Location")); assertThat(response, containsString("/ctx/auth/info")); - session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf(";Path=/ctx")); + session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf("; Path=/ctx")); response = _connector.getResponse("GET /ctx/auth/info HTTP/1.0\r\n" + "Cookie: JSESSIONID=" + session + "\r\n" + @@ -1458,7 +1458,7 @@ public class ConstraintTest response = _connector.getResponse("GET /ctx/auth/info HTTP/1.0\r\n\r\n"); assertThat(response, startsWith("HTTP/1.1 302 ")); assertThat(response, containsString("testLoginPage")); - session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf(";Path=/ctx")); + session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf("; Path=/ctx")); response = _connector.getResponse("POST /ctx/j_security_check HTTP/1.0\r\n" + "Cookie: JSESSIONID=" + session + "\r\n" + @@ -1469,7 +1469,7 @@ public class ConstraintTest assertThat(response, startsWith("HTTP/1.1 302 ")); assertThat(response, containsString("Location")); assertThat(response, containsString("/ctx/auth/info")); - session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf(";Path=/ctx")); + session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf("; Path=/ctx")); response = _connector.getResponse("GET /ctx/auth/info HTTP/1.0\r\n" + @@ -1489,7 +1489,7 @@ public class ConstraintTest response = _connector.getResponse("GET /ctx/starstar/info HTTP/1.0\r\n\r\n"); assertThat(response, startsWith("HTTP/1.1 302 ")); assertThat(response, containsString("testLoginPage")); - session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf(";Path=/ctx")); + session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf("; Path=/ctx")); response = _connector.getResponse("POST /ctx/j_security_check HTTP/1.0\r\n" + "Cookie: JSESSIONID=" + session + "\r\n" + @@ -1500,7 +1500,7 @@ public class ConstraintTest assertThat(response, startsWith("HTTP/1.1 302 ")); assertThat(response, containsString("Location")); assertThat(response, containsString("/ctx/starstar/info")); - session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf(";Path=/ctx")); + session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf("; Path=/ctx")); response = _connector.getResponse("GET /ctx/starstar/info HTTP/1.0\r\n" + "Cookie: JSESSIONID=" + session + "\r\n" + @@ -1512,7 +1512,7 @@ public class ConstraintTest response = _connector.getResponse("GET /ctx/auth/info HTTP/1.0\r\n\r\n"); // assertThat(response,startsWith("HTTP/1.1 302 ")); // assertThat(response,containsString("testLoginPage")); - session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf(";Path=/ctx")); + session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf("; Path=/ctx")); response = _connector.getResponse("POST /ctx/j_security_check HTTP/1.0\r\n" + "Cookie: JSESSIONID=" + session + "\r\n" + @@ -1523,7 +1523,7 @@ public class ConstraintTest assertThat(response, startsWith("HTTP/1.1 302 ")); assertThat(response, containsString("Location")); assertThat(response, containsString("/ctx/auth/info")); - session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf(";Path=/ctx")); + session = response.substring(response.indexOf("JSESSIONID=") + 11, response.indexOf("; Path=/ctx")); response = _connector.getResponse("GET /ctx/auth/info HTTP/1.0\r\n" + "Cookie: JSESSIONID=" + session + "\r\n" + diff --git a/jetty-server/src/main/java/org/eclipse/jetty/server/CookieCutter.java b/jetty-server/src/main/java/org/eclipse/jetty/server/CookieCutter.java index 5dce1cf791a..2c5fd6b1c45 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/CookieCutter.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/CookieCutter.java @@ -17,10 +17,10 @@ // package org.eclipse.jetty.server; + import java.util.ArrayList; import java.util.List; import java.util.Locale; - import javax.servlet.http.Cookie; import org.eclipse.jetty.http.CookieCompliance; @@ -28,14 +28,20 @@ import org.eclipse.jetty.util.log.Log; import org.eclipse.jetty.util.log.Logger; -/* ------------------------------------------------------------ */ -/** Cookie parser - *

Optimized stateful cookie parser. Cookies fields are added with the - * {@link #addCookieField(String)} method and parsed on the next subsequent - * call to {@link #getCookies()}. - * If the added fields are identical to those last added (as strings), then the - * cookies are not re parsed. - * +/** + * Cookie parser + *

+ * Optimized stateful {@code Cookie} header parser. + * Does not support {@code Set-Cookie} header parsing. + *

+ *

> + * Cookies fields are added with the {@link #addCookieField(String)} method and + * parsed on the next subsequent call to {@link #getCookies()}. + *

+ *

+ * If the added fields are identical to those last added (as strings), then the + * cookies are not re parsed. + *

*/ public class CookieCutter { 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 e74986a951e..a64973f3f2f 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 @@ -24,14 +24,12 @@ import java.nio.channels.IllegalSelectorException; import java.util.Collection; import java.util.Collections; import java.util.EnumSet; -import java.util.Iterator; import java.util.List; import java.util.ListIterator; import java.util.Locale; import java.util.concurrent.atomic.AtomicInteger; import java.util.function.Supplier; import java.util.stream.Collectors; - import javax.servlet.RequestDispatcher; import javax.servlet.ServletOutputStream; import javax.servlet.http.Cookie; @@ -214,47 +212,58 @@ public class Response implements HttpServletResponse if (!old_set_cookie.startsWith(name) || old_set_cookie.length()<= name.length() || old_set_cookie.charAt(name.length())!='=') continue; + 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 (getHttpChannel().getHttpConfiguration().getResponseCookieCompliance()==CookieCompliance.RFC2965) + if (responseCookieCompliance == CookieCompliance.RFC2965) { StringBuilder buf = new StringBuilder(); - buf.append(";Domain="); + buf.append(expectedDomainSegment); quoteOnlyOrAppend(buf,domain,isQuoteNeededForCookie(domain)); domain = buf.toString(); } else { - domain = ";Domain="+domain; + domain = expectedDomainSegment+domain; } if (!old_set_cookie.contains(domain)) continue; } - else if (old_set_cookie.contains(";Domain=")) + 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 (getHttpChannel().getHttpConfiguration().getResponseCookieCompliance()==CookieCompliance.RFC2965) + if (responseCookieCompliance == CookieCompliance.RFC2965) { StringBuilder buf = new StringBuilder(); - buf.append(";Path="); + buf.append(expectedPathSegment); quoteOnlyOrAppend(buf,path,isQuoteNeededForCookie(path)); path = buf.toString(); } else { - path = ";Path="+path; + path = expectedPathSegment+path; } if (!old_set_cookie.contains(path)) continue; } - else if (old_set_cookie.contains(";Path=")) + else if (old_set_cookie.contains(expectedPathSegment)) continue; - if (getHttpChannel().getHttpConfiguration().getResponseCookieCompliance() == CookieCompliance.RFC2965) + if (responseCookieCompliance == CookieCompliance.RFC2965) i.set(new HttpField(HttpHeader.CONTENT_ENCODING.SET_COOKIE, newRFC2965SetCookie( cookie.getName(), cookie.getValue(), @@ -377,32 +386,32 @@ public class Response implements HttpServletResponse // Append path if (path!=null && path.length()>0) - buf.append(";Path=").append(path); + buf.append("; Path=").append(path); // Append domain if (domain!=null && domain.length()>0) - buf.append(";Domain=").append(domain); + 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="); + buf.append("; Expires="); if (maxAge == 0) buf.append(__01Jan1970_COOKIE); else DateGenerator.formatCookieDate(buf, System.currentTimeMillis() + 1000L * maxAge); - buf.append(";Max-Age="); + buf.append("; Max-Age="); buf.append(maxAge); } // add the other fields if (isSecure) - buf.append(";Secure"); + buf.append("; Secure"); if (isHttpOnly) - buf.append(";HttpOnly"); + buf.append("; HttpOnly"); return buf.toString(); } diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/CookieCutterTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/CookieCutterTest.java index ec534a1af5c..48780de3c0b 100644 --- a/jetty-server/src/test/java/org/eclipse/jetty/server/CookieCutterTest.java +++ b/jetty-server/src/test/java/org/eclipse/jetty/server/CookieCutterTest.java @@ -18,15 +18,14 @@ package org.eclipse.jetty.server; -import static org.hamcrest.Matchers.is; -import static org.hamcrest.MatcherAssert.assertThat; - import javax.servlet.http.Cookie; import org.eclipse.jetty.http.CookieCompliance; -import org.junit.jupiter.api.Disabled; import org.junit.jupiter.api.Test; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.is; + public class CookieCutterTest { private Cookie[] parseCookieHeaders(CookieCompliance compliance,String... headers) @@ -64,7 +63,24 @@ public class CookieCutterTest assertThat("Cookies.length", cookies.length, is(1)); assertCookie("Cookies[0]", cookies[0], "Customer", "WILE_E_COYOTE", 1, "/acme"); } - + + /** + * Example from RFC2109 and RFC2965. + *

+ * Lenient parsing, input has no spaces after ';' token. + *

+ */ + @Test + public void testRFC_Single_Lenient_NoSpaces() + { + String rawCookie = "$Version=\"1\";Customer=\"WILE_E_COYOTE\";$Path=\"/acme\""; + + Cookie cookies[] = parseCookieHeaders(CookieCompliance.RFC2965,rawCookie); + + assertThat("Cookies.length", cookies.length, is(1)); + assertCookie("Cookies[0]", cookies[0], "Customer", "WILE_E_COYOTE", 1, "/acme"); + } + /** * Example from RFC2109 and RFC2965 */ @@ -156,7 +172,7 @@ public class CookieCutterTest assertCookie("Cookies[0]", cookies[0], "session_id", "1234\", $Version=\"1", 0, null); assertCookie("Cookies[1]", cookies[1], "session_id", "1111", 0, null); } - + /** * Example from RFC6265 */ @@ -185,7 +201,25 @@ public class CookieCutterTest assertCookie("Cookies[0]", cookies[0], "SID", "31d4d96e407aad42", 0, null); assertCookie("Cookies[1]", cookies[1], "lang", "en-US", 0, null); } - + + /** + * Example from RFC6265. + *

+ * Lenient parsing, input has no spaces after ';' token. + *

+ */ + @Test + public void testRFC6265_SidLangExample_Lenient() + { + String rawCookie = "SID=31d4d96e407aad42;lang=en-US"; + + Cookie cookies[] = parseCookieHeaders(CookieCompliance.RFC6265,rawCookie); + + assertThat("Cookies.length", cookies.length, is(2)); + assertCookie("Cookies[0]", cookies[0], "SID", "31d4d96e407aad42", 0, null); + assertCookie("Cookies[1]", cookies[1], "lang", "en-US", 0, null); + } + /** * Basic name=value, following RFC6265 rules */ 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 53d52225560..929a935cec8 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 @@ -33,6 +33,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.Enumeration; import java.util.Iterator; +import java.util.List; import java.util.Locale; import javax.servlet.ServletException; import javax.servlet.ServletOutputStream; @@ -73,9 +74,9 @@ import org.junit.jupiter.api.Test; import static java.nio.charset.StandardCharsets.UTF_8; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.allOf; -import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.hasItems; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.nullValue; @@ -977,7 +978,7 @@ public class ResponseTest String set = response.getHttpFields().get("Set-Cookie"); - assertEquals("name=value;Path=/path;Domain=domain;Secure;HttpOnly", set); + assertEquals("name=value; Path=/path; Domain=domain; Secure; HttpOnly", set); } @Test @@ -1015,7 +1016,7 @@ public class ResponseTest String set = response.getHttpFields().get("Set-Cookie"); - assertEquals("foo=bar%3Bbaz;Path=/secure", set); + assertEquals("foo=bar%3Bbaz; Path=/secure", set); } /** @@ -1056,8 +1057,8 @@ public class ResponseTest assertNotNull(set); ArrayList list = Collections.list(set); assertThat(list, containsInAnyOrder( - "name=value;Path=/path;Domain=domain;Secure;HttpOnly", - "name2=value2;Path=/path;Domain=domain" + "name=value; Path=/path; Domain=domain; Secure; HttpOnly", + "name2=value2; Path=/path; Domain=domain" )); //get rid of the cookies @@ -1088,15 +1089,17 @@ public class ResponseTest response.replaceCookie(new HttpCookie("Foo","value", "A", "/path")); response.replaceCookie(new HttpCookie("Foo","value")); - assertThat(Collections.list(response.getHttpFields().getValues("Set-Cookie")), - contains( - "Foo=value", - "Foo=value;Path=/path;Domain=A", - "Foo=value;Path=/path;Domain=B", - "Bar=value", - "Bar=value;Path=/left", - "Bar=value;Path=/right" - )); + String[] expected = new String[]{ + "Foo=value", + "Foo=value; Path=/path; Domain=A", + "Foo=value; Path=/path; Domain=B", + "Bar=value", + "Bar=value; Path=/left", + "Bar=value; Path=/right" + }; + + List actual = Collections.list(response.getHttpFields().getValues("Set-Cookie")); + assertThat("HttpCookie order", actual, hasItems(expected)); } @Test @@ -1252,8 +1255,8 @@ public class ResponseTest 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()); + 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()); @@ -1264,9 +1267,9 @@ public class ResponseTest 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()); + 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()); + 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 @@ -1275,9 +1278,9 @@ public class ResponseTest 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()); + 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()); + assertEquals("everything=value; Path=path; Expires=Thu, 01-Jan-1970 00:00:00 GMT; Max-Age=0; Secure; HttpOnly",e.nextElement()); assertFalse(e.hasMoreElements()); @@ -1287,9 +1290,9 @@ public class ResponseTest 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()); + 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()); + 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 @@ -1298,9 +1301,9 @@ public class ResponseTest 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()); + 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()); + 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 @@ -1309,8 +1312,8 @@ public class ResponseTest 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()); + 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[] = {