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/apps/showcase/src/main/resources/struts-fileupload.xml b/apps/showcase/src/main/resources/struts-fileupload.xml index 5b500b73f..8f7596d51 100644 --- a/apps/showcase/src/main/resources/struts-fileupload.xml +++ b/apps/showcase/src/main/resources/struts-fileupload.xml @@ -24,6 +24,9 @@ "https://struts.apache.org/dtds/struts-6.0.dtd"> + + + diff --git a/apps/showcase/src/main/webapp/WEB-INF/fileupload/upload-success.jsp b/apps/showcase/src/main/webapp/WEB-INF/fileupload/upload-success.jsp index a1b06277a..576772550 100644 --- a/apps/showcase/src/main/webapp/WEB-INF/fileupload/upload-success.jsp +++ b/apps/showcase/src/main/webapp/WEB-INF/fileupload/upload-success.jsp @@ -41,7 +41,8 @@
  • FileName:
  • Original FileName:
  • File:
    • -
  • Caption:
    • +
  • Size:
    • +
  • Caption:
    • diff --git a/apps/showcase/src/test/java/it/org/apache/struts2/showcase/FileUploadTest.java b/apps/showcase/src/test/java/it/org/apache/struts2/showcase/FileUploadTest.java index fdbc1c311..cb168ab46 100644 --- a/apps/showcase/src/test/java/it/org/apache/struts2/showcase/FileUploadTest.java +++ b/apps/showcase/src/test/java/it/org/apache/struts2/showcase/FileUploadTest.java @@ -18,9 +18,6 @@ */ package it.org.apache.struts2.showcase; -import java.io.File; -import java.io.FileWriter; - import org.htmlunit.WebClient; import org.htmlunit.html.HtmlFileInput; import org.htmlunit.html.HtmlForm; @@ -29,6 +26,10 @@ import org.htmlunit.html.HtmlPage; import org.htmlunit.html.HtmlSubmitInput; import org.junit.Test; +import java.io.File; +import java.io.FileWriter; +import java.security.SecureRandom; + import static org.assertj.core.api.Assertions.assertThat; public class FileUploadTest { @@ -46,6 +47,42 @@ public class FileUploadTest { try (FileWriter writer = new FileWriter(tempFile)) { writer.append("Some strings"); + writer.flush(); + } + + uploadInput.setValue(tempFile.getAbsolutePath()); + final HtmlSubmitInput button = form.getInputByValue("Submit"); + final HtmlPage resultPage = button.click(); + + String content = resultPage.getVisibleText(); + assertThat(content).contains( + "ContentType: text/plain", + "Original FileName: " + tempFile.getName(), + "Caption: some caption", + "Size: 12" + ); + } + } + + @Test + public void testUploadOverMaxSize() throws Exception { + try (final WebClient webClient = new WebClient()) { + final HtmlPage page = webClient.getPage(ParameterUtils.getBaseUrl() + "/fileupload/doUpload.action"); + final HtmlForm form = page.getFormByName("doUpload"); + HtmlInput captionInput = form.getInputByName("caption"); + HtmlFileInput uploadInput = form.getInputByName("upload"); + + captionInput.type("Large file"); + + File tempFile = File.createTempFile("testEmptyFile", ".txt"); + SecureRandom rng = new SecureRandom(); + tempFile.deleteOnExit(); + try (FileWriter writer = new FileWriter(tempFile)) { + for (int i = 0; i < 10240; ++i) { + String line = String.format("%s %s%n", rng.nextInt(), rng.nextInt()); + writer.append(line); + } + writer.flush(); } uploadInput.setValue(tempFile.getAbsolutePath()); @@ -55,9 +92,7 @@ public class FileUploadTest { String content = resultPage.getVisibleText(); System.out.println(content); assertThat(content).contains( - "ContentType: text/plain", - "Original FileName: " + tempFile.getName(), - "Caption:some caption" + "Request exceeded allowed size limit! Max size allowed is: 10,240!" ); } } diff --git a/core/pom.xml b/core/pom.xml index b4dd4b69c..a52453c08 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -19,7 +19,8 @@ * under the License. */ --> - + 4.0.0 org.apache.struts @@ -216,8 +217,7 @@ org.apache.commons - commons-fileupload2-jakarta - 2.0.0-M1 + commons-fileupload2-jakarta-servlet6 commons-io 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..2127bb663 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/Dispatcher.java @@ -147,7 +147,7 @@ public class Dispatcher { private String defaultLocale; /** - * Store state of StrutsConstants.STRUTS_MULTIPART_SAVEDIR setting. + * Store state of {@link StrutsConstants#STRUTS_MULTIPART_SAVE_DIR} setting. */ private String multipartSaveDir; @@ -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; } @@ -323,7 +323,7 @@ public class Dispatcher { /** * @deprecated since 6.4.0, no replacement. */ - @Deprecated(since = "6.4.9", forRemoval = true) + @Deprecated(since = "6.4.0", forRemoval = true) public void setMultipartHandler(String val) { // no-op } @@ -868,11 +868,12 @@ public class Dispatcher { * @return the path to save uploaded files to */ protected String getSaveDir() { - String saveDir = multipartSaveDir.trim(); + String saveDir = Objects.toString(multipartSaveDir, "").trim(); - if (saveDir.equals("")) { - File tempdir = (File) servletContext.getAttribute("jakarta.servlet.context.tempdir"); - LOG.info("Unable to find 'struts.multipart.saveDir' property setting. Defaulting to jakarta.servlet.context.tempdir"); + if (saveDir.isEmpty()) { + File tempdir = (File) servletContext.getAttribute(ServletContext.TEMPDIR); + LOG.info("Unable to find: {} property setting. Defaulting to: {}", + StrutsConstants.STRUTS_MULTIPART_SAVE_DIR, ServletContext.TEMPDIR); if (tempdir != null) { saveDir = tempdir.toString(); @@ -885,9 +886,9 @@ public class Dispatcher { if (!multipartSaveDir.mkdirs()) { String logMessage; try { - logMessage = "Could not find create multipart save directory '" + multipartSaveDir.getCanonicalPath() + "'."; + logMessage = "Could not create multipart save directory '" + multipartSaveDir.getCanonicalPath() + "'."; } catch (IOException e) { - logMessage = "Could not find create multipart save directory '" + multipartSaveDir.toString() + "'."; + logMessage = "Could not create multipart save directory '" + multipartSaveDir + "'."; } if (devMode) { LOG.error(logMessage); 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 256fea051..67ece0514 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 @@ -18,23 +18,35 @@ */ package org.apache.struts2.dispatcher.multipart; -import com.opensymphony.xwork2.LocaleProviderFactory; import com.opensymphony.xwork2.inject.Inject; +import jakarta.servlet.http.HttpServletRequest; +import org.apache.commons.fileupload2.core.FileUploadByteCountLimitException; +import org.apache.commons.fileupload2.core.FileUploadContentTypeException; +import org.apache.commons.fileupload2.core.FileUploadException; +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.StrutsConstants; import org.apache.struts2.dispatcher.LocalizedMessage; -import jakarta.servlet.http.HttpServletRequest; +import java.io.IOException; +import java.nio.charset.Charset; +import java.nio.file.Path; import java.util.ArrayList; +import java.util.Collections; +import java.util.Enumeration; +import java.util.HashMap; import java.util.List; -import java.util.Locale; +import java.util.Map; /** * Abstract class with some helper methods, it should be used * when starting development of another implementation of {@link MultiPartRequest} */ -public abstract class AbstractMultiPartRequest implements MultiPartRequest { +public abstract class AbstractMultiPartRequest implements MultiPartRequest { protected static final String STRUTS_MESSAGES_UPLOAD_ERROR_PARAMETER_TOO_LONG_KEY = "struts.messages.upload.error.parameter.too.long"; @@ -55,6 +67,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. */ @@ -75,17 +92,25 @@ public abstract class AbstractMultiPartRequest implements MultiPartRequest { */ protected int bufferSize = BUFFER_SIZE; + /** + * Defines default encoding to encode data from request used if not provided with request + */ protected String defaultEncoding; /** - * Localization to be used regarding errors. + * Map between file fields and file data. */ - protected Locale defaultLocale = Locale.ENGLISH; + protected Map>> uploadedFiles = new HashMap<>(); + + /** + * Map between non-file fields and values. + */ + protected Map> parameters = new HashMap<>(); /** * @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); } @@ -98,45 +123,154 @@ 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); } - @Inject(StrutsConstants.STRUTS_MULTIPART_MAXFILES) + /** + * @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_MAX_FILES) public void setMaxFiles(String maxFiles) { this.maxFiles = Long.parseLong(maxFiles); } - @Inject(value = StrutsConstants.STRUTS_MULTIPART_MAXFILESIZE, required = false) + /** + * @param maxFileSize Injects the Struts maximum number of files, which can be uploaded. + */ + @Inject(value = StrutsConstants.STRUTS_MULTIPART_MAX_FILE_SIZE, required = false) public void setMaxFileSize(String maxFileSize) { this.maxFileSize = Long.parseLong(maxFileSize); } + /** + * @param maxStringLength Injects the Struts maximum size of single form field. + */ @Inject(StrutsConstants.STRUTS_MULTIPART_MAX_STRING_LENGTH) public void setMaxStringLength(String maxStringLength) { this.maxStringLength = Long.parseLong(maxStringLength); } - @Inject - public void setLocaleProviderFactory(LocaleProviderFactory localeProviderFactory) { - defaultLocale = localeProviderFactory.createLocaleProvider().getLocale(); + /** + * Process the request extract file upload data + * + * @param request current {@link HttpServletRequest} + * @param saveDir a temporary directory to store files + */ + protected abstract void processUpload(HttpServletRequest request, String saveDir) throws IOException; + + /** + * @param request multipart request + * @return character encoding from request or {@link #defaultEncoding} + */ + protected Charset readCharsetEncoding(HttpServletRequest request) { + String charsetStr = StringUtils.isBlank(request.getCharacterEncoding()) + ? defaultEncoding + : request.getCharacterEncoding(); + + return Charset.forName(charsetStr); } /** - * @param request Inspect the servlet request and set the locale if one wasn't provided by - * the Struts2 framework. + * Creates an instance of {@link JakartaServletDiskFileUpload} used by the parser to extract uploaded files + * + * @param charset used charset from incoming request + * @param saveDir a temporary folder to store uploaded files (not always needed) */ - protected void setLocale(HttpServletRequest request) { - if (defaultLocale == null) { - defaultLocale = request.getLocale(); + protected abstract JakartaServletDiskFileUpload createJakartaFileUpload(Charset charset, Path saveDir); + + protected JakartaServletDiskFileUpload prepareServletFileUpload(Charset charset, Path saveDir) { + JakartaServletDiskFileUpload servletFileUpload = createJakartaFileUpload(charset, saveDir); + + if (maxSize != null) { + LOG.debug("Applies max size: {} to file upload request", maxSize); + servletFileUpload.setSizeMax(maxSize); + } + if (maxFiles != null) { + LOG.debug("Applies max files number: {} to file upload request", maxFiles); + servletFileUpload.setFileCountMax(maxFiles); + } + if (maxFileSize != null) { + LOG.debug("Applies max size of single file: {} to file upload request", maxFileSize); + servletFileUpload.setFileSizeMax(maxFileSize); + } + return servletFileUpload; + } + + protected boolean exceedsMaxStringLength(String fieldName, String fieldValue) { + if (maxStringLength != null && fieldValue.length() > maxStringLength) { + if (LOG.isDebugEnabled()) { + LOG.debug("Form field: {} of size: {} bytes exceeds limit of: {}.", + sanitizeNewlines(fieldName), fieldValue.length(), maxStringLength); + } + LocalizedMessage localizedMessage = new LocalizedMessage(this.getClass(), + STRUTS_MESSAGES_UPLOAD_ERROR_PARAMETER_TOO_LONG_KEY, null, + new Object[]{fieldName, maxStringLength, fieldValue.length()}); + if (!errors.contains(localizedMessage)) { + errors.add(localizedMessage); + } + return true; + } + return false; + } + + /** + * Processes the upload. + * + * @param request the servlet request + * @param saveDir location of the save dir + */ + public void parse(HttpServletRequest request, String saveDir) throws IOException { + try { + processUpload(request, saveDir); + } catch (FileUploadException e) { + LOG.debug("Request exceeded size limit!", e); + LocalizedMessage errorMessage; + if (e instanceof FileUploadByteCountLimitException ex) { + errorMessage = buildErrorMessage(e, new Object[]{ + ex.getFieldName(), ex.getFileName(), ex.getPermitted(), ex.getActualSize() + }); + } else if (e instanceof FileUploadFileCountLimitException ex) { + errorMessage = buildErrorMessage(e, new Object[]{ + ex.getPermitted(), ex.getActualSize() + }); + } else if (e instanceof FileUploadSizeException ex) { + errorMessage = buildErrorMessage(e, new Object[]{ + ex.getPermitted(), ex.getActualSize() + }); + } else if (e instanceof FileUploadContentTypeException ex) { + errorMessage = buildErrorMessage(e, new Object[]{ + ex.getContentType() + }); + } else { + errorMessage = buildErrorMessage(e, new Object[]{}); + } + + if (!errors.contains(errorMessage)) { + errors.add(errorMessage); + } + } catch (IOException e) { + LOG.debug("Unable to parse request", e); + LocalizedMessage errorMessage = buildErrorMessage(e, new Object[]{}); + if (!errors.contains(errorMessage)) { + errors.add(errorMessage); + } } } /** * Build error message. * - * @param e the Throwable/Exception + * @param e the Throwable/Exception * @param args arguments * @return error message */ @@ -147,13 +281,6 @@ public abstract class AbstractMultiPartRequest implements MultiPartRequest { return new LocalizedMessage(this.getClass(), errorKey, e.getMessage(), args); } - /* (non-Javadoc) - * @see org.apache.struts2.dispatcher.multipart.MultiPartRequest#getErrors() - */ - public List getErrors() { - return errors; - } - /** * @param originalFileName file name * @return the canonical name based on the supplied filename @@ -171,4 +298,109 @@ public abstract class AbstractMultiPartRequest implements MultiPartRequest { return fileName; } + protected String sanitizeNewlines(String before) { + return before.replaceAll("\\R", "_"); + } + + /* (non-Javadoc) + * @see org.apache.struts2.dispatcher.multipart.MultiPartRequest#getErrors() + */ + public List getErrors() { + return errors; + } + + /* (non-Javadoc) + * @see org.apache.struts2.dispatcher.multipart.MultiPartRequest#getFileParameterNames() + */ + public Enumeration getFileParameterNames() { + return Collections.enumeration(uploadedFiles.keySet()); + } + + /* (non-Javadoc) + * @see org.apache.struts2.dispatcher.multipart.MultiPartRequest#getContentType(java.lang.String) + */ + public String[] getContentType(String fieldName) { + return uploadedFiles.getOrDefault(fieldName, Collections.emptyList()).stream() + .map(UploadedFile::getContentType) + .toArray(String[]::new); + } + + /* (non-Javadoc) + * @see org.apache.struts2.dispatcher.multipart.MultiPartRequest#getFile(java.lang.String) + */ + @SuppressWarnings("unchecked") + public UploadedFile[] getFile(String fieldName) { + return uploadedFiles.getOrDefault(fieldName, Collections.emptyList()) + .toArray(UploadedFile[]::new); + } + + /* (non-Javadoc) + * @see org.apache.struts2.dispatcher.multipart.MultiPartRequest#getFileNames(java.lang.String) + */ + public String[] getFileNames(String fieldName) { + return uploadedFiles.getOrDefault(fieldName, Collections.emptyList()).stream() + .map(file -> getCanonicalName(file.getName())) + .toArray(String[]::new); + } + + /* (non-Javadoc) + * @see org.apache.struts2.dispatcher.multipart.MultiPartRequest#getFilesystemName(java.lang.String) + */ + public String[] getFilesystemName(String fieldName) { + return uploadedFiles.getOrDefault(fieldName, Collections.emptyList()).stream() + .map(UploadedFile::getAbsolutePath) + .toArray(String[]::new); + } + + /* (non-Javadoc) + * @see org.apache.struts2.dispatcher.multipart.MultiPartRequest#getParameter(java.lang.String) + */ + public String getParameter(String name) { + List paramValue = parameters.getOrDefault(name, Collections.emptyList()); + if (!paramValue.isEmpty()) { + return paramValue.get(0); + } + + return null; + } + + /* (non-Javadoc) + * @see org.apache.struts2.dispatcher.multipart.MultiPartRequest#getParameterNames() + */ + public Enumeration getParameterNames() { + return Collections.enumeration(parameters.keySet()); + } + + /* (non-Javadoc) + * @see org.apache.struts2.dispatcher.multipart.MultiPartRequest#getParameterValues(java.lang.String) + */ + public String[] getParameterValues(String name) { + return parameters.getOrDefault(name, Collections.emptyList()) + .toArray(String[]::new); + } + + /* (non-Javadoc) + * @see org.apache.struts2.dispatcher.multipart.MultiPartRequest#cleanUp() + */ + public void cleanUp() { + try { + LOG.debug("Performing File Upload temporary storage cleanup."); + for (List> uploadedFileList : uploadedFiles.values()) { + for (UploadedFile uploadedFile : uploadedFileList) { + if (uploadedFile.isFile()) { + LOG.debug("Deleting file: {}", uploadedFile.getName()); + if (!uploadedFile.delete()) { + LOG.warn("There was a problem attempting to delete file: {}", uploadedFile.getName()); + } + } else { + LOG.debug("File: {} already deleted", uploadedFile.getName()); + } + } + } + } finally { + uploadedFiles = new HashMap<>(); + parameters = new HashMap<>(); + } + } + } 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 ca5afd9dd..c71a1ee25 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 @@ -21,375 +21,108 @@ package org.apache.struts2.dispatcher.multipart; import jakarta.servlet.http.HttpServletRequest; import org.apache.commons.fileupload2.core.DiskFileItem; import org.apache.commons.fileupload2.core.DiskFileItemFactory; -import org.apache.commons.fileupload2.core.FileItem; -import org.apache.commons.fileupload2.core.FileUploadByteCountLimitException; -import org.apache.commons.fileupload2.core.FileUploadContentTypeException; -import org.apache.commons.fileupload2.core.FileUploadException; -import org.apache.commons.fileupload2.core.FileUploadFileCountLimitException; -import org.apache.commons.fileupload2.core.FileUploadSizeException; -import org.apache.commons.fileupload2.core.RequestContext; -import org.apache.commons.fileupload2.jakarta.JakartaServletFileUpload; +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.File; import java.io.IOException; -import java.io.InputStream; -import java.io.UncheckedIOException; import java.nio.charset.Charset; +import java.nio.file.Path; 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. + * Multipart form data request adapter for Jakarta Commons FileUpload package. */ -public class JakartaMultiPartRequest extends AbstractMultiPartRequest { +public class JakartaMultiPartRequest extends AbstractMultiPartRequest { - static final Logger LOG = LogManager.getLogger(JakartaMultiPartRequest.class); - - // maps parameter name -> List of FileItem objects - protected Map> files = new HashMap<>(); - - // maps parameter name -> List of param values - protected Map> params = new HashMap<>(); - - /** - * Creates a new request wrapper to handle multipart data using methods adapted from Jason Pell's - * multipart classes (see class description). - * - * @param saveDir the directory to save off the file - * @param request the request containing the multipart - * @throws java.io.IOException is thrown if encoding fails. - */ - public void parse(HttpServletRequest request, String saveDir) throws IOException { - try { - setLocale(request); - processUpload(request, saveDir); - } catch (FileUploadException e) { - LOG.debug("Request exceeded size limit!", e); - LocalizedMessage errorMessage; - if (e instanceof FileUploadByteCountLimitException) { - FileUploadByteCountLimitException ex = (FileUploadByteCountLimitException) e; - errorMessage = buildErrorMessage(e, new Object[]{ - ex.getFieldName(), ex.getFileName(), ex.getPermitted(), ex.getActualSize() - }); - } else if (e instanceof FileUploadFileCountLimitException) { - FileUploadFileCountLimitException ex = (FileUploadFileCountLimitException) e; - errorMessage = buildErrorMessage(e, new Object[]{ - ex.getPermitted(), ex.getActualSize() - }); - } else if (e instanceof FileUploadSizeException) { - FileUploadSizeException ex = (FileUploadSizeException) e; - errorMessage = buildErrorMessage(e, new Object[]{ - ex.getPermitted(), ex.getActualSize() - }); - } else if (e instanceof FileUploadContentTypeException) { - FileUploadContentTypeException ex = (FileUploadContentTypeException) e; - errorMessage = buildErrorMessage(e, new Object[]{ - ex.getContentType() - }); - } else { - errorMessage = buildErrorMessage(e, new Object[]{}); - } - - if (!errors.contains(errorMessage)) { - errors.add(errorMessage); - } - } catch (Exception e) { - LOG.debug("Unable to parse request", e); - LocalizedMessage errorMessage = buildErrorMessage(e, new Object[]{}); - if (!errors.contains(errorMessage)) { - errors.add(errorMessage); - } - } - } + private static final Logger LOG = LogManager.getLogger(JakartaMultiPartRequest.class); + @Override protected void processUpload(HttpServletRequest request, String saveDir) throws IOException { + Charset charset = readCharsetEncoding(request); - if (JakartaServletFileUpload.isMultipartContent(request)) { - for (FileItem item : parseRequest(request, saveDir)) { - LOG.debug("Found file item: [{}]", sanitizeNewlines(item.getFieldName())); - if (item.isFormField()) { - processNormalFormField(item, request.getCharacterEncoding()); - } else { - processFileField(item); - } + JakartaServletDiskFileUpload servletFileUpload = + prepareServletFileUpload(charset, Path.of(saveDir)); + + for (DiskFileItem item : servletFileUpload.parseRequest(request)) { + LOG.debug(() -> "Processing a form field: " + sanitizeNewlines(item.getFieldName())); + if (item.isFormField()) { + processNormalFormField(item, charset); + } else { + LOG.debug(() -> "Processing a file: " + sanitizeNewlines(item.getFieldName())); + processFileField(item); } } } - protected void processFileField(FileItem item) { - LOG.debug("Item is a file upload"); + protected JakartaServletDiskFileUpload createJakartaFileUpload(Charset charset, Path saveDir) { + DiskFileItemFactory.Builder builder = DiskFileItemFactory.builder(); - // Skip file uploads that don't have a file name - meaning that no file was selected. - if (item.getName() == null || item.getName().trim().isEmpty()) { - LOG.debug("No file has been uploaded for the field: {}", sanitizeNewlines(item.getFieldName())); - return; - } + LOG.debug("Using file save directory: {}", saveDir); + builder.setPath(saveDir); - List values; - if (files.get(item.getFieldName()) != null) { - values = files.get(item.getFieldName()); + LOG.debug("Sets minimal buffer size to always write file to disk"); + builder.setBufferSize(1); + + LOG.debug("Using charset: {}", charset); + builder.setCharset(charset); + + DiskFileItemFactory factory = builder.get(); + return new JakartaServletDiskFileUpload(factory); + } + + protected void processNormalFormField(DiskFileItem item, Charset charset) throws IOException { + LOG.debug("Item: {} is a normal form field", item.getName()); + + List values; + String fieldName = item.getFieldName(); + if (parameters.get(fieldName) != null) { + values = parameters.get(fieldName); } else { values = new ArrayList<>(); } - values.add(item); - files.put(item.getFieldName(), values); + String fieldValue = item.getString(charset); + if (exceedsMaxStringLength(fieldName, fieldValue)) { + return; + } + if (item.getSize() == 0) { + values.add(StringUtils.EMPTY); + } else { + values.add(fieldValue); + } + parameters.put(fieldName, values); } - protected void processNormalFormField(FileItem item, String charset) throws IOException { - try { - LOG.debug("Item is a normal form field"); - Charset encoding = Charset.forName(charset); - - List values; - if (params.get(item.getFieldName()) != null) { - values = params.get(item.getFieldName()); - } else { - values = new ArrayList<>(); - } - - long size = item.getSize(); - if (size > maxStringLength) { - LOG.debug("Form field {} of size {} bytes exceeds limit of {}.", sanitizeNewlines(item.getFieldName()), size, maxStringLength); - LocalizedMessage localizedMessage = new LocalizedMessage(this.getClass(), - STRUTS_MESSAGES_UPLOAD_ERROR_PARAMETER_TOO_LONG_KEY, null, - new Object[]{item.getFieldName(), maxStringLength, size}); - if (!errors.contains(localizedMessage)) { - errors.add(localizedMessage); - } - return; - } - if (size == 0) { - values.add(StringUtils.EMPTY); - } else if (charset == null) { - values.add(item.getString()); // WW-633 - } else { - values.add(item.getString(encoding)); - } - params.put(item.getFieldName(), values); - } finally { - item.delete(); - } - } - - protected List parseRequest(HttpServletRequest servletRequest, String saveDir) throws FileUploadException { - DiskFileItemFactory fac = createDiskFileItemFactory(saveDir); - JakartaServletFileUpload upload = createServletFileUpload(fac); - - - return upload.parseRequest(createRequestContext(servletRequest)); - } - - protected JakartaServletFileUpload createServletFileUpload(DiskFileItemFactory fac) { - JakartaServletFileUpload upload = new JakartaServletFileUpload(fac); - if (maxSize != null) { - upload.setSizeMax(maxSize); - } - if (maxFiles != null) { - upload.setFileCountMax(maxFiles); - } - if (maxFileSize != null) { - upload.setFileSizeMax(maxFileSize); - } - return upload; - } - - protected DiskFileItemFactory createDiskFileItemFactory(String saveDir) { - DiskFileItemFactory.Builder fac = DiskFileItemFactory.builder(); - // Make sure that the data is written to file, even if the file is empty. - //setting 0 or -1 no longer seems to work for fileupload buffer size, so using 1 instead. - fac.setBufferSize(1); - if (saveDir != null) { - fac.setPath(saveDir); - } - return fac.get(); - } - - /* (non-Javadoc) - * @see org.apache.struts2.dispatcher.multipart.MultiPartRequest#getFileParameterNames() - */ - public Enumeration getFileParameterNames() { - return Collections.enumeration(files.keySet()); - } - - /* (non-Javadoc) - * @see org.apache.struts2.dispatcher.multipart.MultiPartRequest#getContentType(java.lang.String) - */ - public String[] getContentType(String fieldName) { - List items = files.get(fieldName); - - if (items == null) { - return null; + protected void processFileField(DiskFileItem item) { + // Skip file uploads that don't have a file name - meaning that no file was selected. + if (item.getName() == null || item.getName().trim().isEmpty()) { + LOG.debug(() -> "No file has been uploaded for the field: " + sanitizeNewlines(item.getFieldName())); + return; } - List contentTypes = new ArrayList<>(items.size()); - for (FileItem fileItem : items) { - contentTypes.add(fileItem.getContentType()); + List> values; + if (uploadedFiles.get(item.getFieldName()) != null) { + values = uploadedFiles.get(item.getFieldName()); + } else { + values = new ArrayList<>(); } - return contentTypes.toArray(new String[0]); - } - - /* (non-Javadoc) - * @see org.apache.struts2.dispatcher.multipart.MultiPartRequest#getFile(java.lang.String) - */ - public UploadedFile[] getFile(String fieldName) { - List items = files.get(fieldName); - - if (items == null) { - return null; - } - - List fileList = new ArrayList<>(items.size()); - for (FileItem fileItem : items) { - DiskFileItem diskFileItem = (DiskFileItem) fileItem; - File storeLocation = diskFileItem.getPath().toFile(); - - // Ensure file exists even if it is empty. - if (diskFileItem.getSize() == 0 && !storeLocation.exists()) { - try { - storeLocation.createNewFile(); - } catch (IOException e) { - LOG.error("Cannot write uploaded empty file to disk: {}", storeLocation.getAbsolutePath(), e); - } - } - UploadedFile uploadedFile = StrutsUploadedFile.Builder.create(storeLocation) - .withContentType(fileItem.getContentType()) - .withOriginalName(fileItem.getName()) + if (item.isInMemory()) { + LOG.warn("Storing uploaded files just in memory isn't supported currently, skipping file: {}!", item.getName()); + } else { + UploadedFile uploadedFile = StrutsUploadedFile.Builder + .create(item.getPath().toFile()) + .withOriginalName(item.getName()) + .withContentType(item.getContentType()) .build(); - fileList.add(uploadedFile); + values.add(uploadedFile); } - return fileList.toArray(new UploadedFile[0]); + uploadedFiles.put(item.getFieldName(), values); } - /* (non-Javadoc) - * @see org.apache.struts2.dispatcher.multipart.MultiPartRequest#getFileNames(java.lang.String) - */ - public String[] getFileNames(String fieldName) { - List items = files.get(fieldName); - - if (items == null) { - return null; - } - - List fileNames = new ArrayList<>(items.size()); - for (FileItem fileItem : items) { - fileNames.add(getCanonicalName(fileItem.getName())); - } - - return fileNames.toArray(new String[0]); - } - - /* (non-Javadoc) - * @see org.apache.struts2.dispatcher.multipart.MultiPartRequest#getFilesystemName(java.lang.String) - */ - public String[] getFilesystemName(String fieldName) { - List items = files.get(fieldName); - - if (items == null) { - return null; - } - - List fileNames = new ArrayList<>(items.size()); - for (FileItem fileItem : items) { - fileNames.add(((DiskFileItem) fileItem).getPath().toFile().getName()); - } - - return fileNames.toArray(new String[0]); - } - - /* (non-Javadoc) - * @see org.apache.struts2.dispatcher.multipart.MultiPartRequest#getParameter(java.lang.String) - */ - public String getParameter(String name) { - List v = params.get(name); - if (v != null && !v.isEmpty()) { - return v.get(0); - } - - return null; - } - - /* (non-Javadoc) - * @see org.apache.struts2.dispatcher.multipart.MultiPartRequest#getParameterNames() - */ - public Enumeration getParameterNames() { - return Collections.enumeration(params.keySet()); - } - - /* (non-Javadoc) - * @see org.apache.struts2.dispatcher.multipart.MultiPartRequest#getParameterValues(java.lang.String) - */ - public String[] getParameterValues(String name) { - List v = params.get(name); - if (v != null && !v.isEmpty()) { - return v.toArray(new String[0]); - } - - return null; - } - - /** - * Creates a RequestContext needed by Jakarta Commons Upload. - * - * @param req the request. - * @return a new request context. - */ - protected RequestContext createRequestContext(final HttpServletRequest req) { - return new RequestContext() { - public String getCharacterEncoding() { - return req.getCharacterEncoding(); - } - - public String getContentType() { - return req.getContentType(); - } - - public long getContentLength() { - return req.getContentLength(); - } - - public InputStream getInputStream() throws IOException { - InputStream in = req.getInputStream(); - if (in == null) { - throw new IOException("Missing content in the request"); - } - return req.getInputStream(); - } - }; - } - - /* (non-Javadoc) - * @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); - if (!item.isInMemory()) { - try { - item.delete(); - } catch (IOException e) { - throw new UncheckedIOException(e); - } - } - } - } - } - - private String sanitizeNewlines(String before) { - return before.replaceAll("[\n\r]", "_"); - } } 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 90ef7a047..53381935b 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 @@ -18,30 +18,28 @@ */ package org.apache.struts2.dispatcher.multipart; -import org.apache.commons.fileupload2.core.DiskFileItem; +import jakarta.servlet.http.HttpServletRequest; import org.apache.commons.fileupload2.core.DiskFileItemFactory; -import org.apache.commons.fileupload2.jakarta.JakartaServletFileUpload; -import org.apache.commons.fileupload2.core.FileUploadSizeException; -import org.apache.commons.fileupload2.core.FileItemInputIterator; 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.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.apache.struts2.dispatcher.LocalizedMessage; -import jakarta.servlet.http.HttpServletRequest; import java.io.BufferedOutputStream; +import java.io.ByteArrayOutputStream; import java.io.File; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.io.Serializable; +import java.nio.charset.Charset; +import java.nio.charset.StandardCharsets; import java.nio.file.Files; +import java.nio.file.Path; 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; /** @@ -52,155 +50,9 @@ import java.util.UUID; * * @since 2.3.18 */ -public class JakartaStreamMultiPartRequest extends AbstractMultiPartRequest { +public class JakartaStreamMultiPartRequest extends AbstractMultiPartRequest { - static final Logger LOG = LogManager.getLogger(JakartaStreamMultiPartRequest.class); - - /** - * Map between file fields and file data. - */ - protected Map> fileInfos = new HashMap<>(); - - /** - * Map between non-file fields and values. - */ - protected Map> parameters = new HashMap<>(); - - /* (non-Javadoc) - * @see org.apache.struts2.dispatcher.multipart.MultiPartRequest#cleanUp() - */ - public void cleanUp() { - LOG.debug("Performing File Upload temporary storage cleanup."); - for (List fileInfoList : fileInfos.values()) { - for (FileInfo fileInfo : fileInfoList) { - File file = fileInfo.getFile(); - LOG.debug("Deleting file '{}'.", file.getName()); - if (!file.delete()) { - LOG.warn("There was a problem attempting to delete file '{}'.", file.getName()); - } - } - } - } - - /* (non-Javadoc) - * @see org.apache.struts2.dispatcher.multipart.MultiPartRequest#getContentType(java.lang.String) - */ - public String[] getContentType(String fieldName) { - List infos = fileInfos.get(fieldName); - if (infos == null) { - return null; - } - - List types = new ArrayList<>(infos.size()); - for (FileInfo fileInfo : infos) { - types.add(fileInfo.getContentType()); - } - - return types.toArray(new String[0]); - } - - /* (non-Javadoc) - * @see org.apache.struts2.dispatcher.multipart.MultiPartRequest#getFile(java.lang.String) - */ - public UploadedFile[] getFile(String fieldName) { - List infos = fileInfos.get(fieldName); - if (infos == null) { - return null; - } - - return infos.stream().map(fileInfo -> - StrutsUploadedFile.Builder.create(fileInfo.getFile()) - .withContentType(fileInfo.contentType) - .withOriginalName(fileInfo.originalName) - .build() - ).toArray(UploadedFile[]::new); - } - - /* (non-Javadoc) - * @see org.apache.struts2.dispatcher.multipart.MultiPartRequest#getFileNames(java.lang.String) - */ - public String[] getFileNames(String fieldName) { - List infos = fileInfos.get(fieldName); - if (infos == null) { - return null; - } - - List names = new ArrayList<>(infos.size()); - for (FileInfo fileInfo : infos) { - names.add(getCanonicalName(fileInfo.getOriginalName())); - } - - return names.toArray(new String[0]); - } - - /* (non-Javadoc) - * @see org.apache.struts2.dispatcher.multipart.MultiPartRequest#getFileParameterNames() - */ - public Enumeration getFileParameterNames() { - return Collections.enumeration(fileInfos.keySet()); - } - - /* (non-Javadoc) - * @see org.apache.struts2.dispatcher.multipart.MultiPartRequest#getFilesystemName(java.lang.String) - */ - public String[] getFilesystemName(String fieldName) { - List infos = fileInfos.get(fieldName); - if (infos == null) { - return null; - } - - List names = new ArrayList<>(infos.size()); - for (FileInfo fileInfo : infos) { - names.add(fileInfo.getFile().getName()); - } - - return names.toArray(new String[0]); - } - - /* (non-Javadoc) - * @see org.apache.struts2.dispatcher.multipart.MultiPartRequest#getParameter(java.lang.String) - */ - public String getParameter(String name) { - List values = parameters.get(name); - if (values != null && !values.isEmpty()) { - return values.get(0); - } - return null; - } - - /* (non-Javadoc) - * @see org.apache.struts2.dispatcher.multipart.MultiPartRequest#getParameterNames() - */ - public Enumeration getParameterNames() { - return Collections.enumeration(parameters.keySet()); - } - - /* (non-Javadoc) - * @see org.apache.struts2.dispatcher.multipart.MultiPartRequest#getParameterValues(java.lang.String) - */ - public String[] getParameterValues(String name) { - List values = parameters.get(name); - if (values != null && !values.isEmpty()) { - return values.toArray(new String[0]); - } - return null; - } - - /* (non-Javadoc) - * @see org.apache.struts2.dispatcher.multipart.MultiPartRequest#parse(jakarta.servlet.http.HttpServletRequest, java.lang.String) - */ - public void parse(HttpServletRequest request, String saveDir) throws IOException { - try { - setLocale(request); - processUpload(request, saveDir); - } catch (Exception e) { - LOG.debug("Error occurred during parsing of multi part request", e); - LocalizedMessage errorMessage = buildErrorMessage(e, new Object[]{}); - if (!errors.contains(errorMessage)) { - errors.add(errorMessage); - } - } - } + private static final Logger LOG = LogManager.getLogger(JakartaStreamMultiPartRequest.class); /** * Processes the upload. @@ -208,153 +60,147 @@ public class JakartaStreamMultiPartRequest extends AbstractMultiPartRequest { * @param request the servlet request * @param saveDir location of the save dir */ - protected void processUpload(HttpServletRequest request, String saveDir) throws Exception { + @Override + protected void processUpload(HttpServletRequest request, String saveDir) throws IOException { + Charset charset = readCharsetEncoding(request); + Path location = Path.of(saveDir); - // Sanity check that the request is a multi-part/form-data request. - if (JakartaServletFileUpload.isMultipartContent(request)) { + JakartaServletDiskFileUpload servletFileUpload = + prepareServletFileUpload(charset, location); - // Sanity check on request size. - boolean requestSizePermitted = isRequestSizePermitted(request); - - // Interface with Commons FileUpload API - // Using the Streaming API - JakartaServletFileUpload servletFileUpload = new JakartaServletFileUpload<>(); - if (maxSize != null) { - servletFileUpload.setSizeMax(maxSize); - } - if (maxFiles != null) { - servletFileUpload.setFileCountMax(maxFiles); - } - if (maxFileSize != null) { - servletFileUpload.setFileSizeMax(maxFileSize); - } - FileItemInputIterator i = servletFileUpload.getItemIterator(request); - - // Iterate the file items - while (i.hasNext()) { - try { - FileItemInput itemStream = i.next(); - - // If the file item stream is a form field, delegate to the - // field item stream handler - if (itemStream.isFormField()) { - processFileItemStreamAsFormField(itemStream); - } - - // Delegate the file item stream for a file field to the - // file item stream handler, but delegation is skipped - // if the requestSizePermitted check failed based on the - // complete content-size of the request. - else { - - // prevent processing file field item if request size not allowed. - if (!requestSizePermitted) { - addFileSkippedError(itemStream.getName(), request); - LOG.debug("Skipped stream '{}', request maximum size ({}) exceeded.", itemStream.getName(), maxSize); - continue; - } - - processFileItemStreamAsFileField(itemStream, saveDir); - } - } catch (IOException e) { - LOG.warn("Error occurred during process upload", e); - } - } - } - } - - /** - * Defines whether the request allowed based on content length. - * - * @param request the servlet request - * @return true if request size is permitted - */ - protected boolean isRequestSizePermitted(HttpServletRequest request) { - // 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) { - return true; - } - return request.getContentLength() < maxSize; - } - - /** - * @param request the servlet request - * @return the request content length. - */ - protected long getRequestSize(HttpServletRequest request) { - return request != null ? request.getContentLength() : 0; - } - - /** - * Add a file skipped message notification for action messages. - * - * @param fileName file name - * @param request the servlet request - */ - protected void addFileSkippedError(String fileName, HttpServletRequest request) { - String exceptionMessage = "Skipped file " + fileName + "; request size limit exceeded."; - long allowedMaxSize = maxSize != null ? maxSize : -1; - FileUploadSizeException exception = new FileUploadSizeException(exceptionMessage, getRequestSize(request), allowedMaxSize); - LocalizedMessage message = buildErrorMessage(exception, new Object[]{fileName, getRequestSize(request), allowedMaxSize}); - if (!errors.contains(message)) { - errors.add(message); - } - } - - /** - * Processes the FileItemStream as a Form Field. - * - * @param itemStream file item stream - */ - protected void processFileItemStreamAsFormField(FileItemInput itemStream) { - String fieldName = itemStream.getFieldName(); - try { - List values; - - String fieldValue = itemStream.getInputStream().toString(); - if (!parameters.containsKey(fieldName)) { - values = new ArrayList<>(); - parameters.put(fieldName, values); + LOG.debug("Using Jakarta Stream API to process request"); + servletFileUpload.getItemIterator(request).forEachRemaining(item -> { + if (item.isFormField()) { + LOG.debug(() -> "Processing a form field: " + sanitizeNewlines(item.getFieldName())); + processFileItemAsFormField(item); } else { - values = parameters.get(fieldName); + LOG.debug(() -> "Processing a file: " + sanitizeNewlines(item.getFieldName())); + processFileItemAsFileField(item, location); } - values.add(fieldValue); - } catch (IOException e) { - LOG.warn("Failed to handle form field '{}'.", fieldName, e); + }); + } + + protected JakartaServletDiskFileUpload createJakartaFileUpload(Charset charset, Path location) { + DiskFileItemFactory.Builder builder = DiskFileItemFactory.builder(); + + LOG.debug("Using file save directory: {}", location); + builder.setPath(location); + + LOG.debug("Sets buffer size: {}", bufferSize); + builder.setBufferSize(bufferSize); + + LOG.debug("Using charset: {}", charset); + builder.setCharset(charset); + + DiskFileItemFactory factory = builder.get(); + return new JakartaServletDiskFileUpload(factory); + } + + private String readStream(InputStream inputStream) throws IOException { + ByteArrayOutputStream result = new ByteArrayOutputStream(); + byte[] buffer = new byte[1024]; + for (int length; (length = inputStream.read(buffer)) != -1; ) { + result.write(buffer, 0, length); } + return result.toString(StandardCharsets.UTF_8); } /** - * Processes the FileItemStream as a file field. + * Processes the FileItem as a normal form field. * - * @param itemStream file item stream - * @param location location + * @param fileItemInput a form field item input */ - protected void processFileItemStreamAsFileField(FileItemInput itemStream, String location) { - // Skip file uploads that don't have a file name - meaning that no file was selected. - if (itemStream.getName() == null || itemStream.getName().trim().isEmpty()) { - LOG.debug("No file has been uploaded for the field: {}", itemStream.getFieldName()); + protected void processFileItemAsFormField(FileItemInput fileItemInput) throws IOException { + String fieldName = fileItemInput.getFieldName(); + String fieldValue = readStream(fileItemInput.getInputStream()); + + if (exceedsMaxStringLength(fieldName, fieldValue)) { return; } - File file = null; - try { - // Create the temporary upload file. - file = createTemporaryFile(itemStream.getName(), location); + List values; + if (parameters.containsKey(fieldName)) { + values = parameters.get(fieldName); + } else { + values = new ArrayList<>(); + parameters.put(fieldName, values); + } + values.add(fieldValue); + } - if (streamFileToDisk(itemStream, file)) { - createFileInfoFromItemStream(itemStream, file); + /** + * @return actual size of already uploaded files + */ + protected Long actualSizeOfUploadedFiles() { + return uploadedFiles.values().stream() + .map(files -> 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); } - } catch (IOException e) { - if (file != null) { - try { - file.delete(); - } catch (SecurityException se) { - LOG.warn("Failed to delete '{}' due to security exception above.", file.getName(), se); - } + 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. + * + * @param fileItemInput file item representing upload file + * @param location location + */ + protected void processFileItemAsFileField(FileItemInput fileItemInput, Path location) throws IOException { + // Skip file uploads that don't have a file name - meaning that no file was selected. + if (fileItemInput.getName() == null || fileItemInput.getName().trim().isEmpty()) { + LOG.debug(() -> "No file has been uploaded for the field: " + sanitizeNewlines(fileItemInput.getFieldName())); + return; + } + + if (exceedsMaxFiles(fileItemInput)) { + return; + } + + File file = createTemporaryFile(fileItemInput.getName(), location); + streamFileToDisk(fileItemInput, file); + + Long currentFilesSize = maxSizeOfFiles != null ? actualSizeOfUploadedFiles() : null; + if (maxSizeOfFiles != null && currentFilesSize + file.length() >= maxSizeOfFiles) { + exceedsMaxSizeOfFiles(fileItemInput, file, currentFilesSize); + } else { + createUploadedFile(fileItemInput, file); } } @@ -363,121 +209,54 @@ public class JakartaStreamMultiPartRequest extends AbstractMultiPartRequest { * * @param fileName file name * @param location location - * @return temporary file based on the given filename and location - * @throws IOException in case of IO errors + * @return a temporary file based on the given filename and location */ - protected File createTemporaryFile(String fileName, String location) throws IOException { - String name = fileName - .substring(fileName.lastIndexOf('/') + 1) - .substring(fileName.lastIndexOf('\\') + 1); - - String prefix = name; - String suffix = ""; - - if (name.contains(".")) { - prefix = name.substring(0, name.lastIndexOf('.')); - suffix = name.substring(name.lastIndexOf('.')); - } - - if (prefix.length() < 3) { - prefix = UUID.randomUUID().toString(); - } - - File file = File.createTempFile(prefix + "_", suffix, new File(location)); - LOG.debug("Creating temporary file '{}' (originally '{}').", file.getName(), fileName); + protected File createTemporaryFile(String fileName, Path location) { + String uid = UUID.randomUUID().toString().replace("-", "_"); + File file = location.resolve("upload_" + uid + ".tmp").toFile(); + LOG.debug("Creating temporary file: {} (originally: {})", file.getName(), fileName); return file; } /** * Streams the file upload stream to the specified file. * - * @param itemStream file item stream - * @param file the file - * @return true if stream was successfully - * @throws IOException in case of IO errors + * @param fileItemInput file item input + * @param file the file */ - protected boolean streamFileToDisk(FileItemInput itemStream, File file) throws IOException { - boolean result; - try (InputStream input = itemStream.getInputStream(); - OutputStream output = new BufferedOutputStream(Files.newOutputStream(file.toPath()), bufferSize)) { + protected void streamFileToDisk(FileItemInput fileItemInput, File file) throws IOException { + InputStream input = fileItemInput.getInputStream(); + try (OutputStream output = new BufferedOutputStream(Files.newOutputStream(file.toPath()), bufferSize)) { byte[] buffer = new byte[bufferSize]; - LOG.debug("Streaming file using buffer size {}.", bufferSize); + LOG.debug("Streaming file: {} using buffer size: {}", fileItemInput.getName(), bufferSize); for (int length; ((length = input.read(buffer)) > 0); ) { output.write(buffer, 0, length); } - result = true; } - return result; } /** - * Creates an internal FileInfo structure used to pass information - * to the FileUploadInterceptor during the interceptor stack - * invocation process. + * Create {@link UploadedFile} abstraction over uploaded file * - * @param itemStream file item stream - * @param file the file + * @param fileItemInput file item stream + * @param file the file */ - protected void createFileInfoFromItemStream(FileItemInput itemStream, File file) { - // gather attributes from file upload stream. - String fileName = itemStream.getName(); - String fieldName = itemStream.getFieldName(); - // create internal structure - FileInfo fileInfo = new FileInfo(file, itemStream.getContentType(), fileName); - // append or create new entry. - if (!fileInfos.containsKey(fieldName)) { - List infos = new ArrayList<>(); - infos.add(fileInfo); - fileInfos.put(fieldName, infos); + protected void createUploadedFile(FileItemInput fileItemInput, File file) { + String fileName = fileItemInput.getName(); + String fieldName = fileItemInput.getFieldName(); + + UploadedFile uploadedFile = StrutsUploadedFile.Builder + .create(file) + .withOriginalName(fileName) + .withContentType(fileItemInput.getContentType()) + .build(); + + if (uploadedFiles.containsKey(fieldName)) { + uploadedFiles.get(fieldName).add(uploadedFile); } else { - fileInfos.get(fieldName).add(fileInfo); - } - } - - /** - * Internal data structure used to store a reference to information needed - * to later pass post processing data to the FileUploadInterceptor. - * - * @since 7.0.0 - */ - public static class FileInfo implements Serializable { - - private final File file; - private final String contentType; - private final String originalName; - - /** - * Default constructor. - * - * @param file the file - * @param contentType content type - * @param originalName original file name - */ - public FileInfo(File file, String contentType, String originalName) { - this.file = file; - this.contentType = contentType; - this.originalName = originalName; - } - - /** - * @return the file - */ - public File getFile() { - return file; - } - - /** - * @return content type - */ - public String getContentType() { - return contentType; - } - - /** - * @return original file name - */ - public String getOriginalName() { - return originalName; + List> infos = new ArrayList<>(); + infos.add(uploadedFile); + uploadedFiles.put(fieldName, infos); } } 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/dispatcher/multipart/StrutsUploadedFile.java b/core/src/main/java/org/apache/struts2/dispatcher/multipart/StrutsUploadedFile.java index 5976f578f..2359b222c 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/multipart/StrutsUploadedFile.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/multipart/StrutsUploadedFile.java @@ -20,7 +20,7 @@ package org.apache.struts2.dispatcher.multipart; import java.io.File; -public class StrutsUploadedFile implements UploadedFile { +public class StrutsUploadedFile implements UploadedFile { private final File file; private final String contentType; @@ -28,6 +28,7 @@ public class StrutsUploadedFile implements UploadedFile { /** * Use builder instead of constructor + * * @param file an uploaded file * @deprecated since Struts 6.4.0 */ @@ -87,9 +88,9 @@ public class StrutsUploadedFile implements UploadedFile { @Override public String toString() { return "StrutsUploadedFile{" + - "contentType='" + contentType + '\'' + - ", originalName='" + originalName + '\'' + - '}'; + "contentType='" + contentType + '\'' + + ", originalName='" + originalName + '\'' + + '}'; } public static class Builder { @@ -115,7 +116,7 @@ public class StrutsUploadedFile implements UploadedFile { return this; } - public UploadedFile build() { + public UploadedFile build() { return new StrutsUploadedFile(this.file, this.contentType, this.originalName); } } diff --git a/core/src/main/java/org/apache/struts2/dispatcher/multipart/UploadedFile.java b/core/src/main/java/org/apache/struts2/dispatcher/multipart/UploadedFile.java index ada27ff6c..c48054765 100644 --- a/core/src/main/java/org/apache/struts2/dispatcher/multipart/UploadedFile.java +++ b/core/src/main/java/org/apache/struts2/dispatcher/multipart/UploadedFile.java @@ -21,24 +21,48 @@ package org.apache.struts2.dispatcher.multipart; import java.io.Serializable; /** - * Virtual representation of a uploaded file used by {@link MultiPartRequest} + * Virtual representation of an uploaded file used by {@link MultiPartRequest} */ -public interface UploadedFile extends Serializable { +public interface UploadedFile extends Serializable { + /** + * @return size of the content of file/stream/array + */ Long length(); + /** + * @return a local name of the file + */ String getName(); + /** + * @return original file name from upload source + */ String getOriginalName(); + /** + * @return indicates if this is a real file or maybe just in-memory stream + */ boolean isFile(); + /** + * @return removes a local copy of the uploaded file/stream + */ boolean delete(); + /** + * @return an absolute path of the file if possible + */ String getAbsolutePath(); - Object getContent(); + /** + * @return content of the upload file + */ + T getContent(); + /** + * @return content type of the uploaded file + */ String getContentType(); } 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 new file mode 100644 index 000000000..7c75a7a4d --- /dev/null +++ b/core/src/test/java/org/apache/struts2/dispatcher/multipart/AbstractMultiPartRequestTest.java @@ -0,0 +1,485 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +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.After; +import org.junit.Before; +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.List; +import java.util.Map; +import java.util.Objects; + +import static org.assertj.core.api.Assertions.assertThat; + +abstract class AbstractMultiPartRequestTest { + + protected static Path tempDir; + + protected MockHttpServletRequest mockRequest; + + protected final String boundary = "_boundary_"; + protected final String endline = "\r\n"; + + protected AbstractMultiPartRequest multiPart; + + abstract protected AbstractMultiPartRequest createMultipartRequest(); + + @BeforeClass + public static void beforeClass() { + String dirProp = System.getProperty("java.io.tmpdir"); + if (Path.of(dirProp).toFile().exists()) { + tempDir = Path.of(dirProp, "multi-part-test"); + } else { + tempDir = Path.of("target", "multi-part-test"); + } + } + + @Before + public void before() { + mockRequest = new MockHttpServletRequest(); + mockRequest.setCharacterEncoding(StandardCharsets.UTF_8.name()); + mockRequest.setMethod("post"); + mockRequest.setContentType("multipart/form-data; boundary=" + boundary); + + multiPart = createMultipartRequest(); + } + + @After + public void after() { + multiPart.cleanUp(); + } + + @Test + public void uploadedFilesToDisk() 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.UTF_8)); + + assertThat(JakartaServletDiskFileUpload.isMultipartContent(mockRequest)).isTrue(); + + // when + multiPart.setBufferSize("1"); // always write files into disk + multiPart.parse(mockRequest, tempDir.toString()); + + // then + assertThat(multiPart.getErrors()) + .isEmpty(); + + assertThat(multiPart.getFileParameterNames().asIterator()).toIterable() + .asList() + .containsOnly("file1", "file2"); + 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.getFile("file2")).allSatisfy(file -> { + assertThat(file.isFile()) + .isTrue(); + assertThat(file.getOriginalName()) + .isEqualTo("test2.csv"); + assertThat(file.getContentType()) + .isEqualTo("text/csv"); + assertThat(file.getContent()) + .asInstanceOf(InstanceOfAssertFactories.FILE) + .exists() + .content() + .isEqualTo("5,6,7,8"); + }); + } + + @Test + public void uploadedMultipleFilesToDisk() throws IOException { + // given + String content = formFile("file1", "test1.csv", "1,2,3,4") + + formFile("file1", "test2.csv", "5,6,7,8") + + endline + "--" + boundary + "--"; + + mockRequest.setContent(content.getBytes(StandardCharsets.UTF_8)); + + assertThat(JakartaServletDiskFileUpload.isMultipartContent(mockRequest)).isTrue(); + + // when + multiPart.setBufferSize("1"); // always write files into disk + multiPart.parse(mockRequest, tempDir.toString()); + + // then + assertThat(multiPart.getErrors()) + .isEmpty(); + + assertThat(multiPart.getFileParameterNames().asIterator()).toIterable() + .asList() + .containsOnly("file1"); + assertThat(multiPart.getFile("file1")).allSatisfy(file -> { + if (Objects.equals(file.getName(), "test1.csv")) { + 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"); + } + if (Objects.equals(file.getName(), "test2.csv")) { + assertThat(file.isFile()) + .isTrue(); + assertThat(file.getOriginalName()) + .isEqualTo("test2.csv"); + assertThat(file.getContentType()) + .isEqualTo("text/csv"); + assertThat(file.getContent()) + .asInstanceOf(InstanceOfAssertFactories.FILE) + .exists() + .content() + .isEqualTo("5,6,7,8"); + } + }); + } + + @Test + public void uploadedFilesWithLargeBuffer() 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.UTF_8)); + + assertThat(JakartaServletDiskFileUpload.isMultipartContent(mockRequest)).isTrue(); + + // when + multiPart.setBufferSize("8192"); // streams files into disk using larger buffer + multiPart.parse(mockRequest, tempDir.toString()); + + // then + assertThat(multiPart.getErrors()) + .isEmpty(); + + assertThat(multiPart.getFileParameterNames().asIterator()).toIterable() + .asList() + .containsOnly("file1", "file2"); + 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.getFile("file2")).allSatisfy(file -> { + assertThat(file.isFile()) + .isTrue(); + assertThat(file.getOriginalName()) + .isEqualTo("test2.csv"); + assertThat(file.getContent()) + .asInstanceOf(InstanceOfAssertFactories.FILE) + .exists() + .content() + .isEqualTo("5,6,7,8"); + }); + } + + @Test + public void cleanUp() 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.UTF_8)); + + assertThat(JakartaServletDiskFileUpload.isMultipartContent(mockRequest)).isTrue(); + + // when + multiPart.parse(mockRequest, tempDir.toString()); + + // then + assertThat(multiPart.getErrors()) + .isEmpty(); + + assertThat(multiPart.getFileParameterNames().asIterator()).toIterable() + .asList() + .containsOnly("file1", "file2"); + 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.getFile("file2")).allSatisfy(file -> { + assertThat(file.isFile()) + .isTrue(); + assertThat(file.getOriginalName()) + .isEqualTo("test2.csv"); + assertThat(file.getContentType()) + .isEqualTo("text/csv"); + assertThat(file.getContent()) + .asInstanceOf(InstanceOfAssertFactories.FILE) + .exists() + .content() + .isEqualTo("5,6,7,8"); + }); + + List> uploadedFiles = new ArrayList<>(); + for (Map.Entry>> entry : multiPart.uploadedFiles.entrySet()) { + uploadedFiles.addAll(entry.getValue()); + } + + // when + multiPart.cleanUp(); + + // then + assertThat(multiPart.uploadedFiles) + .isEmpty(); + assertThat(multiPart.parameters) + .isEmpty(); + assertThat(uploadedFiles).allSatisfy(file -> + assertThat(file.getContent()).asInstanceOf(InstanceOfAssertFactories.FILE) + .doesNotExist() + ); + } + + @Test + public void nonMultiPartUpload() 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.UTF_8)); + + // given + mockRequest.setContentType(""); + + // when + multiPart.parse(mockRequest, tempDir.toString()); + + // then + assertThat(multiPart.getErrors()) + .map(LocalizedMessage::getTextKey) + .containsExactly("struts.messages.upload.error.FileUploadContentTypeException"); + + assertThat(multiPart.getFileParameterNames().asIterator()).toIterable() + .asList() + .isEmpty(); + } + + @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 + "--"; + + mockRequest.setContent(content.getBytes(StandardCharsets.UTF_8)); + + assertThat(JakartaServletDiskFileUpload.isMultipartContent(mockRequest)).isTrue(); + + // when + multiPart.setMaxSize("1"); + multiPart.parse(mockRequest, tempDir.toString()); + + // then + assertThat(multiPart.uploadedFiles) + .isEmpty(); + + assertThat(multiPart.getErrors()) + .map(LocalizedMessage::getTextKey) + .containsExactly("struts.messages.upload.error.FileUploadSizeException"); + } + + @Test + public void maxFilesSize() 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.UTF_8)); + + assertThat(JakartaServletDiskFileUpload.isMultipartContent(mockRequest)).isTrue(); + + multiPart.setMaxFileSize("1"); + multiPart.parse(mockRequest, tempDir.toString()); + + assertThat(multiPart.getErrors()) + .map(LocalizedMessage::getTextKey) + .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") + + formFile("file2", "test2.csv", "5,6,7,8") + + formField("longText", "very long text") + + formField("shortText", "short text") + + endline + "--" + boundary + "--"; + + mockRequest.setContent(content.getBytes(StandardCharsets.UTF_8)); + + assertThat(JakartaServletDiskFileUpload.isMultipartContent(mockRequest)).isTrue(); + + multiPart.setMaxStringLength("10"); + multiPart.parse(mockRequest, tempDir.toString()); + + assertThat(multiPart.getErrors()) + .map(LocalizedMessage::getTextKey) + .containsExactly("struts.messages.upload.error.parameter.too.long"); + } + + @Test + public void mismatchCharset() throws IOException { + // give + String content = formFile("file1", "test1.csv", "Ł,Ś,Ż,Ó") + + endline + "--" + boundary + "--"; + + mockRequest.setContent(content.getBytes(StandardCharsets.UTF_8)); + + assertThat(JakartaServletDiskFileUpload.isMultipartContent(mockRequest)).isTrue(); + + // when + mockRequest.setCharacterEncoding(null); + multiPart.setDefaultEncoding(StandardCharsets.ISO_8859_1.name()); + multiPart.parse(mockRequest, tempDir.toString()); + + // then + assertThat(multiPart.getErrors()) + .isEmpty(); + + assertThat(multiPart.getFileParameterNames().asIterator()).toIterable() + .asList() + .containsOnly("file1"); + 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("Ł,Ś,Ż,Ó"); + }); + } + + @Test + public void normalFields() throws IOException { + String content = formFile("file1", "test1.csv", "1,2,3,4") + + formFile("file2", "test2.csv", "5,6,7,8") + + formField("longText", "very long text") + + formField("shortText", "short text") + + formField("multi", "multi1") + + formField("multi", "multi2") + + endline + "--" + boundary + "--"; + + mockRequest.setContent(content.getBytes(StandardCharsets.UTF_8)); + + assertThat(JakartaServletDiskFileUpload.isMultipartContent(mockRequest)).isTrue(); + + multiPart.parse(mockRequest, tempDir.toString()); + + assertThat(multiPart.getErrors()) + .isEmpty(); + + assertThat(multiPart.getParameterNames().asIterator()).toIterable() + .containsOnly("longText", "shortText", "multi"); + assertThat(multiPart.getParameterValues("longText")) + .contains("very long text"); + assertThat(multiPart.getParameterValues("shortText")) + .contains("short text"); + assertThat(multiPart.getParameter("longText")) + .isEqualTo("very long text"); + assertThat(multiPart.getParameter("shortText")) + .isEqualTo("short text"); + assertThat(multiPart.getParameterValues("multi")) + .containsOnly("multi1", "multi2"); + } + + protected String formFile(String fieldName, String filename, String content) { + return endline + + "--" + boundary + endline + + "Content-Disposition: form-data; name=\"" + fieldName + "\"; filename=\"" + filename + "\"" + + endline + + "Content-Type: text/csv" + + endline + + endline + + content; + } + + protected String formField(String fieldName, String content) { + return endline + + "--" + boundary + endline + + "Content-Disposition: form-data; name=\"" + fieldName + "\"" + + endline + + endline + + content; + } +} 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 new file mode 100644 index 000000000..e24dc133d --- /dev/null +++ b/core/src/test/java/org/apache/struts2/dispatcher/multipart/JakartaMultiPartRequestTest.java @@ -0,0 +1,30 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.struts2.dispatcher.multipart; + +import java.io.File; + +public class JakartaMultiPartRequestTest extends AbstractMultiPartRequestTest { + + @Override + protected AbstractMultiPartRequest createMultipartRequest() { + return new JakartaMultiPartRequest(); + } + +} \ 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 89a01b8a5..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 @@ -18,54 +18,57 @@ */ package org.apache.struts2.dispatcher.multipart; -import java.io.ByteArrayInputStream; +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.IOException; import java.nio.charset.StandardCharsets; -import java.nio.file.Path; -import java.nio.file.Paths; -import org.apache.struts2.dispatcher.LocalizedMessage; -import org.junit.Assert; -import org.junit.Before; -import org.junit.Test; -import org.mockito.Mockito; -import org.springframework.mock.web.DelegatingServletInputStream; +import static org.assertj.core.api.Assertions.assertThat; -import jakarta.servlet.http.HttpServletRequest; +public class JakartaStreamMultiPartRequestTest extends AbstractMultiPartRequestTest { -public class JakartaStreamMultiPartRequestTest { - - private JakartaStreamMultiPartRequest multiPart; - private Path tempDir; - - @Before - public void initialize() { - multiPart = new JakartaStreamMultiPartRequest(); - tempDir = Paths.get("target", "multi-part-test"); + @Override + protected AbstractMultiPartRequest createMultipartRequest() { + return new JakartaStreamMultiPartRequest(); } - /** - * Number of bytes in files greater than 2GB overflow the {@code int} primative. - * The {@link HttpServletRequest#getContentLength()} returns {@literal -1} - * when the header is not present or the size is greater than {@link Integer#MAX_VALUE}. - */ @Test - public void unknownContentLength() throws IOException { - HttpServletRequest request = Mockito.mock(HttpServletRequest.class); - Mockito.when(request.getContentType()).thenReturn("multipart/form-data; charset=utf-8; boundary=__X_BOUNDARY__"); - Mockito.when(request.getMethod()).thenReturn("POST"); - Mockito.when(request.getContentLength()).thenReturn(-1); - String entity = "\r\n--__X_BOUNDARY__\r\n" + - "Content-Disposition: form-data; name=\"upload\"; filename=\"test.csv\"\r\n" + - "Content-Type: text/csv\r\n\r\n1,2\r\n\r\n" + - "--__X_BOUNDARY__\r\n" + - "Content-Disposition: form-data; name=\"upload2\"; filename=\"test2.csv\"\r\n" + - "Content-Type: text/csv\r\n\r\n3,4\r\n\r\n" + - "--__X_BOUNDARY__--\r\n"; - Mockito.when(request.getInputStream()).thenReturn(new DelegatingServletInputStream(new ByteArrayInputStream(entity.getBytes(StandardCharsets.UTF_8)))); - multiPart.setMaxSize("4"); - multiPart.parse(request, tempDir.toString()); - LocalizedMessage next = multiPart.getErrors().iterator().next(); - Assert.assertEquals(next.getTextKey(), "struts.messages.upload.error.FileUploadSizeException"); + 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.UTF_8)); + + assertThat(JakartaServletDiskFileUpload.isMultipartContent(mockRequest)).isTrue(); + + // when + multiPart.setMaxSizeOfFiles("10"); + multiPart.parse(mockRequest, tempDir.toString()); + + // 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) + .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 17de7a040..40c4103f4 100644 --- a/core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java +++ b/core/src/test/java/org/apache/struts2/interceptor/ActionFileUploadInterceptorTest.java @@ -26,8 +26,8 @@ import com.opensymphony.xwork2.mock.MockActionInvocation; import com.opensymphony.xwork2.mock.MockActionProxy; import com.opensymphony.xwork2.util.ClassLoaderUtil; import jakarta.servlet.http.HttpServletRequest; -import org.apache.commons.fileupload2.jakarta.JakartaServletDiskFileUpload; -import org.apache.commons.fileupload2.jakarta.JakartaServletFileUpload; +import org.apache.commons.fileupload2.jakarta.servlet6.JakartaServletDiskFileUpload; +import org.apache.commons.fileupload2.jakarta.servlet6.JakartaServletFileUpload; import org.apache.struts2.ServletActionContext; import org.apache.struts2.StrutsInternalTestCase; import org.apache.struts2.action.UploadedFilesAware; @@ -52,7 +52,7 @@ import static org.assertj.core.api.Assertions.assertThat; */ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase { - public 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); @@ -221,15 +221,15 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase { String msg = errors.get(0); // the error message should contain at least this test assertThat(msg).contains( - "The file is too large to be uploaded", - "inputName", - "log4j2.xml", - "allowed mx size is 10" + "The file is too large to be uploaded", + "inputName", + "log4j2.xml", + "allowed mx size is 10" ); } 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()); @@ -329,33 +329,27 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase { MockHttpServletRequest req = new MockHttpServletRequest(); req.setCharacterEncoding(StandardCharsets.UTF_8.name()); req.setMethod("POST"); - req.addHeader("Content-type", "multipart/form-data; boundary=" + boundary); + req.addHeader("Content-type", "multipart/form-data; boundary=\"" + boundary + "\""); String content = encodeTextFile("test.html", "text/plain", plainContent) + - encodeTextFile("test1.html", "text/html", htmlContent) + - encodeTextFile("test2.html", "text/html", htmlContent) + - endline + - endline + - endline + - "--" + - boundary + - "--" + - endline; + encodeTextFile("test1.html", "text/html", htmlContent) + + encodeTextFile("test2.html", "text/html", htmlContent) + + endline + "--" + boundary + "--"; req.setContent(content.getBytes()); assertTrue(JakartaServletDiskFileUpload.isMultipartContent(req)); - MyFileUploadAction action = new MyFileUploadAction(); + MyFileUploadAction action = new MyFileUploadAction<>(); container.inject(action); 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.setAllowedTypes("text/html"); interceptor.intercept(mai); - List files = action.getUploadFiles(); + List> files = action.getUploadFiles(); assertNotNull(files); assertEquals("files accepted ", 2, files.size()); @@ -369,19 +363,19 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase { req.setMethod("POST"); req.addHeader("Content-type", "multipart/form-data; boundary=" + boundary); String content = encodeTextFile("test.html", "text/plain", plainContent) + - encodeTextFile("test1.html", "text/html", htmlContent) + - encodeTextFile("test2.html", "text/html", htmlContent) + - encodeTextFile("test3.html", "text/html", htmlContent) + - endline + - "--" + - boundary + - "--" + - endline; + encodeTextFile("test1.html", "text/html", htmlContent) + + encodeTextFile("test2.html", "text/html", htmlContent) + + encodeTextFile("test3.html", "text/html", htmlContent) + + endline + + "--" + + boundary + + "--" + + endline; req.setContent(content.getBytes()); assertTrue(JakartaServletFileUpload.isMultipartContent(req)); - MyFileUploadAction action = new MyFileUploadAction(); + MyFileUploadAction action = new MyFileUploadAction<>(); container.inject(action); MockActionInvocation mai = new MockActionInvocation(); mai.setAction(action); @@ -417,14 +411,14 @@ 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); mai.setResultCode("success"); mai.setInvocationContext(ActionContext.getContext()); ActionContext.getContext() - .withServletRequest(createMultipartRequestMaxFileSize(req)); + .withServletRequest(createMultipartRequestMaxFileSize(req)); interceptor.intercept(mai); @@ -435,8 +429,8 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase { String msg = errors.iterator().next(); // FIXME: the expected size is 40 - length of the string assertEquals( - "File deleteme.txt assigned to file exceeded allowed size limit! Max size allowed is: 10 but file was: 10!", - msg); + "File deleteme.txt assigned to file exceeded allowed size limit! Max size allowed is: 10 but file was: 11!", + msg); } public void testMultipartRequestMaxStringLength() throws Exception { @@ -464,14 +458,14 @@ 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); mai.setResultCode("success"); mai.setInvocationContext(ActionContext.getContext()); ActionContext.getContext() - .withServletRequest(createMultipartRequestMaxStringLength(req)); + .withServletRequest(createMultipartRequestMaxStringLength(req)); interceptor.intercept(mai); @@ -481,8 +475,8 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase { assertEquals(1, errors.size()); String msg = errors.iterator().next(); assertEquals( - "The request parameter \"normalFormField2\" was too long. Max length allowed is 20, but found 27!", - msg); + "The request parameter \"normalFormField2\" was too long. Max length allowed is 20, but found 27!", + msg); } public void testMultipartRequestLocalizedError() throws Exception { @@ -502,15 +496,15 @@ 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); mai.setResultCode("success"); mai.setInvocationContext(ActionContext.getContext()); ActionContext.getContext() - .withLocale(Locale.GERMAN) - .withServletRequest(createMultipartRequestMaxSize(req, 10)); + .withLocale(Locale.GERMAN) + .withServletRequest(createMultipartRequestMaxSize(req, 10)); interceptor.intercept(mai); @@ -524,20 +518,15 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase { } private String encodeTextFile(String filename, String contentType, String content) { - return "\r\n" + - "--" + - "simple boundary" + - "\r\n" + - "Content-Disposition: form-data; name=\"" + - "file" + - "\"; filename=\"" + - filename + - "\r\n" + - "Content-Type: " + - contentType + - "\r\n" + - "\r\n" + - content; + return endline + + "--" + boundary + + endline + + "Content-Disposition: form-data; name=\"" + "file" + "\"; filename=\"" + filename + + endline + + "Content-Type: " + contentType + + endline + + endline + + content; } private MultiPartRequestWrapper createMultipartRequestMaxFileSize(HttpServletRequest req) { @@ -557,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()); } @@ -582,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 cb40a4b7a..2f3145979 100644 --- a/core/src/test/java/org/apache/struts2/interceptor/FileUploadInterceptorTest.java +++ b/core/src/test/java/org/apache/struts2/interceptor/FileUploadInterceptorTest.java @@ -18,8 +18,24 @@ */ package org.apache.struts2.interceptor; +import com.opensymphony.xwork2.ActionContext; +import com.opensymphony.xwork2.ActionSupport; +import com.opensymphony.xwork2.DefaultLocaleProvider; +import com.opensymphony.xwork2.ValidationAwareSupport; +import com.opensymphony.xwork2.mock.MockActionInvocation; +import com.opensymphony.xwork2.util.ClassLoaderUtil; +import jakarta.servlet.http.HttpServletRequest; +import org.apache.commons.fileupload2.jakarta.servlet6.JakartaServletFileUpload; +import org.apache.struts2.ServletActionContext; +import org.apache.struts2.StrutsInternalTestCase; +import org.apache.struts2.dispatcher.HttpParameters; +import org.apache.struts2.dispatcher.multipart.JakartaMultiPartRequest; +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; + import java.io.File; -import java.io.IOException; import java.net.URI; import java.net.URL; import java.nio.charset.StandardCharsets; @@ -29,35 +45,14 @@ import java.util.List; import java.util.Locale; import java.util.Map; -import org.apache.commons.fileupload2.jakarta.JakartaServletFileUpload; -import org.apache.struts2.ServletActionContext; -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.MultiPartRequestWrapper; -import org.apache.struts2.dispatcher.multipart.StrutsUploadedFile; -import org.apache.struts2.dispatcher.multipart.UploadedFile; -import org.springframework.mock.web.MockHttpServletRequest; - -import com.opensymphony.xwork2.ActionContext; -import com.opensymphony.xwork2.ActionSupport; -import com.opensymphony.xwork2.DefaultLocaleProvider; -import com.opensymphony.xwork2.ValidationAwareSupport; -import com.opensymphony.xwork2.mock.MockActionInvocation; -import com.opensymphony.xwork2.util.ClassLoaderUtil; - -import jakarta.servlet.http.HttpServletRequest; - import static org.assertj.core.api.Assertions.assertThat; /** * Test case for FileUploadInterceptor. */ - public class FileUploadInterceptorTest extends StrutsInternalTestCase { - public static final UploadedFile EMPTY_FILE = new UploadedFile() { + private static final UploadedFile EMPTY_FILE = new UploadedFile<>() { @Override public Long length() { return 0L; @@ -84,8 +79,8 @@ public class FileUploadInterceptorTest extends StrutsInternalTestCase { } @Override - public byte[] getContent() { - return new byte[0]; + public String getContent() { + return ""; } @Override @@ -210,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); @@ -221,10 +220,10 @@ public class FileUploadInterceptorTest extends StrutsInternalTestCase { String msg = errors.get(0); // the error message should contain at least this test assertThat(msg).contains( - "The file is too large to be uploaded", - "inputName", - "log4j2.xml", - "allowed mx size is 10" + "The file is too large to be uploaded", + "inputName", + "log4j2.xml", + "allowed mx size is 10" ); } @@ -253,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); @@ -275,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); @@ -306,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); @@ -314,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(); @@ -343,15 +342,15 @@ public class FileUploadInterceptorTest extends StrutsInternalTestCase { req.setMethod("POST"); req.addHeader("Content-type", "multipart/form-data; boundary=" + bondary); String content = encodeTextFile("test.html", "text/plain", plainContent) + - encodeTextFile("test1.html", "text/html", htmlContent) + - encodeTextFile("test2.html", "text/html", htmlContent) + - endline + - endline + - endline + - "--" + - bondary + - "--" + - endline; + encodeTextFile("test1.html", "text/html", htmlContent) + + encodeTextFile("test2.html", "text/html", htmlContent) + + endline + + endline + + endline + + "--" + + bondary + + "--" + + endline; req.setContent(content.getBytes()); assertTrue(JakartaServletFileUpload.isMultipartContent(req)); @@ -364,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(); @@ -396,14 +395,14 @@ public class FileUploadInterceptorTest extends StrutsInternalTestCase { req.setMethod("POST"); req.addHeader("Content-type", "multipart/form-data; boundary=" + boundary); String content = encodeTextFile("test.html", "text/plain", plainContent) + - encodeTextFile("test1.html", "text/html", htmlContent) + - encodeTextFile("test2.html", "text/html", htmlContent) + - encodeTextFile("test3.html", "text/html", htmlContent) + - endline + - "--" + - boundary + - "--" + - endline; + encodeTextFile("test1.html", "text/html", htmlContent) + + encodeTextFile("test2.html", "text/html", htmlContent) + + encodeTextFile("test3.html", "text/html", htmlContent) + + endline + + "--" + + boundary + + "--" + + endline; req.setContent(content.getBytes()); assertTrue(JakartaServletFileUpload.isMultipartContent(req)); @@ -416,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); @@ -466,7 +465,7 @@ public class FileUploadInterceptorTest extends StrutsInternalTestCase { String msg = errors.iterator().next(); // FIXME: the expected size is 40 - length of the string assertEquals( - "File deleteme.txt assigned to file exceeded allowed size limit! Max size allowed is: 10 but file was: 10!", + "File deleteme.txt assigned to file exceeded allowed size limit! Max size allowed is: 10 but file was: 11!", msg); } @@ -543,9 +542,9 @@ public class FileUploadInterceptorTest extends StrutsInternalTestCase { mai.setInvocationContext(ActionContext.getContext()); Map param = new HashMap<>(); ActionContext.getContext() - .withParameters(HttpParameters.create(param).build()) - .withLocale(Locale.GERMAN) - .withServletRequest(createMultipartRequestMaxSize(req, 10)); + .withParameters(HttpParameters.create(param).build()) + .withLocale(Locale.GERMAN) + .withServletRequest(createMultipartRequestMaxSize(req, 10)); interceptor.intercept(mai); @@ -560,19 +559,19 @@ public class FileUploadInterceptorTest extends StrutsInternalTestCase { private String encodeTextFile(String filename, String contentType, String content) { return "\r\n" + - "--" + - "simple boundary" + - "\r\n" + - "Content-Disposition: form-data; name=\"" + - "file" + - "\"; filename=\"" + - filename + - "\r\n" + - "Content-Type: " + - contentType + - "\r\n" + - "\r\n" + - content; + "--" + + "simple boundary" + + "\r\n" + + "Content-Disposition: form-data; name=\"" + + "file" + + "\"; filename=\"" + + filename + + "\r\n" + + "Content-Type: " + + contentType + + "\r\n" + + "\r\n" + + content; } private MultiPartRequestWrapper createMultipartRequestMaxFileSize(HttpServletRequest req) { @@ -592,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()); } @@ -618,11 +617,6 @@ public class FileUploadInterceptorTest extends StrutsInternalTestCase { } public static class MyFileupAction extends ActionSupport { - - private static final long serialVersionUID = 6255238895447968889L; - - // no methods } - } diff --git a/core/src/test/resources/log4j2.xml b/core/src/test/resources/log4j2.xml index 4c4061dbf..88dd6e0f6 100644 --- a/core/src/test/resources/log4j2.xml +++ b/core/src/test/resources/log4j2.xml @@ -29,5 +29,6 @@ + \ No newline at end of file diff --git a/pom.xml b/pom.xml index 9305818c2..a4a1ede00 100644 --- a/pom.xml +++ b/pom.xml @@ -818,8 +818,8 @@ org.apache.commons - commons-fileupload2-jakarta - 2.0.0-M1 + commons-fileupload2-jakarta-servlet6 + 2.0.0-M2 commons-io