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.

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

Improve error handling for Jersey's Exception that caused by client #1982

Merged
merged 1 commit into from Mar 27, 2017

Conversation

Projects
None yet
3 participants
@t-tsutsumi
Contributor

t-tsutsumi commented Mar 26, 2017

If sending a POST request with a Content-Type header that is not application/x-www-form-urlencoded and using @FormParam in resource method, an IllegalStateException will be thrown by Jersey and logged at ERROR level by LoggingExceptionMapper. Also, this is the same behavior as https://java.net/jira/browse/JERSEY-2636.

This indicates that users can easily output ERROR level logs in server side. Therefore, I would like to prevent this.

Steps to reproduce

  1. Create a new resource class and method that given the @POST and @FormParam.
    Note: @Consumes is optional because ignoring.
@Path("/")
public class EchoResource {
    @POST
    @Consumes(MediaType.APPLICATION_FORM_URLENCODED)
    public String echo(@FormParam("msg") String msg) {
        return msg;
    }
}
  1. Send the POST request without a Content-Type header, the 500 Internal Server Error is returned and logged at ERROR level.
$ curl -i -XPOST localhost:8080/
HTTP/1.1 500 Internal Server Error
Date: Sun, 26 Mar 2017 03:20:17 GMT
Content-Type: application/json
Content-Length: 110

{"code":500,"message":"There was an error processing your request. It has been logged (ID 76abeb6446b86576)."}
ERROR [2017-03-26 03:20:17,068] io.dropwizard.jersey.errors.LoggingExceptionMapper: Error handling a request: 76abeb6446b86576
! java.lang.IllegalStateException: The @FormParam is utilized when the content type of the request entity is not application/x-www-form-urlencoded
! at org.glassfish.jersey.server.internal.inject.FormParamValueFactoryProvider$FormParamValueFactory.ensureValidRequest(FormParamValueFactoryProvider.java:183)
! at org.glassfish.jersey.server.internal.inject.FormParamValueFactoryProvider$FormParamValueFactory.getForm(FormParamValueFactoryProvider.java:167)
! at org.glassfish.jersey.server.internal.inject.FormParamValueFactoryProvider$FormParamValueFactory.provide(FormParamValueFactoryProvider.java:118)
! at org.glassfish.jersey.server.spi.internal.ParamValueFactoryWithSource.provide(ParamValueFactoryWithSource.java:71)
! at org.glassfish.jersey.server.spi.internal.ParameterValueHelper.getParameterValues(ParameterValueHelper.java:90)
! at org.glassfish.jersey.server.model.internal.JavaResourceMethodDispatcherProvider$AbstractMethodParamInvoker.getParamValues(JavaResourceMethodDispatcherProvider.java:127)
...

Impact

Probably, this change does not affected to users using the past version, because If bound an ExceptionMapper<IllegalStateException> at Application#run(Configuration, Environment), this mappers register in the HK2 container before Dropwizard's ExceptionMapper(s) that is bound by the ExceptionMapperBinder.

Please refer to the following references for details.

Reference 1: HK2 API documentation

Services are sorted by (in order) the service ranking, the largest locator id (so that services in children are picked before services in parents) and smallest service id (so that older services are picked prior to newer services). Therefore the best instance of a service is a service with the highest ranking or largest service locator id or the lowest service id. The ranking of a service is found in its Descriptor and can be changed at any time at run time. The locator id of a service is a system assigned value for the Descriptor when it is bound into the ServiceLocator and is the id of that ServiceLocator. The service id of a service is a system assigned value for the Descriptor when it is bound into the ServiceLocator. The system assigned value is a monotonically increasing value. Thus if two services have the same ranking the best service will be associated with the oldest Descriptor bound into the system.

Reference 2: org.glassfish.jersey.internal.ExceptionMapperFactory#find(Class, T)

@coveralls

This comment has been minimized.

coveralls commented Mar 26, 2017

Coverage Status

Coverage increased (+0.01%) to 84.391% when pulling 74a69ee on t-tsutsumi:rfe/jersey-exception-handling into 8c27102 on dropwizard:master.

@arteam arteam merged commit dcd521a into dropwizard:master Mar 27, 2017

@arteam

This comment has been minimized.

Member

arteam commented Mar 27, 2017

Looks good. Thank you for your detailed explanation of this issue. Hopefully https://java.net/jira/browse/JERSEY-2992 get resolved in the future.

@arteam arteam added blocked improvement and removed blocked labels Mar 27, 2017

@arteam arteam added this to the 1.2.0 milestone Mar 27, 2017

arteam added a commit that referenced this pull request Mar 27, 2017

@t-tsutsumi t-tsutsumi deleted the t-tsutsumi:rfe/jersey-exception-handling branch Mar 27, 2017

arteam added a commit that referenced this pull request Mar 27, 2017

aaanders added a commit to aaanders/dropwizard that referenced this pull request Sep 20, 2018

aaanders added a commit to aaanders/dropwizard that referenced this pull request Sep 20, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment