diff --git a/core/src/main/java/org/apache/struts2/StrutsConstants.java b/core/src/main/java/org/apache/struts2/StrutsConstants.java index 8d44b9837..dc5720b71 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 0fe1e300c..107f35e51 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)); @@ -579,6 +581,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..e46cecc00 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 @@ -51,8 +51,12 @@ public abstract class AbstractMultiPartRequest implements MultiPartRequest { /** * Specifies the maximum size of the entire request. */ - protected long maxSize; - protected boolean maxSizeProvided; + protected Long maxSize; + + /** + * Specifies the maximum number of files in one request. + */ + protected Long maxFiles; /** * Specifies the buffer size to use during streaming. @@ -84,10 +88,14 @@ public abstract class AbstractMultiPartRequest implements MultiPartRequest { */ @Inject(StrutsConstants.STRUTS_MULTIPART_MAXSIZE) public void setMaxSize(String maxSize) { - this.maxSizeProvided = true; this.maxSize = Long.parseLong(maxSize); } + @Inject(StrutsConstants.STRUTS_MULTIPART_MAXFILES) + public void setMaxFiles(String maxFiles) { + this.maxFiles = Long.parseLong(maxFiles); + } + @Inject public void setLocaleProviderFactory(LocaleProviderFactory localeProviderFactory) { defaultLocale = localeProviderFactory.createLocaleProvider().getLocale(); @@ -134,9 +142,9 @@ public abstract class AbstractMultiPartRequest implements MultiPartRequest { int forwardSlash = fileName.lastIndexOf('/'); int backwardSlash = fileName.lastIndexOf('\\'); if (forwardSlash != -1 && forwardSlash > backwardSlash) { - fileName = fileName.substring(forwardSlash + 1, fileName.length()); + fileName = fileName.substring(forwardSlash + 1); } else { - fileName = fileName.substring(backwardSlash + 1, fileName.length()); + fileName = fileName.substring(backwardSlash + 1); } return fileName; } 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..00d922401 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[]{}); } @@ -150,7 +160,12 @@ public class JakartaMultiPartRequest extends AbstractMultiPartRequest { protected ServletFileUpload createServletFileUpload(DiskFileItemFactory fac) { ServletFileUpload upload = new ServletFileUpload(fac); - upload.setSizeMax(maxSize); + if (maxSize != null) { + upload.setSizeMax(maxSize); + } + if (maxFiles != null) { + upload.setFileCountMax(maxFiles); + } return upload; } @@ -316,14 +331,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 names = files.keySet(); for (String name : names) { List 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..c7311ab0a 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 @@ -30,15 +30,15 @@ import org.apache.struts2.dispatcher.LocalizedMessage; import javax.servlet.http.HttpServletRequest; import java.io.*; +import java.nio.file.Files; import java.util.*; /** * Multi-part form data request adapter for Jakarta Commons FileUpload package that * leverages the streaming API rather than the traditional non-streaming API. - * + *

* For more details see WW-3025 * - * @author Chris Cranford * @since 2.3.18 */ public class JakartaStreamMultiPartRequest extends AbstractMultiPartRequest { @@ -85,7 +85,7 @@ public class JakartaStreamMultiPartRequest extends AbstractMultiPartRequest { types.add(fileInfo.getContentType()); } - return types.toArray(new String[types.size()]); + return types.toArray(new String[0]); } /* (non-Javadoc) @@ -102,7 +102,7 @@ public class JakartaStreamMultiPartRequest extends AbstractMultiPartRequest { files.add(new StrutsUploadedFile(fileInfo.getFile())); } - return files.toArray(new UploadedFile[files.size()]); + return files.toArray(new UploadedFile[0]); } /* (non-Javadoc) @@ -119,7 +119,7 @@ public class JakartaStreamMultiPartRequest extends AbstractMultiPartRequest { names.add(getCanonicalName(fileInfo.getOriginalName())); } - return names.toArray(new String[names.size()]); + return names.toArray(new String[0]); } /* (non-Javadoc) @@ -143,7 +143,7 @@ public class JakartaStreamMultiPartRequest extends AbstractMultiPartRequest { names.add(fileInfo.getFile().getName()); } - return names.toArray(new String[names.size()]); + return names.toArray(new String[0]); } /* (non-Javadoc) @@ -170,7 +170,7 @@ public class JakartaStreamMultiPartRequest extends AbstractMultiPartRequest { public String[] getParameterValues(String name) { List values = parameters.get(name); if (values != null && values.size() > 0) { - return values.toArray(new String[values.size()]); + return values.toArray(new String[0]); } return null; } @@ -209,9 +209,12 @@ public class JakartaStreamMultiPartRequest extends AbstractMultiPartRequest { // Interface with Commons FileUpload API // Using the Streaming API ServletFileUpload servletFileUpload = new ServletFileUpload(); - if (maxSizeProvided) { + if (maxSize != null) { servletFileUpload.setSizeMax(maxSize); } + if (maxFiles != null) { + servletFileUpload.setFileCountMax(maxFiles); + } FileItemIterator i = servletFileUpload.getItemIterator(request); // Iterate the file items @@ -258,7 +261,7 @@ 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 == -1 || request == null) { + if ((maxSize != null && maxSize == -1) || request == null) { return true; } @@ -286,8 +289,9 @@ public class JakartaStreamMultiPartRequest extends AbstractMultiPartRequest { */ protected void addFileSkippedError(String fileName, HttpServletRequest request) { String exceptionMessage = "Skipped file " + fileName + "; request size limit exceeded."; - FileSizeLimitExceededException exception = new FileUploadBase.FileSizeLimitExceededException(exceptionMessage, getRequestSize(request), maxSize); - LocalizedMessage message = buildErrorMessage(exception, new Object[]{fileName, getRequestSize(request), maxSize}); + long allowedMaxSize = maxSize != null ? maxSize : -1; + FileSizeLimitExceededException exception = new FileUploadBase.FileSizeLimitExceededException(exceptionMessage, getRequestSize(request), allowedMaxSize); + LocalizedMessage message = buildErrorMessage(exception, new Object[]{fileName, getRequestSize(request), allowedMaxSize}); if (!errors.contains(message)) { errors.add(message); } @@ -386,12 +390,12 @@ public class JakartaStreamMultiPartRequest extends AbstractMultiPartRequest { * @throws IOException in case of IO errors */ protected boolean streamFileToDisk(FileItemStream itemStream, File file) throws IOException { - boolean result = false; + boolean result; try (InputStream input = itemStream.openStream(); - OutputStream output = new BufferedOutputStream(new FileOutputStream(file), bufferSize)) { + OutputStream output = new BufferedOutputStream(Files.newOutputStream(file.toPath()), bufferSize)) { byte[] buffer = new byte[bufferSize]; LOG.debug("Streaming file using buffer size {}.", bufferSize); - for (int length = 0; ((length = input.read(buffer)) > 0); ) { + for (int length; ((length = input.read(buffer)) > 0); ) { output.write(buffer, 0, length); } result = true; @@ -433,9 +437,9 @@ public class JakartaStreamMultiPartRequest extends AbstractMultiPartRequest { private static final long serialVersionUID = 1083158552766906037L; - private File file; - private String contentType; - private String originalName; + private final File file; + private final String contentType; + private final String originalName; /** * Default constructor. 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 content"; + 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 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/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 aaf1f8b12..4eb931231 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 @@ -51,15 +51,15 @@ public class PellMultiPartRequest extends AbstractMultiPartRequest { //calling the constructor. See javadoc for MultipartRequest.setEncoding(). synchronized (this) { setEncoding(); - if (maxSizeProvided){ - int intMaxSize = (maxSize >= Integer.MAX_VALUE ? Integer.MAX_VALUE : Long.valueOf(maxSize).intValue()); + 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); } } } - + public Enumeration getFileParameterNames() { return multi.getFileParameterNames(); } diff --git a/pom.xml b/pom.xml index 42f0f42c8..e2b32e529 100644 --- a/pom.xml +++ b/pom.xml @@ -917,7 +917,7 @@ commons-fileupload commons-fileupload - 1.4 + 1.5 commons-io