Skip to content

Commit

Permalink
Merge remote-tracking branch 'upstream/jetty-12.0.x' into jetty-12.0.…
Browse files Browse the repository at this point in the history 
…x-old-docs-remove-logging-sections

* upstream/jetty-12.0.x:
  Issue jetty#9403 sendError(404) not setError(404) for DefaultServlet (jetty#9404)
  Fix bad merges from jetty-11
  revert upgrade from 3aaa65c
  Fix osgi dependencies for update to org.eclipse.osgi.services.
  Add ignore .checkstyle
  fix style
  Fixed Merge remote-tracking branch 'origin/jetty-11.0.x' into jetty-12.0.x
  fixing merge
  Fix jetty#9334 Cookie Compliance (jetty#9402)
  Bump maven-deploy-plugin from 3.0.0 to 3.1.0
  Bump asciidoctorj-diagram from 2.2.3 to 2.2.4
  Bump jakarta.servlet.jsp-api from 3.0.0 to 3.1.1
  Bump maven-invoker-plugin from 3.4.0 to 3.5.0
  Bump maven.surefire.plugin.version from 3.0.0-M8 to 3.0.0-M9
  Bump maven-javadoc-plugin from 3.4.1 to 3.5.0
  Bump tycho-p2-repository-plugin from 3.0.1 to 3.0.2
  Bump maven.version from 3.8.7 to 3.9.0
  • Loading branch information
Greg Poulos committed Feb 23, 2023
2 parents 0780b1b + 4e97a59 commit d260492
Show file tree
Hide file tree
Showing 14 changed files with 384 additions and 142 deletions.
9 changes: 8 additions & 1 deletion jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/CookieCache.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,14 @@ public List<HttpCookie> getCookies(HttpFields headers)
if (building)
{
_cookieList = new ArrayList<>();
_parser.parseFields(_rawFields);
try
{
_parser.parseFields(_rawFields);
}
catch (CookieParser.InvalidCookieException invalidCookieException)
{
throw new BadMessageException(HttpStatus.BAD_REQUEST_400, invalidCookieException.getMessage(), invalidCookieException);
}
}

