Skip to content

Issue #12806 - UriCompliance listener reporting updates #12807

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.

Sign up for GitHub

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

Draft
wants to merge 6 commits into
base: jetty-12.0.x
Choose a base branch
from
+323 −160
Draft

Issue #12806 - UriCompliance listener reporting updates #12807

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9371948
Issue #12806 - UriCompliance listener reporting updates
joakime Feb 19, 2025
11a98c9
HttpCompliance updates
joakime Feb 21, 2025
9481a42
RFC6265CookieParser updates
joakime Feb 21, 2025
e19413e
Merge remote-tracking branch 'origin/jetty-12.0.x' into fix/12.0.x/re…
joakime Mar 26, 2025
19da892
Merge remote-tracking branch 'origin/jetty-12.0.x' into fix/12.0.x/re…
joakime Jul 28, 2025
0d05bc5
Attempting to bring a consistent API to how Compliance violations are…
joakime Jul 29, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 4 additions & 5 deletions jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/HttpCompliance.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,11 +410,10 @@ public static void checkHttpCompliance(MetaData.Request request, HttpCompliance

private static void assertAllowed(Violation violation, HttpCompliance mode, ComplianceViolation.Listener listener)
{
if (mode.allows(violation))
listener.onComplianceViolation(new ComplianceViolation.Event(
mode, violation, violation.getDescription()
));
else
listener.onComplianceViolation(new ComplianceViolation.Event(
mode, violation, violation.getDescription()
));
if (!mode.allows(violation))
throw new BadMessageException(violation.getDescription());
}
}
79 changes: 33 additions & 46 deletions jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/RFC6265CookieParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public void parseField(String field)

if (token == null)
{
if (!_complianceMode.allows(INVALID_COOKIES))
if (!complianceAllows(INVALID_COOKIES))
throw new InvalidCookieException("Invalid Cookie character");
state = State.INVALID_COOKIE;
continue;
Expand All @@ -99,7 +99,7 @@ public void parseField(String field)

if (token.isRfc2616Token())
{
if (!StringUtil.isBlank(cookieName) && !(c == '$' && (_complianceMode.allows(ATTRIBUTES) || _complianceMode.allows(ATTRIBUTE_VALUES))))
if (!StringUtil.isBlank(cookieName) && !(c == '$' && (complianceAllows(ATTRIBUTES) || complianceAllows(ATTRIBUTE_VALUES))))
{
_handler.addCookie(cookieName, cookieValue, cookieVersion, cookieDomain, cookiePath, cookieComment);
cookieName = null;
Expand All @@ -112,9 +112,8 @@ public void parseField(String field)
string.append(c);
state = State.IN_NAME;
}
else if (_complianceMode.allows(INVALID_COOKIES))
else if (complianceAllows(INVALID_COOKIES, field))
{
reportComplianceViolation(INVALID_COOKIES, field);
state = State.INVALID_COOKIE;
}
else
Expand All @@ -135,9 +134,8 @@ else if (_complianceMode.allows(INVALID_COOKIES))
continue;
}

if ((c == ' ' || c == '\t') && _complianceMode.allows(OPTIONAL_WHITE_SPACE))
if ((c == ' ' || c == '\t') && complianceAllows(OPTIONAL_WHITE_SPACE, field))
{
reportComplianceViolation(OPTIONAL_WHITE_SPACE, field);
if (string.charAt(0) == '$')
attributeName = string.toString();
else
Expand All @@ -150,9 +148,8 @@ else if (_complianceMode.allows(INVALID_COOKIES))
{
string.append(c);
}
else if (_complianceMode.allows(INVALID_COOKIES))
else if (complianceAllows(INVALID_COOKIES, field))
{
reportComplianceViolation(INVALID_COOKIES, field);
state = c == ';' ? State.START : State.INVALID_COOKIE;
}
else
Expand All @@ -173,9 +170,8 @@ else if (_complianceMode.allows(INVALID_COOKIES))
continue;
}

if (_complianceMode.allows(INVALID_COOKIES))
if (complianceAllows(INVALID_COOKIES, field))
{
reportComplianceViolation(INVALID_COOKIES, field);
state = State.INVALID_COOKIE;
}
else
Expand All @@ -185,9 +181,8 @@ else if (_complianceMode.allows(INVALID_COOKIES))
break;

case VALUE:
if (c == ' ' && _complianceMode.allows(OPTIONAL_WHITE_SPACE))
if (c == ' ' && complianceAllows(OPTIONAL_WHITE_SPACE, field))
{
reportComplianceViolation(OPTIONAL_WHITE_SPACE, field);
continue;
}

Expand All @@ -207,9 +202,8 @@ else if (token.isRfc6265CookieOctet())
string.append(c);
state = State.IN_VALUE;
}
else if (_complianceMode.allows(INVALID_COOKIES))
else if (complianceAllows(INVALID_COOKIES, field))
{
reportComplianceViolation(INVALID_COOKIES, field);
state = State.INVALID_COOKIE;
}
else
Expand All @@ -219,9 +213,8 @@ else if (_complianceMode.allows(INVALID_COOKIES))
break;

