diff --git a/apps/showcase/src/main/java/org/apache/struts2/showcase/fileupload/FileUploadAction.java b/apps/showcase/src/main/java/org/apache/struts2/showcase/fileupload/FileUploadAction.java index c2ac471f4..be6f38669 100644 --- a/apps/showcase/src/main/java/org/apache/struts2/showcase/fileupload/FileUploadAction.java +++ b/apps/showcase/src/main/java/org/apache/struts2/showcase/fileupload/FileUploadAction.java @@ -24,6 +24,7 @@ import com.opensymphony.xwork2.ActionSupport; import org.apache.struts2.action.UploadedFilesAware; import org.apache.struts2.dispatcher.multipart.UploadedFile; +import java.io.File; import java.util.List; /** @@ -32,7 +33,7 @@ import java.util.List; public class FileUploadAction extends ActionSupport implements UploadedFilesAware { private String contentType; - private UploadedFile uploadedFile; + private UploadedFile uploadedFile; private String fileName; private String caption; private String originalName; @@ -78,7 +79,7 @@ public class FileUploadAction extends ActionSupport implements UploadedFilesAwar } @Override - public void withUploadedFiles(List uploadedFiles) { + public void withUploadedFiles(List> uploadedFiles) { this.uploadedFile = uploadedFiles.get(0); this.fileName = uploadedFile.getName(); this.contentType = uploadedFile.getContentType(); diff --git a/core/src/main/java/org/apache/struts2/StrutsConstants.java b/core/src/main/java/org/apache/struts2/StrutsConstants.java index 939b3bddb..8c0ccbec6 100644 --- a/core/src/main/java/org/apache/struts2/StrutsConstants.java +++ b/core/src/main/java/org/apache/struts2/StrutsConstants.java @@ -140,21 +140,28 @@ public final class StrutsConstants { public static final String STRUTS_UI_ESCAPE_HTML_BODY = "struts.ui.escapeHtmlBody"; /** The maximum size of a multipart request (file upload) */ - public static final String STRUTS_MULTIPART_MAXSIZE = "struts.multipart.maxSize"; + public static final String STRUTS_MULTIPART_MAX_SIZE = "struts.multipart.maxSize"; + + /** The maximum size of all uploaded files. + Used only with {@link org.apache.struts2.dispatcher.multipart.JakartaStreamMultiPartRequest} */ + public static final String STRUTS_MULTIPART_MAX_SIZE_OF_FILES = "struts.multipart.maxSizeOfFiles"; /** The maximum number of files allowed in a multipart request */ - public static final String STRUTS_MULTIPART_MAXFILES = "struts.multipart.maxFiles"; + public static final String STRUTS_MULTIPART_MAX_FILES = "struts.multipart.maxFiles"; /** The maximum length of a string parameter in a multipart request. */ public static final String STRUTS_MULTIPART_MAX_STRING_LENGTH = "struts.multipart.maxStringLength"; /** 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"; + public static final String STRUTS_MULTIPART_MAX_FILE_SIZE = "struts.multipart.maxFileSize"; - /** Declares the buffer size to be used during streaming multipart content to disk. Used only with {@link org.apache.struts2.dispatcher.multipart.JakartaStreamMultiPartRequest} */ - public static final String STRUTS_MULTIPART_BUFFERSIZE = "struts.multipart.bufferSize"; + /** The directory to use for storing uploaded files */ + public static final String STRUTS_MULTIPART_SAVE_DIR = "struts.multipart.saveDir"; + + /** Declares the buffer size to be used during streaming multipart content to disk. + * Used only with {@link org.apache.struts2.dispatcher.multipart.JakartaStreamMultiPartRequest} + */ + public static final String STRUTS_MULTIPART_BUFFER_SIZE = "struts.multipart.bufferSize"; /** * The org.apache.struts2.dispatcher.multipart.MultiPartRequest parser implementation diff --git a/core/src/main/java/org/apache/struts2/action/UploadedFilesAware.java b/core/src/main/java/org/apache/struts2/action/UploadedFilesAware.java index 92ec9c98b..2946af23a 100644 --- a/core/src/main/java/org/apache/struts2/action/UploadedFilesAware.java +++ b/core/src/main/java/org/apache/struts2/action/UploadedFilesAware.java @@ -20,6 +20,7 @@ package org.apache.struts2.action; import org.apache.struts2.dispatcher.multipart.UploadedFile; +import java.io.File; import java.util.List; /** @@ -35,6 +36,6 @@ public interface UploadedFilesAware { * * @param uploadedFiles a list of {@link UploadedFile}, cannot be null. It can be empty. */ - void withUploadedFiles(List uploadedFiles); + void withUploadedFiles(List> uploadedFiles); } 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 2b854243d..96252a029 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 @@ -199,12 +199,12 @@ public class ConstantConfig { map.put(StrutsConstants.STRUTS_UI_TEMPLATEDIR, uiTemplateDir); 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_MAXFILESIZE, Objects.toString(multipartMaxFileSize, null)); + map.put(StrutsConstants.STRUTS_MULTIPART_MAX_SIZE, Objects.toString(multipartMaxSize, null)); + map.put(StrutsConstants.STRUTS_MULTIPART_MAX_FILES, Objects.toString(multipartMaxFiles, null)); + map.put(StrutsConstants.STRUTS_MULTIPART_MAX_FILE_SIZE, Objects.toString(multipartMaxFileSize, null)); map.put(StrutsConstants.STRUTS_MULTIPART_MAX_STRING_LENGTH, Objects.toString(multipartMaxStringLength, null)); - map.put(StrutsConstants.STRUTS_MULTIPART_SAVEDIR, multipartSaveDir); - map.put(StrutsConstants.STRUTS_MULTIPART_BUFFERSIZE, Objects.toString(multipartBufferSize, null)); + map.put(StrutsConstants.STRUTS_MULTIPART_SAVE_DIR, multipartSaveDir); + map.put(StrutsConstants.STRUTS_MULTIPART_BUFFER_SIZE, Objects.toString(multipartBufferSize, null)); map.put(StrutsConstants.STRUTS_MULTIPART_PARSER, beanConfToString(multipartParser)); map.put(StrutsConstants.STRUTS_MULTIPART_ENABLED, Objects.toString(multipartEnabled, null)); map.put(StrutsConstants.STRUTS_MULTIPART_VALIDATION_REGEX, Objects.toString(multipartValidationRegex, null)); diff --git a/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java b/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java index 8668616c2..37659d5a7 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java @@ -315,7 +315,7 @@ public class Dispatcher { * * @param val New setting */ - @Inject(StrutsConstants.STRUTS_MULTIPART_SAVEDIR) + @Inject(StrutsConstants.STRUTS_MULTIPART_SAVE_DIR) public void setMultipartSaveDir(String val) { multipartSaveDir = val; } 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 97cb1827e..618407d0a 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 @@ -66,6 +66,11 @@ public abstract class AbstractMultiPartRequest implements MultiPartRequest { */ protected Long maxSize; + /** + * Specifies the maximum size of all the uploaded files. + */ + protected Long maxSizeOfFiles; + /** * Specifies the maximum number of files in one request. */ @@ -104,7 +109,7 @@ public abstract class AbstractMultiPartRequest implements MultiPartRequest { /** * @param bufferSize Sets the buffer size to be used. */ - @Inject(value = StrutsConstants.STRUTS_MULTIPART_BUFFERSIZE, required = false) + @Inject(value = StrutsConstants.STRUTS_MULTIPART_BUFFER_SIZE, required = false) public void setBufferSize(String bufferSize) { this.bufferSize = Integer.parseInt(bufferSize); } @@ -117,15 +122,23 @@ public abstract class AbstractMultiPartRequest implements MultiPartRequest { /** * @param maxSize Injects the Struts multipart request maximum size. */ - @Inject(StrutsConstants.STRUTS_MULTIPART_MAXSIZE) + @Inject(StrutsConstants.STRUTS_MULTIPART_MAX_SIZE) public void setMaxSize(String maxSize) { this.maxSize = Long.parseLong(maxSize); } + /** + * @param maxSizeOfFiles Injects the Struts maximum size of all uploaded files. + */ + @Inject(value = StrutsConstants.STRUTS_MULTIPART_MAX_SIZE_OF_FILES, required = false) + public void setMaxSizeOfFiles(String maxSizeOfFiles) { + this.maxSizeOfFiles = Long.parseLong(maxSizeOfFiles); + } + /** * @param maxFiles Injects the Struts maximum size of an individual file uploaded. */ - @Inject(StrutsConstants.STRUTS_MULTIPART_MAXFILES) + @Inject(StrutsConstants.STRUTS_MULTIPART_MAX_FILES) public void setMaxFiles(String maxFiles) { this.maxFiles = Long.parseLong(maxFiles); } @@ -133,7 +146,7 @@ public abstract class AbstractMultiPartRequest implements MultiPartRequest { /** * @param maxFileSize Injects the Struts maximum number of files, which can be uploaded. */ - @Inject(value = StrutsConstants.STRUTS_MULTIPART_MAXFILESIZE, required = false) + @Inject(value = StrutsConstants.STRUTS_MULTIPART_MAX_FILE_SIZE, required = false) public void setMaxFileSize(String maxFileSize) { this.maxFileSize = Long.parseLong(maxFileSize); } @@ -232,7 +245,7 @@ public abstract class AbstractMultiPartRequest implements MultiPartRequest { if (!errors.contains(errorMessage)) { errors.add(errorMessage); } - } catch (Exception e) { + } catch (IOException e) { LOG.debug("Unable to parse request", e); LocalizedMessage errorMessage = buildErrorMessage(e, new Object[]{}); if (!errors.contains(errorMessage)) { 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 3514652e4..6135afb4e 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 @@ -21,10 +21,13 @@ package org.apache.struts2.dispatcher.multipart; import jakarta.servlet.http.HttpServletRequest; import org.apache.commons.fileupload2.core.DiskFileItemFactory; import org.apache.commons.fileupload2.core.FileItemInput; +import org.apache.commons.fileupload2.core.FileUploadFileCountLimitException; +import org.apache.commons.fileupload2.core.FileUploadSizeException; import org.apache.commons.fileupload2.jakarta.servlet6.JakartaServletDiskFileUpload; import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.apache.struts2.dispatcher.LocalizedMessage; import java.io.BufferedOutputStream; import java.io.ByteArrayOutputStream; @@ -128,6 +131,54 @@ public class JakartaStreamMultiPartRequest extends AbstractMultiPartRequest files.stream().map(UploadedFile::length).reduce(0L, Long::sum)) + .reduce(0L, Long::sum); + } + + private boolean exceedsMaxFiles(FileItemInput fileItemInput) { + if (maxFiles != null && maxFiles == uploadedFiles.size()) { + if (LOG.isDebugEnabled()) { + LOG.debug("Cannot accept another file: {} as it will exceed max files: {}", + sanitizeNewlines(fileItemInput.getName()), maxFiles); + } + LocalizedMessage errorMessage = buildErrorMessage(new FileUploadFileCountLimitException( + String.format("File %s exceeds allowed maximum number of files %s", + fileItemInput.getName(), maxFiles), + maxFiles, uploadedFiles.size()), + new Object[]{maxFiles, uploadedFiles.size()}); + if (!errors.contains(errorMessage)) { + errors.add(errorMessage); + } + return true; + } + return false; + } + + private void exceedsMaxSizeOfFiles(FileItemInput fileItemInput, File file, Long currentFilesSize) { + if (LOG.isDebugEnabled()) { + LOG.debug("File: {} of size: {} exceeds allowed max size: {}, actual size of already uploaded files: {}", + sanitizeNewlines(fileItemInput.getName()), file.length(), maxSizeOfFiles, currentFilesSize + ); + } + LocalizedMessage errorMessage = buildErrorMessage(new FileUploadSizeException( + String.format("Size %s of file %s exceeds allowed max size %s", + file.length(), fileItemInput.getName(), maxSizeOfFiles), + maxSizeOfFiles, currentFilesSize), + new Object[]{maxSizeOfFiles, currentFilesSize}); + if (!errors.contains(errorMessage)) { + errors.add(errorMessage); + } + if (!file.delete() && LOG.isWarnEnabled()) { + LOG.warn("Cannot delete file: {} which exceeds maximum size: {} of all files!", + sanitizeNewlines(fileItemInput.getName()), maxSizeOfFiles); + } + } + /** * Processes the FileItem as a file field. * @@ -141,9 +192,19 @@ public class JakartaStreamMultiPartRequest extends AbstractMultiPartRequest= maxSizeOfFiles) { + exceedsMaxSizeOfFiles(fileItemInput, file, currentFilesSize); + } else { + createUploadedFile(fileItemInput, file); + } } /** diff --git a/core/src/main/java/org/apache/struts2/dispatcher/multipart/MultiPartRequest.java b/core/src/main/java/org/apache/struts2/dispatcher/multipart/MultiPartRequest.java index cacfb750a..d366bd5e3 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/multipart/MultiPartRequest.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/multipart/MultiPartRequest.java @@ -32,7 +32,7 @@ import jakarta.servlet.http.HttpServletRequest; public interface MultiPartRequest { void parse(HttpServletRequest request, String saveDir) throws IOException; - + /** * Returns an enumeration of the parameter names for uploaded files * @@ -58,7 +58,7 @@ public interface MultiPartRequest { * @param fieldName input field name * @return a UploadedFile[] object for files associated with the specified input field name */ - UploadedFile[] getFile(String fieldName); + UploadedFile[] getFile(String fieldName); /** * Returns a String[] of file names for files associated with the specified input field name diff --git a/core/src/main/java/org/apache/struts2/dispatcher/multipart/MultiPartRequestWrapper.java b/core/src/main/java/org/apache/struts2/dispatcher/multipart/MultiPartRequestWrapper.java index c4f7b9dc6..7ddadb8c5 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/multipart/MultiPartRequestWrapper.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/multipart/MultiPartRequestWrapper.java @@ -25,6 +25,8 @@ import org.apache.struts2.dispatcher.LocalizedMessage; import org.apache.struts2.dispatcher.StrutsRequestWrapper; import jakarta.servlet.http.HttpServletRequest; + +import java.io.File; import java.io.IOException; import java.util.*; @@ -137,7 +139,7 @@ public class MultiPartRequestWrapper extends StrutsRequestWrapper { * @param fieldName input field name * @return a File[] object for files associated with the specified input field name */ - public UploadedFile[] getFiles(String fieldName) { + public UploadedFile[] getFiles(String fieldName) { if (multi == null) { return null; } diff --git a/core/src/main/java/org/apache/struts2/interceptor/ActionFileUploadInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/ActionFileUploadInterceptor.java index 3cd0529ae..72fe043da 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/ActionFileUploadInterceptor.java +++ b/core/src/main/java/org/apache/struts2/interceptor/ActionFileUploadInterceptor.java @@ -28,6 +28,7 @@ import org.apache.struts2.action.UploadedFilesAware; import org.apache.struts2.dispatcher.multipart.MultiPartRequestWrapper; import org.apache.struts2.dispatcher.multipart.UploadedFile; +import java.io.File; import java.util.ArrayList; import java.util.Enumeration; import java.util.List; @@ -153,19 +154,19 @@ public class ActionFileUploadInterceptor extends AbstractFileUploadInterceptor { // bind allowed Files Enumeration fileParameterNames = multiWrapper.getFileParameterNames(); - List acceptedFiles = new ArrayList<>(); + List> acceptedFiles = new ArrayList<>(); while (fileParameterNames != null && fileParameterNames.hasMoreElements()) { // get the value of this input tag String inputName = fileParameterNames.nextElement(); - UploadedFile[] uploadedFiles = multiWrapper.getFiles(inputName); + UploadedFile[] uploadedFiles = multiWrapper.getFiles(inputName); if (uploadedFiles == null || uploadedFiles.length == 0) { if (LOG.isWarnEnabled()) { LOG.warn(getTextMessage(action, STRUTS_MESSAGES_INVALID_FILE_KEY, new String[]{inputName})); } } else { - for (UploadedFile uploadedFile : uploadedFiles) { + for (UploadedFile uploadedFile : uploadedFiles) { if (acceptFile(action, uploadedFile, uploadedFile.getOriginalName(), uploadedFile.getContentType(), inputName)) { acceptedFiles.add(uploadedFile); } diff --git a/core/src/test/java/org/apache/struts2/config/SettingsTest.java b/core/src/test/java/org/apache/struts2/config/SettingsTest.java index 90b824ae4..f4bf9c801 100644 --- a/core/src/test/java/org/apache/struts2/config/SettingsTest.java +++ b/core/src/test/java/org/apache/struts2/config/SettingsTest.java @@ -35,8 +35,8 @@ public class SettingsTest extends StrutsInternalTestCase { public void testSettings() { Settings settings = new DefaultSettings(); - assertEquals("12345", settings.get(StrutsConstants.STRUTS_MULTIPART_MAXSIZE)); - assertEquals("\\temp", settings.get(StrutsConstants.STRUTS_MULTIPART_SAVEDIR)); + assertEquals("12345", settings.get(StrutsConstants.STRUTS_MULTIPART_MAX_SIZE)); + assertEquals("\\temp", settings.get(StrutsConstants.STRUTS_MULTIPART_SAVE_DIR)); assertEquals("test,org/apache/struts2/othertest", settings.get( StrutsConstants.STRUTS_CUSTOM_PROPERTIES)); assertEquals("testvalue", settings.get("testkey")); diff --git a/core/src/test/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequestTest.java b/core/src/test/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequestTest.java index 2ce832cda..7c75a7a4d 100644 --- a/core/src/test/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequestTest.java +++ b/core/src/test/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequestTest.java @@ -27,18 +27,18 @@ import org.junit.BeforeClass; import org.junit.Test; import org.springframework.mock.web.MockHttpServletRequest; +import java.io.File; import java.io.IOException; import java.nio.charset.StandardCharsets; import java.nio.file.Path; import java.util.ArrayList; -import java.util.Arrays; import java.util.List; import java.util.Map; import java.util.Objects; import static org.assertj.core.api.Assertions.assertThat; -abstract class AbstractMultiPartRequestTest { +abstract class AbstractMultiPartRequestTest { protected static Path tempDir; @@ -47,9 +47,9 @@ abstract class AbstractMultiPartRequestTest { protected final String boundary = "_boundary_"; protected final String endline = "\r\n"; - protected AbstractMultiPartRequest multiPart; + protected AbstractMultiPartRequest multiPart; - abstract protected AbstractMultiPartRequest createMultipartRequest(); + abstract protected AbstractMultiPartRequest createMultipartRequest(); @BeforeClass public static void beforeClass() { @@ -271,8 +271,8 @@ abstract class AbstractMultiPartRequestTest { .isEqualTo("5,6,7,8"); }); - List> uploadedFiles = new ArrayList<>(); - for (Map.Entry>> entry : multiPart.uploadedFiles.entrySet()) { + List> uploadedFiles = new ArrayList<>(); + for (Map.Entry>> entry : multiPart.uploadedFiles.entrySet()) { uploadedFiles.addAll(entry.getValue()); } @@ -284,10 +284,10 @@ abstract class AbstractMultiPartRequestTest { .isEmpty(); assertThat(multiPart.parameters) .isEmpty(); - assertThat(uploadedFiles).allSatisfy(file -> { - assertThat(file.getContent()).asInstanceOf(InstanceOfAssertFactories.FILE) - .doesNotExist(); - }); + assertThat(uploadedFiles).allSatisfy(file -> + assertThat(file.getContent()).asInstanceOf(InstanceOfAssertFactories.FILE) + .doesNotExist() + ); } @Test @@ -315,6 +315,7 @@ abstract class AbstractMultiPartRequestTest { @Test public void maxSize() throws IOException { + // given String content = formFile("file1", "test1.csv", "1,2,3,4") + formFile("file2", "test2.csv", "5,6,7,8") + endline + "--" + boundary + "--"; @@ -323,10 +324,13 @@ abstract class AbstractMultiPartRequestTest { assertThat(JakartaServletDiskFileUpload.isMultipartContent(mockRequest)).isTrue(); + // when multiPart.setMaxSize("1"); multiPart.parse(mockRequest, tempDir.toString()); - Arrays.stream(multiPart.getFile("file1")).findFirst().map(UploadedFile::length); + // then + assertThat(multiPart.uploadedFiles) + .isEmpty(); assertThat(multiPart.getErrors()) .map(LocalizedMessage::getTextKey) @@ -351,6 +355,24 @@ abstract class AbstractMultiPartRequestTest { .containsExactly("struts.messages.upload.error.FileUploadByteCountLimitException"); } + @Test + public void maxFiles() throws IOException { + String content = formFile("file1", "test1.csv", "1,2,3,4") + + formFile("file2", "test2.csv", "5,6,7,8") + + endline + "--" + boundary + "--"; + + mockRequest.setContent(content.getBytes(StandardCharsets.US_ASCII)); + + assertThat(JakartaServletDiskFileUpload.isMultipartContent(mockRequest)).isTrue(); + + multiPart.setMaxFiles("1"); + multiPart.parse(mockRequest, tempDir.toString()); + + assertThat(multiPart.errors) + .map(LocalizedMessage::getTextKey) + .containsExactly("struts.messages.upload.error.FileUploadFileCountLimitException"); + } + @Test public void maxStringLength() throws IOException { String content = formFile("file1", "test1.csv", "1,2,3,4") + diff --git a/core/src/test/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequestTest.java b/core/src/test/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequestTest.java index 056f68cf3..e24dc133d 100644 --- a/core/src/test/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequestTest.java +++ b/core/src/test/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequestTest.java @@ -18,39 +18,13 @@ */ package org.apache.struts2.dispatcher.multipart; -import org.apache.commons.fileupload2.jakarta.servlet6.JakartaServletDiskFileUpload; -import org.apache.struts2.dispatcher.LocalizedMessage; -import org.junit.Test; - import java.io.File; -import java.io.IOException; -import java.nio.charset.StandardCharsets; -import static org.assertj.core.api.Assertions.assertThat; - -public class JakartaMultiPartRequestTest extends AbstractMultiPartRequestTest { +public class JakartaMultiPartRequestTest extends AbstractMultiPartRequestTest { @Override protected AbstractMultiPartRequest createMultipartRequest() { return new JakartaMultiPartRequest(); } - @Test - public void maxFiles() throws IOException { - String content = formFile("file1", "test1.csv", "1,2,3,4") + - formFile("file2", "test2.csv", "5,6,7,8") + - endline + "--" + boundary + "--"; - - mockRequest.setContent(content.getBytes(StandardCharsets.US_ASCII)); - - assertThat(JakartaServletDiskFileUpload.isMultipartContent(mockRequest)).isTrue(); - - multiPart.setMaxFiles("1"); - multiPart.parse(mockRequest, tempDir.toString()); - - assertThat(multiPart.errors) - .map(LocalizedMessage::getTextKey) - .containsExactly("struts.messages.upload.error.FileUploadFileCountLimitException"); - } - } \ No newline at end of file diff --git a/core/src/test/java/org/apache/struts2/dispatcher/multipart/JakartaStreamMultiPartRequestTest.java b/core/src/test/java/org/apache/struts2/dispatcher/multipart/JakartaStreamMultiPartRequestTest.java index 111466fb6..aa54316f5 100644 --- a/core/src/test/java/org/apache/struts2/dispatcher/multipart/JakartaStreamMultiPartRequestTest.java +++ b/core/src/test/java/org/apache/struts2/dispatcher/multipart/JakartaStreamMultiPartRequestTest.java @@ -20,40 +20,55 @@ package org.apache.struts2.dispatcher.multipart; import org.apache.commons.fileupload2.jakarta.servlet6.JakartaServletDiskFileUpload; import org.apache.struts2.dispatcher.LocalizedMessage; +import org.assertj.core.api.InstanceOfAssertFactories; import org.junit.Test; -import java.io.File; import java.io.IOException; import java.nio.charset.StandardCharsets; import static org.assertj.core.api.Assertions.assertThat; -public class JakartaStreamMultiPartRequestTest extends AbstractMultiPartRequestTest { +public class JakartaStreamMultiPartRequestTest extends AbstractMultiPartRequestTest { @Override - protected AbstractMultiPartRequest createMultipartRequest() { + protected AbstractMultiPartRequest createMultipartRequest() { return new JakartaStreamMultiPartRequest(); } @Test - public void maxFilesNotSupportedInJakartaStreamMultiPartRequest() throws IOException { + public void maxSizeOfFiles() throws IOException { + // given String content = formFile("file1", "test1.csv", "1,2,3,4") + formFile("file2", "test2.csv", "5,6,7,8") + endline + "--" + boundary + "--"; - mockRequest.setContent(content.getBytes(StandardCharsets.US_ASCII)); + mockRequest.setContent(content.getBytes(StandardCharsets.UTF_8)); assertThat(JakartaServletDiskFileUpload.isMultipartContent(mockRequest)).isTrue(); - multiPart.setMaxFiles("1"); + // when + multiPart.setMaxSizeOfFiles("10"); multiPart.parse(mockRequest, tempDir.toString()); - assertThat(multiPart.errors) + // then + assertThat(multiPart.uploadedFiles) + .hasSize(1); + assertThat(multiPart.getFile("file1")).allSatisfy(file -> { + assertThat(file.isFile()) + .isTrue(); + assertThat(file.getOriginalName()) + .isEqualTo("test1.csv"); + assertThat(file.getContentType()) + .isEqualTo("text/csv"); + assertThat(file.getContent()) + .asInstanceOf(InstanceOfAssertFactories.FILE) + .exists() + .content() + .isEqualTo("1,2,3,4"); + }); + assertThat(multiPart.getErrors()) .map(LocalizedMessage::getTextKey) - .isEmpty(); - assertThat(multiPart.getFileParameterNames().asIterator()).toIterable() - .hasSize(2) - .contains("file1", "file2"); + .containsExactly("struts.messages.upload.error.FileUploadSizeException"); } } diff --git a/core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java b/core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java index fe9468b37..40c4103f4 100644 --- a/core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java +++ b/core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java @@ -52,7 +52,7 @@ import static org.assertj.core.api.Assertions.assertThat; */ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase { - private static final UploadedFile EMPTY_FILE = new UploadedFile() { + private static final UploadedFile EMPTY_FILE = new UploadedFile<>() { @Override public Long length() { return 0L; @@ -79,8 +79,8 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase { } @Override - public byte[] getContent() { - return new byte[0]; + public String getContent() { + return ""; } @Override @@ -210,7 +210,7 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase { URL url = ClassLoaderUtil.getResource("log4j2.xml", ActionFileUploadInterceptorTest.class); File file = new File(new URI(url.toString())); assertTrue("log4j2.xml should be in src/test folder", file.exists()); - UploadedFile uploadedFile = StrutsUploadedFile.Builder.create(file).withContentType("text/html").withOriginalName("filename").build(); + UploadedFile uploadedFile = StrutsUploadedFile.Builder.create(file).withContentType("text/html").withOriginalName("filename").build(); boolean notOk = interceptor.acceptFile(validation, uploadedFile, "filename", "text/html", "inputName"); assertFalse(notOk); @@ -229,7 +229,7 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase { } public void testNoMultipartRequest() throws Exception { - MyFileUploadAction action = new MyFileUploadAction(); + MyFileUploadAction action = new MyFileUploadAction<>(); MockActionInvocation mai = new MockActionInvocation(); mai.setAction(action); @@ -250,13 +250,13 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase { req.setContentType("multipart/form-data"); // not a multipart contentype req.setMethod("post"); - MyFileUploadAction action = container.inject(MyFileUploadAction.class); + MyFileUploadAction action = container.inject(MyFileUploadAction.class); MockActionInvocation mai = new MockActionInvocation(); mai.setAction(action); mai.setResultCode("success"); mai.setInvocationContext(ActionContext.getContext()); - ActionContext.getContext().put(ServletActionContext.HTTP_REQUEST, createMultipartRequestMaxSize(req, 2000)); + ActionContext.getContext().withServletRequest(createMultipartRequestMaxSize(req, 2000)); interceptor.intercept(mai); @@ -271,13 +271,13 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase { req.addHeader("Content-type", "multipart/form-data"); req.setContent(null); // there is no content - MyFileUploadAction action = container.inject(MyFileUploadAction.class); + MyFileUploadAction action = container.inject(MyFileUploadAction.class); MockActionInvocation mai = new MockActionInvocation(); mai.setAction(action); mai.setResultCode("success"); mai.setInvocationContext(ActionContext.getContext()); - ActionContext.getContext().put(ServletActionContext.HTTP_REQUEST, createMultipartRequestMaxSize(req, 2000)); + ActionContext.getContext().withServletRequest(createMultipartRequestMaxSize(req, 2000)); interceptor.intercept(mai); @@ -301,19 +301,19 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase { """); req.setContent(content.getBytes(StandardCharsets.US_ASCII)); - MyFileUploadAction action = new MyFileUploadAction(); + MyFileUploadAction action = new MyFileUploadAction<>(); MockActionInvocation mai = new MockActionInvocation(); mai.setAction(action); mai.setResultCode("success"); mai.setInvocationContext(ActionContext.getContext()); - ActionContext.getContext().put(ServletActionContext.HTTP_REQUEST, createMultipartRequestMaxSize(req, 2000)); + ActionContext.getContext().withServletRequest(createMultipartRequestMaxSize(req, 2000)); interceptor.intercept(mai); assertFalse(action.hasErrors()); - List files = action.getUploadFiles(); + List> files = action.getUploadFiles(); assertNotNull(files); assertEquals(1, files.size()); @@ -338,7 +338,7 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase { assertTrue(JakartaServletDiskFileUpload.isMultipartContent(req)); - MyFileUploadAction action = new MyFileUploadAction(); + MyFileUploadAction action = new MyFileUploadAction<>(); container.inject(action); MockActionInvocation mai = new MockActionInvocation(); mai.setAction(action); @@ -349,7 +349,7 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase { interceptor.setAllowedTypes("text/html"); interceptor.intercept(mai); - List files = action.getUploadFiles(); + List> files = action.getUploadFiles(); assertNotNull(files); assertEquals("files accepted ", 2, files.size()); @@ -375,7 +375,7 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase { assertTrue(JakartaServletFileUpload.isMultipartContent(req)); - MyFileUploadAction action = new MyFileUploadAction(); + MyFileUploadAction action = new MyFileUploadAction<>(); container.inject(action); MockActionInvocation mai = new MockActionInvocation(); mai.setAction(action); @@ -411,7 +411,7 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase { """); req.setContent(content.getBytes(StandardCharsets.US_ASCII)); - MyFileUploadAction action = container.inject(MyFileUploadAction.class); + MyFileUploadAction action = container.inject(MyFileUploadAction.class); MockActionInvocation mai = new MockActionInvocation(); mai.setAction(action); @@ -458,7 +458,7 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase { """); req.setContent(content.getBytes(StandardCharsets.US_ASCII)); - MyFileUploadAction action = container.inject(MyFileUploadAction.class); + MyFileUploadAction action = container.inject(MyFileUploadAction.class); MockActionInvocation mai = new MockActionInvocation(); mai.setAction(action); @@ -496,7 +496,7 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase { """); req.setContent(content.getBytes(StandardCharsets.US_ASCII)); - MyFileUploadAction action = container.inject(MyFileUploadAction.class); + MyFileUploadAction action = container.inject(MyFileUploadAction.class); MockActionInvocation mai = new MockActionInvocation(); mai.setAction(action); @@ -546,12 +546,12 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase { } private MultiPartRequestWrapper createMultipartRequest(HttpServletRequest req, int maxsize, int maxfilesize, int maxfiles, int maxStringLength) { - JakartaMultiPartRequest jak = new JakartaMultiPartRequest(); jak.setMaxSize(String.valueOf(maxsize)); jak.setMaxFileSize(String.valueOf(maxfilesize)); jak.setMaxFiles(String.valueOf(maxfiles)); jak.setMaxStringLength(String.valueOf(maxStringLength)); + jak.setDefaultEncoding(StandardCharsets.UTF_8.name()); return new MultiPartRequestWrapper(jak, req, tempDir.getAbsolutePath(), new DefaultLocaleProvider()); } @@ -571,15 +571,15 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase { super.tearDown(); } - public static class MyFileUploadAction extends ActionSupport implements UploadedFilesAware { - private List uploadedFiles; + public static class MyFileUploadAction extends ActionSupport implements UploadedFilesAware { + private List> uploadedFiles; @Override - public void withUploadedFiles(List uploadedFiles) { + public void withUploadedFiles(List> uploadedFiles) { this.uploadedFiles = uploadedFiles; } - public List getUploadFiles() { + public List> getUploadFiles() { return this.uploadedFiles; } } 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 64caa523a..2f3145979 100644 --- a/core/src/test/java/org/apache/struts2/interceptor/FileUploadInterceptorTest.java +++ b/core/src/test/java/org/apache/struts2/interceptor/FileUploadInterceptorTest.java @@ -50,10 +50,9 @@ import static org.assertj.core.api.Assertions.assertThat; /** * Test case for FileUploadInterceptor. */ - public class FileUploadInterceptorTest extends StrutsInternalTestCase { - private static final UploadedFile EMPTY_FILE = new UploadedFile() { + private static final UploadedFile EMPTY_FILE = new UploadedFile<>() { @Override public Long length() { return 0L; @@ -80,8 +79,8 @@ public class FileUploadInterceptorTest extends StrutsInternalTestCase { } @Override - public byte[] getContent() { - return new byte[0]; + public String getContent() { + return ""; } @Override @@ -206,7 +205,11 @@ public class FileUploadInterceptorTest extends StrutsInternalTestCase { URL url = ClassLoaderUtil.getResource("log4j2.xml", FileUploadInterceptorTest.class); File file = new File(new URI(url.toString())); assertTrue("log4j2.xml should be in src/test folder", file.exists()); - UploadedFile uploadedFile = StrutsUploadedFile.Builder.create(file).withContentType("text/html").withOriginalName("filename").build(); + UploadedFile uploadedFile = StrutsUploadedFile.Builder.create(file) + .withContentType("text/html") + .withOriginalName("filename") + .build(); + boolean notOk = interceptor.acceptFile(validation, uploadedFile, "filename", "text/html", "inputName"); assertFalse(notOk); @@ -249,7 +252,7 @@ public class FileUploadInterceptorTest extends StrutsInternalTestCase { mai.setInvocationContext(ActionContext.getContext()); ActionContext.getContext().withParameters(HttpParameters.create().build()); - ActionContext.getContext().put(ServletActionContext.HTTP_REQUEST, createMultipartRequestMaxSize(req, 2000)); + ActionContext.getContext().withServletRequest(createMultipartRequestMaxSize(req, 2000)); interceptor.intercept(mai); @@ -271,7 +274,7 @@ public class FileUploadInterceptorTest extends StrutsInternalTestCase { mai.setInvocationContext(ActionContext.getContext()); ActionContext.getContext().withParameters(HttpParameters.create().build()); - ActionContext.getContext().put(ServletActionContext.HTTP_REQUEST, createMultipartRequestMaxSize(req, 2000)); + ActionContext.getContext().withServletRequest(createMultipartRequestMaxSize(req, 2000)); interceptor.intercept(mai); @@ -302,7 +305,7 @@ public class FileUploadInterceptorTest extends StrutsInternalTestCase { mai.setInvocationContext(ActionContext.getContext()); Map param = new HashMap<>(); ActionContext.getContext().withParameters(HttpParameters.create(param).build()); - ActionContext.getContext().put(ServletActionContext.HTTP_REQUEST, createMultipartRequestMaxSize(req, 2000)); + ActionContext.getContext().withServletRequest(createMultipartRequestMaxSize(req, 2000)); interceptor.intercept(mai); @@ -310,7 +313,7 @@ public class FileUploadInterceptorTest extends StrutsInternalTestCase { HttpParameters parameters = mai.getInvocationContext().getParameters(); assertEquals(3, parameters.keySet().size()); - UploadedFile[] files = (UploadedFile[]) parameters.get("file").getObject(); + UploadedFile[] files = (UploadedFile[]) parameters.get("file").getObject(); String[] fileContentTypes = parameters.get("fileContentType").getMultipleValues(); String[] fileRealFilenames = parameters.get("fileFileName").getMultipleValues(); @@ -360,14 +363,14 @@ public class FileUploadInterceptorTest extends StrutsInternalTestCase { mai.setInvocationContext(ActionContext.getContext()); Map param = new HashMap<>(); ActionContext.getContext().withParameters(HttpParameters.create(param).build()); - ActionContext.getContext().put(ServletActionContext.HTTP_REQUEST, createMultipartRequestMaxSize(req, 2000)); + ActionContext.getContext().withServletRequest(createMultipartRequestMaxSize(req, 2000)); interceptor.setAllowedTypes("text/html"); interceptor.intercept(mai); HttpParameters parameters = mai.getInvocationContext().getParameters(); assertEquals(3, parameters.keySet().size()); - UploadedFile[] files = (UploadedFile[]) parameters.get("file").getObject(); + UploadedFile[] files = (UploadedFile[]) parameters.get("file").getObject(); String[] fileContentTypes = parameters.get("fileContentType").getMultipleValues(); String[] fileRealFilenames = parameters.get("fileFileName").getMultipleValues(); @@ -412,7 +415,7 @@ public class FileUploadInterceptorTest extends StrutsInternalTestCase { mai.setInvocationContext(ActionContext.getContext()); Map param = new HashMap<>(); ActionContext.getContext().withParameters(HttpParameters.create(param).build()); - ActionContext.getContext().put(ServletActionContext.HTTP_REQUEST, createMultipartRequestMaxFiles(req)); + ActionContext.getContext().withServletRequest(createMultipartRequestMaxFiles(req)); interceptor.setAllowedTypes("text/html"); interceptor.intercept(mai); @@ -588,12 +591,12 @@ public class FileUploadInterceptorTest extends StrutsInternalTestCase { } private MultiPartRequestWrapper createMultipartRequest(HttpServletRequest req, int maxsize, int maxfilesize, int maxfiles, int maxStringLength) { - JakartaMultiPartRequest jak = new JakartaMultiPartRequest(); jak.setMaxSize(String.valueOf(maxsize)); jak.setMaxFileSize(String.valueOf(maxfilesize)); jak.setMaxFiles(String.valueOf(maxfiles)); jak.setMaxStringLength(String.valueOf(maxStringLength)); + jak.setDefaultEncoding(StandardCharsets.UTF_8.name()); return new MultiPartRequestWrapper(jak, req, tempDir.getAbsolutePath(), new DefaultLocaleProvider()); } @@ -614,11 +617,6 @@ public class FileUploadInterceptorTest extends StrutsInternalTestCase { } public static class MyFileupAction extends ActionSupport { - - private static final long serialVersionUID = 6255238895447968889L; - - // no methods } - }