Skip to content
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

DispatcherServlet's multipart request parsing fails during Jetty error dispatch [SPR-15231] #19796

Closed
spring-issuemaster opened this issue Feb 8, 2017 · 3 comments

Comments

@spring-issuemaster
Copy link
Collaborator

commented Feb 8, 2017

Gunnar Hillert opened SPR-15231 and commented

I have a simple example where I upload files using Jetty. If a user uploads a file larger than permitted, an @ExceptionHandler(MultipartException.class) will perform a redirect to a page informing the user of the issue.

This works as expected using Jetty 9.2.11.v20150529 but fails with 9.4.1.v20170120.

In fact the ExceptionHandler is being triggered, but the redirect is ignored and the user gets a ERR_CONNECTION_RESET page (Chrome).

Not sure how far this is related to #19507


Affects: 4.3.5

Issue Links:

  • #19744 DispatcherServet.checkMultipart() does not consider javax.servlet.error.exception that has a MultipartException cause

Referenced from: commits d44325e, 4d2360e

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 10, 2017

Brian Clozel commented

Rossen Stoyanchev Juergen Hoeller

It seems there was a behavior change between Jetty 9.3.7.v20160115 and 9.3.8.v20160314.

In a Spring Boot application configured to reject multipart files bigger than a certain size, when such a file is uploaded, different things happen. Note that Spring Boot configures Jetty Error handlers to dispatch back errors to the "/error" page and the ErrorController.

  1. the container throws an IllegalStateException saying that the file exceeds the allowed size
  2. this request is dispatched to the configured error page, and the dedicated error request attribute contains the IllegalStateException
  3. the DispatcherServlet detects that request as a multipart request with no MultipartException in the dedicated error request attribute
  4. the StandardServletMultipartResolver attempts to create a StandardMultipartHttpServletRequest, which calls request.getParts()

In Jetty 9.3.7.v20160115, the parts are properly returned and the request is dispatched to the error controller for error rendering.

In Jetty 9.3.8.v20160314, the getParts call throws an IllegalStateException and the processing ends with a dispatchException, no custom error is rendered, only the default HTTP 500.

The current behavior is odd - why should we even process the request as multipart if the request is already an error dispatch? Shouldn't we check for that case and not process the request?

It seems we're already bypassing all this if the request is already a multipart request:

if(WebUtils.getNativeRequest(request, MultipartHttpServletRequest.class) != null) {
                this.logger.debug("Request is already a MultipartHttpServletRequest - if not in a forward, this typically results from an additional MultipartFilter in web.xml");
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 11, 2017

Juergen Hoeller commented

Alright, I'll revise that DispatcherServlet multipart check to be more defensive...

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 13, 2017

Juergen Hoeller commented

As of 4.3.7, we're still trying to resolve a multipart request but defensively catch and log a failure there now, proceeding with the regular request for the error dispatch in such a case.

It is indeed questionable whether we even need to bother processing a multipart request for an error dispatch in the first place. However, since even plain request parameters (from a POST body) will only be accessible after multipart parsing, it seems beneficial to nevertheless try (but not insist).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.