Skip to content

Commit

Permalink
Improve CookieCompliance testing (#9399)
Browse files Browse the repository at this point in the history 
Improved handling of CookieCompliance.from method
Added tests for request and response cookie handling
Use from in jetty.xml

Signed-off-by: gregw <[email protected]>
  • Loading branch information
@gregw
gregw authored Feb 23, 2023
1 parent 791c6a7 commit 6c114b0
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 80 deletions.
22 changes: 9 additions & 13 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 @@ -218,25 +218,21 @@ public static CookieCompliance from(String spec)
if (compliance == null)
{
String[] elements = spec.split("\\s*,\\s*");
Set<Violation> violations;
switch (elements[0])
Set<Violation> violations = switch (elements[0])
{
case "0" :
violations = noneOf(Violation.class);
break;

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

default :
case "0" -> noneOf(Violation.class);
case "*" -> allOf(Violation.class);
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());
if (mode.getAllowed().isEmpty())
yield noneOf(Violation.class);
else
yield copyOf(mode.getAllowed());
}
}
};

for (int i = 1; i < elements.length; i++)
{
Expand Down
4 changes: 2 additions & 2 deletions jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/CookieCutter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ else if (tokenstart >= 0)
if (_complianceMode.allows(INVALID_COOKIES))
reportComplianceViolation(INVALID_COOKIES, hdr);
else
throw new IllegalArgumentException("Bad Cookie");
throw new InvalidCookieException("Bad Cookie");
}
else
{
Expand Down Expand Up @@ -353,7 +353,7 @@ else if (tokenstart >= 0)
if (_complianceMode.allows(INVALID_COOKIES))
reportComplianceViolation(INVALID_COOKIES, hdr);
else
throw new IllegalArgumentException("Bad Cookie");
throw new InvalidCookieException("Bad Cookie");
}
else
{
Expand Down
62 changes: 30 additions & 32 deletions jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpCompliance.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
*/
public final class HttpCompliance implements ComplianceViolation.Mode
{
protected static final Logger LOG = LoggerFactory.getLogger(HttpCompliance.class);
private static final Logger LOG = LoggerFactory.getLogger(HttpCompliance.class);

// These are compliance violations, which may optionally be allowed by the compliance mode, which mean that
// the relevant section of the RFC is not strictly adhered to.
Expand Down Expand Up @@ -222,9 +222,9 @@ public static HttpCompliance valueOf(String name)
* <dl>
* <dt>0</dt><dd>No {@link Violation}s</dd>
* <dt>*</dt><dd>All {@link Violation}s</dd>
* <dt>RFC2616</dt><dd>The set of {@link Violation}s application to https://tools.ietf.org/html/rfc2616,
* but not https://tools.ietf.org/html/rfc7230</dd>
* <dt>RFC7230</dt><dd>The set of {@link Violation}s application to https://tools.ietf.org/html/rfc7230</dd>
* <dt>RFC2616</dt><dd>The set of {@link Violation}s application to <a href="https://tools.ietf.org/html/rfc2616">RFC2616</a>,
* but not <a href="https://tools.ietf.org/html/rfc7230">RFC7230</a></dd>
* <dt>RFC7230</dt><dd>The set of {@link Violation}s application to <a href="https://tools.ietf.org/html/rfc7230">RFC7230</a></dd>
* <dt>name</dt><dd>Any of the known modes defined in {@link HttpCompliance#KNOWN_MODES}</dd>
* </dl>
* <p>
Expand All @@ -237,39 +237,37 @@ public static HttpCompliance valueOf(String name)
*/
public static HttpCompliance from(String spec)
{
Set<Violation> sections;
String[] elements = spec.split("\\s*,\\s*");
switch (elements[0])
HttpCompliance compliance = valueOf(spec);
if (compliance == null)
{
case "0":
sections = noneOf(Violation.class);
break;

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

default:
String[] elements = spec.split("\\s*,\\s*");
Set<Violation> sections = switch (elements[0])
{
case "0" -> noneOf(Violation.class);
case "*" -> allOf(Violation.class);
default ->
{
HttpCompliance mode = HttpCompliance.valueOf(elements[0]);
yield (mode == null) ? noneOf(Violation.class) : copyOf(mode.getAllowed());
}
};

for (int i = 1; i < elements.length; i++)
{
HttpCompliance mode = HttpCompliance.valueOf(elements[0]);
sections = (mode == null) ? noneOf(HttpCompliance.Violation.class) : copyOf(mode.getAllowed());
String element = elements[i];
boolean exclude = element.startsWith("-");
if (exclude)
element = element.substring(1);
Violation section = Violation.valueOf(element);
if (exclude)
sections.remove(section);
else
sections.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)
sections.remove(section);
else
sections.add(section);
compliance = new HttpCompliance("CUSTOM" + __custom.getAndIncrement(), sections);
}

return new HttpCompliance("CUSTOM" + __custom.getAndIncrement(), sections);
return compliance;
}

private final String _name;
Expand Down
63 changes: 30 additions & 33 deletions jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/UriCompliance.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -202,45 +202,42 @@ public static UriCompliance from(Set<Violation> violations)
*/
public static UriCompliance from(String spec)
{
Set<Violation> violations;
String[] elements = spec.split("\\s*,\\s*");
switch (elements[0])
UriCompliance compliance = valueOf(spec);
if (compliance == null)
{
case "0":
violations = noneOf(Violation.class);
break;
String[] elements = spec.split("\\s*,\\s*");

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

default:
Set<Violation> violations = switch (elements[0])
{
case "0" -> noneOf(Violation.class);
case "*" -> allOf(Violation.class);
default ->
{
UriCompliance mode = UriCompliance.valueOf(elements[0]);
yield (mode == null) ? noneOf(Violation.class) : copyOf(mode.getAllowed());
}
};

for (int i = 1; i < elements.length; i++)
{
UriCompliance mode = UriCompliance.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);

// Ignore removed name. TODO: remove in future release.
if (element.equals("NON_CANONICAL_AMBIGUOUS_PATHS"))
continue;

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);

// Ignore removed name. TODO: remove in future release.
if (element.equals("NON_CANONICAL_AMBIGUOUS_PATHS"))
continue;

Violation section = Violation.valueOf(element);
if (exclude)
violations.remove(section);
else
violations.add(section);
compliance = new UriCompliance("CUSTOM" + __custom.getAndIncrement(), violations);
}

UriCompliance compliance = new UriCompliance("CUSTOM" + __custom.getAndIncrement(), violations);
if (LOG.isDebugEnabled())
LOG.debug("UriCompliance from {}->{}", spec, compliance);
return compliance;
Expand Down

0 comments on commit 6c114b0

Please sign in to comment.