6

Using Struts 2.3.15.1

Implementing file upload in struts2. This is something I've done a number of times, however, I'm trying to include some sanity checks (i.e. max file size primarily). I have the fileUpload interceptor in place as the last interceptor in my stack (i.e. struts.xml). My stack includes a few in-house interceptors as well as the validationWorkflowStack. I've set the following property in my struts.properties file:

struts.multipart.maxSize = 2000000

In addition to the file upload, I'm passing a few other params in my form. Form is defined as:

<s:form action="addResource" method="post" enctype="multipart/form-data"> 
  <s:hidden name="rfqId" value='%{rfq.id}' />
  <s:file name="uploadFile" id="uploadFile" label="File" size="40" value=""/>
  ....
</s:form>

As I'm sure we all know, the validationWorkflowStack includes the params interceptor, which sets the request params onto the action. Here's the issue, when the file being uploaded exceeds the maxSize, there are no params for the params interceptor to set. I've stepped through is and there's nothing in the actionContext. This is not good, because I need those params to handle the INPUT error that will result.

Am I missing something?

Roman C
  • 49,761
  • 33
  • 66
  • 176
fmpdmb
  • 1,370
  • 3
  • 21
  • 34
  • Any reason to include the `fileUpload` interceptor as the last interceptor in your stack? – Aleksandr M Aug 06 '13 at 17:12
  • As opposed to a different location or not at all? Is it now in the default stack or something? For what it's worth, I'm tried having the interceptor first/middle/last/etc to no avail. – fmpdmb Aug 06 '13 at 17:38
  • As opposed to first or somewhere in the middle like in the `defaultStack`. Order of interceptors matters. The `fileUpload` is in the `defaultStack` but not in the `basicStack`. – Aleksandr M Aug 06 '13 at 17:48
  • You should post the configuration files: web.xml, struts.xml, action code. Without it is difficult to tell what are you asking. – Roman C Aug 06 '13 at 20:43
  • @Roman C - yes and no... I'm fairly certain the issue I'm encountering is related to how the struts2 MultiPartRequestWrapper is handling the error or the MultiPartRequest itself. I really don't think the action code or the other files you mention will help. – fmpdmb Aug 06 '13 at 23:15
  • @fmpdmb I'm not sure it's an issue, that why I was needed aditional info but if you persist on it you will be happy to receive a bunch of answers like below. – Roman C Aug 07 '13 at 07:24
  • 1
    What's wrong with the answer below? Say what you think is happening, I'll be happy to be proven wrong and learn something new. Looking at the source code, it seems like that to me, I'll be happy to debug it when I'll be back to my pc next month – Andrea Ligios Aug 07 '13 at 19:55
  • What happens if you put the upload field last in the form, so that all other form fields come before it? Are the other parameters preserved then? – erickson Aug 08 '13 at 16:25
  • @erickson - Currently, the upload field in nearly the last one (ie. there are others before it). I could certainly try moving it down, but I don't see how that would help. I'm not sure how I'd go about rearranging the order of the fields on submit, but I'm sure it's doable. In looking at the implementation of `JakartaMultiPartRequest.processUpload()`, the struts default impl, I don't think this approach would help. One approach I considered was implementing my own `MultiPartRequest`. – fmpdmb Aug 09 '13 at 14:59
  • @fmpdmb please check out the edited answer, hope that helps ;) – Andrea Ligios Sep 15 '14 at 10:08

3 Answers3

6

Problem solved !

From the updated documentation, now the problem can be solved by using the new JakartaStreamMultiPartRequest :

As from Struts version 2.3.18 a new implementation of MultiPartRequest was added - JakartaStreamMultiPartRequest. It can be used to handle large files, see WW-3025 for more details, but you can simple set

<constant name="struts.multipart.parser" value="jakarta-stream" />

in struts.xml to start using it.

From the linked JIRA body :

When any size limits exceed, immediately a FileUploadBase.SizeLimitExceededException or FileUploadBase.FileSizeLimitExceededException is thrown and parsing of the multipart request terminates without providing request parameters for further processing.

This basically makes it impossible for any web application to handle size limit exceeded cases gracefully.

My proposal is that request parsing should always complete to deliver the request parameters. Size limit exceeded cases/exceptions might be collected for later retrieval, FileSizeLimitExeededException should be mapped to the FileItem to allow some validation on the FileItem on application level. This would allow to mark upload input fields as erronous if the uploaded file was too big.

Actually I made a patch for that (see attachment). With this patch, commons-fileupload always completes request parsing in case of size limit exceedings and only after complete parsing will throw an exception if one was detected.

and Chris Cranford's comment:

I am working on a new multipart parser for Struts2 I am calling JakartaStreamMultiPartRequest.

This multi-part parser behaves identical to the existing Jakarta multi-part parser except that it uses the Commons FileUpload Streaming API and rather than delegating maximum request size check to the File Upload API, it's done internally to avoid the existing problem of the Upload API breaking the loop iteration and parameters being lost.

Awesome, thanks guys :)


Old answer

I guess it is due to the different behavior of

  • a single file (or more files) that is exceeding its maximum defined size, and then can be redirected back at the end of a normal process with the INPUT result, and
  • the violation of the maximum size of the entire Request, that will (probably?) break any other element parsing, because it is a security mechanism, not a feature like the file size check;

When the files are parsed first (it should depend on their order in the page), if a file breaks the limit of the multipart request size, the other fields (the form fields) won't be read and hence not returned back with the INPUT result.

Struts2 uses the Jakarta implementation for the MultiPartRequestWrapper:

struts.multipart.parser - This property should be set to a class that extends MultiPartRequest. Currently, the framework ships with the Jakarta FileUpload implementation.

