diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/CookieCompliance.java b/jetty-http/src/main/java/org/eclipse/jetty/http/CookieCompliance.java
index f8696d48f582..40853a6f0104 100644
--- a/jetty-http/src/main/java/org/eclipse/jetty/http/CookieCompliance.java
+++ b/jetty-http/src/main/java/org/eclipse/jetty/http/CookieCompliance.java
@@ -24,5 +24,6 @@
public enum CookieCompliance
{
RFC6265,
+ RFC6265_LEGACY, // Forgiving of bad quotes.
RFC2965
}
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 3753a8253064..60a9f5208db1 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
@@ -154,6 +154,13 @@ protected void parseFields()
// Handle quoted values for name or value
if (inQuoted)
{
+ boolean eol = c == 0 && i == hdr.length();
+ if (!eol && _compliance != CookieCompliance.RFC2965 && isRFC6265RejectedCharacter(inQuoted, c))
+ {
+ reject = true;
+ continue;
+ }
+
if (escaped)
{
escaped = false;
@@ -182,15 +189,24 @@ protected void parseFields()
continue;
case 0:
- // unterminated quote, let's ignore quotes
+ // unterminated quote
+ if (_compliance == CookieCompliance.RFC6265)
+ continue;
+ // let's ignore quotes
unquoted.setLength(0);
inQuoted = false;
i--;
continue;
+ case ';':
+ if (_compliance == CookieCompliance.RFC6265)
+ reject = true;
+ else
+ unquoted.append(c);
+ continue;
+
default:
unquoted.append(c);
- continue;
}
}
else
@@ -198,6 +214,13 @@ protected void parseFields()
// Handle name and value state machines
if (invalue)
{
+ boolean eol = c == 0 && i == hdr.length();
+ if (!eol && _compliance == CookieCompliance.RFC6265 && isRFC6265RejectedCharacter(inQuoted, c))
+ {
+ reject = true;
+ continue;
+ }
+
// parse the cookie-value
switch (c)
{
@@ -300,9 +323,20 @@ else if (tokenstart >= 0)
unquoted = new StringBuilder();
break;
}
+ else if (_compliance == CookieCompliance.RFC6265)
+ {
+ reject = true;
+ continue;
+ }
// fall through to default case
default:
+ if (_compliance == CookieCompliance.RFC6265 && quoted)
+ {
+ reject = true;
+ continue;
+ }
+
if (quoted)
{
// must have been a bad internal quote. let's fix as best we can
@@ -312,18 +346,12 @@ else if (tokenstart >= 0)
continue;
}
- if (_compliance == CookieCompliance.RFC6265)
- {
- if (isRFC6265RejectedCharacter(inQuoted, c))
- {
- reject = true;
- }
- }
+ if (_compliance == CookieCompliance.RFC6265_LEGACY && isRFC6265RejectedCharacter(inQuoted, c))
+ reject = true;
if (tokenstart < 0)
tokenstart = i;
tokenend = i;
- continue;
}
}
else
@@ -366,13 +394,8 @@ else if (tokenstart >= 0)
continue;
}
- if (_compliance == CookieCompliance.RFC6265)
- {
- if (isRFC6265RejectedCharacter(inQuoted, c))
- {
- reject = true;
- }
- }
+ if (_compliance != CookieCompliance.RFC2965 && isRFC6265RejectedCharacter(inQuoted, c))
+ reject = true;
if (tokenstart < 0)
tokenstart = i;
@@ -390,28 +413,37 @@ else if (tokenstart >= 0)
protected boolean isRFC6265RejectedCharacter(boolean inQuoted, char c)
{
- if (inQuoted)
+ // LEGACY test
+ if (_compliance == CookieCompliance.RFC6265_LEGACY)
{
- // We only reject if a Control Character is encountered
- if (Character.isISOControl(c))
+ if (inQuoted)
{
- return true;
+ // We only reject if a Control Character is encountered
+ if (Character.isISOControl(c))
+ return true;
}
- }
- else
- {
- /* From RFC6265 - Section 4.1.1 - Syntax
- * cookie-octet = %x21 / %x23-2B / %x2D-3A / %x3C-5B / %x5D-7E
- * ; US-ASCII characters excluding CTLs,
- * ; whitespace DQUOTE, comma, semicolon,
- * ; and backslash
- */
- return Character.isISOControl(c) || // control characters
- c > 127 || // 8-bit characters
- c == ',' || // comma
- c == ';'; // semicolon
+ else
+ {
+ return Character.isISOControl(c) || // control characters
+ c > 127 || // 8-bit characters
+ c == ',' || // comma
+ c == ';'; // semicolon
+ }
+ return false;
}
- return false;
+ /* From RFC6265 - Section 4.1.1 - Syntax
+ * cookie-octet = %x21 / %x23-2B / %x2D-3A / %x3C-5B / %x5D-7E
+ * ; US-ASCII characters excluding CTLs,
+ * ; whitespace DQUOTE, comma, semicolon,
+ * ; and backslash
+ *
+ * Note: DQUOTE and semicolon are used as separator by the parser,
+ * so we can consider them authorized.
+ */
+ return c > 127 || // 8-bit characters
+ Character.isISOControl(c) || // control characters
+ c == ',' || // comma
+ c == '\\'; // backslash
}
}
diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/CookieCutterLenientTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/CookieCutterLenientTest.java
index 3eceac6e3974..1d06c82a5ada 100644
--- a/jetty-server/src/test/java/org/eclipse/jetty/server/CookieCutterLenientTest.java
+++ b/jetty-server/src/test/java/org/eclipse/jetty/server/CookieCutterLenientTest.java
@@ -21,6 +21,7 @@
import java.util.stream.Stream;
import javax.servlet.http.Cookie;
+import org.eclipse.jetty.http.CookieCompliance;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
@@ -162,7 +163,7 @@ public static Stream<Arguments> data()
@MethodSource("data")
public void testLenientBehavior(String rawHeader, String expectedName, String expectedValue)
{
- CookieCutter cutter = new CookieCutter();
+ CookieCutter cutter = new CookieCutter(CookieCompliance.RFC6265_LEGACY);
cutter.addCookieField(rawHeader);
Cookie[] cookies = cutter.getCookies();
if (expectedName == null)
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 494ff03282e9..3e84ce6ce467 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
@@ -19,10 +19,13 @@
package org.eclipse.jetty.server;
import java.util.Arrays;
+import java.util.List;
import javax.servlet.http.Cookie;
import org.eclipse.jetty.http.CookieCompliance;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;
@@ -162,15 +165,13 @@ public void testRFC2965CookieSpoofingExample()
"$Version=\"1\"; session_id=\"1111\"; $Domain=\".cracker.edu\"";
Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC2965, rawCookie);
-
assertThat("Cookies.length", cookies.length, is(2));
assertCookie("Cookies[0]", cookies[0], "session_id", "1234", 1, null);
assertCookie("Cookies[1]", cookies[1], "session_id", "1111", 1, null);
cookies = parseCookieHeaders(CookieCompliance.RFC6265, rawCookie);
- assertThat("Cookies.length", cookies.length, is(2));
- assertCookie("Cookies[0]", cookies[0], "session_id", "1234\", $Version=\"1", 0, null);
- assertCookie("Cookies[1]", cookies[1], "session_id", "1111", 0, null);
+ assertThat("Cookies.length", cookies.length, is(1));
+ assertCookie("Cookies[0]", cookies[0], "session_id", "1111", 0, null);
}
/**
@@ -266,7 +267,7 @@ public void testExcessiveSemicolons()
{
char[] excessive = new char[65535];
Arrays.fill(excessive, ';');
- String rawCookie = "foo=bar; " + excessive + "; xyz=pdq";
+ String rawCookie = "foo=bar; " + new String(excessive) + "; xyz=pdq";
Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC6265, rawCookie);
@@ -274,4 +275,83 @@ public void testExcessiveSemicolons()
assertCookie("Cookies[0]", cookies[0], "foo", "bar", 0, null);
assertCookie("Cookies[1]", cookies[1], "xyz", "pdq", 0, null);
}
+
+ @ParameterizedTest
+ @MethodSource("rfc6265Cookies")
+ public void testRFC6265CookieParsing(Param param)
+ {
+ Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC6265, param.input);
+
+ assertThat("Cookies.length (" + dump(cookies) + ")", cookies.length, is(param.expected.size()));
+ for (int i = 0; i < cookies.length; i++)
+ {
+ Cookie cookie = cookies[i];
+ assertThat("Cookies[" + i + "] (" + dump(cookies) + ")", cookie.getName() + "=" + cookie.getValue(), is(param.expected.get(i)));
+ }
+ }
+
+ public static List<Param> rfc6265Cookies()
+ {
+ return Arrays.asList(
+ new Param("A=1; B=2; C=3", "A=1", "B=2", "C=3"),
+ new Param("A=\"1\"; B=2; C=3", "A=1", "B=2", "C=3"),
+ new Param("A=\"1\"; B=\"2\"; C=\"3\"", "A=1", "B=2", "C=3"),
+ new Param("A=1; B=2; C=\"3", "A=1", "B=2"),
+ new Param("A=1 ; B=2; C=3", "A=1", "B=2", "C=3"),
+ new Param("A= 1; B=2; C=3", "A=1", "B=2", "C=3"),
+ new Param("A=\"1; B=2\"; C=3", "C=3"),
+ new Param("A=\"1; B=2; C=3"),
+ new Param("A=\"1 B=2\"; C=3", "A=1 B=2", "C=3"),
+ new Param("A=\"\"1; B=2; C=3", "B=2", "C=3"),
+ new Param("A=\"\" ; B=2; C=3", "A=", "B=2", "C=3"),
+ new Param("A=\"\"; B=2; C=3", "A=", "B=2", "C=3"),
+ new Param("A=1\"\"; B=2; C=3", "B=2", "C=3"),
+ new Param("A=1\"; B=2; C=3", "B=2", "C=3"),
+ new Param("A=1\"1; B=2; C=3", "B=2", "C=3"),
+ new Param("A=\" 1\"; B=2; C=3", "A= 1", "B=2", "C=3"),
+ new Param("A=\"1 \"; B=2; C=3", "A=1 ", "B=2", "C=3"),
+ new Param("A=\" 1 \"; B=2; C=3", "A= 1 ", "B=2", "C=3"),
+ new Param("A=\" 1 1 \"; B=2; C=3", "A= 1 1 ", "B=2", "C=3"),
+ new Param("A=1,; B=2; C=3", "B=2", "C=3"),
+ new Param("A=\"1,\"; B=2; C=3", "B=2", "C=3"),
+ new Param("A=\\1; B=2; C=3", "B=2", "C=3"),
+ new Param("A=\"\\1\"; B=2; C=3", "B=2", "C=3"),
+ new Param("A=1\u0007; B=2; C=3", "B=2", "C=3"),
+ new Param("A=\"1\u0007\"; B=2; C=3", "B=2", "C=3"),
+ new Param("€"),
+ new Param("@={}"),
+ new Param("$X=Y; N=V", "N=V"),
+ new Param("N=V; $X=Y", "N=V")
+ );
+ }
+
+ private static String dump(Cookie[] cookies)
+ {
+ StringBuilder sb = new StringBuilder();
+ for (Cookie cookie : cookies)
+ {
+ sb.append("<").append(cookie.getName()).append(">=<").append(cookie.getValue()).append("> | ");
+ }
+ if (sb.length() > 0)
+ sb.delete(sb.length() - 2, sb.length() - 1);
+ return sb.toString();
+ }
+
+ private static class Param
+ {
+ private final String input;
+ private final List<String> expected;
+
+ public Param(String input, String... expected)
+ {
+ this.input = input;
+ this.expected = Arrays.asList(expected);
+ }
+
+ @Override
+ public String toString()
+ {
+ return input + " -> " + expected.toString();
+ }
+ }
}
diff --git a/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java b/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java
index f77f39b63018..a4e9125964fc 100644
--- a/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java
+++ b/jetty-server/src/test/java/org/eclipse/jetty/server/RequestTest.java
@@ -53,6 +53,7 @@
import javax.servlet.http.Part;
import org.eclipse.jetty.http.BadMessageException;
+import org.eclipse.jetty.http.CookieCompliance;
import org.eclipse.jetty.http.HttpCompliance;
import org.eclipse.jetty.http.HttpComplianceSection;
import org.eclipse.jetty.http.HttpStatus;
@@ -140,6 +141,7 @@ protected String formatAddrOrHost(String addr)
http.getHttpConfiguration().setRequestHeaderSize(512);
http.getHttpConfiguration().setResponseHeaderSize(512);
http.getHttpConfiguration().setOutputBufferSize(2048);
+ http.getHttpConfiguration().setRequestCookieCompliance(CookieCompliance.RFC6265_LEGACY);
http.getHttpConfiguration().addCustomizer(new ForwardedRequestCustomizer());
_connector = new LocalConnector(_server, http);
_server.addConnector(_connector);