From 7b8c2c1bf081c7f408e5da3371c8c3c9802f09fd Mon Sep 17 00:00:00 2001 From: Simone Bordet Date: Tue, 14 Feb 2023 22:38:24 +0100 Subject: [PATCH] Fix/jetty 10 9334 review cookie cutter (#9339) Cookie cleanup + New Cookie parser with clearer focus on RFC6265. + Better compliance modes for RFC2965 + Introduced CookieParser interface so that old and new parsers can coexist and be selected by compliance mode. --------- Signed-off-by: Simone Bordet Signed-off-by: Greg Wilkins Co-authored-by: Greg Wilkins Co-authored-by: Simone Bordet --- .../eclipse/jetty/http/CookieCompliance.java | 111 ++++- .../org/eclipse/jetty/http/CookieCutter.java | 142 ++++-- .../org/eclipse/jetty/http/CookieParser.java | 60 +++ .../org/eclipse/jetty/http/HttpCookie.java | 6 +- .../org/eclipse/jetty/http/HttpParser.java | 3 +- .../org/eclipse/jetty/http/HttpTokens.java | 48 ++ .../jetty/http/RFC6265CookieParser.java | 421 +++++++++++++++++ .../jetty/http/CookieCutterLenientTest.java | 26 +- .../eclipse/jetty/http/CookieCutterTest.java | 35 +- .../eclipse/jetty/http/CookieParserTest.java | 37 ++ .../jetty/http/RFC6265CookieParserTest.java | 422 ++++++++++++++++++ .../org/eclipse/jetty/server/Cookies.java | 12 +- .../org/eclipse/jetty/server/RequestTest.java | 1 + 13 files changed, 1226 insertions(+), 98 deletions(-) create mode 100644 jetty-http/src/main/java/org/eclipse/jetty/http/CookieParser.java create mode 100644 jetty-http/src/main/java/org/eclipse/jetty/http/RFC6265CookieParser.java create mode 100644 jetty-http/src/test/java/org/eclipse/jetty/http/CookieParserTest.java create mode 100644 jetty-http/src/test/java/org/eclipse/jetty/http/RFC6265CookieParserTest.java 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 2a084bfecb7..294beb74708 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 @@ -25,8 +25,10 @@ import org.slf4j.LoggerFactory; import static java.util.Collections.unmodifiableSet; import static java.util.EnumSet.allOf; +import static java.util.EnumSet.complementOf; import static java.util.EnumSet.copyOf; import static java.util.EnumSet.noneOf; +import static java.util.EnumSet.of; /** * The compliance mode for Cookie handling. @@ -38,14 +40,61 @@ public class CookieCompliance implements ComplianceViolation.Mode public enum Violation implements ComplianceViolation { /** - * Allow a comma as part of a cookie value + * A comma was found in a cookie value. + * + * @deprecated Use SPECIAL_CHARS_IN_QUOTES */ - COMMA_NOT_VALID_OCTET("https://tools.ietf.org/html/rfc6265#section-4.1.1", "Comma not valid as cookie-octet or separator"), + @Deprecated + COMMA_NOT_VALID_OCTET("https://tools.ietf.org/html/rfc6265#section-4.2.1", "Comma not valid as cookie-octet or separator"), /** - * Allow cookies to have $ prefixed reserved parameters + * A comma was found as separator between cookies. */ - RESERVED_NAMES_NOT_DOLLAR_PREFIXED("https://tools.ietf.org/html/rfc6265#section-4.1.1", "Reserved names no longer use '$' prefix"); + COMMA_SEPARATOR("https://www.rfc-editor.org/rfc/rfc2965.html", "Comma cookie separator"), + + /** + * @deprecated no replacement because was mistakenly considered a violation + */ + @Deprecated + RESERVED_NAMES_NOT_DOLLAR_PREFIXED("https://tools.ietf.org/html/rfc6265#section-4.2.1", "Reserved name no longer use '$' prefix"), + + /** + * Special characters were found in a quoted cookie value. + */ + SPECIAL_CHARS_IN_QUOTES("https://www.rfc-editor.org/rfc/rfc6265#section-4.2.1", "Special character cannot be quoted"), + + /** + * A backslash was found in a quoted cookie value. + */ + ESCAPE_IN_QUOTES("https://www.rfc-editor.org/rfc/rfc2616#section-2.2", "Escaped character in quotes"), + + /** + * Quotes are not balanced or are embedded in value. + */ + BAD_QUOTES("https://www.rfc-editor.org/rfc/rfc2616#section-2.2", "Bad quotes"), + + /** + * An invalid cookie was found, without a more specific violation. + * When this violation is not allowed, an exception is thrown. + */ + INVALID_COOKIES("https://tools.ietf.org/html/rfc6265", "Invalid cookie"), + + /** + * A cookie attribute was found. + * The attribute value is retained only if {@link #ATTRIBUTE_VALUES} is allowed. + */ + ATTRIBUTES("https://www.rfc-editor.org/rfc/rfc6265#section-4.2.1", "Cookie attribute"), + + /** + * A cookie attribute value was found and its value is retained. + * Allowing {@code ATTRIBUTE_VALUE} implies allowing {@link #ATTRIBUTES}. + */ + ATTRIBUTE_VALUES("https://www.rfc-editor.org/rfc/rfc6265#section-4.2.1", "Cookie attribute value"), + + /** + * Whitespace was found around the cookie name and/or around the cookie value. + */ + OPTIONAL_WHITE_SPACE("https://www.rfc-editor.org/rfc/rfc6265#section-5.2", "White space around name/value"); private final String url; private final String description; @@ -75,17 +124,58 @@ public class CookieCompliance implements ComplianceViolation.Mode } } + /** + *

A CookieCompliance mode that enforces RFC 6265 compliance, + * but allows:

+ *
    + *
  • {@link Violation#ATTRIBUTES}
    • + *
  • {@link Violation#INVALID_COOKIES}
    • + *
  • {@link Violation#OPTIONAL_WHITE_SPACE}
    • + *
+ */ + public static final CookieCompliance RFC6265 = new CookieCompliance("RFC6265", of( + Violation.ATTRIBUTES, Violation.INVALID_COOKIES, Violation.OPTIONAL_WHITE_SPACE) + ); + /** * A CookieCompliance mode that enforces RFC 6265 compliance. */ - public static final CookieCompliance RFC6265 = new CookieCompliance("RFC6265", noneOf(Violation.class)); + public static final CookieCompliance RFC6265_STRICT = new CookieCompliance("RFC6265_STRICT", noneOf(Violation.class)); + + /** + *

A CookieCompliance mode that enforces RFC 6265 compliance, + * but allows:

+ *
    + *
  • {@link Violation#BAD_QUOTES}
    • + *
  • {@link Violation#ESCAPE_IN_QUOTES}
    • + *
  • {@link Violation#INVALID_COOKIES}
    • + *
  • {@link Violation#OPTIONAL_WHITE_SPACE}
    • + *
  • {@link Violation#SPECIAL_CHARS_IN_QUOTES}
    • + *
+ */ + public static final CookieCompliance RFC6265_LEGACY = new CookieCompliance("RFC6265_LEGACY", of( + Violation.BAD_QUOTES, Violation.ESCAPE_IN_QUOTES, Violation.INVALID_COOKIES, Violation.OPTIONAL_WHITE_SPACE, Violation.SPECIAL_CHARS_IN_QUOTES) + ); /** * A CookieCompliance mode that allows RFC 2965 compliance. */ - public static final CookieCompliance RFC2965 = new CookieCompliance("RFC2965", allOf(Violation.class)); + public static final CookieCompliance RFC2965_LEGACY = new CookieCompliance("RFC2965_LEGACY", allOf(Violation.class)); - private static final List KNOWN_MODES = Arrays.asList(RFC6265, RFC2965); + /** + * A CookieCompliance mode that allows RFC 2965 compliance + * but does not allow: + *
    + *
  • {@link Violation#BAD_QUOTES}
    • + *
  • {@link Violation#COMMA_NOT_VALID_OCTET}
    • + *
  • {@link Violation#RESERVED_NAMES_NOT_DOLLAR_PREFIXED}
    • + *
+ */ + public static final CookieCompliance RFC2965 = new CookieCompliance("RFC2965", complementOf(of( + Violation.BAD_QUOTES, Violation.COMMA_NOT_VALID_OCTET, Violation.RESERVED_NAMES_NOT_DOLLAR_PREFIXED) + )); + + private static final List KNOWN_MODES = Arrays.asList(RFC6265, RFC6265_STRICT, RFC6265_LEGACY, RFC2965, RFC2965_LEGACY); private static final AtomicInteger __custom = new AtomicInteger(); public static CookieCompliance valueOf(String name) @@ -166,7 +256,7 @@ public class CookieCompliance implements ComplianceViolation.Mode private final String _name; private final Set _violations; - private CookieCompliance(String name, Set violations) + public CookieCompliance(String name, Set violations) { _name = name; _violations = unmodifiableSet(copyOf(Objects.requireNonNull(violations))); @@ -195,4 +285,9 @@ public class CookieCompliance implements ComplianceViolation.Mode { return _violations; } + + public boolean compliesWith(CookieCompliance mode) + { + return this == mode || getAllowed().containsAll(mode.getAllowed()); + } } diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/CookieCutter.java b/jetty-http/src/main/java/org/eclipse/jetty/http/CookieCutter.java index bdd08083c4f..e30d3f5302c 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/CookieCutter.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/CookieCutter.java @@ -13,32 +13,47 @@ package org.eclipse.jetty.http; +import java.util.Collections; import java.util.List; import java.util.Locale; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import static org.eclipse.jetty.http.CookieCompliance.Violation.BAD_QUOTES; import static org.eclipse.jetty.http.CookieCompliance.Violation.COMMA_NOT_VALID_OCTET; +import static org.eclipse.jetty.http.CookieCompliance.Violation.ESCAPE_IN_QUOTES; +import static org.eclipse.jetty.http.CookieCompliance.Violation.INVALID_COOKIES; import static org.eclipse.jetty.http.CookieCompliance.Violation.RESERVED_NAMES_NOT_DOLLAR_PREFIXED; +import static org.eclipse.jetty.http.CookieCompliance.Violation.SPECIAL_CHARS_IN_QUOTES; /** * Cookie parser */ -public abstract class CookieCutter +@Deprecated +public class CookieCutter implements CookieParser { protected static final Logger LOG = LoggerFactory.getLogger(CookieCutter.class); - protected final CookieCompliance _complianceMode; + private final CookieParser.Handler _handler; + private final CookieCompliance _complianceMode; private final ComplianceViolation.Listener _complianceListener; - protected CookieCutter(CookieCompliance compliance, ComplianceViolation.Listener complianceListener) + public CookieCutter(CookieParser.Handler handler, CookieCompliance compliance, ComplianceViolation.Listener complianceListener) { + _handler = handler; _complianceMode = compliance; _complianceListener = complianceListener; } - protected void parseFields(List rawFields) + @Override + public void parseField(String field) + { + parseFields(Collections.singletonList(field)); + } + + @Override + public void parseFields(List rawFields) { StringBuilder unquoted = null; @@ -93,19 +108,33 @@ public abstract class CookieCutter break; case '\\': + if (_complianceMode.allows(ESCAPE_IN_QUOTES)) + reportComplianceViolation(ESCAPE_IN_QUOTES, hdr); + else + reject = true; escaped = true; continue; case 0: // unterminated quote, let's ignore quotes + if (_complianceMode.allows(BAD_QUOTES)) + reportComplianceViolation(BAD_QUOTES, hdr); + else + reject = true; unquoted.setLength(0); inQuoted = false; i--; continue; default: + if (isRFC6265RejectedCharacter(c)) + { + if (_complianceMode.allows(SPECIAL_CHARS_IN_QUOTES)) + reportComplianceViolation(SPECIAL_CHARS_IN_QUOTES, hdr); + else + reject = true; + } unquoted.append(c); - continue; } } else @@ -128,6 +157,10 @@ public abstract class CookieCutter if (quoted) { // must have been a bad internal quote. let's fix as best we can + if (_complianceMode.allows(BAD_QUOTES)) + reportComplianceViolation(BAD_QUOTES, hdr); + else + reject = true; unquoted.append(hdr, tokenstart, i--); inQuoted = true; quoted = false; @@ -157,7 +190,7 @@ public abstract class CookieCutter try { - if (name.startsWith("$")) + if (name != null && name.startsWith("$")) { if (RESERVED_NAMES_NOT_DOLLAR_PREFIXED.isAllowedBy(_complianceMode)) { @@ -187,11 +220,18 @@ public abstract class CookieCutter // This is a new cookie, so add the completed last cookie if we have one if (cookieName != null) { - if (!reject) + if (reject) { - addCookie(cookieName, cookieValue, cookieDomain, cookiePath, cookieVersion, cookieComment); - reject = false; + if (_complianceMode.allows(INVALID_COOKIES)) + reportComplianceViolation(INVALID_COOKIES, hdr); + else + throw new IllegalArgumentException("Bad Cookie"); } + else + { + _handler.addCookie(cookieName, cookieValue, cookieVersion, cookieDomain, cookiePath, cookieComment); + } + reject = false; cookieDomain = null; cookiePath = null; cookieComment = null; @@ -227,24 +267,27 @@ public abstract class CookieCutter if (quoted) { // must have been a bad internal quote. let's fix as best we can + if (_complianceMode.allows(BAD_QUOTES)) + reportComplianceViolation(BAD_QUOTES, hdr); + else + reject = true; unquoted.append(hdr, tokenstart, i--); inQuoted = true; quoted = false; continue; } - if (_complianceMode == CookieCompliance.RFC6265) + if (isRFC6265RejectedCharacter(c)) { - if (isRFC6265RejectedCharacter(inQuoted, c)) - { + if (c < 128 && _complianceMode.allows(SPECIAL_CHARS_IN_QUOTES)) + reportComplianceViolation(SPECIAL_CHARS_IN_QUOTES, hdr); + else reject = true; - } } if (tokenstart < 0) tokenstart = i; tokenend = i; - continue; } } else @@ -287,68 +330,69 @@ public abstract class CookieCutter if (quoted) { // must have been a bad internal quote. let's fix as best we can + if (_complianceMode.allows(BAD_QUOTES)) + reportComplianceViolation(BAD_QUOTES, hdr); + else + reject = true; unquoted.append(hdr, tokenstart, i--); inQuoted = true; quoted = false; continue; } - if (_complianceMode == CookieCompliance.RFC6265) + if (isRFC6265RejectedCharacter(c)) { - if (isRFC6265RejectedCharacter(inQuoted, c)) - { + if (_complianceMode.allows(SPECIAL_CHARS_IN_QUOTES)) + reportComplianceViolation(SPECIAL_CHARS_IN_QUOTES, hdr); + else reject = true; - } } if (tokenstart < 0) tokenstart = i; tokenend = i; - continue; } } } } - if (cookieName != null && !reject) - addCookie(cookieName, cookieValue, cookieDomain, cookiePath, cookieVersion, cookieComment); + if (cookieName != null) + { + if (reject) + { + if (_complianceMode.allows(INVALID_COOKIES)) + reportComplianceViolation(INVALID_COOKIES, hdr); + else + throw new IllegalArgumentException("Bad Cookie"); + } + else + { + _handler.addCookie(cookieName, cookieValue, cookieVersion, cookieDomain, cookiePath, cookieComment); + } + } } } protected void reportComplianceViolation(CookieCompliance.Violation violation, String reason) { if (_complianceListener != null) - { _complianceListener.onComplianceViolation(_complianceMode, violation, reason); - } } - protected abstract void addCookie(String cookieName, String cookieValue, String cookieDomain, String cookiePath, int cookieVersion, String cookieComment); - - protected boolean isRFC6265RejectedCharacter(boolean inQuoted, char c) + protected boolean isRFC6265RejectedCharacter(char c) { - if (inQuoted) - { - // 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 - } - - 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 + */ + return Character.isISOControl(c) || // control characters + c > 127 || // 8-bit characters + c == ' ' || // whitespace + c == '"' || // DQUOTE + c == ',' || // comma + c == ';' || // semicolon + c == '\\'; // backslash } } diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/CookieParser.java b/jetty-http/src/main/java/org/eclipse/jetty/http/CookieParser.java new file mode 100644 index 00000000000..b214b02c828 --- /dev/null +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/CookieParser.java @@ -0,0 +1,60 @@ +// +// ======================================================================== +// Copyright (c) 1995-2022 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License v. 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 +// which is available at https://www.apache.org/licenses/LICENSE-2.0. +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== +// + +package org.eclipse.jetty.http; + +import java.util.List; + +import static org.eclipse.jetty.http.CookieCompliance.Violation.BAD_QUOTES; +import static org.eclipse.jetty.http.CookieCompliance.Violation.Listener; + +/** + *

Cookie parser.

+ *

An interface for variations of a cookie parser.

+ */ +public interface CookieParser +{ + /** + *

A factory method to create a new parser suitable for the compliance mode.

+ * @param compliance The compliance mode to use for parsing. + * @param complianceListener A listener for compliance violations or null. + * @return A CookieParser instance. + */ + static CookieParser newParser(Handler handler, CookieCompliance compliance, Listener complianceListener) + { + // The RFC6265CookieParser is primarily a RFC6265 parser, but it can handle most + // defined "violations" so that it effectively becomes a RFC2965 parser. However, it + // cannot forgive bad quotes. Thus, we use the legacy CookieCutter parser only if + // the compliance mode requires BAD QUOTES. + if (compliance.allows(BAD_QUOTES)) + return new CookieCutter(handler, compliance, complianceListener); + return new RFC6265CookieParser(handler, compliance, complianceListener); + } + + void parseField(String field); + + default void parseFields(List rawFields) + { + // For each cookie field + for (String field : rawFields) + parseField(field); + } + + /** + * The handler of parsed cookies. + */ + interface Handler + { + void addCookie(String name, String value, int version, String domain, String path, String comment); + } +} 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 e282e05fb82..edc7f89df6b 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 @@ -281,11 +281,9 @@ public class HttpCookie public String getSetCookie(CookieCompliance compliance) { - if (compliance == CookieCompliance.RFC6265) + if (CookieCompliance.RFC6265.compliesWith(compliance)) return getRFC6265SetCookie(); - if (compliance == CookieCompliance.RFC2965) - return getRFC2965SetCookie(); - throw new IllegalStateException(); + return getRFC2965SetCookie(); } public String getRFC2965SetCookie() diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java index 7e802c62824..a7265c8173a 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpParser.java @@ -441,8 +441,7 @@ public class HttpParser private HttpTokens.Token next(ByteBuffer buffer) { byte ch = buffer.get(); - - HttpTokens.Token t = HttpTokens.TOKENS[0xff & ch]; + HttpTokens.Token t = HttpTokens.getToken(ch); switch (t.getType()) { diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpTokens.java b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpTokens.java index bbc56688372..5ccb16e2fc7 100644 --- a/jetty-http/src/main/java/org/eclipse/jetty/http/HttpTokens.java +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/HttpTokens.java @@ -47,6 +47,16 @@ public class HttpTokens OTEXT // Obsolete text } + public static Token getToken(byte b) + { + return TOKENS[0xFF & b]; + } + + public static Token getToken(char c) + { + return c <= 0xFF ? TOKENS[c] : null; + } + public static class Token { private final Type _type; @@ -54,6 +64,10 @@ public class HttpTokens private final char _c; private final int _x; + private final boolean _rfc2616Token; + + private final boolean _rfc6265CookieOctet; + private Token(byte b, Type type) { _type = type; @@ -61,6 +75,30 @@ public class HttpTokens _c = (char)(0xff & b); char lc = (_c >= 'A' & _c <= 'Z') ? ((char)(_c - 'A' + 'a')) : _c; _x = (_type == Type.DIGIT || _type == Type.ALPHA && lc >= 'a' && lc <= 'f') ? TypeUtil.convertHexDigit(b) : -1; + + // token = 1* + // separators = "(" | ")" | "<" | ">" | "@" + // | "," | ";" | ":" | "\" | <"> + // | "/" | "[" | "]" | "?" | "=" + // | "{" | "}" | SP | HT + // CTL = + _rfc2616Token = b >= 32 && b < 127 && + b != '(' && b != ')' && b != '<' && b != '>' && b != '@' && + b != ',' && b != ';' && b != ':' && b != '\\' && b != '"' && + b != '/' && b != '[' && b != ']' && b != '?' && b != '=' && + b != '{' && b != '}' && b != ' '; + + // cookie-octet = %x21 / %x23-2B / %x2D-3A / %x3C-5B / %x5D-7E + // ; US-ASCII characters excluding CTLs, + // ; whitespace DQUOTE, comma, semicolon, + // ; and backslash + _rfc6265CookieOctet = + b == 0x21 || + b >= 0x23 && b <= 0x2B || + b >= 0x2D && b <= 0x3A || + b >= 0x3C && b <= 0x5B || + b >= 0x5D && b <= 0x7E; } public Type getType() @@ -83,6 +121,16 @@ public class HttpTokens return _x >= 0; } + public boolean isRfc2616Token() + { + return _rfc2616Token; + } + + public boolean isRfc6265CookieOctet() + { + return _rfc6265CookieOctet; + } + public int getHexDigit() { return _x; diff --git a/jetty-http/src/main/java/org/eclipse/jetty/http/RFC6265CookieParser.java b/jetty-http/src/main/java/org/eclipse/jetty/http/RFC6265CookieParser.java new file mode 100644 index 00000000000..6015abb43dc --- /dev/null +++ b/jetty-http/src/main/java/org/eclipse/jetty/http/RFC6265CookieParser.java @@ -0,0 +1,421 @@ +// +// ======================================================================== +// Copyright (c) 1995-2022 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License v. 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 +// which is available at https://www.apache.org/licenses/LICENSE-2.0. +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== +// + +package org.eclipse.jetty.http; + +import java.util.Locale; + +import org.eclipse.jetty.util.StringUtil; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import static org.eclipse.jetty.http.CookieCompliance.Violation.ATTRIBUTES; +import static org.eclipse.jetty.http.CookieCompliance.Violation.ATTRIBUTE_VALUES; +import static org.eclipse.jetty.http.CookieCompliance.Violation.COMMA_NOT_VALID_OCTET; +import static org.eclipse.jetty.http.CookieCompliance.Violation.COMMA_SEPARATOR; +import static org.eclipse.jetty.http.CookieCompliance.Violation.ESCAPE_IN_QUOTES; +import static org.eclipse.jetty.http.CookieCompliance.Violation.INVALID_COOKIES; +import static org.eclipse.jetty.http.CookieCompliance.Violation.OPTIONAL_WHITE_SPACE; +import static org.eclipse.jetty.http.CookieCompliance.Violation.SPECIAL_CHARS_IN_QUOTES; + +/** + * Cookie parser + */ +public class RFC6265CookieParser implements CookieParser +{ + protected static final Logger LOG = LoggerFactory.getLogger(RFC6265CookieParser.class); + + private final CookieParser.Handler _handler; + private final CookieCompliance _complianceMode; + private final ComplianceViolation.Listener _complianceListener; + + protected RFC6265CookieParser(CookieParser.Handler handler, CookieCompliance compliance, ComplianceViolation.Listener complianceListener) + { + _handler = handler; + _complianceMode = compliance; + _complianceListener = complianceListener; + } + + private enum State + { + START, + IN_NAME, + AFTER_NAME, + VALUE, + IN_VALUE, + IN_QUOTED_VALUE, + ESCAPED_VALUE, + AFTER_QUOTED_VALUE, + END, + INVALID_COOKIE + } + + @Override + public void parseField(String field) + { + State state = State.START; + + String attributeName = null; + String value = null; + String cookieName = null; + String cookieValue = null; + String cookiePath = null; + String cookieDomain = null; + String cookieComment = null; + int cookieVersion = 0; + boolean cookieInvalid = false; + + int length = field.length(); + StringBuilder string = new StringBuilder(); + for (int i = 0; i <= length; i++) + { + char c = i == length ? ';' : field.charAt(i); + HttpTokens.Token token = HttpTokens.getToken(c); + + if (token == null) + { + if (!_complianceMode.allows(INVALID_COOKIES)) + throw new IllegalArgumentException("Invalid Cookie character"); + state = State.INVALID_COOKIE; + continue; + } + + switch (state) + { + case START: + if (c == ' ' || c == '\t' || c == ';') + continue; + + string.setLength(0); + + if (token.isRfc2616Token()) + { + if (!StringUtil.isBlank(cookieName) && c != '$') + { + _handler.addCookie(cookieName, cookieValue, cookieVersion, cookieDomain, cookiePath, cookieComment); + cookieName = null; + cookieValue = null; + cookieDomain = null; + cookiePath = null; + cookieComment = null; + } + + string.append(c); + state = State.IN_NAME; + } + else if (_complianceMode.allows(INVALID_COOKIES)) + { + reportComplianceViolation(INVALID_COOKIES, field); + state = State.INVALID_COOKIE; + } + else + { + throw new IllegalArgumentException("Bad Cookie name"); + } + + break; + + case IN_NAME: + if (c == '=') + { + if (string.charAt(0) == '$') + attributeName = string.toString(); + else + cookieName = string.toString(); + state = State.VALUE; + continue; + } + + if ((c == ' ' || c == '\t') && _complianceMode.allows(OPTIONAL_WHITE_SPACE)) + { + reportComplianceViolation(OPTIONAL_WHITE_SPACE, field); + if (string.charAt(0) == '$') + attributeName = string.toString(); + else + cookieName = string.toString(); + state = State.AFTER_NAME; + continue; + } + + if (token.isRfc2616Token()) + { + string.append(c); + } + else if (_complianceMode.allows(INVALID_COOKIES)) + { + reportComplianceViolation(INVALID_COOKIES, field); + state = c == ';' ? State.START : State.INVALID_COOKIE; + } + else + { + throw new IllegalArgumentException("Bad Cookie name"); + } + break; + + case AFTER_NAME: + if (c == '=') + { + state = State.VALUE; + continue; + } + if (c == ';' || c == ',') + { + state = State.START; + continue; + } + + if (_complianceMode.allows(INVALID_COOKIES)) + { + reportComplianceViolation(INVALID_COOKIES, field); + state = State.INVALID_COOKIE; + } + else + { + throw new IllegalArgumentException("Bad Cookie"); + } + break; + + case VALUE: + if (c == ' ' && _complianceMode.allows(OPTIONAL_WHITE_SPACE)) + { + reportComplianceViolation(OPTIONAL_WHITE_SPACE, field); + continue; + } + + string.setLength(0); + if (c == '"') + { + state = State.IN_QUOTED_VALUE; + } + else if (c == ';') + { + value = ""; + i--; + state = State.END; + } + else if (token.isRfc6265CookieOctet()) + { + string.append(c); + state = State.IN_VALUE; + } + else if (_complianceMode.allows(INVALID_COOKIES)) + { + reportComplianceViolation(INVALID_COOKIES, field); + state = State.INVALID_COOKIE; + } + else + { + throw new IllegalArgumentException("Bad Cookie value"); + } + break; + + case IN_VALUE: + if (c == ';' || c == ',' || c == ' ' || c == '\t') + { + value = string.toString(); + i--; + state = State.END; + } + else if (token.isRfc6265CookieOctet()) + { + string.append(c); + } + else if (_complianceMode.allows(INVALID_COOKIES)) + { + reportComplianceViolation(INVALID_COOKIES, field); + state = State.INVALID_COOKIE; + } + else + { + throw new IllegalArgumentException("Bad Cookie value"); + } + break; + + case IN_QUOTED_VALUE: + if (c == '"') + { + value = string.toString(); + state = State.AFTER_QUOTED_VALUE; + } + else if (c == '\\' && _complianceMode.allows(ESCAPE_IN_QUOTES)) + { + state = State.ESCAPED_VALUE; + } + else if (token.isRfc6265CookieOctet()) + { + string.append(c); + } + else if (_complianceMode.allows(SPECIAL_CHARS_IN_QUOTES)) + { + reportComplianceViolation(SPECIAL_CHARS_IN_QUOTES, field); + string.append(c); + } + else if (c == ',' && _complianceMode.allows(COMMA_NOT_VALID_OCTET)) + { + reportComplianceViolation(COMMA_NOT_VALID_OCTET, field); + string.append(c); + } + else if (_complianceMode.allows(INVALID_COOKIES)) + { + string.append(c); + if (!cookieInvalid) + { + cookieInvalid = true; + reportComplianceViolation(INVALID_COOKIES, field); + } + // Try to find the closing double quote by staying in the current state. + } + else + { + throw new IllegalArgumentException("Bad Cookie quoted value"); + } + break; + + case ESCAPED_VALUE: + string.append(c); + state = State.IN_QUOTED_VALUE; + break; + + case AFTER_QUOTED_VALUE: + if (c == ';' || c == ',' || c == ' ' || c == '\t') + { + i--; + state = cookieInvalid ? State.INVALID_COOKIE : State.END; + } + else if (_complianceMode.allows(INVALID_COOKIES)) + { + reportComplianceViolation(INVALID_COOKIES, field); + state = State.INVALID_COOKIE; + } + else + { + throw new IllegalArgumentException("Bad Cookie quoted value"); + } + break; + + case END: + if (c == ';') + { + state = State.START; + } + else if (c == ',') + { + if (_complianceMode.allows(COMMA_SEPARATOR)) + { + reportComplianceViolation(COMMA_SEPARATOR, field); + state = State.START; + } + else if (_complianceMode.allows(INVALID_COOKIES)) + { + reportComplianceViolation(INVALID_COOKIES, field); + state = State.INVALID_COOKIE; + continue; + } + else + { + throw new IllegalStateException("Comma cookie separator"); + } + } + else if ((c == ' ' || c == '\t') && _complianceMode.allows(OPTIONAL_WHITE_SPACE)) + { + reportComplianceViolation(OPTIONAL_WHITE_SPACE, field); + continue; + } + + boolean knownAttribute = true; + if (StringUtil.isBlank(attributeName)) + { + cookieValue = value; + } + else + { + // We have an attribute. + if (_complianceMode.allows(ATTRIBUTE_VALUES)) + { + reportComplianceViolation(ATTRIBUTES, field); + switch (attributeName.toLowerCase(Locale.ENGLISH)) + { + case "$path": + cookiePath = value; + break; + case "$domain": + cookieDomain = value; + break; + case "$port": + cookieComment = "$port=" + value; + break; + case "$version": + cookieVersion = Integer.parseInt(value); + break; + default: + knownAttribute = false; + break; + } + } + else if (_complianceMode.allows(ATTRIBUTES)) + { + reportComplianceViolation(ATTRIBUTES, field); + } + else if (_complianceMode.allows(INVALID_COOKIES)) + { + reportComplianceViolation(INVALID_COOKIES, field); + state = State.INVALID_COOKIE; + continue; + } + else + { + throw new IllegalArgumentException("Invalid Cookie with attributes"); + } + attributeName = null; + } + value = null; + + if (!knownAttribute) + { + if (!_complianceMode.allows(INVALID_COOKIES)) + throw new IllegalArgumentException("Invalid Cookie attribute"); + reportComplianceViolation(INVALID_COOKIES, field); + state = State.INVALID_COOKIE; + continue; + } + + if (state == State.END) + throw new IllegalStateException("Invalid cookie"); + break; + + case INVALID_COOKIE: + attributeName = null; + value = null; + cookieName = null; + cookieValue = null; + cookiePath = null; + cookieDomain = null; + cookieComment = null; + cookieInvalid = false; + if (c == ';') + state = State.START; + break; + } + } + + if (!cookieInvalid && !StringUtil.isBlank(cookieName)) + _handler.addCookie(cookieName, cookieValue, cookieVersion, cookieDomain, cookiePath, cookieComment); + } + + protected void reportComplianceViolation(CookieCompliance.Violation violation, String reason) + { + if (_complianceListener != null) + _complianceListener.onComplianceViolation(_complianceMode, violation, reason); + } + +} diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/CookieCutterLenientTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/CookieCutterLenientTest.java index aeb60c6f40d..b19ac53b08f 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/CookieCutterLenientTest.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/CookieCutterLenientTest.java @@ -14,7 +14,6 @@ package org.eclipse.jetty.http; import java.util.ArrayList; -import java.util.Collections; import java.util.List; import java.util.stream.Stream; @@ -64,6 +63,8 @@ public class CookieCutterLenientTest // lenient with spaces and EOF Arguments.of("abc=", "abc", ""), + Arguments.of("abc= ", "abc", ""), + Arguments.of("abc= x", "abc", "x"), Arguments.of("abc = ", "abc", ""), Arguments.of("abc = ;", "abc", ""), Arguments.of("abc = ; ", "abc", ""), @@ -173,28 +174,27 @@ public class CookieCutterLenientTest } } - class TestCutter extends CookieCutter + static class TestCutter implements CookieParser.Handler { + CookieCutter cutter; List names = new ArrayList<>(); List values = new ArrayList<>(); protected TestCutter() { - super(CookieCompliance.RFC6265, null); - } - - @Override - protected void addCookie(String cookieName, String cookieValue, String cookieDomain, String cookiePath, int cookieVersion, String cookieComment) - { - names.add(cookieName); - values.add(cookieValue); + cutter = new CookieCutter(this, CookieCompliance.RFC6265_LEGACY, null); } public void parseField(String field) { - super.parseFields(Collections.singletonList(field)); + cutter.parseField(field); + } + + @Override + public void addCookie(String cookieName, String cookieValue, int cookieVersion, String cookieDomain, String cookiePath, String cookieComment) + { + names.add(cookieName); + values.add(cookieValue); } } - - ; } diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/CookieCutterTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/CookieCutterTest.java index d2c41e28099..1b760c5023c 100644 --- a/jetty-http/src/test/java/org/eclipse/jetty/http/CookieCutterTest.java +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/CookieCutterTest.java @@ -54,7 +54,7 @@ public class CookieCutterTest { String rawCookie = "$Version=\"1\"; Customer=\"WILE_E_COYOTE\"; $Path=\"/acme\""; - Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC2965, rawCookie); + Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC2965_LEGACY, rawCookie); assertThat("Cookies.length", cookies.length, is(1)); assertCookie("Cookies[0]", cookies[0], "Customer", "WILE_E_COYOTE", 1, "/acme"); @@ -70,7 +70,7 @@ public class CookieCutterTest "Customer=\"WILE_E_COYOTE\"; $Path=\"/acme\"; " + "Part_Number=\"Rocket_Launcher_0001\"; $Path=\"/acme\""; - Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC2965, rawCookie); + Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC2965_LEGACY, rawCookie); assertThat("Cookies.length", cookies.length, is(2)); assertCookie("Cookies[0]", cookies[0], "Customer", "WILE_E_COYOTE", 1, "/acme"); @@ -88,7 +88,7 @@ public class CookieCutterTest "Part_Number=\"Rocket_Launcher_0001\"; $Path=\"/acme\"; " + "Shipping=\"FedEx\"; $Path=\"/acme\""; - Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC2965, rawCookie); + Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC2965_LEGACY, rawCookie); assertThat("Cookies.length", cookies.length, is(3)); assertCookie("Cookies[0]", cookies[0], "Customer", "WILE_E_COYOTE", 1, "/acme"); @@ -106,7 +106,7 @@ public class CookieCutterTest "Part_Number=\"Riding_Rocket_0023\"; $Path=\"/acme/ammo\"; " + "Part_Number=\"Rocket_Launcher_0001\"; $Path=\"/acme\""; - Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC2965, rawCookie); + Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC2965_LEGACY, rawCookie); assertThat("Cookies.length", cookies.length, is(2)); assertCookie("Cookies[0]", cookies[0], "Part_Number", "Riding_Rocket_0023", 1, "/acme/ammo"); @@ -123,7 +123,7 @@ public class CookieCutterTest "session_id=\"1234\"; " + "session_id=\"1111\"; $Domain=\".cracker.edu\""; - Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC2965, rawCookie); + Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC2965_LEGACY, rawCookie); assertThat("Cookies.length", cookies.length, is(2)); assertCookie("Cookies[0]", cookies[0], "session_id", "1234", 1, null); @@ -139,13 +139,13 @@ public class CookieCutterTest String rawCookie = "$Version=\"1\"; session_id=\"1234\", " + "$Version=\"1\"; session_id=\"1111\"; $Domain=\".cracker.edu\""; - Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC2965, rawCookie); + Cookie[] cookies /* = parseCookieHeaders(CookieCompliance.RFC2965_LEGACY, 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); + cookies */ = parseCookieHeaders(CookieCompliance.RFC6265_LEGACY, 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); @@ -159,7 +159,7 @@ public class CookieCutterTest { String rawCookie = "SID=31d4d96e407aad42"; - Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC6265, rawCookie); + Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC6265_LEGACY, rawCookie); assertThat("Cookies.length", cookies.length, is(1)); assertCookie("Cookies[0]", cookies[0], "SID", "31d4d96e407aad42", 0, null); @@ -173,7 +173,7 @@ public class CookieCutterTest { String rawCookie = "SID=31d4d96e407aad42; lang=en-US"; - Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC6265, rawCookie); + Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC6265_LEGACY, rawCookie); assertThat("Cookies.length", cookies.length, is(2)); assertCookie("Cookies[0]", cookies[0], "SID", "31d4d96e407aad42", 0, null); @@ -188,7 +188,7 @@ public class CookieCutterTest { String rawCookie = "key=value"; - Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC6265, rawCookie); + Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC6265_LEGACY, rawCookie); assertThat("Cookies.length", cookies.length, is(1)); assertCookie("Cookies[0]", cookies[0], "key", "value", 0, null); @@ -202,7 +202,7 @@ public class CookieCutterTest { String rawCookie = "$key=value"; - Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC6265, rawCookie); + Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC6265_LEGACY, rawCookie); assertThat("Cookies.length", cookies.length, is(0)); } @@ -214,7 +214,7 @@ public class CookieCutterTest // The first cookie "testcookie" should be ignored, per RFC6265, as it's missing the "=" sign. - Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC6265, rawCookie); + Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC6265_LEGACY, rawCookie); assertThat("Cookies.length", cookies.length, is(2)); assertCookie("Cookies[0]", cookies[0], "server.id", "abcd", 0, null); @@ -228,7 +228,7 @@ public class CookieCutterTest Arrays.fill(excessive, ';'); String rawCookie = "foo=bar; " + excessive + "; xyz=pdq"; - Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC6265, rawCookie); + Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC6265_LEGACY, rawCookie); assertThat("Cookies.length", cookies.length, is(2)); assertCookie("Cookies[0]", cookies[0], "foo", "bar", 0, null); @@ -285,24 +285,25 @@ public class CookieCutterTest } } - class TestCutter extends CookieCutter + class TestCutter implements CookieParser.Handler { List cookies = new ArrayList<>(); + CookieCutter cutter; public TestCutter(CookieCompliance compliance, ComplianceViolation.Listener complianceListener) { - super(compliance, complianceListener); + cutter = new CookieCutter(this, compliance, complianceListener); } @Override - protected void addCookie(String cookieName, String cookieValue, String cookieDomain, String cookiePath, int cookieVersion, String cookieComment) + public void addCookie(String cookieName, String cookieValue, int cookieVersion, String cookieDomain, String cookiePath, String cookieComment) { cookies.add(new Cookie(cookieName, cookieValue, cookieDomain, cookiePath, cookieVersion, cookieComment)); } public void parseFields(String... fields) { - super.parseFields(Arrays.asList(fields)); + cutter.parseFields(Arrays.asList(fields)); } } } diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/CookieParserTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/CookieParserTest.java new file mode 100644 index 00000000000..e996cf91e92 --- /dev/null +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/CookieParserTest.java @@ -0,0 +1,37 @@ +// +// ======================================================================== +// Copyright (c) 1995-2022 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License v. 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 +// which is available at https://www.apache.org/licenses/LICENSE-2.0. +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== +// + +package org.eclipse.jetty.http; + +import java.util.EnumSet; + +import org.junit.jupiter.api.Test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.instanceOf; + +public class CookieParserTest +{ + @Test + public void testNewCookieParser() + { + assertThat(CookieParser.newParser(null, CookieCompliance.RFC2965_LEGACY, null), instanceOf(CookieCutter.class)); + assertThat(CookieParser.newParser(null, CookieCompliance.RFC6265_LEGACY, null), instanceOf(CookieCutter.class)); + assertThat(CookieParser.newParser(null, new CookieCompliance("custom", EnumSet.of(CookieCompliance.Violation.COMMA_SEPARATOR, CookieCompliance.Violation.BAD_QUOTES)), null), instanceOf(CookieCutter.class)); + + assertThat(CookieParser.newParser(null, CookieCompliance.RFC2965, null), instanceOf(RFC6265CookieParser.class)); + assertThat(CookieParser.newParser(null, CookieCompliance.RFC6265, null), instanceOf(RFC6265CookieParser.class)); + assertThat(CookieParser.newParser(null, CookieCompliance.RFC6265_STRICT, null), instanceOf(RFC6265CookieParser.class)); + assertThat(CookieParser.newParser(null, new CookieCompliance("custom", EnumSet.of(CookieCompliance.Violation.COMMA_SEPARATOR)), null), instanceOf(RFC6265CookieParser.class)); + } +} diff --git a/jetty-http/src/test/java/org/eclipse/jetty/http/RFC6265CookieParserTest.java b/jetty-http/src/test/java/org/eclipse/jetty/http/RFC6265CookieParserTest.java new file mode 100644 index 00000000000..ed56e3e0f86 --- /dev/null +++ b/jetty-http/src/test/java/org/eclipse/jetty/http/RFC6265CookieParserTest.java @@ -0,0 +1,422 @@ +// +// ======================================================================== +// Copyright (c) 1995-2022 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License v. 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 +// which is available at https://www.apache.org/licenses/LICENSE-2.0. +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== +// + +package org.eclipse.jetty.http; + +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; + +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; +import static org.junit.jupiter.api.Assertions.assertThrows; + +public class RFC6265CookieParserTest +{ + /** + * Example from RFC2109 and RFC2965 + */ + @Test + public void testRFC2965Single() + { + String rawCookie = "$Version=\"1\"; Customer=\"WILE_E_COYOTE\"; $Path=\"/acme\""; + + // Test with RFC 2965. + TestCookieParser parser = new TestCookieParser(CookieCompliance.RFC2965_LEGACY); + List cookies = parser.parseFields(rawCookie); + + assertThat("Cookies.length", cookies.size(), is(1)); + assertCookie("Cookies[0]", cookies.get(0), "Customer", "WILE_E_COYOTE", 1, "/acme"); + // There are 2 attributes, so 2 violations. + assertThat(parser.violations.size(), is(2)); + + // Same test with RFC 6265. + parser = new TestCookieParser(CookieCompliance.RFC6265); + cookies = parser.parseFields(rawCookie); + + assertThat("Cookies.length", cookies.size(), is(1)); + assertCookie("Cookies[0]", cookies.get(0), "Customer", "WILE_E_COYOTE", 0, null); + // There are 2 attributes, so 2 violations. + assertThat(parser.violations.size(), is(2)); + + // Same test with RFC 6265 strict should throw. + TestCookieParser strictParser = new TestCookieParser(CookieCompliance.RFC6265_STRICT); + assertThrows(IllegalArgumentException.class, () -> strictParser.parseFields(rawCookie)); + } + + /** + * Example from RFC2109 and RFC2965 + */ + @Test + public void testRFC2965Double() + { + String rawCookie = "$Version=\"1\"; " + + "Customer=\"WILE_E_COYOTE\"; $Path=\"/acme\"; " + + "Part_Number=\"Rocket_Launcher_0001\"; $Path=\"/acme\""; + + Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC2965_LEGACY, rawCookie); + + assertThat("Cookies.length", cookies.length, is(2)); + assertCookie("Cookies[0]", cookies[0], "Customer", "WILE_E_COYOTE", 1, "/acme"); + assertCookie("Cookies[1]", cookies[1], "Part_Number", "Rocket_Launcher_0001", 1, "/acme"); + } + + /** + * Example from RFC2109 and RFC2965 + */ + @Test + public void testRFCTriple() + { + String rawCookie = "$Version=\"1\"; " + + "Customer=\"WILE_E_COYOTE\"; $Path=\"/acme\"; " + + "Part_Number=\"Rocket_Launcher_0001\"; $Path=\"/acme\"; " + + "Shipping=\"FedEx\"; $Path=\"/acme\""; + + Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC2965_LEGACY, rawCookie); + + assertThat("Cookies.length", cookies.length, is(3)); + assertCookie("Cookies[0]", cookies[0], "Customer", "WILE_E_COYOTE", 1, "/acme"); + assertCookie("Cookies[1]", cookies[1], "Part_Number", "Rocket_Launcher_0001", 1, "/acme"); + assertCookie("Cookies[2]", cookies[2], "Shipping", "FedEx", 1, "/acme"); + } + + /** + * Example from RFC2109 and RFC2965 + */ + @Test + public void testRFCPathExample() + { + String rawCookie = "$Version=\"1\"; " + + "Part_Number=\"Riding_Rocket_0023\"; $Path=\"/acme/ammo\"; " + + "Part_Number=\"Rocket_Launcher_0001\"; $Path=\"/acme\""; + + Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC2965_LEGACY, rawCookie); + + assertThat("Cookies.length", cookies.length, is(2)); + assertCookie("Cookies[0]", cookies[0], "Part_Number", "Riding_Rocket_0023", 1, "/acme/ammo"); + assertCookie("Cookies[1]", cookies[1], "Part_Number", "Rocket_Launcher_0001", 1, "/acme"); + } + + /** + * Example from RFC2109 + */ + @Test + public void testRFC2109CookieSpoofingExample() + { + String rawCookie = "$Version=\"1\"; " + + "session_id=\"1234\"; " + + "session_id=\"1111\"; $Domain=\".cracker.edu\""; + + Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC2965_LEGACY, 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); + } + + /** + * Example from RFC2965 + */ + @Test + public void testRFC2965CookieSpoofingExample() + { + String rawCookie = "$Version=\"1\"; session_id=\"1234\", " + + "$Version=\"1\"; session_id=\"1111\"; $Domain=\".cracker.edu\""; + + Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC2965_LEGACY, 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(1)); + assertCookie("Cookies[1]", cookies[0], "session_id", "1111", 0, null); + } + + /** + * Example from RFC6265 + */ + @Test + public void testRFC6265SidExample() + { + String rawCookie = "SID=31d4d96e407aad42"; + + Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC6265, rawCookie); + + assertThat("Cookies.length", cookies.length, is(1)); + assertCookie("Cookies[0]", cookies[0], "SID", "31d4d96e407aad42", 0, null); + } + + /** + * Example from RFC6265 + */ + @Test + public void testRFC6265SidLangExample() + { + 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 + */ + @Test + public void testKeyValue() + { + String rawCookie = "key=value"; + + Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC6265, rawCookie); + + assertThat("Cookies.length", cookies.length, is(1)); + assertCookie("Cookies[0]", cookies[0], "key", "value", 0, null); + } + + /** + * Basic name=value, following RFC6265 rules + */ + @Test + public void testDollarName() + { + String rawCookie = "$key=value"; + + Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC6265, rawCookie); + + assertThat("Cookies.length", cookies.length, is(0)); + } + + @Test + public void testMultipleCookies() + { + String rawCookie = "testcookie; server.id=abcd; server.detail=cfg"; + + // The first cookie "testcookie" should be ignored, per RFC6265, as it's missing the "=" sign. + + Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC6265, rawCookie); + + assertThat("Cookies.length", cookies.length, is(2)); + assertCookie("Cookies[0]", cookies[0], "server.id", "abcd", 0, null); + assertCookie("Cookies[1]", cookies[1], "server.detail", "cfg", 0, null); + } + + @Test + public void testExcessiveSemicolons() + { + char[] excessive = new char[65535]; + Arrays.fill(excessive, ';'); + String rawCookie = "foo=bar; " + new String(excessive) + "; xyz=pdq"; + + Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC6265, rawCookie); + + assertThat("Cookies.length", cookies.length, is(2)); + assertCookie("Cookies[0]", cookies[0], "foo", "bar", 0, null); + assertCookie("Cookies[1]", cookies[1], "xyz", "pdq", 0, null); + } + + @Test + public void testRFC2965QuotedEscape() + { + String rawCookie = "A=\"double\\\"quote\""; + Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC2965_LEGACY, rawCookie); + + assertThat("Cookies.length", cookies.length, is(1)); + assertCookie("Cookies[0]", cookies[0], "A", "double\"quote", 0, null); + } + + @Test + public void testRFC2965QuotedSpecial() + { + String rawCookie = "A=\", ;\""; + Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC2965_LEGACY, rawCookie); + + assertThat("Cookies.length", cookies.length, is(1)); + assertCookie("Cookies[0]", cookies[0], "A", ", ;", 0, null); + } + + // TODO: + // $X; N=V + // $X=Y; N=V + + public static List parameters() + { + 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", "C=3"), + new Param("A=\"1; B=2; C=3"), + new Param("A=\"1 B=2\"; C=3", "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=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", "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\"; 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") + ); + } + + @ParameterizedTest + @MethodSource("parameters") + public void testRFC6265Cookie(Param param) + { + Cookie[] cookies = parseCookieHeaders(CookieCompliance.RFC6265, param.input); + + assertThat("Cookies.length", cookies.length, is(param.expected.size())); + for (int i = 0; i < cookies.length; i++) + { + Cookie cookie = cookies[i]; + assertThat(cookie.getName() + "=" + cookie.getValue(), is(param.expected.get(i))); + } + } + + static class Cookie + { + String name; + String value; + String domain; + String path; + int version; + String comment; + + public Cookie(String name, String value, String domain, String path, int version, String comment) + { + this.name = name; + this.value = value; + this.domain = domain; + this.path = path; + this.version = version; + this.comment = comment; + } + + public String getName() + { + return name; + } + + public String getValue() + { + return value; + } + + public String getDomain() + { + return domain; + } + + public String getPath() + { + return path; + } + + public int getVersion() + { + return version; + } + + public String getComment() + { + return comment; + } + } + + private Cookie[] parseCookieHeaders(CookieCompliance compliance, String... headers) + { + TestCookieParser cutter = new TestCookieParser(compliance); + for (String header : headers) + { + cutter.parseFields(header); + } + return cutter.cookies.toArray(Cookie[]::new); + } + + private void assertCookie(String prefix, Cookie cookie, + String expectedName, + String expectedValue, + int expectedVersion, + String expectedPath) + { + assertThat(prefix + ".name", cookie.getName(), is(expectedName)); + assertThat(prefix + ".value", cookie.getValue(), is(expectedValue)); + assertThat(prefix + ".version", cookie.getVersion(), is(expectedVersion)); + assertThat(prefix + ".path", cookie.getPath(), is(expectedPath)); + } + + private static class TestCookieParser implements ComplianceViolation.Listener, CookieParser.Handler + { + private final CookieParser parser; + private final List cookies = new ArrayList<>(); + private final List violations = new ArrayList<>(); + + private TestCookieParser(CookieCompliance compliance) + { + parser = new RFC6265CookieParser(this, compliance, this); + } + + @Override + public void onComplianceViolation(ComplianceViolation.Mode mode, ComplianceViolation violation, String details) + { + violations.add((CookieCompliance.Violation)violation); + } + + private List parseFields(String... fields) + { + parser.parseFields(Arrays.asList(fields)); + return cookies; + } + + @Override + public void addCookie(String cookieName, String cookieValue, int cookieVersion, String cookieDomain, String cookiePath, String cookieComment) + { + cookies.add(new Cookie(cookieName, cookieValue, cookieDomain, cookiePath, cookieVersion, cookieComment)); + } + } + + private static class Param + { + private final String input; + private final List 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/main/java/org/eclipse/jetty/server/Cookies.java b/jetty-server/src/main/java/org/eclipse/jetty/server/Cookies.java index 4dc61cb1d44..18986a77b9e 100644 --- a/jetty-server/src/main/java/org/eclipse/jetty/server/Cookies.java +++ b/jetty-server/src/main/java/org/eclipse/jetty/server/Cookies.java @@ -19,7 +19,7 @@ import javax.servlet.http.Cookie; import org.eclipse.jetty.http.ComplianceViolation; import org.eclipse.jetty.http.CookieCompliance; -import org.eclipse.jetty.http.CookieCutter; +import org.eclipse.jetty.http.CookieParser; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -31,7 +31,7 @@ import org.slf4j.LoggerFactory; * If the added fields are identical to those last added (as strings), then the * cookies are not re parsed. */ -public class Cookies extends CookieCutter +public class Cookies implements CookieParser.Handler { protected static final Logger LOG = LoggerFactory.getLogger(Cookies.class); protected final List _rawFields = new ArrayList<>(); @@ -41,6 +41,8 @@ public class Cookies extends CookieCutter private Cookie[] _cookies; private boolean _set = false; + private final CookieParser _parser; + public Cookies() { this(CookieCompliance.RFC6265, null); @@ -48,7 +50,7 @@ public class Cookies extends CookieCutter public Cookies(CookieCompliance compliance, ComplianceViolation.Listener complianceListener) { - super(compliance, complianceListener); + _parser = CookieParser.newParser(this, compliance, complianceListener); } public void addCookieField(String rawField) @@ -93,7 +95,7 @@ public class Cookies extends CookieCutter if (_parsed) return _cookies; - parseFields(_rawFields); + _parser.parseFields(_rawFields); _cookies = (Cookie[])_cookieList.toArray(new Cookie[_cookieList.size()]); _cookieList.clear(); _parsed = true; @@ -115,7 +117,7 @@ public class Cookies extends CookieCutter } @Override - protected void addCookie(String name, String value, String domain, String path, int version, String comment) + public void addCookie(String name, String value, int version, String domain, String path, String comment) { try { 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 ccea038e573..8a26350bd3c 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 @@ -154,6 +154,7 @@ public class RequestTest http.getHttpConfiguration().setResponseHeaderSize(512); http.getHttpConfiguration().setOutputBufferSize(2048); http.getHttpConfiguration().addCustomizer(new ForwardedRequestCustomizer()); + http.getHttpConfiguration().setRequestCookieCompliance(CookieCompliance.RFC6265_LEGACY); _connector = new LocalConnector(_server, http); _server.addConnector(_connector); _connector.setIdleTimeout(500);