You can find the source code on Struts2 official site or here (faster to google); this is what is called when posting a multipart form:

 public void parse(HttpServletRequest request, String saveDir) throws IOException {
        try {
            setLocale(request);
            processUpload(request, saveDir);
        } catch (FileUploadBase.SizeLimitExceededException e) {
            if (LOG.isWarnEnabled()) {
                LOG.warn("Request exceeded size limit!", e);
            }
            String errorMessage = buildErrorMessage(e, new Object[]{e.getPermittedSize(), e.getActualSize()});
            if (!errors.contains(errorMessage)) {
                errors.add(errorMessage);
            }
        } catch (Exception e) {
            if (LOG.isWarnEnabled()) {
                LOG.warn("Unable to parse request", e);
            }
            String errorMessage = buildErrorMessage(e, new Object[]{});
            if (!errors.contains(errorMessage)) {
                errors.add(errorMessage);
            }
        }
    }

then, this is where it cycles the multipart Items, both files and form fields:

   private void processUpload(HttpServletRequest request, String saveDir) throws FileUploadException, UnsupportedEncodingException {
        for (FileItem item : parseRequest(request, saveDir)) {
            if (LOG.isDebugEnabled()) {
                LOG.debug("Found item " + item.getFieldName());
            }
            if (item.isFormField()) {
                processNormalFormField(item, request.getCharacterEncoding());
            } else {
                processFileField(item);
            }
        }
    }

that will end, in the FileUploadBase, in this implementation for each item:

 FileItemStreamImpl(String pName, String pFieldName,
                    String pContentType, boolean pFormField,
                    long pContentLength) throws IOException {
                name = pName;
                fieldName = pFieldName;
                contentType = pContentType;
                formField = pFormField;
                final ItemInputStream itemStream = multi.newInputStream();
                InputStream istream = itemStream;
                if (fileSizeMax != -1) {
                    if (pContentLength != -1
                            &&  pContentLength > fileSizeMax) {
                        FileSizeLimitExceededException e =
                            new FileSizeLimitExceededException(
                                format("The field %s exceeds its maximum permitted size of %s bytes.",
                                       fieldName, fileSizeMax),
                                pContentLength, fileSizeMax);
                        e.setFileName(pName);
                        e.setFieldName(pFieldName);
                        throw new FileUploadIOException(e);
                    }
                    istream = new LimitedInputStream(istream, fileSizeMax) {
                        @Override
                        protected void raiseError(long pSizeMax, long pCount)
                                throws IOException {
                            itemStream.close(true);
                            FileSizeLimitExceededException e =
                                new FileSizeLimitExceededException(
                                    format("The field %s exceeds its maximum permitted size of %s bytes.",
                                           fieldName, pSizeMax),
                                    pCount, pSizeMax);
                            e.setFieldName(fieldName);
                            e.setFileName(name);
                            throw new FileUploadIOException(e);
                        }
                    };
                }
                stream = istream;
            }

as you can see, it handles pretty differently the file size cap and the request size cap;

I've looked at the source for fun but you could really confirm (or correct) this assumptions, trying to debug the MultiPartRequestWrapper to see if what happens inside is what I think is going on... good luck and have fun.

Marco Borchert
  • 1,038
  • 1
  • 11
  • 17
Andrea Ligios
  • 49,480
  • 26
  • 114
  • 243
  • 2
    I don't see a suggestion here, rather an explanation of what's going on, which I knew. The MultiPartRequest implementation is adding an action error and bailing out before processing the other form variables. The flow continues down the interceptor stack which ends up causing a problem. My attempted solution has involved putting the fileUpload first and following it by the workflow interceptor to bail out if any errors exist and having a unique result type that I map to some very unfriendly page for the user since I've lost my request params. – fmpdmb Aug 07 '13 at 22:09
  • **IF** this is what is going on (the request being cut due to security reason) **THEN** there is nothing you can do; there will always be a limit that, once hit, will cut the request, even not setting a limit (it will be the default limit). IF it is not, THEN explain why not and how. I'm not 100% sure it is like that, just confident that it is like that by looking at the source code. Not every question can be answered with "suggestions", but IF it is like that, this is the right answer the same. The only suggestion is to check file size client side with the HTML5 way. – Andrea Ligios Aug 16 '13 at 13:05
  • The solution I came up with seems to be working well at this point. – fmpdmb Aug 22 '13 at 17:12
  • I tested it now, it is like that ;) In your answer you have simply chosen to redirect an user to an error page instead of redirecting it to the source page with the fields blank. It could be better or not, but I'd rather call that a "solution". Multipart size check is definitely a security measure, it should not be handled nicely; just use a value high enough, put the **file size** check both server and client side, and if a user is trying to break your multipart limit, he is not necessarily deserving to be redirected back with all the fields populated, because probably is trying to flood. – Andrea Ligios Sep 02 '13 at 09:42
1

Here's how I've worked around this issue. I wouldn't call this a solution.

enter image description here

fmpdmb
  • 1,370
  • 3
  • 21
  • 34
0

Try putting a javascript check at at the early stage :

<!DOCTYPE html>
<html>

    <head>
    <script type="text/javascript">
    function checkSize(max_img_size)
    {
        var input = document.getElementById("upload");
        // check for browser support (may need to be modified)
        if(input.files && input.files.length == 1)
        {           
            if (input.files[0].size > max_img_size) 
            {
                alert("The file must be less than " + (max_img_size/1024/1024) + "MB");
                return false;
            }
        }

        return true;
    }
    </script>
    </head>
    <body>
    <form action="demo_post_enctype.asp" method="post" enctype="multipart/form-data" 
    onsubmit="return checkSize(2097152)">    
    <input type="file" id="upload" />
    <input type="submit" />

    </body>
    </html>
Nikhil Maheshwari
  • 2,198
  • 18
  • 25