return _cookieList == null ? Collections.emptyList() : _cookieList;
Expand Down
75 changes: 43 additions & 32 deletions jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/CookieCompliance.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,13 +128,12 @@ public String getDescription()
* <p>A CookieCompliance mode that enforces <a href="https://tools.ietf.org/html/rfc6265">RFC 6265</a> compliance,
* but allows:</p>
* <ul>
* <li>{@link Violation#ATTRIBUTES}</li>
* <li>{@link Violation#INVALID_COOKIES}</li>
* <li>{@link Violation#OPTIONAL_WHITE_SPACE}</li>
* </ul>
*/
public static final CookieCompliance RFC6265 = new CookieCompliance("RFC6265", of(
Violation.ATTRIBUTES, Violation.INVALID_COOKIES, Violation.OPTIONAL_WHITE_SPACE)
Violation.INVALID_COOKIES, Violation.OPTIONAL_WHITE_SPACE)
);

/**
Expand All @@ -146,15 +145,16 @@ public String getDescription()
* <p>A CookieCompliance mode that enforces <a href="https://tools.ietf.org/html/rfc6265">RFC 6265</a> compliance,
* but allows:</p>
* <ul>
* <li>{@link Violation#ATTRIBUTES}</li>
* <li>{@link Violation#BAD_QUOTES}</li>
* <li>{@link Violation#ESCAPE_IN_QUOTES}</li>
* <li>{@link Violation#INVALID_COOKIES}</li>
* <li>{@link Violation#OPTIONAL_WHITE_SPACE}</li>
* <li>{@link Violation#SPECIAL_CHARS_IN_QUOTES}</li>
* </ul>
*/
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)
public static final CookieCompliance RFC6265_LEGACY = new CookieCompliance("RFC6265_LEGACY", EnumSet.of(
Violation.ATTRIBUTES, Violation.BAD_QUOTES, Violation.ESCAPE_IN_QUOTES, Violation.INVALID_COOKIES, Violation.OPTIONAL_WHITE_SPACE, Violation.SPECIAL_CHARS_IN_QUOTES)
);

/**
Expand Down Expand Up @@ -214,40 +214,45 @@ public static CookieCompliance valueOf(String name)
*/
public static CookieCompliance from(String spec)
{
Set<Violation> violations;
String[] elements = spec.split("\\s*,\\s*");
switch (elements[0])
CookieCompliance compliance = valueOf(spec);
if (compliance == null)
{
case "0":
violations = noneOf(Violation.class);
break;

case "*":
violations = allOf(Violation.class);
break;
String[] elements = spec.split("\\s*,\\s*");
Set<Violation> violations;
switch (elements[0])
{
case "0" :
violations = noneOf(Violation.class);
break;

case "*" :
violations = allOf(Violation.class);
break;

default :
{
CookieCompliance mode = valueOf(elements[0]);
if (mode == null)
throw new IllegalArgumentException("Unknown base mode: " + elements[0]);
violations = (mode.getAllowed().isEmpty()) ? noneOf(Violation.class) : copyOf(mode.getAllowed());
}
}

default:
for (int i = 1; i < elements.length; i++)
{
CookieCompliance mode = valueOf(elements[0]);
violations = (mode == null) ? noneOf(Violation.class) : copyOf(mode.getAllowed());
break;
String element = elements[i];
boolean exclude = element.startsWith("-");
if (exclude)
element = element.substring(1);
Violation section = Violation.valueOf(element);
if (exclude)
violations.remove(section);
else
violations.add(section);
}
}

for (int i = 1; i < elements.length; i++)
{
String element = elements[i];
boolean exclude = element.startsWith("-");
if (exclude)
element = element.substring(1);
Violation section = Violation.valueOf(element);
if (exclude)
violations.remove(section);
else
violations.add(section);
compliance = new CookieCompliance("CUSTOM" + __custom.getAndIncrement(), violations);
}

CookieCompliance compliance = new CookieCompliance("CUSTOM" + __custom.getAndIncrement(), violations);
if (LOG.isDebugEnabled())
LOG.debug("CookieCompliance from {}->{}", spec, compliance);
return compliance;
Expand Down Expand Up @@ -290,4 +295,10 @@ public boolean compliesWith(CookieCompliance mode)
{
return this == mode || getAllowed().containsAll(mode.getAllowed());
}

@Override
public String toString()
{
return String.format("%s@%x%s", _name, hashCode(), _violations);
}
}
30 changes: 28 additions & 2 deletions jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/CookieParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@ static CookieParser newParser(Handler handler, CookieCompliance compliance, List
return new RFC6265CookieParser(handler, compliance, complianceListener);
}

void parseField(String field);
void parseField(String field) throws InvalidCookieException;

default void parseFields(List<String> rawFields)
default void parseFields(List<String> rawFields) throws InvalidCookieException
{
// For each cookie field
for (String field : rawFields)
Expand All @@ -57,4 +57,30 @@ interface Handler
{
void addCookie(String name, String value, int version, String domain, String path, String comment);
}

/**
* <p>The exception thrown when a cookie cannot be parsed and {@link CookieCompliance.Violation#INVALID_COOKIES} is not allowed.</p>
*/
class InvalidCookieException extends IllegalArgumentException
{
public InvalidCookieException()
{
super();
}

public InvalidCookieException(String s)
{
super(s);
}

public InvalidCookieException(String message, Throwable cause)
{
super(message, cause);
}

public InvalidCookieException(Throwable cause)
{
super(cause);
}
}
}
3 changes: 3 additions & 0 deletions jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpCookie.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -823,6 +823,9 @@ private static String asString(HttpCookie httpCookie)
{
StringBuilder builder = new StringBuilder();
builder.append(httpCookie.getName()).append("=").append(httpCookie.getValue());
int version = httpCookie.getVersion();
if (version > 0)
builder.append(";Version=").append(version);
String domain = httpCookie.getDomain();
if (domain != null)
builder.append(";Domain=").append(domain);
Expand Down
46 changes: 17 additions & 29 deletions jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/RFC6265CookieParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ public void parseField(String field)
if (token == null)
{
if (!_complianceMode.allows(INVALID_COOKIES))
throw new IllegalArgumentException("Invalid Cookie character");
throw new InvalidCookieException("Invalid Cookie character");
state = State.INVALID_COOKIE;
continue;
}
Expand All @@ -100,7 +100,7 @@ public void parseField(String field)

if (token.isRfc2616Token())
{
if (!StringUtil.isBlank(cookieName) && c != '$')
if (!StringUtil.isBlank(cookieName) && !(c == '$' && (_complianceMode.allows(ATTRIBUTES) || _complianceMode.allows(ATTRIBUTE_VALUES))))
{
_handler.addCookie(cookieName, cookieValue, cookieVersion, cookieDomain, cookiePath, cookieComment);
cookieName = null;
Expand All @@ -120,7 +120,7 @@ else if (_complianceMode.allows(INVALID_COOKIES))
}
else
{
throw new IllegalArgumentException("Bad Cookie name");
throw new InvalidCookieException("Bad Cookie name");
}

break;
Expand Down Expand Up @@ -158,7 +158,7 @@ else if (_complianceMode.allows(INVALID_COOKIES))
}
else
{
throw new IllegalArgumentException("Bad Cookie name");
throw new InvalidCookieException("Bad Cookie name");
}
break;

Expand All @@ -181,7 +181,7 @@ else if (_complianceMode.allows(INVALID_COOKIES))
}
else
{
throw new IllegalArgumentException("Bad Cookie");
throw new InvalidCookieException("Bad Cookie");
}
break;

Expand Down Expand Up @@ -215,7 +215,7 @@ else if (_complianceMode.allows(INVALID_COOKIES))
}
else
{
throw new IllegalArgumentException("Bad Cookie value");
throw new InvalidCookieException("Bad Cookie value");
}
break;