case IN_VALUE:
if (c == ' ' && _complianceMode.allows(SPACE_IN_VALUES))
if (c == ' ' && complianceAllows(SPACE_IN_VALUES, field))
{
reportComplianceViolation(SPACE_IN_VALUES, field);
spaces = 1;
state = State.SPACE_IN_VALUE;
}
Expand All @@ -235,9 +228,8 @@ else if (token.isRfc6265CookieOctet())
{
string.append(c);
}
else if (_complianceMode.allows(INVALID_COOKIES))
else if (complianceAllows(INVALID_COOKIES, field))
{
reportComplianceViolation(INVALID_COOKIES, field);
state = State.INVALID_COOKIE;
}
else
Expand All @@ -262,9 +254,8 @@ else if (token.isRfc6265CookieOctet())
string.append(" ".repeat(spaces)).append(c);
state = State.IN_VALUE;
}
else if (_complianceMode.allows(INVALID_COOKIES))
else if (complianceAllows(INVALID_COOKIES, field))
{
reportComplianceViolation(INVALID_COOKIES, field);
state = State.INVALID_COOKIE;
}
else
Expand All @@ -279,36 +270,32 @@ else if (_complianceMode.allows(INVALID_COOKIES))
value = string.toString();
state = State.AFTER_QUOTED_VALUE;
}
else if (c == '\\' && _complianceMode.allows(ESCAPE_IN_QUOTES))
else if (c == '\\' && complianceAllows(ESCAPE_IN_QUOTES))
{
state = State.ESCAPED_VALUE;
}
else if (token.isRfc6265CookieOctet())
{
string.append(c);
}
else if (_complianceMode.allows(SPECIAL_CHARS_IN_QUOTES))
else if (complianceAllows(SPECIAL_CHARS_IN_QUOTES, field))
{
reportComplianceViolation(SPECIAL_CHARS_IN_QUOTES, field);
string.append(c);
}
else if (c == ',' && _complianceMode.allows(COMMA_NOT_VALID_OCTET))
else if (c == ',' && complianceAllows(COMMA_NOT_VALID_OCTET, field))
{
reportComplianceViolation(COMMA_NOT_VALID_OCTET, field);
string.append(c);
}
else if (c == ' ' && _complianceMode.allows(SPACE_IN_VALUES))
else if (c == ' ' && complianceAllows(SPACE_IN_VALUES, field))
{
reportComplianceViolation(SPACE_IN_VALUES, field);
string.append(c);
}
else if (_complianceMode.allows(INVALID_COOKIES))
else if (complianceAllows(INVALID_COOKIES, field))
{
string.append(c);
if (!cookieInvalid)
{
cookieInvalid = true;
reportComplianceViolation(INVALID_COOKIES, field);
}
// Try to find the closing double quote by staying in the current state.
}
Expand All @@ -329,9 +316,8 @@ else if (_complianceMode.allows(INVALID_COOKIES))
i--;
state = cookieInvalid ? State.INVALID_COOKIE : State.END;
}
else if (_complianceMode.allows(INVALID_COOKIES))
else if (complianceAllows(INVALID_COOKIES, field))
{
reportComplianceViolation(INVALID_COOKIES, field);
state = State.INVALID_COOKIE;
}
else
Expand All @@ -347,14 +333,12 @@ else if (_complianceMode.allows(INVALID_COOKIES))
}
else if (c == ',')
{
if (_complianceMode.allows(COMMA_SEPARATOR))
if (complianceAllows(COMMA_SEPARATOR, field))
{
reportComplianceViolation(COMMA_SEPARATOR, field);
state = State.START;
}
else if (_complianceMode.allows(INVALID_COOKIES))
else if (complianceAllows(INVALID_COOKIES, field))
{
reportComplianceViolation(INVALID_COOKIES, field);
state = State.INVALID_COOKIE;
continue;
}
Expand All @@ -363,9 +347,8 @@ else if (_complianceMode.allows(INVALID_COOKIES))
throw new InvalidCookieException("Comma cookie separator");
}
}
else if ((c == ' ' || c == '\t') && _complianceMode.allows(OPTIONAL_WHITE_SPACE))
else if ((c == ' ' || c == '\t') && complianceAllows(OPTIONAL_WHITE_SPACE, field))
{
reportComplianceViolation(OPTIONAL_WHITE_SPACE, field);
continue;
}

