Thursday, April 12, 2007

I know why a Struts 2 File Upload fails

Yesterday I began some work on using Struts2 to do a file upload. The way it works is pretty slick. At least when it works.

The problem was that when uploading files, I'd be surprised with null values. However, when pressing the back button and re-submitting the form, it would work fine. Odd behavior indeed, but I'm not alone. Other people have had the same problem uploading files with Struts 2.0.6.

I dug into it a little bit. To quite a deep level actually, and I found the bug. The good news is that it seems to be related specifically to Struts 2.0.6. There's a point in the Filter, which serves as the starting point of the whole framework, where the HttpServletRequest is supposed to be wrapped by the appropriate class for later data retrieval.

In the case of the mysterious broken file upload, the interceptor (FileUploadInterceptor) that handles populating the struts action with the values checks to see if the Request object is an instance of the MultiPartRequestWrapper. If it is, the interceptor attempts to pull out the multipart data; that is, the actual file bytes. If it's not, it does nothing. When the file uploads don't work it means that the Request has not been wrapped in the MultiPartRequestWrapper. Therefore the FileUploadFilter doesn't do anything, even though the data is actually there.

The reason why it doesn't get wrapped relates to the FilterDispatcher (org.apache.struts2.dispatcher package) and the prepareDispatcherAndWrapRequest method. In that method, there's a logic block which deals with the Dispatcher instance. When the instance is null, it sets the Dispatcher instance to one that will work. The peculiar thing about this block, is that the part of the method that does the request wrapping is also in that logic block. Therefore, the request doesn't get wrapped in the MultiPartRequestWrapper unless the Dispatcher instance is null. When the file uploads failed, this instance was NOT null, causing the Request to push through as the RequestFacade. The FileUploadInterceptor doesn't work with that data type.

The fix? Just put the code that does the wrapping outside of the logic block that deals with the dispatcher instance.

To the Struts team's credit, it looks like this bug has been fixed. The code in the repository after the 2.0.6 tag does the wrapping correctly.

Here's some code for those who care.

First, the code that comes with Struts 2.0.6

    protected HttpServletRequest prepareDispatcherAndWrapRequest(HttpServletRequest request, HttpServletResponse response) throws ServletException {

Dispatcher du = Dispatcher.getInstance();

// Prepare and wrap the request if the cleanup filter hasn't already, cleanup filter should be
// configured first before struts2 dispatcher filter, hence when its cleanup filter's turn,
// static instance of Dispatcher should be null.
if (du == null) {

Dispatcher.setInstance(dispatcher);

// prepare the request no matter what - this ensures that the proper character encoding
// is used before invoking the mapper (see WW-9127)
dispatcher.prepare(request, response);

try {
// Wrap request first, just in case it is multipart/form-data
// parameters might not be accessible through before encoding (ww-1278)
request = dispatcher.wrapRequest(request, getServletContext());
} catch (IOException e) {
String message = "Could not wrap servlet request with MultipartRequestWrapper!";
LOG.error(message, e);
throw new ServletException(message, e);
}
}
else {
dispatcher = du;
}
return request;
}

Code that works:


    protected HttpServletRequest prepareDispatcherAndWrapRequest(HttpServletRequest request, HttpServletResponse response) throws ServletException {

Dispatcher du = Dispatcher.getInstance();

// Prepare and wrap the request if the cleanup filter hasn't already, cleanup filter should be
// configured first before struts2 dispatcher filter, hence when its cleanup filter's turn,
// static instance of Dispatcher should be null.
if (du == null) {

Dispatcher.setInstance(dispatcher);

// prepare the request no matter what - this ensures that the proper character encoding
// is used before invoking the mapper (see WW-9127)
dispatcher.prepare(request, response);

}
else {
dispatcher = du;
}


//Note that this wrapping code is where it should be
try {
// Wrap request first, just in case it is multipart/form-data
// parameters might not be accessible through before encoding (ww-1278)
request = dispatcher.wrapRequest(request, getServletContext());
} catch (IOException e) {
String message = "Could not wrap servlet request with MultipartRequestWrapper!";
LOG.error(message, e);
throw new ServletException(message, e);
}
return request;
}

14 comments:

João Guilherme said...

Good one! These null parameters were pissing me off. :)

Joe Pemberton said...

many, many, many thanks!

Anonymous said...
This comment has been removed by a blog administrator.
Anonymous said...

Hi
thanx its realy a very good one
and its solve my problem

Christian Jensen said...

Thanks a lot!

I am using the JARs from Apache... What is the lightest way to integrate you changes into my app - do I have to go and get full source and rebuild it?

Can you post up a 2.0.6 + fix jar file?

Eric said...

Christian -

The good news is that this is really easy to fix. If you add a fixed FilterDispatcher java file to your own source path for your project, it will override the code in the struts.jar. This is what I do.

No need to build the whole struts from source.

So, you'll add this package to your project:

org.apache.struts2.dispatcher


Then grab the source code for the FilterDispatcher from the repository, after it has been fixed. This one should work:


org.apache.struts2.dispatcher.FilterDispatcher


Then, when you deploy your project, you'll have a fixed FilterDispatcher in your WEB-INF/classes folder, which overrides the one in the struts.jar

Hilbert said...

Hello! I read this article! Big thanks to author, very interesting. Write more.

Say said...

Does this problem exist in S2.0.11?

Eric said...

As far as I know, this problem does not exist since 2.0.7. But, I have not tested file uploads in 2.0.11 yet.

Chandana said...

Article is Nice. Iam using Struts 2.0.9. Above mentioned problem didn't exist in that.

Anonymous said...

I changed the FilterDispatcher class acc. to the one u mentioned. but its not working.. what all versions of jars do i need to take?

Anonymous said...

after reading the comment from the author i did more research and i found that you can add the following filter / filter mapping in the web.xml instead of overwriting the Dispacth class



filter:
filter-name: struts-cleanup
filter-class: org.apache.struts2.dispatcher.ActionContextCleanUp



filter-mapping:
filter-name:struts-cleanup
url-pattern:/*



This will force struts 2 version 2.0.6 to create a new dispatch instance.

Guhan said...

Hi,

Currently I am facing same problem in 2.1.8 struts. I am getting all the values as null in action class.

Could you please provide your help to thamayanthi.g@gmail.com

Thanks
Thamayanthi

Valon said...

Thanks very much for your explanation and the Anonymous post re. adding the struts-cleanup filter! File upload seems to be working consistently now with Struts 2.06 (and I now have another reason to upgrade to a later version of Struts 2).