Expand All @@ -237,7 +237,7 @@ else if (_complianceMode.allows(INVALID_COOKIES))
}
else
{
throw new IllegalArgumentException("Bad Cookie value");
throw new InvalidCookieException("Bad Cookie value");
}
break;

Expand Down Expand Up @@ -277,7 +277,7 @@ else if (_complianceMode.allows(INVALID_COOKIES))
}
else
{
throw new IllegalArgumentException("Bad Cookie quoted value");
throw new InvalidCookieException("Bad Cookie quoted value");
}
break;

Expand All @@ -299,7 +299,7 @@ else if (_complianceMode.allows(INVALID_COOKIES))
}
else
{
throw new IllegalArgumentException("Bad Cookie quoted value");
throw new InvalidCookieException("Bad Cookie quoted value");
}
break;

Expand All @@ -323,7 +323,7 @@ else if (_complianceMode.allows(INVALID_COOKIES))
}
else
{
throw new IllegalStateException("Comma cookie separator");
throw new InvalidCookieException("Comma cookie separator");
}
}
else if ((c == ' ' || c == '\t') && _complianceMode.allows(OPTIONAL_WHITE_SPACE))
Expand All @@ -332,7 +332,6 @@ else if ((c == ' ' || c == '\t') && _complianceMode.allows(OPTIONAL_WHITE_SPACE)
continue;
}