Expand All @@ -376,9 +359,8 @@ else if ((c == ' ' || c == '\t') && _complianceMode.allows(OPTIONAL_WHITE_SPACE)
else
{
// We have an attribute.
if (_complianceMode.allows(ATTRIBUTE_VALUES))
if (complianceAllows(ATTRIBUTE_VALUES, field))
{
reportComplianceViolation(ATTRIBUTES, field);
switch (attributeName.toLowerCase(Locale.ENGLISH))
{
case "$path":
Expand All @@ -394,16 +376,15 @@ else if ((c == ' ' || c == '\t') && _complianceMode.allows(OPTIONAL_WHITE_SPACE)
cookieVersion = Integer.parseInt(value);
break;
default:
if (!_complianceMode.allows(INVALID_COOKIES))
if (!complianceAllows(INVALID_COOKIES, field))
throw new IllegalArgumentException("Invalid Cookie attribute");
reportComplianceViolation(INVALID_COOKIES, field);
state = State.INVALID_COOKIE;
break;
}
}
else if (_complianceMode.allows(ATTRIBUTES))
else if (complianceAllows(ATTRIBUTES, field))
{
reportComplianceViolation(ATTRIBUTES, field);
// ignore
}
else
{
Expand Down Expand Up @@ -437,10 +418,16 @@ else if (_complianceMode.allows(ATTRIBUTES))
_handler.addCookie(cookieName, cookieValue, cookieVersion, cookieDomain, cookiePath, cookieComment);
}

protected void reportComplianceViolation(CookieCompliance.Violation violation, String reason)
private boolean complianceAllows(ComplianceViolation violation)
{
if (_complianceListener != null)
_complianceListener.onComplianceViolation(new ComplianceViolation.Event(_complianceMode, violation, reason));
return complianceAllows(violation, violation.getDescription());
}

private boolean complianceAllows(ComplianceViolation violation, String reason)
{
boolean ret = _complianceMode.allows(violation);
if (ret && _complianceListener != null)
_complianceListener.onComplianceViolation(new ComplianceViolation.Event(_complianceMode, violation, reason));
return ret;
}
}
23 changes: 19 additions & 4 deletions jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/UriCompliance.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,9 @@ public enum Violation implements ComplianceViolation

/**
* Allow truncated UTF-8 encodings to be substituted by the replacement character.
* <p>
* Note: This violation allows a subset of {@link #BAD_UTF8_ENCODING} behaviors meant to replicate LEGACY behaviors, and will be reported as {@link #BAD_UTF8_ENCODING}.
* </p>
*/
TRUNCATED_UTF8_ENCODING("https://datatracker.ietf.org/doc/html/rfc5987#section-3.2.1", "Truncated UTF-8 encoding"),

Expand All @@ -112,7 +115,7 @@ public enum Violation implements ComplianceViolation

/**
* Allow path characters not allowed in the path portion of the URI and HTTP specs.
* <p>This would allow characters that fall outside of the {@code unreserved / pct-encoded / sub-delims / ":" / "@"} ABNF</p>
* <p>This would allow characters that fall outside the {@code unreserved / pct-encoded / sub-delims / ":" / "@"} ABNF</p>
*/
ILLEGAL_PATH_CHARACTERS("https://datatracker.ietf.org/doc/html/rfc3986#section-3.3", "Illegal Path Character"),

Expand Down Expand Up @@ -426,18 +429,30 @@ private static Set<Violation> copyOf(Set<Violation> violations)
return EnumSet.copyOf(violations);
}

/**
* Check the {@link HttpURI} against a configured {@link UriCompliance} to see if any detected violations
* are allowed by the configured {@link UriCompliance}.
*
* @param compliance the configured {@link UriCompliance}.
* @param uri the HttpURI.
* @param listener listener to report violations to.
* @return A string representing the violations that were not allowed by the configured {@link UriCompliance}, null if
* the provided HttpURI either has no violations, or only had violations that are allowed by the {@link UriCompliance}
*/
public static String checkUriCompliance(UriCompliance compliance, HttpURI uri, ComplianceViolation.Listener listener)
{
if (uri.hasViolations())
{
StringBuilder violations = null;
for (UriCompliance.Violation violation : uri.getViolations())
{
// Always report violation to listeners
if (listener != null)
listener.onComplianceViolation(new ComplianceViolation.Event(compliance, violation, uri.toString()));

// Only trigger a failure of the HttpURI for compliance reasons if the compliance doesn't allow for violation detected
if (compliance == null || !compliance.allows(violation))
{
if (listener != null)
listener.onComplianceViolation(new ComplianceViolation.Event(compliance, violation, uri.toString()));

if (violations == null)
violations = new StringBuilder();
else
Expand Down
6 changes: 3 additions & 3 deletions jetty-core/jetty-http/src/test/java/org/eclipse/jetty/http/RFC6265CookieParserTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ public void testRFC2965Single()
assertThat("Cookies.length", cookies.size(), is(1));
assertCookie("Cookies[0]", cookies.get(0), "Customer", "WILE_E_COYOTE", 1, "/acme");
// There are 2 attributes, so 2 violations.
assertThat(parser.violations.size(), is(2));
assertThat(parser.violations.size(), is(3));

// Same test with RFC6265.
parser = new TestCookieParser(CookieCompliance.RFC6265);
Expand All @@ -61,7 +61,7 @@ public void testRFC2965Single()
assertCookie("Cookies[0]", cookies.get(0), "Customer", "WILE_E_COYOTE", 0, null);

// There are 2 attributes that are seen as violations
assertThat(parser.violations.size(), is(2));
assertThat(parser.violations.size(), is(3));

// Same again, but allow attributes which are not ignored
parser = new TestCookieParser(CookieCompliance.from("RFC6265,ATTRIBUTE_VALUES"));
Expand All @@ -70,7 +70,7 @@ public void testRFC2965Single()
assertCookie("Cookies[0]", cookies.get(0), "Customer", "WILE_E_COYOTE", 1, "/acme");

// There are 2 attributes that are seen as violations
assertThat(parser.violations.size(), is(2));
assertThat(parser.violations.size(), is(3));

// Same test, but with RFC 6265 strict.
parser = new TestCookieParser(CookieCompliance.RFC6265_STRICT);
Expand Down