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

Missing content for multipart request after upgrade to Jetty > 9.2.7 #240

Closed
jmcc0nn3ll opened this issue Feb 16, 2016 · 7 comments
Closed
Labels
Bug For general bugs on Jetty side

Comments

@jmcc0nn3ll
Copy link
Contributor

migrated from Bugzilla #480063
status ASSIGNED severity normal in component server for 9.3.x
Reported in version 9.2.7 on platform All
Assigned to: Project Inbox

On 2015-10-18 09:30:22 -0400, Yann Biancheri wrote:

I've upgraded my application server from Jetty 9.2.3 to 9.2.13. Shortly after, it appears that file upload using tomahawk and jsf 2.2 stopped working.
Before the upgrade I had this warning "Missing content for multipart request". After the Jetty upgrade the warning is promoted to an exception and I have a 500 error page.

After some investigations the last working version for my application is Jetty 9.2.6. I believe the issue is related to the resolution of this bug https://bugs.eclipse.org/bugs/show_bug.cgi?id=455655

From my understanding it seems that tomahawk's extension filter has consumed the request stream before the servlet has a chance to do so but I'm no expert.

I have produced a minimal test case with instructions to reproduce the issue here https://github.com/yannooo/jetty-tomahawk-issue
I've deployed this app to the latest version of Tomcat 7 and 8 and it worked without issue.

On 2015-10-21 05:24:24 -0400, Jan Bartel wrote:

Yann,

Hmm. The difficulty is that the FacesServlet is annotated with @MultiPartConfig, which means that it is expecting to access the parts. We delay parsing of the content until code calls one of the getParameter methods, or getPart/s. If a filter has already hungrily consumed all of the content beforehand, then that's a problem.

We already have an issue open to change the point at which the input is parsed, before a filter could get to it: https://bugs.eclipse.org/bugs/show_bug.cgi?id=458126

If you're only using the ExtensionFilter to do the multipart processing, then you can remove it because jetty will parse it for you. If you're using it for some other reason, then that won't work for you.

Jan

On 2015-10-28 16:07:39 -0400, Yann Biancheri wrote:

Thanks for your answer but my application is using the ExtensionFilter for other purposes. Also if I remove it from my sample app it causes an error when uploading files.

In my case, the best seems to disable the scanning of @MultiPartConfig annotation.

Our production package embeds jetty and I just removed org.eclipse.jetty.annotations.AnnotationConfiguration and can confirm everything works fine.

For development purpose we are using jetty-maven-plugin but I haven't found a way to prevent the scanning of annotation. Do you have any recommendations?

On 2015-10-28 19:54:37 -0400, Jan Bartel wrote:

Yann,

I am surprised that removing AnnotationConfiguration is working - all modern jsp engines use a ServletContainerInitializer to set themselves up, and AnnotationConfiguration is the class that makes sure they are found and applied.

Seems like the easiest thing to do would be to let jetty do the parsing for you by adding a filter as the first one in the chain that just calls one of the getParameter() methods. That way jetty does the parsing, the parts are available to the FacesServlet, or any other code that calls getParts() or getParameter().

You could also add a filter that was the last one in the chain and it called getParameter() too, catching a RuntimeException, opening it to check if it is wrapping an IOException with message of "Missing content for multipart request" and ignoring it if so.

I've tested both of these solutions and they both work, so you pick which you prefer.

The best and final solution would be as I mentioned before, an implementation for bug https://bugs.eclipse.org/bugs/show_bug.cgi?id=458126.

Jan

@jmcc0nn3ll jmcc0nn3ll added the Bug For general bugs on Jetty side label Feb 16, 2016
@joakime
Copy link
Contributor

joakime commented Mar 2, 2016

Closing this issue, as its a duplicate of Issue #182

@joakime joakime closed this as completed Mar 2, 2016
@janbartel janbartel reopened this Nov 2, 2016
janbartel added a commit that referenced this issue Nov 2, 2016
No error if no parts because input stream already consumed.
@janbartel
Copy link
Contributor

I committed a change for jetty-9.3 and beyond. The change will NOT throw an exception if the content input stream has already been consumed: in this case Request.getParts() will just return an empty collection. However, if the input stream has not already been consumed, the existing multipart parsing behaviour is retained: that is, you may get an exception if the multipart is badly formatted/missing.

@deki
Copy link

deki commented Dec 14, 2016

I've just tested it with jetty-9.4.0.v20161208 and it stills fails. Looking at the code the exception is still thrown in line 554:
java.io.IOException: Missing content for multipart request
at org.eclipse.jetty.util.MultiPartInputStreamParser.parse(MultiPartInputStreamParser.java:554)
at org.eclipse.jetty.util.MultiPartInputStreamParser.getParts(MultiPartInputStreamParser.java:441)
at org.eclipse.jetty.server.Request.getParts(Request.java:2310)
at org.eclipse.jetty.server.Request.extractMultipartParameters(Request.java:517)
at org.eclipse.jetty.server.Request.extractContentParameters(Request.java:439)
at org.eclipse.jetty.server.Request.getParameters(Request.java:363)
at org.eclipse.jetty.server.Request.getParameterNames(Request.java:1015)

@joakime
Copy link
Contributor

joakime commented Dec 14, 2016

@deki please file a new issue.

Include how you are using jetty.
What client / client-libs are you using, and their versions.

@deki
Copy link

deki commented Dec 14, 2016

Hmm does it make sense to have three issues for the same root cause? I already described how to reproduce it in #182

@joakime
Copy link
Contributor

joakime commented Dec 14, 2016

@deki then keep the conversation in issue #182, no point resurrecting this older issue.

@joakime
Copy link
Contributor

joakime commented Dec 14, 2016

@deki I've changed my mind. file a new issue. This new issue can be labeled as a regression introduced in the 9.4.0 release.

We still need reproduction information though, as the reproduction steps in Issue #182 from Nov 1st still do not apply with the mix of Jetty 9 and Servlet 3.0+ and @MultiPartConfig together (as @janbartel and @gregw pointed out already)

To state this a different way ...

If a servlet is annotated with @MultiPartConfig then the HttpServletRequest.getInputStream() is parsed before the servlet (and its associated filterchain) is called, and the various Servlet 3.0+ fields (such as) HttpServletRequest.getParts() is populated, you cannot then use commons-fileupload and its ServletFileUpload.parseRequest(request) to parse the Request InputStream as its already been consumed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side
Projects
None yet
Development

No branches or pull requests

4 participants