Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #9334 Cookie Compliance #9402

Merged
merged 3 commits into from
Feb 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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