Skip to content

Commit

Permalink
Fix #9334 Cookie Compliance
Browse files Browse the repository at this point in the history 
Fix incorrect change to RFC6265 to not support dollars in cookie names.
Included updates and tests from #9399

Signed-off-by: gregw <gregw@webtide.com>
  • Loading branch information
@gregw
gregw committed Feb 21, 2023
1 parent 298a881 commit 78c9074
Show file tree
Hide file tree
Showing 8 changed files with 306 additions and 47 deletions.
72 changes: 42 additions & 30 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 @@ -145,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 @@ -213,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 @@ -289,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
20 changes: 10 additions & 10 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 Down Expand Up @@ -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 Down Expand Up @@ -378,7 +378,7 @@ else if (_complianceMode.allows(ATTRIBUTES))
value = null;

if (state == State.END)
throw new IllegalStateException("Invalid cookie");
throw new InvalidCookieException("Invalid cookie");
break;

case INVALID_COOKIE:
Expand Down
4 changes: 2 additions & 2 deletions jetty-server/src/main/config/etc/jetty.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,8 @@
<Set name="persistentConnectionsEnabled" property="jetty.httpConfig.persistentConnectionsEnabled"/>
<Set name="httpCompliance"><Call class="org.eclipse.jetty.http.HttpCompliance" name="from"><Arg><Property name="jetty.httpConfig.compliance" deprecated="jetty.http.compliance" default="RFC7230"/></Arg></Call></Set>
<Set name="uriCompliance"><Call class="org.eclipse.jetty.http.UriCompliance" name="from"><Arg><Property name="jetty.httpConfig.uriCompliance" default="SAFE"/></Arg></Call></Set>
<Set name="requestCookieCompliance"><Call class="org.eclipse.jetty.http.CookieCompliance" name="valueOf"><Arg><Property name="jetty.httpConfig.requestCookieCompliance" default="RFC6265"/></Arg></Call></Set>
<Set name="responseCookieCompliance"><Call class="org.eclipse.jetty.http.CookieCompliance" name="valueOf"><Arg><Property name="jetty.httpConfig.responseCookieCompliance" default="RFC6265"/></Arg></Call></Set>
<Set name="requestCookieCompliance"><Call class="org.eclipse.jetty.http.CookieCompliance" name="from"><Arg><Property name="jetty.httpConfig.requestCookieCompliance" default="RFC6265"/></Arg></Call></Set>
<Set name="responseCookieCompliance"><Call class="org.eclipse.jetty.http.CookieCompliance" name="from"><Arg><Property name="jetty.httpConfig.responseCookieCompliance" default="RFC6265"/></Arg></Call></Set>
<Set name="multiPartFormDataCompliance"><Call class="org.eclipse.jetty.server.MultiPartFormDataCompliance" name="valueOf"><Arg><Property name="jetty.httpConfig.multiPartFormDataCompliance" default="RFC7578"/></Arg></Call></Set>
<Set name="relativeRedirectAllowed"><Property name="jetty.httpConfig.relativeRedirectAllowed" default="false"/></Set>
<Set name="useInputDirectByteBuffers" property="jetty.httpConfig.useInputDirectByteBuffers"/>
Expand Down
2 changes: 1 addition & 1 deletion jetty-server/src/main/config/modules/server.mod
View file
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ etc/jetty.xml
## URI Compliance: DEFAULT, LEGACY, RFC3986, RFC3986_UNAMBIGUOUS, UNSAFE
# jetty.httpConfig.uriCompliance=DEFAULT

## Cookie compliance mode for parsing request Cookie headers: RFC2965, RFC6265
## Cookie compliance mode for parsing request Cookie headers: RFC6265_STRICT, RFC6265, RFC6265_LEGACY, RFC2965, RFC2965_LEGACY
# jetty.httpConfig.requestCookieCompliance=RFC6265

## Cookie compliance mode for generating response Set-Cookie: RFC2965, RFC6265
Expand Down
11 changes: 10 additions & 1 deletion jetty-server/src/main/java/org/eclipse/jetty/server/Cookies.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,11 @@
import java.util.List;
import javax.servlet.http.Cookie;

import org.eclipse.jetty.http.BadMessageException;
import org.eclipse.jetty.http.ComplianceViolation;
import org.eclipse.jetty.http.CookieCompliance;
import org.eclipse.jetty.http.CookieParser;
import org.eclipse.jetty.http.HttpStatus;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand Down Expand Up @@ -95,7 +97,14 @@ public Cookie[] getCookies()
if (_parsed)
return _cookies;

_parser.parseFields(_rawFields);
try
{
_parser.parseFields(_rawFields);
}
catch (CookieParser.InvalidCookieException invalidCookieException)
{
throw new BadMessageException(HttpStatus.BAD_REQUEST_400, invalidCookieException.getMessage(), invalidCookieException);
}
_cookies = (Cookie[])_cookieList.toArray(new Cookie[_cookieList.size()]);
_cookieList.clear();
_parsed = true;
Expand Down

0 comments on commit 78c9074

Please sign in to comment.