Skip to content

Commit

Permalink
Fix #9334 Cookie Compliance (#9402)
Browse files Browse the repository at this point in the history 
Fix incorrect change to RFC6265 to not support dollars in cookie names.

Signed-off-by: gregw <gregw@webtide.com>
  • Loading branch information
@gregw
gregw authored Feb 21, 2023
1 parent f01d538 commit 4d14641
Show file tree
Hide file tree
Showing 10 changed files with 524 additions and 90 deletions.
75 changes: 43 additions & 32 deletions 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-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);
}
}
}
2 changes: 1 addition & 1 deletion 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 @@ -281,7 +281,7 @@ private static boolean isQuoteNeededForCookie(String s)

public String getSetCookie(CookieCompliance compliance)
{
if (CookieCompliance.RFC6265.compliesWith(compliance))
if (compliance == null || CookieCompliance.RFC6265_LEGACY.compliesWith(compliance))
return getRFC6265SetCookie();
return getRFC2965SetCookie();
}
Expand Down
46 changes: 17 additions & 29 deletions 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
Loading

0 comments on commit 4d14641

Please sign in to comment.