You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@struts.apache.org by lu...@apache.org on 2023/03/08 16:15:50 UTC
[struts] 01/02: WW-5285 Limits max number of files to upload at once Upgrades commons-fileupload to ver. 1.5 and sets default limit to 256 files
This is an automated email from the ASF dual-hosted git repository.
lukaszlenart pushed a commit to branch release/6.1.x
in repository https://gitbox.apache.org/repos/asf/struts.git
commit 483a0c8d012924b78343dd35e56037c0d48a84cf
Author: Lukasz Lenart <lu...@apache.org>
AuthorDate: Sat Feb 25 10:42:21 2023 +0100
WW-5285 Limits max number of files to upload at once
Upgrades commons-fileupload to ver. 1.5 and sets default limit to 256 files
---
.../java/org/apache/struts2/StrutsConstants.java | 3 ++
.../struts2/config/entities/ConstantConfig.java | 10 +++++
.../multipart/AbstractMultiPartRequest.java | 12 ++++++
.../multipart/JakartaMultiPartRequest.java | 21 ++++++++---
.../multipart/JakartaStreamMultiPartRequest.java | 3 ++
.../org/apache/struts2/default.properties | 1 +
.../org/apache/struts2/struts-messages.properties | 1 +
.../interceptor/FileUploadInterceptorTest.java | 44 ++++++++++++++++++++++
pom.xml | 2 +-
9 files changed, 91 insertions(+), 6 deletions(-)
diff --git a/core/src/main/java/org/apache/struts2/StrutsConstants.java b/core/src/main/java/org/apache/struts2/StrutsConstants.java
index c75ff3d54..15eb43d87 100644
--- a/core/src/main/java/org/apache/struts2/StrutsConstants.java
+++ b/core/src/main/java/org/apache/struts2/StrutsConstants.java
@@ -142,6 +142,9 @@ public final class StrutsConstants {
/** The maximize 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 */
+ public static final String STRUTS_MULTIPART_MAXFILES = "struts.multipart.maxFiles";
+
/** 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 63c30ce2f..f728d9115 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
@@ -64,6 +64,7 @@ public class ConstantConfig {
private String uiTheme;
private String uiThemeExpansionToken;
private Long multipartMaxSize;
+ private Long multipartMaxFiles;
private String multipartSaveDir;
private Integer multipartBufferSize;
private BeanConfig multipartParser;
@@ -195,6 +196,7 @@ public class ConstantConfig {
map.put(StrutsConstants.STRUTS_UI_THEME, uiTheme);
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_SAVEDIR, multipartSaveDir);
map.put(StrutsConstants.STRUTS_MULTIPART_BUFFERSIZE, Objects.toString(multipartBufferSize, null));
map.put(StrutsConstants.STRUTS_MULTIPART_PARSER, beanConfToString(multipartParser));
@@ -580,6 +582,14 @@ public class ConstantConfig {
this.multipartMaxSize = multipartMaxSize;
}
+ public Long getMultipartMaxFiles() {
+ return multipartMaxFiles;
+ }
+
+ public void setMultipartMaxFiles(Long multipartMaxFiles) {
+ this.multipartMaxFiles = multipartMaxFiles;
+ }
+
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 700364047..b3410e578 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
@@ -54,6 +54,12 @@ public abstract class AbstractMultiPartRequest implements MultiPartRequest {
protected long maxSize;
protected boolean maxSizeProvided;
+ /**
+ * Specifies the maximum number of files in one request.
+ */
+ protected long maxFiles;
+ protected boolean maxFilesProvided;
+
/**
* Specifies the buffer size to use during streaming.
*/
@@ -88,6 +94,12 @@ public abstract class AbstractMultiPartRequest implements MultiPartRequest {
this.maxSize = Long.parseLong(maxSize);
}
+ @Inject(StrutsConstants.STRUTS_MULTIPART_MAXFILES)
+ public void setMaxFiles(String maxFiles) {
+ this.maxFilesProvided = true;
+ this.maxFiles = Long.parseLong(maxFiles);
+ }
+
@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 c629ec043..c20b1de19 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
@@ -18,6 +18,7 @@
*/
package org.apache.struts2.dispatcher.multipart;
+import org.apache.commons.fileupload.FileCountLimitExceededException;
import org.apache.commons.fileupload.FileItem;
import org.apache.commons.fileupload.FileUploadBase;
import org.apache.commons.fileupload.FileUploadException;
@@ -35,7 +36,13 @@ import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.io.UnsupportedEncodingException;
-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.Set;
/**
* Multipart form data request adapter for Jakarta Commons Fileupload package.
@@ -65,9 +72,12 @@ public class JakartaMultiPartRequest extends AbstractMultiPartRequest {
} catch (FileUploadException e) {
LOG.warn("Request exceeded size limit!", e);
LocalizedMessage errorMessage;
- if(e instanceof FileUploadBase.SizeLimitExceededException) {
+ if (e instanceof FileUploadBase.SizeLimitExceededException) {
FileUploadBase.SizeLimitExceededException ex = (FileUploadBase.SizeLimitExceededException) e;
errorMessage = buildErrorMessage(e, new Object[]{ex.getPermittedSize(), ex.getActualSize()});
+ } else if (e instanceof FileCountLimitExceededException) {
+ FileCountLimitExceededException ex = (FileCountLimitExceededException) e;
+ errorMessage = buildErrorMessage(e, new Object[]{ex.getLimit()});
} else {
errorMessage = buildErrorMessage(e, new Object[]{});
}
@@ -151,6 +161,7 @@ public class JakartaMultiPartRequest extends AbstractMultiPartRequest {
protected ServletFileUpload createServletFileUpload(DiskFileItemFactory fac) {
ServletFileUpload upload = new ServletFileUpload(fac);
upload.setSizeMax(maxSize);
+ upload.setFileCountMax(maxFiles);
return upload;
}
@@ -316,14 +327,14 @@ public class JakartaMultiPartRequest extends AbstractMultiPartRequest {
}
/* (non-Javadoc)
- * @see org.apache.struts2.dispatcher.multipart.MultiPartRequest#cleanUp()
- */
+ * @see org.apache.struts2.dispatcher.multipart.MultiPartRequest#cleanUp()
+ */
public void cleanUp() {
Set<String> names = files.keySet();
for (String name : names) {
List<FileItem> items = files.get(name);
for (FileItem item : items) {
- LOG.debug("Removing file {} {}", name, item );
+ LOG.debug("Removing file {} {}", name, item);
if (!item.isInMemory()) {
item.delete();
}
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 2d016f56b..f709b3416 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
@@ -212,6 +212,9 @@ public class JakartaStreamMultiPartRequest extends AbstractMultiPartRequest {
if (maxSizeProvided) {
servletFileUpload.setSizeMax(maxSize);
}
+ if (maxFilesProvided) {
+ servletFileUpload.setFileCountMax(maxFiles);
+ }
FileItemIterator i = servletFileUpload.getItemIterator(request);
// Iterate the file items
diff --git a/core/src/main/resources/org/apache/struts2/default.properties b/core/src/main/resources/org/apache/struts2/default.properties
index 5847db3be..8c4144de8 100644
--- a/core/src/main/resources/org/apache/struts2/default.properties
+++ b/core/src/main/resources/org/apache/struts2/default.properties
@@ -68,6 +68,7 @@ struts.multipart.parser=jakarta
### Uses javax.servlet.context.tempdir by default
struts.multipart.saveDir=
struts.multipart.maxSize=2097152
+struts.multipart.maxFiles=256
### 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 aa6e842e4..bf75f05b4 100644
--- a/core/src/main/resources/org/apache/struts2/struts-messages.properties
+++ b/core/src/main/resources/org/apache/struts2/struts-messages.properties
@@ -31,6 +31,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.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 b6a41010e..842a7db3a 100644
--- a/core/src/test/java/org/apache/struts2/interceptor/FileUploadInterceptorTest.java
+++ b/core/src/test/java/org/apache/struts2/interceptor/FileUploadInterceptorTest.java
@@ -370,6 +370,49 @@ public class FileUploadInterceptorTest extends StrutsInternalTestCase {
assertNotNull("test1.html", fileRealFilenames[0]);
}
+ public void testUnacceptedNumberOfFiles() throws Exception {
+ final String htmlContent = "<html><head></head><body>html content</body></html>";
+ final String plainContent = "plain content";
+ final String bondary = "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);
+ 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(endline);
+ content.append("--");
+ content.append(bondary);
+ content.append("--");
+ content.append(endline);
+ req.setContent(content.toString().getBytes());
+
+ assertTrue(ServletFileUpload.isMultipartContent(req));
+
+ MyFileupAction action = new MyFileupAction();
+ container.inject(action);
+ MockActionInvocation mai = new MockActionInvocation();
+ mai.setAction(action);
+ mai.setResultCode("success");
+ mai.setInvocationContext(ActionContext.getContext());
+ Map<String, Object> param = new HashMap<>();
+ ActionContext.getContext().setParameters(HttpParameters.create(param).build());
+ ActionContext.getContext().put(ServletActionContext.HTTP_REQUEST, createMultipartRequest(req, 2000));
+
+ interceptor.setAllowedTypes("text/html");
+ interceptor.intercept(mai);
+
+ HttpParameters parameters = mai.getInvocationContext().getParameters();
+ assertEquals(0, parameters.keySet().size());
+ assertEquals(1, action.getActionErrors().size());
+ assertEquals("Request exceeded allowed number of files! Max allowed files number is: 3!", action.getActionErrors().iterator().next());
+ }
+
public void testMultipartRequestLocalizedError() throws Exception {
MockHttpServletRequest req = new MockHttpServletRequest();
req.setCharacterEncoding(StandardCharsets.UTF_8.name());
@@ -432,6 +475,7 @@ public class FileUploadInterceptorTest extends StrutsInternalTestCase {
private MultiPartRequestWrapper createMultipartRequest(HttpServletRequest req, int maxsize) throws IOException {
JakartaMultiPartRequest jak = new JakartaMultiPartRequest();
jak.setMaxSize(String.valueOf(maxsize));
+ jak.setMaxFiles("3");
return new MultiPartRequestWrapper(jak, req, tempDir.getAbsolutePath(), new DefaultLocaleProvider());
}
diff --git a/pom.xml b/pom.xml
index 3453bd034..7d8d61935 100644
--- a/pom.xml
+++ b/pom.xml
@@ -908,7 +908,7 @@
<dependency>
<groupId>commons-fileupload</groupId>
<artifactId>commons-fileupload</artifactId>
- <version>1.4</version>
+ <version>1.5</version>
</dependency>
<dependency>
<groupId>commons-io</groupId>