malformed uri#198
Conversation
kyleroush
requested review from
b-boogaard,
johnlcox,
nathanschile and
venki09
as code owners
| * @author Kyle Roush | ||
| */ | ||
| @Provider | ||
| @Priority(Priorities.AUTHENTICATION) |
There was a problem hiding this comment.
Could we lower this to a USER priority? Probably Not. I see that ForwardedHeaderFilter also uses UriInfo.
There was a problem hiding this comment.
Does this error occur at the first call of UriInfo in the stack?
| try { | ||
| URI.create(requestContext.getUriInfo().getAbsolutePath().toString()); | ||
| } catch (IllegalArgumentException e) { | ||
| requestContext.abortWith( |
There was a problem hiding this comment.
Let's log the exception.
| Object o, Class<?> type, Type genericType, Annotation[] annotations, MediaType mediaType, | ||
| MultivaluedMap<String, Object> httpHeaders, OutputStream os) throws IOException { | ||
|
|
||
| if (httpServletResponse.getStatus() >= 400 ) { |
There was a problem hiding this comment.
Instead of pulling in the servlet-api, can we update the code to something like
String fields = null;
try {
fields = uriInfo.getQueryParameters() == null ? null
: uriInfo.getQueryParameters().getFirst("fields");
} catch (Throwable e) {
// Nothing to do. URI does not conform to grammar of RFC 2396.
}
| .in(Singleton.class); | ||
| } | ||
|
|
||
| @Provides |
| @@ -20,4 +22,9 @@ protected void configure() { | |||
| bind(CorrelationIdFilter.class).toProvider(CorrelationIdFilterProvider.class) | |||
There was a problem hiding this comment.
Update the class Javadoc to indicate that the module now provides the MalformedRequestFilter
| @Override | ||
| public void filter(ContainerRequestContext requestContext) { | ||
| try { | ||
| URI.create(requestContext.getUriInfo().getAbsolutePath().toString()); |
There was a problem hiding this comment.
You can do requestContext.getUriInfo().getRequestUri(); so it doesn't create a new URI.
| import javax.ws.rs.core.{MediaType, Response, UriInfo} | ||
| import org.jboss.resteasy.specimpl.ResteasyUriInfo | ||
| import org.mockito | ||
| import org.mockito.{ArgumentCaptor, Mockito} |
| it("does not call abortWith") { | ||
| malformedRequestFilter.filter(containerRequestContext) | ||
|
|
||
| Mockito.verify(containerRequestContext).getUriInfo(); |
There was a problem hiding this comment.
nit. This can be verify(containerRequestContext).getUriInfo, since Scala doesn't require the parens and semicolon. You can also import org.mockito.Mockito.{verify, when} so you don't need to have Mockito.
| package com.cerner.beadledom.jaxrs; | ||
|
|
||
| import com.cerner.beadledom.jaxrs.provider.CorrelationIdFilter; | ||
| import com.cerner.beadledom.jaxrs.provider.MalformedRequestFilter; |
There was a problem hiding this comment.
Update CHANGELOG.md with the added class.
3 participants
What was changed? Why is this necessary?
I have added a new jaxrs filter that will check to see if the request is a correctly formatted URI and if it is not it will short circuit the request and return a 400 status code.
I have also updated the JSON field filtering logic to only apply the filter when the request is successful.
How was it tested?
How to test
./mvnw clean install -U