diff --git a/core/src/main/java/org/apache/struts2/StrutsConstants.java b/core/src/main/java/org/apache/struts2/StrutsConstants.java index dc5720b71..c49ceda38 100644 --- a/core/src/main/java/org/apache/struts2/StrutsConstants.java +++ b/core/src/main/java/org/apache/struts2/StrutsConstants.java @@ -139,12 +139,14 @@ public final class StrutsConstants { /** A global flag to enable/disable html body escaping in tags, can be overwritten per tag */ public static final String STRUTS_UI_ESCAPE_HTML_BODY = "struts.ui.escapeHtmlBody"; - /** The maximize size of a multipart request (file upload) */ + /** The maximum size of a multipart request (file upload) */ public static final String STRUTS_MULTIPART_MAXSIZE = "struts.multipart.maxSize"; - /** The maximized number of files allowed to upload */ + /** The maximum number of files allowed in a multipart request */ public static final String STRUTS_MULTIPART_MAXFILES = "struts.multipart.maxFiles"; + /** The maximum size per file in a multipart request */ + public static final String STRUTS_MULTIPART_MAXFILESIZE = "struts.multipart.maxFileSize"; /** The directory to use for storing uploaded files */ public static final String STRUTS_MULTIPART_SAVEDIR = "struts.multipart.saveDir"; diff --git a/core/src/main/java/org/apache/struts2/config/entities/ConstantConfig.java b/core/src/main/java/org/apache/struts2/config/entities/ConstantConfig.java index 107f35e51..85e1ef69c 100644 --- a/core/src/main/java/org/apache/struts2/config/entities/ConstantConfig.java +++ b/core/src/main/java/org/apache/struts2/config/entities/ConstantConfig.java @@ -18,6 +18,10 @@ */ package org.apache.struts2.config.entities; +import org.apache.commons.lang3.StringUtils; +import org.apache.struts2.StrutsConstants; +import org.apache.struts2.dispatcher.StaticContentLoader; + import java.util.ArrayList; import java.util.HashMap; import java.util.List; @@ -27,10 +31,6 @@ import java.util.Objects; import java.util.Set; import java.util.regex.Pattern; -import org.apache.commons.lang3.StringUtils; -import org.apache.struts2.StrutsConstants; -import org.apache.struts2.dispatcher.StaticContentLoader; - public class ConstantConfig { private Boolean devMode; private Boolean i18nReload; @@ -65,6 +65,7 @@ public class ConstantConfig { private String uiThemeExpansionToken; private Long multipartMaxSize; private Long multipartMaxFiles; + private Long multipartMaxFileSize; private String multipartSaveDir; private Integer multipartBufferSize; private BeanConfig multipartParser; @@ -197,6 +198,7 @@ public class ConstantConfig { map.put(StrutsConstants.STRUTS_UI_THEME_EXPANSION_TOKEN, uiThemeExpansionToken); map.put(StrutsConstants.STRUTS_MULTIPART_MAXSIZE, Objects.toString(multipartMaxSize, null)); map.put(StrutsConstants.STRUTS_MULTIPART_MAXFILES, Objects.toString(multipartMaxFiles, null)); + map.put(StrutsConstants.STRUTS_MULTIPART_MAXFILESIZE, Objects.toString(multipartMaxFileSize, null)); map.put(StrutsConstants.STRUTS_MULTIPART_SAVEDIR, multipartSaveDir); map.put(StrutsConstants.STRUTS_MULTIPART_BUFFERSIZE, Objects.toString(multipartBufferSize, null)); map.put(StrutsConstants.STRUTS_MULTIPART_PARSER, beanConfToString(multipartParser)); @@ -589,6 +591,14 @@ public class ConstantConfig { this.multipartMaxFiles = multipartMaxFiles; } + public Long getMultipartMaxFileSize() { + return multipartMaxFileSize; + } + + public void setMultipartMaxFileSize(Long multipartMaxFileSize) { + this.multipartMaxFileSize = multipartMaxFileSize; + } + public String getMultipartSaveDir() { return multipartSaveDir; } diff --git a/core/src/main/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequest.java b/core/src/main/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequest.java index e46cecc00..a4616a0ca 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequest.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequest.java @@ -58,6 +58,11 @@ public abstract class AbstractMultiPartRequest implements MultiPartRequest { */ protected Long maxFiles; + /** + * Specifies the maximum size per file in the request. + */ + protected Long maxFileSize; + /** * Specifies the buffer size to use during streaming. */ @@ -84,7 +89,7 @@ public abstract class AbstractMultiPartRequest implements MultiPartRequest { } /** - * @param maxSize Injects the Struts multiple part maximum size. + * @param maxSize Injects the Struts multipart request maximum size. */ @Inject(StrutsConstants.STRUTS_MULTIPART_MAXSIZE) public void setMaxSize(String maxSize) { @@ -96,6 +101,11 @@ public abstract class AbstractMultiPartRequest implements MultiPartRequest { this.maxFiles = Long.parseLong(maxFiles); } + @Inject(StrutsConstants.STRUTS_MULTIPART_MAXFILESIZE) + public void setMaxFileSize(String maxFileSize) { + this.maxFileSize = Long.parseLong(maxFileSize); + } + @Inject public void setLocaleProviderFactory(LocaleProviderFactory localeProviderFactory) { defaultLocale = localeProviderFactory.createLocaleProvider().getLocale(); diff --git a/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequest.java b/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequest.java index 00d922401..551e1d51c 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequest.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequest.java @@ -75,6 +75,9 @@ public class JakartaMultiPartRequest extends AbstractMultiPartRequest { if (e instanceof FileUploadBase.SizeLimitExceededException) { FileUploadBase.SizeLimitExceededException ex = (FileUploadBase.SizeLimitExceededException) e; errorMessage = buildErrorMessage(e, new Object[]{ex.getPermittedSize(), ex.getActualSize()}); + } else if (e instanceof FileUploadBase.FileSizeLimitExceededException) { + FileUploadBase.FileSizeLimitExceededException ex = (FileUploadBase.FileSizeLimitExceededException) e; + errorMessage = buildErrorMessage(e, new Object[]{ex.getFileName(), ex.getPermittedSize(), ex.getActualSize()}); } else if (e instanceof FileCountLimitExceededException) { FileCountLimitExceededException ex = (FileCountLimitExceededException) e; errorMessage = buildErrorMessage(e, new Object[]{ex.getLimit()}); @@ -166,6 +169,9 @@ public class JakartaMultiPartRequest extends AbstractMultiPartRequest { if (maxFiles != null) { upload.setFileCountMax(maxFiles); } + if (maxFileSize != null) { + upload.setFileSizeMax(maxFileSize); + } return upload; } diff --git a/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaStreamMultiPartRequest.java b/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaStreamMultiPartRequest.java index c7311ab0a..7eb867392 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaStreamMultiPartRequest.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/multipart/JakartaStreamMultiPartRequest.java @@ -29,9 +29,20 @@ import org.apache.logging.log4j.Logger; import org.apache.struts2.dispatcher.LocalizedMessage; import javax.servlet.http.HttpServletRequest; -import java.io.*; +import java.io.BufferedOutputStream; +import java.io.File; +import java.io.IOException; +import java.io.InputStream; +import java.io.OutputStream; +import java.io.Serializable; import java.nio.file.Files; -import java.util.*; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Enumeration; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.UUID; /** * Multi-part form data request adapter for Jakarta Commons FileUpload package that @@ -215,6 +226,9 @@ public class JakartaStreamMultiPartRequest extends AbstractMultiPartRequest { if (maxFiles != null) { servletFileUpload.setFileCountMax(maxFiles); } + if (maxFileSize != null) { + servletFileUpload.setFileSizeMax(maxFileSize); + } FileItemIterator i = servletFileUpload.getItemIterator(request); // Iterate the file items @@ -261,10 +275,9 @@ public class JakartaStreamMultiPartRequest extends AbstractMultiPartRequest { // if maxSize is specified as -1, there is no sanity check and it's // safe to return true for any request, delegating the failure // checks later in the upload process. - if ((maxSize != null && maxSize == -1) || request == null) { + if (maxSize == null || maxSize == -1 || request == null) { return true; } - return request.getContentLength() < maxSize; } @@ -273,12 +286,7 @@ public class JakartaStreamMultiPartRequest extends AbstractMultiPartRequest { * @return the request content length. */ protected long getRequestSize(HttpServletRequest request) { - long requestSize = 0; - if (request != null) { - requestSize = request.getContentLength(); - } - - return requestSize; + return request != null ? request.getContentLength() : 0; } /** diff --git a/core/src/main/resources/org/apache/struts2/default.properties b/core/src/main/resources/org/apache/struts2/default.properties index 8c4144de8..0b904f186 100644 --- a/core/src/main/resources/org/apache/struts2/default.properties +++ b/core/src/main/resources/org/apache/struts2/default.properties @@ -69,6 +69,7 @@ struts.multipart.parser=jakarta struts.multipart.saveDir= struts.multipart.maxSize=2097152 struts.multipart.maxFiles=256 +struts.multipart.maxFileSize=2097152 ### Load custom property files (does not override struts.properties!) # struts.custom.properties=application,org/apache/struts2/extension/custom diff --git a/core/src/main/resources/org/apache/struts2/struts-messages.properties b/core/src/main/resources/org/apache/struts2/struts-messages.properties index bf75f05b4..1d8f2a7a9 100644 --- a/core/src/main/resources/org/apache/struts2/struts-messages.properties +++ b/core/src/main/resources/org/apache/struts2/struts-messages.properties @@ -32,6 +32,7 @@ struts.messages.error.file.extension.not.allowed=File extension not allowed: {0} # dedicated messages used to handle various problems with file upload - check {@link JakartaMultiPartRequest#parse(HttpServletRequest, String)} struts.messages.upload.error.SizeLimitExceededException=Request exceeded allowed size limit! Max size allowed is: {0} but request was: {1}! struts.messages.upload.error.FileCountLimitExceededException=Request exceeded allowed number of files! Max allowed files number is: {0}! +struts.messages.upload.error.FileSizeLimitExceededException=File in request exceeded allowed file size limit! Max file size allowed is: {1} but file {0} was: {2}! struts.messages.upload.error.IOException=Error uploading: {0}! devmode.notification=Developer Notification (set struts.devMode to false to disable this message):\n{0} diff --git a/core/src/test/java/org/apache/struts2/interceptor/FileUploadInterceptorTest.java b/core/src/test/java/org/apache/struts2/interceptor/FileUploadInterceptorTest.java index 842a7db3a..5c5f6c985 100644 --- a/core/src/test/java/org/apache/struts2/interceptor/FileUploadInterceptorTest.java +++ b/core/src/test/java/org/apache/struts2/interceptor/FileUploadInterceptorTest.java @@ -30,8 +30,8 @@ import org.apache.struts2.StrutsInternalTestCase; import org.apache.struts2.TestAction; import org.apache.struts2.dispatcher.HttpParameters; import org.apache.struts2.dispatcher.multipart.JakartaMultiPartRequest; -import org.apache.struts2.dispatcher.multipart.StrutsUploadedFile; import org.apache.struts2.dispatcher.multipart.MultiPartRequestWrapper; +import org.apache.struts2.dispatcher.multipart.StrutsUploadedFile; import org.apache.struts2.dispatcher.multipart.UploadedFile; import org.springframework.mock.web.MockHttpServletRequest; @@ -201,7 +201,7 @@ public class FileUploadInterceptorTest extends StrutsInternalTestCase { assertFalse(notOk); assertFalse(validation.getFieldErrors().isEmpty()); assertTrue(validation.hasErrors()); - List errors = (List) validation.getFieldErrors().get("inputName"); + List errors = validation.getFieldErrors().get("inputName"); assertEquals(1, errors.size()); String msg = (String) errors.get(0); // the error message should contain at least this test @@ -235,7 +235,7 @@ public class FileUploadInterceptorTest extends StrutsInternalTestCase { mai.setInvocationContext(ActionContext.getContext()); ActionContext.getContext().setParameters(HttpParameters.create().build()); - ActionContext.getContext().put(ServletActionContext.HTTP_REQUEST, createMultipartRequest(req, 2000)); + ActionContext.getContext().put(ServletActionContext.HTTP_REQUEST, createMultipartRequestMaxSize(req, 2000)); interceptor.intercept(mai); @@ -257,7 +257,7 @@ public class FileUploadInterceptorTest extends StrutsInternalTestCase { mai.setInvocationContext(ActionContext.getContext()); ActionContext.getContext().setParameters(HttpParameters.create().build()); - ActionContext.getContext().put(ServletActionContext.HTTP_REQUEST, createMultipartRequest(req, 2000)); + ActionContext.getContext().put(ServletActionContext.HTTP_REQUEST, createMultipartRequestMaxSize(req, 2000)); interceptor.intercept(mai); @@ -288,7 +288,7 @@ public class FileUploadInterceptorTest extends StrutsInternalTestCase { mai.setInvocationContext(ActionContext.getContext()); Map param = new HashMap<>(); ActionContext.getContext().setParameters(HttpParameters.create(param).build()); - ActionContext.getContext().put(ServletActionContext.HTTP_REQUEST, createMultipartRequest(req, 2000)); + ActionContext.getContext().put(ServletActionContext.HTTP_REQUEST, createMultipartRequestMaxSize(req, 2000)); interceptor.intercept(mai); @@ -349,7 +349,7 @@ public class FileUploadInterceptorTest extends StrutsInternalTestCase { mai.setInvocationContext(ActionContext.getContext()); Map param = new HashMap(); ActionContext.getContext().setParameters(HttpParameters.create(param).build()); - ActionContext.getContext().put(ServletActionContext.HTTP_REQUEST, createMultipartRequest(req, 2000)); + ActionContext.getContext().put(ServletActionContext.HTTP_REQUEST, createMultipartRequestMaxSize(req, 2000)); interceptor.setAllowedTypes("text/html"); interceptor.intercept(mai); @@ -373,21 +373,21 @@ public class FileUploadInterceptorTest extends StrutsInternalTestCase { public void testUnacceptedNumberOfFiles() throws Exception { final String htmlContent = "html content"; final String plainContent = "plain content"; - final String bondary = "simple boundary"; + final String boundary = "simple boundary"; final String endline = "\r\n"; MockHttpServletRequest req = new MockHttpServletRequest(); req.setCharacterEncoding(StandardCharsets.UTF_8.name()); req.setMethod("POST"); - req.addHeader("Content-type", "multipart/form-data; boundary=" + bondary); + req.addHeader("Content-type", "multipart/form-data; boundary=" + boundary); StringBuilder content = new StringBuilder(128); - content.append(encodeTextFile(bondary, endline, "file", "test.html", "text/plain", plainContent)); - content.append(encodeTextFile(bondary, endline, "file", "test1.html", "text/html", htmlContent)); - content.append(encodeTextFile(bondary, endline, "file", "test2.html", "text/html", htmlContent)); - content.append(encodeTextFile(bondary, endline, "file", "test3.html", "text/html", htmlContent)); + content.append(encodeTextFile(boundary, endline, "file", "test.html", "text/plain", plainContent)); + content.append(encodeTextFile(boundary, endline, "file", "test1.html", "text/html", htmlContent)); + content.append(encodeTextFile(boundary, endline, "file", "test2.html", "text/html", htmlContent)); + content.append(encodeTextFile(boundary, endline, "file", "test3.html", "text/html", htmlContent)); content.append(endline); content.append("--"); - content.append(bondary); + content.append(boundary); content.append("--"); content.append(endline); req.setContent(content.toString().getBytes()); @@ -402,7 +402,7 @@ public class FileUploadInterceptorTest extends StrutsInternalTestCase { mai.setInvocationContext(ActionContext.getContext()); Map param = new HashMap<>(); ActionContext.getContext().setParameters(HttpParameters.create(param).build()); - ActionContext.getContext().put(ServletActionContext.HTTP_REQUEST, createMultipartRequest(req, 2000)); + ActionContext.getContext().put(ServletActionContext.HTTP_REQUEST, createMultipartRequestMaxFiles(req, 3)); interceptor.setAllowedTypes("text/html"); interceptor.intercept(mai); @@ -413,6 +413,45 @@ public class FileUploadInterceptorTest extends StrutsInternalTestCase { assertEquals("Request exceeded allowed number of files! Max allowed files number is: 3!", action.getActionErrors().iterator().next()); } + public void testMultipartRequestMaxFileSize() throws Exception { + MockHttpServletRequest req = new MockHttpServletRequest(); + req.setCharacterEncoding(StandardCharsets.UTF_8.name()); + req.setMethod("post"); + req.addHeader("Content-type", "multipart/form-data; boundary=---1234"); + + // inspired by the unit tests for jakarta commons fileupload + String content = ("-----1234\r\n" + + "Content-Disposition: form-data; name=\"file\"; filename=\"deleteme.txt\"\r\n" + + "Content-Type: text/html\r\n" + + "\r\n" + + "Unit test of FileUploadInterceptor" + + "\r\n" + + "-----1234--\r\n"); + req.setContent(content.getBytes("US-ASCII")); + + MyFileupAction action = container.inject(MyFileupAction.class); + + MockActionInvocation mai = new MockActionInvocation(); + mai.setAction(action); + mai.setResultCode("success"); + mai.setInvocationContext(ActionContext.getContext()); + Map param = new HashMap<>(); + ActionContext.getContext() + .withParameters(HttpParameters.create(param).build()) + .withServletRequest(createMultipartRequestMaxFileSize(req, 10)); + + interceptor.intercept(mai); + + assertTrue(action.hasActionErrors()); + + Collection errors = action.getActionErrors(); + assertEquals(1, errors.size()); + String msg = errors.iterator().next(); + assertEquals( + "File in request exceeded allowed file size limit! Max file size allowed is: 10 but file deleteme.txt was: 34!", + msg); + } + public void testMultipartRequestLocalizedError() throws Exception { MockHttpServletRequest req = new MockHttpServletRequest(); req.setCharacterEncoding(StandardCharsets.UTF_8.name()); @@ -439,7 +478,7 @@ public class FileUploadInterceptorTest extends StrutsInternalTestCase { ActionContext.getContext() .withParameters(HttpParameters.create(param).build()) .withLocale(Locale.GERMAN) - .withServletRequest(createMultipartRequest(req, 10)); + .withServletRequest(createMultipartRequestMaxSize(req, 10)); interceptor.intercept(mai); @@ -472,10 +511,23 @@ public class FileUploadInterceptorTest extends StrutsInternalTestCase { return sb.toString(); } - private MultiPartRequestWrapper createMultipartRequest(HttpServletRequest req, int maxsize) throws IOException { + private MultiPartRequestWrapper createMultipartRequestMaxFileSize(HttpServletRequest req, int maxfilesize) throws IOException { + return createMultipartRequest(req, -1, maxfilesize, -1); + } + + private MultiPartRequestWrapper createMultipartRequestMaxFiles(HttpServletRequest req, int maxfiles) throws IOException { + return createMultipartRequest(req, -1, -1, maxfiles); + } + + private MultiPartRequestWrapper createMultipartRequestMaxSize(HttpServletRequest req, int maxsize) throws IOException { + return createMultipartRequest(req, maxsize, -1, -1); + } + + private MultiPartRequestWrapper createMultipartRequest(HttpServletRequest req, int maxsize, int maxfilesize, int maxfiles) throws IOException { JakartaMultiPartRequest jak = new JakartaMultiPartRequest(); jak.setMaxSize(String.valueOf(maxsize)); - jak.setMaxFiles("3"); + jak.setMaxFileSize(String.valueOf(maxfilesize)); + jak.setMaxFiles(String.valueOf(maxfiles)); return new MultiPartRequestWrapper(jak, req, tempDir.getAbsolutePath(), new DefaultLocaleProvider()); } diff --git a/plugins/pell-multipart/src/main/java/org/apache/struts2/dispatcher/multipart/PellMultiPartRequest.java b/plugins/pell-multipart/src/main/java/org/apache/struts2/dispatcher/multipart/PellMultiPartRequest.java index 4eb931231..9688be65e 100644 --- a/plugins/pell-multipart/src/main/java/org/apache/struts2/dispatcher/multipart/PellMultiPartRequest.java +++ b/plugins/pell-multipart/src/main/java/org/apache/struts2/dispatcher/multipart/PellMultiPartRequest.java @@ -18,9 +18,9 @@ */ package org.apache.struts2.dispatcher.multipart; -import org.apache.logging.log4j.Logger; -import org.apache.logging.log4j.LogManager; import http.utils.multipartrequest.ServletMultipartRequest; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.Logger; import javax.servlet.http.HttpServletRequest; import java.io.IOException; @@ -51,11 +51,11 @@ public class PellMultiPartRequest extends AbstractMultiPartRequest { //calling the constructor. See javadoc for MultipartRequest.setEncoding(). synchronized (this) { setEncoding(); - if (maxSize != null && maxSize > -1){ + if (maxSize != null && maxSize > -1) { int intMaxSize = (maxSize >= Integer.MAX_VALUE ? Integer.MAX_VALUE : maxSize.intValue()); - multi = new ServletMultipartRequest(servletRequest, saveDir, intMaxSize); - }else{ - multi = new ServletMultipartRequest(servletRequest, saveDir); + multi = new ServletMultipartRequest(servletRequest, saveDir, intMaxSize); + } else { + multi = new ServletMultipartRequest(servletRequest, saveDir); } } }