Improve Error messages for Ambiguous URIs (#11457)

* Some testing of HttpURI for Issue #11448
* Issue #11448 - improved stacktrace message for ambiguous URI
This commit is contained in:
Joakim Erdfelt 2024-02-28 04:15:56 -08:00 committed by GitHub
parent 98ceb73cc6
commit 97cb50ead9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 47 additions and 7 deletions

View File

@ -1024,6 +1024,8 @@ public class HttpURITest
// Path choices
Arguments.of("http", "example.org", 0, "/a/b/c/d", null, null, "http://example.org/a/b/c/d"),
Arguments.of("http", "example.org", 0, "/a%20b/c%20d", null, null, "http://example.org/a%20b/c%20d"),
Arguments.of("http", "example.org", 0, "/foo%2Fbaz", null, null, "http://example.org/foo%2Fbaz"),
Arguments.of("http", "example.org", 0, "/foo%252Fbaz", null, null, "http://example.org/foo%252Fbaz"),
// Query specified
Arguments.of("http", "example.org", 0, "/", "a=b", null, "http://example.org/?a=b"),
Arguments.of("http", "example.org", 0, "/documentation/latest/", "a=b", null, "http://example.org/documentation/latest/?a=b"),
@ -1046,6 +1048,24 @@ public class HttpURITest
assertThat(httpURI.asString(), is(expectedStr));
}
public static Stream<Arguments> fromStringAsStringCases()
{
return Stream.of(
Arguments.of("http://localhost:4444/", "http://localhost:4444/"),
Arguments.of("/foo/baz", "/foo/baz"),
Arguments.of("/foo%2Fbaz", "/foo%2Fbaz"),
Arguments.of("/foo%252Fbaz", "/foo%252Fbaz")
);
}
@ParameterizedTest
@MethodSource("fromStringAsStringCases")
public void testFromStringAsString(String input, String expected)
{
HttpURI httpURI = HttpURI.from(input);
assertThat(httpURI.asString(), is(expected));
}
/**
* Tests of parameters that result in undesired behaviors.
* {@link HttpURI#from(String, String, int, String)}

View File

@ -1307,21 +1307,24 @@ public class ServletApiRequest implements HttpServletRequest
static class AmbiguousURI extends ServletApiRequest
{
protected AmbiguousURI(ServletContextRequest servletContextRequest)
private final String msg;
protected AmbiguousURI(ServletContextRequest servletContextRequest, String msg)
{
super(servletContextRequest);
this.msg = msg;
}
@Override
public String getPathInfo()
{
throw new HttpException.IllegalArgumentException(HttpStatus.BAD_REQUEST_400, "Ambiguous URI encoding");
throw new HttpException.IllegalArgumentException(HttpStatus.BAD_REQUEST_400, msg);
}
@Override
public String getServletPath()
{
throw new HttpException.IllegalArgumentException(HttpStatus.BAD_REQUEST_400, "Ambiguous URI encoding");
throw new HttpException.IllegalArgumentException(HttpStatus.BAD_REQUEST_400, msg);
}
}

View File

@ -197,11 +197,28 @@ public class ServletContextRequest extends ContextRequest implements ServletCont
if (getHttpURI().hasViolations() && !getServletChannel().getServletContextHandler().getServletHandler().isDecodeAmbiguousURIs())
{
// TODO we should check if current compliance mode allows all the violations?
for (UriCompliance.Violation violation : getHttpURI().getViolations())
if (getHttpURI().hasViolations())
{
if (UriCompliance.AMBIGUOUS_VIOLATIONS.contains(violation))
return new ServletApiRequest.AmbiguousURI(this);
StringBuilder msg = null;
for (UriCompliance.Violation violation : getHttpURI().getViolations())
{
if (UriCompliance.AMBIGUOUS_VIOLATIONS.contains(violation))
{
if (msg == null)
{
msg = new StringBuilder();
msg.append("Ambiguous URI encoding: ");
}
else
{
msg.append(", ");
}
msg.append(violation.name());
}
}
if (msg != null)
return new ServletApiRequest.AmbiguousURI(this, msg.toString());
}
}