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 <[email protected]>
  • Loading branch information
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.
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
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);
}
}
}
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
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.