malformed uri #198
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.
| @@ -52,6 +56,12 @@ public long getSize( | |||
| public void writeTo( | |||
| 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.
}
| @@ -20,4 +22,9 @@ protected void configure() { | |||
| bind(CorrelationIdFilter.class).toProvider(CorrelationIdFilterProvider.class) | |||
| .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.
| @@ -1,7 +1,9 @@ | |||
| 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.
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