Parse multipart/form-data before it enters the servlet

[jmcc0nn3ll]

migrated from Bugzilla #458126
status ASSIGNED severity normal in component server for 9.2.x
Reported in version 9.3.0 on platform All
Assigned to: Jan Bartel

On 2015-01-22 07:18:16 -0500, Jan Bartel wrote:

As reported in https://bugs.eclipse.org/bugs/show_bug.cgi?id=455655 there is a problem reporting parsing errors of multipart/form-data to the servlet. Jetty-9.2 was modified to throw runtime exceptions when the servlet attempts to retrieve the parts with any of the getPart/Parts methods.

The solution for jetty-9.3 may be to throw any exception much earlier in the request processing, so that servlet is not even entered if there is a serious formatting error in the multipart data.

On 2015-01-22 09:39:13 -0500, Bill Mair wrote:

It should be possible to register a factory to create parser instances that parse "multipart/form-data" (e.g. MultiPartInputStreamParser and MultiPartInputStreamParserFactory).

The current implementation should be changed to implement the interface and if no Factory has been registered by the servlet then the default implementation should be used.

This would allow servlets to parse and verify the data and compensate for erroneous input.

Maybe some kind of Content-Type based request processor registration already exists?

[deki]

Any progress here? Apache MyFaces Tobago uses Commons Fileupload. However it seems that Jetty only allows Servlet API 3.x and we can no longer use Commons Fileupload?!

java.io.IOException: Missing content for multipart request
at org.eclipse.jetty.util.MultiPartInputStreamParser.parse(MultiPartInputStreamParser.java:534)
at org.eclipse.jetty.util.MultiPartInputStreamParser.getParts(MultiPartInputStreamParser.java:422)
at org.eclipse.jetty.server.Request.getParts(Request.java:2311)
at org.eclipse.jetty.server.Request.extractMultipartParameters(Request.java:520)
at org.eclipse.jetty.server.Request.extractContentParameters(Request.java:442)
at org.eclipse.jetty.server.Request.getParameters(Request.java:366)
at org.eclipse.jetty.server.Request.getParameterNames(Request.java:1017)
at javax.servlet.ServletRequestWrapper.getParameterNames(ServletRequestWrapper.java:212)
at javax.servlet.ServletRequestWrapper.getParameterNames(ServletRequestWrapper.java:212)
at javax.servlet.ServletRequestWrapper.getParameterNames(ServletRequestWrapper.java:212)

[sbordet]

@deki, so the client sends crap data and Jetty throws an exception; perhaps it's the client that should be fixed ?

We will happily analyze the issue if we had a reproducible test case. You can attach it to this issue.

We also accept contributions, see https://www.eclipse.org/jetty/documentation/9.3.x/contributing-patches.html.

Thanks !

[joakime]

@deki What version of Jetty? and what version of commons-fileupload?

For troubleshooting, alternatively, you can capture the raw commons-fileupload produced content and provide it for this issue.

[deki]

@joakime Jetty 9.3.13.v20161014, Commons Fileupload 1.3.2

It's reproducible using the Tobago demo with the following steps:

• svn co http://svn.apache.org/repos/asf/myfaces/tobago/branches/tobago-2.0.x/
• edit tobago-example/pom.xml and change jetty version in line 92 to 9.3.13.v20161014
• run mvn clean install
• goto tobago-example/tobago-example-demo/
• run mvn jetty:run -Djsf=myfaces-2.2
• open http://localhost:8080
• navigate to upload page and upload any file
• upload fails with the exception above

As far as I can see the issue and possible solutions were already discussed in #240. I'm happy to contribute, just let me know which one is the preferred solution.

@sbordet it's not related to crap data, the same example is working with Tomcat.

[gregw]

@deki - working in tomcat doesn't necessarily mean compliance with the RFCs and specifications.

My understanding of this issue is that both the tobago filter and something annotated with @MultiPartConfig are expecting to read/parse the multipart data.

In this case, it might just be that tomcat is more forgiving of missing content if something else has consumed it.

Anyway, we do need to look at this in more detail to work out exactly what is happening. However, you could expedite the process by having a little deeper look - what is annotated with @MultiPartConfig? who is calling getParameterNames and are they expecting the parts to be in those names? Did the filter read the content?

[janbartel]

@deki The problem seems to be that Tobago has its own multipart filter processing that I believe is going to clash with the @MultipartConfig annotation that I believe is on the FacesServlet (but please check the src for FacesServlet of the version you are using to confirm that).

As I can see from the Tobago code, there is a TobagoMultipartFormdataFilter configured in web.xml. That filter wraps the inbound request into a TobagoMultipartFormdataRequest, which in its initialization looks like it reads all of the content from the request and does its own multipart parsing.

Thus, by the time that jetty needs to honour the @MultipartConfig annotation on the FacesServlet, there is no content for us to read.

Very difficult for us to tell whether that is because the request is really missing content or some other part of the webapp gobbled up all of the input first.

The only thing I can think of at the moment is for jetty to log the exception but not throw an error: I assume that tomcat is ignoring the lack of content for it to parse for the @MultipartConfig too.

[janbartel]

@deki I've commented over on #240 about a change I've just checked in to jetty 9.3.x and above. I think it should do what you want now. IMHO it would still be better if there wasn't a conflict between the @MultipartConfig annotation on FacesServlet and other filters that consume all the input.

[deki]

@janbartel thanks for the fix. We already solved the conflict in Tobago 3.x. For Tobago 2.x we can't really fix it as the user can use e.g. a JSF 2.0.x implementation without @MultipartConfig annotation on FacesServlet whereas a JSF 2.2.x implementation has the annotation.