diff --git a/core/pom.xml b/core/pom.xml index b4dd4b69c..9b9b94dff 100644 --- a/core/pom.xml +++ b/core/pom.xml @@ -216,8 +216,8 @@ org.apache.commons - commons-fileupload2-jakarta - 2.0.0-M1 + commons-fileupload2-jakarta-servlet6 + 2.0.0-M2 commons-io 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..69c61d619 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 @@ -20,12 +20,12 @@ 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.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.util.ArrayList; import java.util.List; import java.util.Locale; @@ -125,7 +125,7 @@ public abstract class AbstractMultiPartRequest implements MultiPartRequest { /** * @param request Inspect the servlet request and set the locale if one wasn't provided by - * the Struts2 framework. + * the Struts2 framework. */ protected void setLocale(HttpServletRequest request) { if (defaultLocale == null) { @@ -136,7 +136,7 @@ public abstract class AbstractMultiPartRequest implements MultiPartRequest { /** * Build error message. * - * @param e the Throwable/Exception + * @param e the Throwable/Exception * @param args arguments * @return error message */ @@ -149,7 +149,7 @@ public abstract class AbstractMultiPartRequest implements MultiPartRequest { /* (non-Javadoc) * @see org.apache.struts2.dispatcher.multipart.MultiPartRequest#getErrors() - */ + */ public List getErrors() { return errors; } @@ -171,4 +171,8 @@ public abstract class AbstractMultiPartRequest implements MultiPartRequest { return fileName; } + protected String sanitizeNewlines(String before) { + return before.replaceAll("[\n\r]", "_"); + } + } 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..1c4d628cd 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 @@ -19,25 +19,23 @@ package org.apache.struts2.dispatcher.multipart; import jakarta.servlet.http.HttpServletRequest; +import org.apache.commons.fileupload2.core.AbstractFileUpload; 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.JakartaServletFileUpload; 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.util.ArrayList; import java.util.Collections; @@ -52,12 +50,13 @@ import java.util.Set; */ public class JakartaMultiPartRequest extends AbstractMultiPartRequest { - static final Logger LOG = LogManager.getLogger(JakartaMultiPartRequest.class); + private static final Logger LOG = LogManager.getLogger(JakartaMultiPartRequest.class); - // maps parameter name -> List of FileItem objects - protected Map> files = new HashMap<>(); + protected Map> uploadedFiles = new HashMap<>(); - // maps parameter name -> List of param values + /** + * Keeps info about normal form fields + */ protected Map> params = new HashMap<>(); /** @@ -112,40 +111,46 @@ public class JakartaMultiPartRequest extends AbstractMultiPartRequest { } protected void processUpload(HttpServletRequest request, String saveDir) throws IOException { - - 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); - } + if (!JakartaServletFileUpload.isMultipartContent(request)) { + LOG.debug("Http request isn't: {}, stop processing", AbstractFileUpload.MULTIPART_FORM_DATA); + return; + } + for (DiskFileItem item : parseRequest(request, saveDir)) { + LOG.debug("Processing a form field: {}", item.getFieldName()); + if (item.isFormField()) { + processNormalFormField(item, request.getCharacterEncoding()); + } else { + LOG.debug("Processing a file: {}", item.getFieldName()); + processFileField(item); } } } - protected void processFileField(FileItem item) { - LOG.debug("Item is a file upload"); - + 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 values; - if (files.get(item.getFieldName()) != null) { - values = files.get(item.getFieldName()); + List values; + if (uploadedFiles.get(item.getFieldName()) != null) { + values = uploadedFiles.get(item.getFieldName()); } else { values = new ArrayList<>(); } - values.add(item); - files.put(item.getFieldName(), values); + UploadedFile uploadedFile = StrutsUploadedFile.Builder + .create(item.getPath().toFile()) + .withOriginalName(item.getName()) + .withContentType(item.getContentType()) + .build(); + values.add(uploadedFile); + + uploadedFiles.put(item.getFieldName(), values); } - protected void processNormalFormField(FileItem item, String charset) throws IOException { + protected void processNormalFormField(DiskFileItem item, String charset) throws IOException { try { LOG.debug("Item is a normal form field"); Charset encoding = Charset.forName(charset); @@ -159,7 +164,7 @@ public class JakartaMultiPartRequest extends AbstractMultiPartRequest { long size = item.getSize(); if (size > maxStringLength) { - LOG.debug("Form field {} of size {} bytes exceeds limit of {}.", sanitizeNewlines(item.getFieldName()), 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}); @@ -170,8 +175,6 @@ public class JakartaMultiPartRequest extends AbstractMultiPartRequest { } if (size == 0) { values.add(StringUtils.EMPTY); - } else if (charset == null) { - values.add(item.getString()); // WW-633 } else { values.add(item.getString(encoding)); } @@ -181,140 +184,99 @@ public class JakartaMultiPartRequest extends AbstractMultiPartRequest { } } - protected List parseRequest(HttpServletRequest servletRequest, String saveDir) throws FileUploadException { - DiskFileItemFactory fac = createDiskFileItemFactory(saveDir); - JakartaServletFileUpload upload = createServletFileUpload(fac); - - + protected List parseRequest(HttpServletRequest servletRequest, String saveDir) throws FileUploadException { + DiskFileItemFactory fileItemFactory = createDiskFileItemFactory(saveDir); + JakartaServletFileUpload upload = createServletFileUpload(fileItemFactory); return upload.parseRequest(createRequestContext(servletRequest)); } - protected JakartaServletFileUpload createServletFileUpload(DiskFileItemFactory fac) { - JakartaServletFileUpload upload = new JakartaServletFileUpload(fac); + protected JakartaServletFileUpload createServletFileUpload(DiskFileItemFactory fileItemFactory) { + JakartaServletFileUpload servletFileUpload = new JakartaServletFileUpload<>(fileItemFactory); if (maxSize != null) { - upload.setSizeMax(maxSize); + servletFileUpload.setSizeMax(maxSize); } if (maxFiles != null) { - upload.setFileCountMax(maxFiles); + servletFileUpload.setFileCountMax(maxFiles); } if (maxFileSize != null) { - upload.setFileSizeMax(maxFileSize); + servletFileUpload.setFileSizeMax(maxFileSize); } - return upload; + return servletFileUpload; } 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); + DiskFileItemFactory.Builder builder = DiskFileItemFactory.builder(); if (saveDir != null) { - fac.setPath(saveDir); + LOG.debug("Using file save directory: {}", saveDir); + builder.setPath(saveDir); } - return fac.get(); + return builder.get(); } /* (non-Javadoc) * @see org.apache.struts2.dispatcher.multipart.MultiPartRequest#getFileParameterNames() */ public Enumeration getFileParameterNames() { - return Collections.enumeration(files.keySet()); + return Collections.enumeration(uploadedFiles.keySet()); } /* (non-Javadoc) * @see org.apache.struts2.dispatcher.multipart.MultiPartRequest#getContentType(java.lang.String) */ public String[] getContentType(String fieldName) { - List items = files.get(fieldName); + List uploadedFilesList = uploadedFiles.get(fieldName); - if (items == null) { + if (uploadedFilesList == null) { return null; } - - List contentTypes = new ArrayList<>(items.size()); - for (FileItem fileItem : items) { - contentTypes.add(fileItem.getContentType()); - } - - return contentTypes.toArray(new String[0]); + return uploadedFilesList.stream().map(UploadedFile::getContentType).toArray(String[]::new); } /* (non-Javadoc) * @see org.apache.struts2.dispatcher.multipart.MultiPartRequest#getFile(java.lang.String) */ public UploadedFile[] getFile(String fieldName) { - List items = files.get(fieldName); + List uploadedFileList = uploadedFiles.get(fieldName); - if (items == null) { + if (uploadedFileList == 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()) - .build(); - fileList.add(uploadedFile); - } - - return fileList.toArray(new UploadedFile[0]); + return uploadedFileList.toArray(UploadedFile[]::new); } /* (non-Javadoc) * @see org.apache.struts2.dispatcher.multipart.MultiPartRequest#getFileNames(java.lang.String) */ public String[] getFileNames(String fieldName) { - List items = files.get(fieldName); + List uploadedFileList = uploadedFiles.get(fieldName); - if (items == null) { + if (uploadedFileList == null) { return null; } - - List fileNames = new ArrayList<>(items.size()); - for (FileItem fileItem : items) { - fileNames.add(getCanonicalName(fileItem.getName())); - } - - return fileNames.toArray(new String[0]); + return uploadedFileList.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) { - List items = files.get(fieldName); + List uploadedFileList = uploadedFiles.get(fieldName); - if (items == null) { + if (uploadedFileList == 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]); + return uploadedFileList.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 v = params.get(name); - if (v != null && !v.isEmpty()) { - return v.get(0); + List paramValue = params.get(name); + if (paramValue != null && !paramValue.isEmpty()) { + return paramValue.get(0); } return null; @@ -373,23 +335,14 @@ public class JakartaMultiPartRequest extends AbstractMultiPartRequest { * @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); - } - } - } - } + Set names = uploadedFiles.keySet(); + names.forEach(name -> { + List uploadedFileList = uploadedFiles.get(name); + uploadedFileList.forEach(file -> { + LOG.debug("Removing file: {}", file.getName()); + file.delete(); + }); + }); } - 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..b0f6c6024 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,23 +18,22 @@ */ package org.apache.struts2.dispatcher.multipart; +import jakarta.servlet.http.HttpServletRequest; +import org.apache.commons.fileupload2.core.AbstractFileUpload; import org.apache.commons.fileupload2.core.DiskFileItem; 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.jakarta.servlet6.JakartaServletFileUpload; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.apache.logging.log4j.message.ParameterizedMessage; import org.apache.struts2.dispatcher.LocalizedMessage; -import jakarta.servlet.http.HttpServletRequest; import java.io.BufferedOutputStream; import java.io.File; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; -import java.io.Serializable; import java.nio.file.Files; import java.util.ArrayList; import java.util.Collections; @@ -54,12 +53,12 @@ import java.util.UUID; */ public class JakartaStreamMultiPartRequest extends AbstractMultiPartRequest { - static final Logger LOG = LogManager.getLogger(JakartaStreamMultiPartRequest.class); + private static final Logger LOG = LogManager.getLogger(JakartaStreamMultiPartRequest.class); /** * Map between file fields and file data. */ - protected Map> fileInfos = new HashMap<>(); + protected Map> uploadedFiles = new HashMap<>(); /** * Map between non-file fields and values. @@ -71,12 +70,11 @@ public class JakartaStreamMultiPartRequest extends AbstractMultiPartRequest { */ 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()); + for (List uploadedFileList : uploadedFiles.values()) { + for (UploadedFile uploadedFile : uploadedFileList) { + LOG.debug("Deleting file '{}'.", uploadedFile.getName()); + if (!uploadedFile.delete()) { + LOG.warn("There was a problem attempting to delete file '{}'.", uploadedFile.getName()); } } } @@ -86,14 +84,14 @@ public class JakartaStreamMultiPartRequest extends AbstractMultiPartRequest { * @see org.apache.struts2.dispatcher.multipart.MultiPartRequest#getContentType(java.lang.String) */ public String[] getContentType(String fieldName) { - List infos = fileInfos.get(fieldName); - if (infos == null) { + List uploadedFileList = uploadedFiles.get(fieldName); + if (uploadedFileList == null) { return null; } - List types = new ArrayList<>(infos.size()); - for (FileInfo fileInfo : infos) { - types.add(fileInfo.getContentType()); + List types = new ArrayList<>(uploadedFileList.size()); + for (UploadedFile uploadedFile : uploadedFileList) { + types.add(uploadedFile.getContentType()); } return types.toArray(new String[0]); @@ -103,31 +101,25 @@ public class JakartaStreamMultiPartRequest extends AbstractMultiPartRequest { * @see org.apache.struts2.dispatcher.multipart.MultiPartRequest#getFile(java.lang.String) */ public UploadedFile[] getFile(String fieldName) { - List infos = fileInfos.get(fieldName); - if (infos == null) { + List uploadedFileList = uploadedFiles.get(fieldName); + if (uploadedFileList == null) { return null; } - - return infos.stream().map(fileInfo -> - StrutsUploadedFile.Builder.create(fileInfo.getFile()) - .withContentType(fileInfo.contentType) - .withOriginalName(fileInfo.originalName) - .build() - ).toArray(UploadedFile[]::new); + return uploadedFileList.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) { + List uploadedFileList = uploadedFiles.get(fieldName); + if (uploadedFileList == null) { return null; } - List names = new ArrayList<>(infos.size()); - for (FileInfo fileInfo : infos) { - names.add(getCanonicalName(fileInfo.getOriginalName())); + List names = new ArrayList<>(uploadedFileList.size()); + for (UploadedFile uploadedFile : uploadedFileList) { + names.add(getCanonicalName(uploadedFile.getOriginalName())); } return names.toArray(new String[0]); @@ -137,24 +129,18 @@ public class JakartaStreamMultiPartRequest extends AbstractMultiPartRequest { * @see org.apache.struts2.dispatcher.multipart.MultiPartRequest#getFileParameterNames() */ public Enumeration getFileParameterNames() { - return Collections.enumeration(fileInfos.keySet()); + return Collections.enumeration(uploadedFiles.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) { + List uploadedFileList = uploadedFiles.get(fieldName); + if (uploadedFileList == null) { return null; } - - List names = new ArrayList<>(infos.size()); - for (FileInfo fileInfo : infos) { - names.add(fileInfo.getFile().getName()); - } - - return names.toArray(new String[0]); + return uploadedFileList.stream().map(UploadedFile::getAbsolutePath).toArray(String[]::new); } /* (non-Javadoc) @@ -209,111 +195,51 @@ public class JakartaStreamMultiPartRequest extends AbstractMultiPartRequest { * @param saveDir location of the save dir */ protected void processUpload(HttpServletRequest request, String saveDir) throws Exception { - // Sanity check that the request is a multi-part/form-data request. - if (JakartaServletFileUpload.isMultipartContent(request)) { - - // 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); - } - } + if (!JakartaServletFileUpload.isMultipartContent(request)) { + LOG.debug("Http request isn't: {}, stop processing", AbstractFileUpload.MULTIPART_FORM_DATA); + return; } - } - /** - * 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; + // Interface with Commons FileUpload API + // Using the Streaming API + JakartaServletFileUpload servletFileUpload = new JakartaServletFileUpload<>(); + if (maxSize != null) { + LOG.debug("Applies max size: {} to file upload request", maxSize); + servletFileUpload.setSizeMax(maxSize); } - 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); + 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); + } + + // Iterate the file items + servletFileUpload.getItemIterator(request).forEachRemaining(item -> { + if (item.isFormField()) { + LOG.debug("Processing a form field: {}", sanitizeNewlines(item.getFieldName())); + processFileItemAsFormField(item); + } else { + LOG.debug("Processing a file: {}", sanitizeNewlines(item.getFieldName())); + processFileItemAsFileField(item, saveDir); + } + }); } /** - * Processes the FileItemStream as a Form Field. + * Processes the FileItem as a normal form field. * - * @param itemStream file item stream + * @param fileItemInput a form field item input */ - protected void processFileItemStreamAsFormField(FileItemInput itemStream) { - String fieldName = itemStream.getFieldName(); + protected void processFileItemAsFormField(FileItemInput fileItemInput) { + String fieldName = fileItemInput.getFieldName(); try { List values; - String fieldValue = itemStream.getInputStream().toString(); + String fieldValue = fileItemInput.getInputStream().toString(); if (!parameters.containsKey(fieldName)) { values = new ArrayList<>(); parameters.put(fieldName, values); @@ -322,35 +248,37 @@ public class JakartaStreamMultiPartRequest extends AbstractMultiPartRequest { } values.add(fieldValue); } catch (IOException e) { - LOG.warn("Failed to handle form field '{}'.", fieldName, e); + LOG.warn(new ParameterizedMessage("Failed to handle form field: '{}'", sanitizeNewlines(fieldName)), e); } } /** - * Processes the FileItemStream as a file field. + * Processes the FileItem as a file field. * - * @param itemStream file item stream - * @param location location + * @param fileItemInput file item representing upload file + * @param location location */ - protected void processFileItemStreamAsFileField(FileItemInput itemStream, String location) { + protected void processFileItemAsFileField(FileItemInput fileItemInput, 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()); + if (fileItemInput.getName() == null || fileItemInput.getName().trim().isEmpty()) { + LOG.debug("No file has been uploaded for the field: {}", sanitizeNewlines(fileItemInput.getFieldName())); return; } File file = null; try { // Create the temporary upload file. - file = createTemporaryFile(itemStream.getName(), location); + file = createTemporaryFile(fileItemInput.getName(), location); - if (streamFileToDisk(itemStream, file)) { - createFileInfoFromItemStream(itemStream, file); + if (streamFileToDisk(fileItemInput, file)) { + createUploadFile(fileItemInput, file); } } catch (IOException e) { if (file != null) { try { - file.delete(); + if (!file.delete()) { + LOG.warn("Could not delete the file: {}", file.getAbsoluteFile()); + } } catch (SecurityException se) { LOG.warn("Failed to delete '{}' due to security exception above.", file.getName(), se); } @@ -363,13 +291,13 @@ public class JakartaStreamMultiPartRequest extends AbstractMultiPartRequest { * * @param fileName file name * @param location location - * @return temporary file based on the given filename and location + * @return a temporary file based on the given filename and location * @throws IOException in case of IO errors */ protected File createTemporaryFile(String fileName, String location) throws IOException { String name = fileName - .substring(fileName.lastIndexOf('/') + 1) - .substring(fileName.lastIndexOf('\\') + 1); + .substring(fileName.lastIndexOf('/') + 1) + .substring(fileName.lastIndexOf('\\') + 1); String prefix = name; String suffix = ""; @@ -391,93 +319,50 @@ public class JakartaStreamMultiPartRequest extends AbstractMultiPartRequest { /** * Streams the file upload stream to the specified file. * - * @param itemStream file item stream - * @param file the file + * @param fileItemInput file item input + * @param file the file * @return true if stream was successfully * @throws IOException in case of IO errors */ - 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 boolean streamFileToDisk(FileItemInput fileItemInput, File file) throws IOException { + try (InputStream input = fileItemInput.getInputStream(); + 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: {}", bufferSize); for (int length; ((length = input.read(buffer)) > 0); ) { output.write(buffer, 0, length); } - result = true; + } catch (IOException e) { + LOG.error(new ParameterizedMessage("Cannot write input file: {} into file stream: {}", + fileItemInput.getName(), file.getAbsolutePath()), e); + return false; } - return result; + return true; } /** - * 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) { + protected void createUploadFile(FileItemInput fileItemInput, File file) { // gather attributes from file upload stream. - String fileName = itemStream.getName(); - String fieldName = itemStream.getFieldName(); + String fileName = fileItemInput.getName(); + String fieldName = fileItemInput.getFieldName(); // create internal structure - FileInfo fileInfo = new FileInfo(file, itemStream.getContentType(), fileName); + UploadedFile uploadedFile = StrutsUploadedFile.Builder + .create(file) + .withOriginalName(fileName) + .withContentType(fileItemInput.getContentType()) + .build(); // append or create new entry. - if (!fileInfos.containsKey(fieldName)) { - List infos = new ArrayList<>(); - infos.add(fileInfo); - fileInfos.put(fieldName, infos); + if (!uploadedFiles.containsKey(fieldName)) { + List infos = new ArrayList<>(); + infos.add(uploadedFile); + uploadedFiles.put(fieldName, infos); } 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; + uploadedFiles.get(fieldName).add(uploadedFile); } } 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..3a9f650d5 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; @@ -221,10 +221,10 @@ 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" ); } @@ -331,15 +331,15 @@ 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) + - endline + - endline + - endline + - "--" + - boundary + - "--" + - endline; + encodeTextFile("test1.html", "text/html", htmlContent) + + encodeTextFile("test2.html", "text/html", htmlContent) + + endline + + endline + + endline + + "--" + + boundary + + "--" + + endline; req.setContent(content.getBytes()); assertTrue(JakartaServletDiskFileUpload.isMultipartContent(req)); @@ -369,14 +369,14 @@ 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)); @@ -424,7 +424,7 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase { mai.setResultCode("success"); mai.setInvocationContext(ActionContext.getContext()); ActionContext.getContext() - .withServletRequest(createMultipartRequestMaxFileSize(req)); + .withServletRequest(createMultipartRequestMaxFileSize(req)); interceptor.intercept(mai); @@ -435,8 +435,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: 10!", + msg); } public void testMultipartRequestMaxStringLength() throws Exception { @@ -471,7 +471,7 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase { mai.setResultCode("success"); mai.setInvocationContext(ActionContext.getContext()); ActionContext.getContext() - .withServletRequest(createMultipartRequestMaxStringLength(req)); + .withServletRequest(createMultipartRequestMaxStringLength(req)); interceptor.intercept(mai); @@ -481,8 +481,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 { @@ -509,8 +509,8 @@ public class ActionFileUploadInterceptorTest extends StrutsInternalTestCase { 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); @@ -525,19 +525,19 @@ 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; + "--" + + "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) { 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..40d4f92d7 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,26 +45,6 @@ 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; /** @@ -221,10 +217,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" ); } @@ -343,15 +339,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)); @@ -396,14 +392,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)); @@ -543,9 +539,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 +556,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) { diff --git a/pom.xml b/pom.xml index f8b0a6e35..51eee01c7 100644 --- a/pom.xml +++ b/pom.xml @@ -823,8 +823,8 @@ org.apache.commons - commons-fileupload2-jakarta - 2.0.0-M1 + commons-fileupload2-jakarta-servlet6 + 2.0.0-M2 commons-io