[jira] Commented: (TOMAHAWK-1381) HtmlInputFileUpload does not fail gracefully when filesize exceeds uploadMaxFileSize web.xml value

["Leonardo Uribe (JIRA)" <de...@myfaces.apache.org>]

    [ https://issues.apache.org/jira/browse/TOMAHAWK-1381?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12716096#action_12716096 ] 

Leonardo Uribe commented on TOMAHAWK-1381:
------------------------------------------

I have checked the patch and made some modifications to make it even better:

1. There is no need to replace UploadedFileDefaultFileImpl. Really, there is an error related on TOMAHAWK-1208, and the solution is do what is proposed on the patch: put a serialVersionUID to prevent java.io.NotSerializableException (I'm not sure if this works, so I'll do some tests and/or corrections and update the patch)

2. Create a new param called cacheFileSizeErrors on ExtensionsFilter (org.apache.myfaces.UPLOAD_CACHE_FILE_SIZE_ERRORS when TomahawkFacesContextWrapper is used instead) so the user can decide when process non file upload fields even if a file upload size is exceeded. A new class called org.apache.myfaces.custom.fileupload.ServletChacheFileSizeErrorsFileUpload was created to hold the related code there instead on MultipartRequestWrapper. Note that the objective of do this is preserve previous behavior and provide the enhanced one.

3. Now, getStorage() property has the following behavior:

    /**
     * This setting was intended to allow control over how the contents of the
     * file get temporarily stored during processing.
     * <p> It allows three options<p>
     * <ul>
     * <li>"default": The file is handled on memory while the file size is below 
     * uploadThresholdSize value, otherwise is handled on disk or file storage when
     * decode occur (set submitted value)</li>
     * <li>"memory": The file is loaded to memory when decode occur 
     * (set submitted value). In other words, before set the uploaded file as 
     * submitted value it is loaded to memory. Use with caution, because it
     * could cause OutOfMemory exceptions when the uploaded files are too big. </li>
     * <li>"file": The file is handled on disk or file storage.</li>
     * </ul>
     * 
     * @JSFProperty
     */
    public abstract String getStorage();

   This is more consistent to the user expected behavior (see TOMAHAWK-1420).

The only problem with this patch is that changes the semantic of uploadMaxFileSize (see FILEUPLOAD-168 for related discussions). Take a look at this one from Martin Cooper:

     "...  The semantic of sizeMax is specifically "the maximum allowed size of a complete request". That is, if the complete request is larger than   
     sizeMax, regardless of how many files or fields are being uploaded within it, it is rejected, because that is exactly what sizeMax is defined to do.

     The semantic of fileSizeMax is "the maximum size permitted for a single uploaded file", which allows the processing of the entire request, but 
     rejecting any individual files that exceed the specified maximum size for a single file.

     Most of the issues being raised in this report could be resolved by simply using fileSizeMax instead of sizeMax. Trying to use sizeMax for this 
     does not make sense, because it does not make sense to allow a request to be processed that specifically violates the semantic of sizeMax.

     If that still doesn't satisfy, the streaming mode was implemented to provide more flexibility than sizeMax and fileSize Max can do on their own, 
     so if you need that additional flexibility, the streaming mode was designed for you.....".

Theorically, t:inputFileUpload just handle one file per request, so there is no problem and since the default option is use ServletFileUpload.parseRequest my opinion is that we can commit this code.

Suggestions are welcome. If no objections, I'll commit this patch soon (I'll review some parts first).


> HtmlInputFileUpload does not fail gracefully when filesize exceeds uploadMaxFileSize web.xml value
> --------------------------------------------------------------------------------------------------
>
>                 Key: TOMAHAWK-1381
>                 URL: https://issues.apache.org/jira/browse/TOMAHAWK-1381
>             Project: MyFaces Tomahawk
>          Issue Type: Bug
>    Affects Versions: 1.1.7, 1.1.8
>            Reporter: Phillip Webb
>         Attachments: file-upload.patch, file-upload2.patch, TOMAHAWK-1381-fileupload3.patch
>
>
> When uploading a file using the HtmlInputFileUpload that exceeds that uploadMaxFileSize web.xml setting the system fails without error and no form data is not processed by JSF.
> I think that there are a number of reasons for this:
> - There is a bug in commons-fileupload that prevents failures from being handled correctly (see FILEUPLOAD-169)
> - MultipartRequestWrapper calls FileUploads setSizeMax, this is the size for the total upload, and not individual files, it should call setFileSizeMax
> - HtmlFileUploadRenderer calls fileUpload.parseRequest(request), this will fail if any size exceptions are thrown, it would be better to use fileUpload.getItemIterator and catch each exception.

-- 
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.