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

Spring MVC MultiPartException not thrown in Jetty as it is in Tomcat when upload part is too big #81

Closed
joakime opened this issue Feb 16, 2016 · 5 comments
Assignees
Milestone

Comments

@joakime
Copy link
Contributor

joakime commented Feb 16, 2016

http://stackoverflow.com/questions/35413971/spring-boot-jetty-catch-upload-error-file-too-big

It seems that Spring's org.springframework.web.multipart.MultipartException is not thrown when running under Jetty, as it is on Tomcat.
In Jetty 9.2.14, we throw a IllegalStateException, which isn't what Spring is expecting.

java.lang.IllegalStateException: Multipart Mime part file exceeds max filesize
at org.eclipse.jetty.util.MultiPartInputStreamParser$MultiPart.write(MultiPartInputStreamParser.java:111) ~[jetty-util-9.2.14.v20151106.jar:9.2.14.v20151106]
at org.eclipse.jetty.util.MultiPartInputStreamParser.parse(MultiPartInputStreamParser.java:681) ~[jetty-util-9.2.14.v20151106.jar:9.2.14.v20151106]
at org.eclipse.jetty.util.MultiPartInputStreamParser.getParts(MultiPartInputStreamParser.java:400) ~[jetty-util-9.2.14.v20151106.jar:9.2.14.v20151106]
at org.eclipse.jetty.server.Request.getParts(Request.java:2146) ~[jetty-server-9.2.14.v20151106.jar:9.2.14.v20151106]
at org.eclipse.jetty.server.Request.extractMultipartParameters(Request.java:386) ~[jetty-server-9.2.14.v20151106.jar:9.2.14.v20151106]
at org.eclipse.jetty.server.Request.extractContentParameters(Request.java:309) ~[jetty-server-9.2.14.v20151106.jar:9.2.14.v20151106]
at org.eclipse.jetty.server.Request.extractParameters(Request.java:257) ~[jetty-server-9.2.14.v20151106.jar:9.2.14.v20151106]
at org.eclipse.jetty.server.Request.getParameter(Request.java:826) ~[jetty-server-9.2.14.v20151106.jar:9.2.14.v20151106]
at javax.servlet.ServletRequestWrapper.getParameter(ServletRequestWrapper.java:194) ~[javax.servlet-api-3.1.0.jar:3.1.0]
at org.springframework.web.filter.HiddenHttpMethodFilter.doFilterInternal(HiddenHttpMethodFilter.java:70) ~[spring-web-4.2.3.RELEASE.jar:4.2.3.RELEASE]
at org.springframework.web.filter.OncePerRequestFilter.doFilter(OncePerRequestFilter.java:107) ~[spring-web-4.2.3.RELEASE.jar:4.2.3.RELEASE]
@joakime joakime modified the milestone: 9.2.x Feb 16, 2016
@wilkinsona
Copy link
Contributor

@joakime Thanks for opening this and #82.

In Jetty 9.2.14, we throw a IllegalStateException, which isn't what Spring is expecting.

It's not so much the Spring doesn't expect an IllegalStateException, but that the exception doesn't happen at the same time as it does when using Tomcat. Very early in the handling of a request, DispatcherServlet tries to create a StandardMultipartHttpServletRequest. It tries to retrieve all of the request's parts. Any exception that's thrown at this stage will be wrapped in a MultipartException. Tomcat throws an exception at this point but Jetty does not.

@janbartel janbartel self-assigned this Feb 16, 2016
@janbartel
Copy link
Contributor

Related bug: https://bugs.eclipse.org/bugs/show_bug.cgi?id=455655

Jetty parses the multipart data either when one of the Request.getParameter/s() methods are called, or when Request.getParts() is called. If an exception happens during the parsing as a result of calling Request.getParameter/s() it was difficult to communicate that back to the client, due to the signature of getParameter/s(), so we added a throw of a RuntimeException to wrap the real exception.

So to clarify, I expect the current behaviour for jetty to be:

  • if parsing initiated on a call to getParameter/s() then log the Exception and throw it wrapped in a RuntimeException. Subsequent calls to getParameter/s() will not throw, and neither would subsequent calls to getPart/s().
  • if parsing initiated on a call to getPart/s() then an exception would be thrown. Subsequent calls to getPart/s() or getParameter/s() would not throw an exception.

So if I understand the bug report correctly, you're asking that if an exception occurs during multipart parsing, you want that exception returned every time getPart/s() is called? So that even if your code causes the parsing by calling getParameter/s() initially and the RuntimeException is thrown, your code is still likely to go on to call getPart/s() and you want the IOException or ServletException at that time, yes?

janbartel added a commit that referenced this issue Feb 16, 2016
…load part is too big

Issue #82 Request.getPart() that results in Exception still allows other parts to be fetched
@janbartel
Copy link
Contributor

Assuming my understanding of the issue is correct as per my previous message, I've pushed a fix for this to the jetty-9.3.x branch: 11d3448

If you'd like to test it out, that would be great.

@joakime
Copy link
Contributor Author

joakime commented Feb 16, 2016

The bug was reported against Jetty 9.2.x, can this fix be cherry-picked back to the jetty-9.2.x branch?

@wilkinsona
Copy link
Contributor

@janbartel The latest 9.3.8 snapshot looks good. Thank you. The IllegalStateException is now thrown as part of Spring's DispatcherServlet's checkMultipart processing which gives it a chance to catch it and wrap it in a MultipartException.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants