File upload with JakartaMultipartRequest fails if file size less than DiskFileItem.sizeThreshold

• Key: WW-1549
• Type: Bug
• Status: Resolved
• Resolution: Fixed
• Priority: Minor
• Assignee: Don Brown
• Reporter: Tomas Carlsson
• Component/s: Core Interceptors
• Affects Version/s: 2.0.10, 2.1.0
• Fix Version/s: 2.0.2

Description

commons-fileupload DiskFileItem will not write any data to the file system until its sizeThreshold has been reached. This is not considered in JakartaMultipartRequest. It should check this when getting the File object that is later injected to an Action.

Comments

[Tomas Carlsson]

Quick fix for this problem.

[Tomas Carlsson]

IGNORE the first patch that I attached.

I realized that the real reason for this bug is that the property "struts.multipart.maxSize" is used in the wrong way. Instead of actually limiting the upload size by calling ServletFileUpload.setSizeMax() it is used to set the amount of bytes to store in memory before writing to disk (DiskFileItemFactory.setSizeThreshold()).

This new patch (s2_28850_v2.patch) fixes this problem.

[Don Brown]

Fixed, thanks for the patch!

By adding upload.setSizeMax to JakartaMultiPartRequest, the file upload interceptor max size validation has been disabled. Furthermore, whenever there is a single file above the allowed size none of the request parameters are set on the action This happens due to upload.parseRequest(createRequestContext(servletRequest)) which throws exception (o.a.c.fileupload.FileItemIteratorImpl checks max size).
If the application requires the parameters it will fail.

There is also slight confusion and disconnect between:
struts.multipart.maxSize and fileInterceptor.maxsize

If you upload file that exceeds allowed struts.multipart.maxSize fileInterceptor will never kick in.

IMHO file upload interceptor should be responsible for max size validation (esp. that it reports errors as field errors and not as action errors).
Furthermore, we should drop 'struts.multipart.maxSize' since is confusing along with fileInterceptor.maxsize, also fileInterceptor.maxsize should get a new default max size. The other option is to use struts.multipart.maxSize to set fileInterceptor.maxsize.

This line should be removed so file interceptor can validate files:
------
Index: src/main/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequest.java
===================================================================
--- src/main/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequest.java (revision 560614)
+++ src/main/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequest.java (working copy)
@@ -86,7 +86,6 @@
         // Parse the request
         try {
             ServletFileUpload upload = new ServletFileUpload(fac);
- upload.setSizeMax(maxSize);
             List items = upload.parseRequest(createRequestContext(servletRequest));

--------------

[James Holmes]

Lukasz, you need to re-open this ticket or create another one if you want changes to be made. This ticket will not "show up on the radar" because it currently has a status of resolved.

[Lukasz Racon]

New ticked has been created for this issue: https://issues.apache.org/struts/browse/WW-2073

PS. Thanks for the reminder. I was trying to check if original issue reporter/person who fixed it, had more reasons behind adding "upload.setSizeMax(maxSize); "