boolean knownAttribute = true;
if (StringUtil.isBlank(attributeName))
{
cookieValue = value;
Expand All @@ -358,39 +357,28 @@ else if ((c == ' ' || c == '\t') && _complianceMode.allows(OPTIONAL_WHITE_SPACE)
cookieVersion = Integer.parseInt(value);
break;
default:
knownAttribute = false;
if (!_complianceMode.allows(INVALID_COOKIES))
throw new IllegalArgumentException("Invalid Cookie attribute");
reportComplianceViolation(INVALID_COOKIES, field);
state = State.INVALID_COOKIE;
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");
cookieName = attributeName;
cookieValue = value;
}
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");
throw new InvalidCookieException("Invalid cookie");
break;

case INVALID_COOKIE:
Expand Down
48 changes: 1 addition & 47 deletions ...-core/jetty-http/src/test/java/org/eclipse/jetty/http/RFC6265CookieParserLenientTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,58 +85,12 @@ public static Stream<Arguments> data()
Arguments.of("!f!o!o!=wat", "!f!o!o!", "wat"),
Arguments.of("__MyHost=Foo", "__MyHost", "Foo"),
Arguments.of("some-thing-else=to-parse", "some-thing-else", "to-parse"),
// RFC2109 - names with attr/token syntax starting with '$' (and not a cookie reserved word)
// See https://tools.ietf.org/html/draft-ietf-httpbis-cookie-prefixes-00#section-5.2
// Cannot pass names through as jakarta.servlet.http.Cookie class does not allow them
Arguments.of("$foo=bar", null, null),
Arguments.of("$foo=bar", "$foo", "bar"),

// Tests that conform to RFC6265
Arguments.of("abc=foobar!", "abc", "foobar!"),
Arguments.of("abc=\"foobar!\"", "abc", "foobar!")

/* TODO need to discuss if we should support these cases
,
// UTF-8 raw values (not encoded) - VIOLATION of RFC6265
Arguments.of("2sides=\u262F", null, null), // 2 byte (YIN YANG) - rejected due to not being DQUOTED
Arguments.of("currency=\"\u20AC\"", "currency", "\u20AC"), // 3 byte (EURO SIGN)
Arguments.of("gothic=\"\uD800\uDF48\"", "gothic", "\uD800\uDF48"), // 4 byte (GOTHIC LETTER HWAIR)
// Spaces
Arguments.of("foo=bar baz", "foo", "bar baz"),
Arguments.of("foo=\"bar baz\"", "foo", "bar baz"),
Arguments.of("z=a b c d e f g", "z", "a b c d e f g"),
// Bad tspecials usage - VIOLATION of RFC6265
Arguments.of("foo=bar;baz", "foo", "bar"),
Arguments.of("foo=\"bar;baz\"", "foo", "bar;baz"),
Arguments.of("z=a;b,c:d;e/f[g]", "z", "a"),
Arguments.of("z=\"a;b,c:d;e/f[g]\"", "z", "a;b,c:d;e/f[g]"),
Arguments.of("name=quoted=\"\\\"badly\\\"\"", "name", "quoted=\"\\\"badly\\\"\""), // someone attempting to escape a DQUOTE from within a DQUOTED pair)
// Quoted with other Cookie keywords
Arguments.of("x=\"$Version=0\"", "x", "$Version=0"),
Arguments.of("x=\"$Path=/\"", "x", "$Path=/"),
Arguments.of("x=\"$Path=/ $Domain=.foo.com\"", "x", "$Path=/ $Domain=.foo.com"),
Arguments.of("x=\" $Path=/ $Domain=.foo.com \"", "x", " $Path=/ $Domain=.foo.com "),
Arguments.of("a=\"b; $Path=/a; c=d; $PATH=/c; e=f\"; $Path=/e/", "a", "b; $Path=/a; c=d; $PATH=/c; e=f"), // VIOLATES RFC6265
// Lots of equals signs
Arguments.of("query=b=c&d=e", "query", "b=c&d=e"),
// Escaping
Arguments.of("query=%7B%22sessionCount%22%3A5%2C%22sessionTime%22%3A14151%7D", "query", "%7B%22sessionCount%22%3A5%2C%22sessionTime%22%3A14151%7D"),
// Google cookies (seen in wild, has `tspecials` of ':' in value)
Arguments.of("GAPS=1:A1aaaAaAA1aaAAAaa1a11a:aAaaAa-aaA1-", "GAPS", "1:A1aaaAaAA1aaAAAaa1a11a:aAaaAa-aaA1-"),
// Strong abuse of cookie spec (lots of tspecials) - VIOLATION of RFC6265
Arguments.of("$Version=0; rToken=F_TOKEN''!--\"</a>=&{()}", "rToken", "F_TOKEN''!--\"</a>=&{()}"),
// Commas that were not commas
Arguments.of("name=foo,bar", "name", "foo,bar"),
Arguments.of("name=foo , bar", "name", "foo , bar"),
Arguments.of("name=foo , bar, bob", "name", "foo , bar, bob")
*/
);
}

Expand Down

0 comments on commit d260492

Please sign in to comment.