This is an automated email from the ASF dual-hosted git repository. lukaszlenart pushed a commit to branch feature/WW-5371-modern-upload in repository https://gitbox.apache.org/repos/asf/struts.git
The following commit(s) were added to refs/heads/feature/WW-5371-modern-upload by this push: new 3ce3f8264 WW-5371 Simplifies file upload logic and extracts constants 3ce3f8264 is described below commit 3ce3f82646f667be262fadc1420a2d9251577c17 Author: Lukasz Lenart <lukaszlen...@apache.org> AuthorDate: Mon Dec 11 08:55:20 2023 +0100 WW-5371 Simplifies file upload logic and extracts constants --- .../showcase/fileupload/FileUploadAction.java | 3 -- .../dispatcher/multipart/StrutsUploadedFile.java | 5 ++ .../struts2/dispatcher/multipart/UploadedFile.java | 4 +- .../interceptor/AbstractFileUploadInterceptor.java | 20 +++++--- .../interceptor/ActionFileUploadInterceptor.java | 56 +++++++++------------- .../struts2/interceptor/FileUploadInterceptor.java | 6 +-- 6 files changed, 47 insertions(+), 47 deletions(-) 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 246f8ad70..c2ac471f4 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,7 +24,6 @@ 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,8 +31,6 @@ import java.util.List; */ public class FileUploadAction extends ActionSupport implements UploadedFilesAware { - private static final long serialVersionUID = 5156288255337069381L; - private String contentType; private UploadedFile uploadedFile; private String fileName; 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 55233d7d7..5976f578f 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 @@ -26,6 +26,11 @@ public class StrutsUploadedFile implements UploadedFile { private final String contentType; private final String originalName; + /** + * Use builder instead of constructor + * @param file an uploaded file + * @deprecated since Struts 6.4.0 + */ @Deprecated public StrutsUploadedFile(File file) { this.file = file; 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 896c681be..ada27ff6c 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 @@ -18,10 +18,12 @@ */ package org.apache.struts2.dispatcher.multipart; +import java.io.Serializable; + /** * Virtual representation of a uploaded file used by {@link MultiPartRequest} */ -public interface UploadedFile { +public interface UploadedFile extends Serializable { Long length(); diff --git a/core/src/main/java/org/apache/struts2/interceptor/AbstractFileUploadInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/AbstractFileUploadInterceptor.java index 3857a7b4e..c8da384d2 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/AbstractFileUploadInterceptor.java +++ b/core/src/main/java/org/apache/struts2/interceptor/AbstractFileUploadInterceptor.java @@ -45,6 +45,14 @@ public abstract class AbstractFileUploadInterceptor extends AbstractInterceptor private static final Logger LOG = LogManager.getLogger(AbstractFileUploadInterceptor.class); + public static final String STRUTS_MESSAGES_BYPASS_REQUEST_KEY = "struts.messages.bypass.request"; + public static final String STRUTS_MESSAGES_ERROR_UPLOADING_KEY = "struts.messages.error.uploading"; + public static final String STRUTS_MESSAGES_ERROR_FILE_TOO_LARGE_KEY = "struts.messages.error.file.too.large"; + public static final String STRUTS_MESSAGES_INVALID_FILE_KEY = "struts.messages.invalid.file"; + public static final String STRUTS_MESSAGES_INVALID_CONTENT_TYPE_KEY = "struts.messages.invalid.content.type"; + public static final String STRUTS_MESSAGES_ERROR_CONTENT_TYPE_NOT_ALLOWED_KEY = "struts.messages.error.content.type.not.allowed"; + public static final String STRUTS_MESSAGES_ERROR_FILE_EXTENSION_NOT_ALLOWED_KEY = "struts.messages.error.file.extension.not.allowed"; + protected Long maximumSize; protected Set<String> allowedTypesSet = Collections.emptySet(); protected Set<String> allowedExtensionsSet = Collections.emptySet(); @@ -109,31 +117,31 @@ public abstract class AbstractFileUploadInterceptor extends AbstractInterceptor // If it's null the upload failed if (file == null) { - String errMsg = getTextMessage(action, "struts.messages.error.uploading", new String[]{inputName}); + String errMsg = getTextMessage(action, STRUTS_MESSAGES_ERROR_UPLOADING_KEY, new String[]{inputName}); if (validation != null) { validation.addFieldError(inputName, errMsg); } LOG.warn(errMsg); } else if (file.getContent() == null) { - String errMsg = getTextMessage(action, "struts.messages.error.uploading", new String[]{filename}); + String errMsg = getTextMessage(action, STRUTS_MESSAGES_ERROR_UPLOADING_KEY, new String[]{filename}); if (validation != null) { validation.addFieldError(inputName, errMsg); } LOG.warn(errMsg); } else if (maximumSize != null && maximumSize < file.length()) { - String errMsg = getTextMessage(action, "struts.messages.error.file.too.large", new String[]{inputName, filename, file.getName(), "" + file.length(), getMaximumSizeStr(action)}); + String errMsg = getTextMessage(action, STRUTS_MESSAGES_ERROR_FILE_TOO_LARGE_KEY, new String[]{inputName, filename, file.getName(), "" + file.length(), getMaximumSizeStr(action)}); if (validation != null) { validation.addFieldError(inputName, errMsg); } LOG.warn(errMsg); } else if ((!allowedTypesSet.isEmpty()) && (!containsItem(allowedTypesSet, contentType))) { - String errMsg = getTextMessage(action, "struts.messages.error.content.type.not.allowed", new String[]{inputName, filename, file.getName(), contentType}); + String errMsg = getTextMessage(action, STRUTS_MESSAGES_ERROR_CONTENT_TYPE_NOT_ALLOWED_KEY, new String[]{inputName, filename, file.getName(), contentType}); if (validation != null) { validation.addFieldError(inputName, errMsg); } LOG.warn(errMsg); } else if ((!allowedExtensionsSet.isEmpty()) && (!hasAllowedExtension(allowedExtensionsSet, filename))) { - String errMsg = getTextMessage(action, "struts.messages.error.file.extension.not.allowed", new String[]{inputName, filename, file.getName(), contentType}); + String errMsg = getTextMessage(action, STRUTS_MESSAGES_ERROR_FILE_EXTENSION_NOT_ALLOWED_KEY, new String[]{inputName, filename, file.getName(), contentType}); if (validation != null) { validation.addFieldError(inputName, errMsg); } @@ -237,7 +245,7 @@ public abstract class AbstractFileUploadInterceptor extends AbstractInterceptor if (textProvider.hasKey(error.getTextKey())) { errorMessage = textProvider.getText(error.getTextKey(), Arrays.asList(error.getArgs())); } else { - errorMessage = textProvider.getText("struts.messages.error.uploading", error.getDefaultMessage()); + errorMessage = textProvider.getText(STRUTS_MESSAGES_ERROR_UPLOADING_KEY, error.getDefaultMessage()); } validation.addActionError(errorMessage); } 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 4cff8617e..1642a890b 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/ActionFileUploadInterceptor.java +++ b/core/src/main/java/org/apache/struts2/interceptor/ActionFileUploadInterceptor.java @@ -180,8 +180,10 @@ public class ActionFileUploadInterceptor extends AbstractFileUploadInterceptor { public String intercept(ActionInvocation invocation) throws Exception { HttpServletRequest request = invocation.getInvocationContext().getServletRequest(); if (!(request instanceof MultiPartRequestWrapper)) { - ActionProxy proxy = invocation.getProxy(); - LOG.debug(getTextMessage("struts.messages.bypass.request", new String[]{proxy.getNamespace(), proxy.getActionName()})); + if (LOG.isDebugEnabled()) { + ActionProxy proxy = invocation.getProxy(); + LOG.debug(getTextMessage(STRUTS_MESSAGES_BYPASS_REQUEST_KEY, new String[]{proxy.getNamespace(), proxy.getActionName()})); + } return invocation.invoke(); } @@ -199,50 +201,36 @@ public class ActionFileUploadInterceptor extends AbstractFileUploadInterceptor { // bind allowed Files Enumeration<String> fileParameterNames = multiWrapper.getFileParameterNames(); + List<UploadedFile> acceptedFiles = new ArrayList<>(); + while (fileParameterNames != null && fileParameterNames.hasMoreElements()) { // get the value of this input tag String inputName = fileParameterNames.nextElement(); + UploadedFile[] uploadedFiles = multiWrapper.getFiles(inputName); - // get the content type - String[] contentType = multiWrapper.getContentTypes(inputName); - - if (isNonEmpty(contentType)) { - // get the name of the file from the input tag - String[] fileName = multiWrapper.getFileNames(inputName); - - if (isNonEmpty(fileName)) { - // get a File object for the uploaded File - UploadedFile[] files = multiWrapper.getFiles(inputName); - if (files != null && files.length > 0) { - List<UploadedFile> acceptedFiles = new ArrayList<>(files.length); - - for (int index = 0; index < files.length; index++) { - if (acceptFile(action, files[index], fileName[index], contentType[index], inputName)) { - acceptedFiles.add(files[index]); - } - } - - if (acceptedFiles.isEmpty()) { - LOG.debug("No files have been uploaded/accepted"); - } else { - LOG.debug("Passing: {} uploaded file(s) to action", acceptedFiles.size()); - action.withUploadedFiles(acceptedFiles); - } - } - } else { - if (LOG.isWarnEnabled()) { - LOG.warn(getTextMessage(action, "struts.messages.invalid.file", new String[]{inputName})); - } + if (uploadedFiles == null || uploadedFiles.length == 0) { + if (LOG.isWarnEnabled()) { + LOG.warn(getTextMessage(action, STRUTS_MESSAGES_INVALID_FILE_KEY, new String[]{inputName})); } } else { - if (LOG.isWarnEnabled()) { - LOG.warn(getTextMessage(action, "struts.messages.invalid.content.type", new String[]{inputName})); + for (UploadedFile uploadedFile : uploadedFiles) { + if (acceptFile(action, uploadedFile, uploadedFile.getOriginalName(), uploadedFile.getContentType(), inputName)) { + acceptedFiles.add(uploadedFile); + } } } } + if (acceptedFiles.isEmpty()) { + LOG.debug("No files have been uploaded/accepted"); + } else { + LOG.debug("Passing: {} uploaded file(s) to action", acceptedFiles.size()); + action.withUploadedFiles(acceptedFiles); + } + // invoke action return invocation.invoke(); } } + diff --git a/core/src/main/java/org/apache/struts2/interceptor/FileUploadInterceptor.java b/core/src/main/java/org/apache/struts2/interceptor/FileUploadInterceptor.java index 4bcdec07b..287e66078 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/FileUploadInterceptor.java +++ b/core/src/main/java/org/apache/struts2/interceptor/FileUploadInterceptor.java @@ -191,7 +191,7 @@ public class FileUploadInterceptor extends AbstractFileUploadInterceptor { if (!(request instanceof MultiPartRequestWrapper)) { if (LOG.isDebugEnabled()) { ActionProxy proxy = invocation.getProxy(); - LOG.debug(getTextMessage("struts.messages.bypass.request", new String[]{proxy.getNamespace(), proxy.getActionName()})); + LOG.debug(getTextMessage(STRUTS_MESSAGES_BYPASS_REQUEST_KEY, new String[]{proxy.getNamespace(), proxy.getActionName()})); } return invocation.invoke(); @@ -243,12 +243,12 @@ public class FileUploadInterceptor extends AbstractFileUploadInterceptor { } } else { if (LOG.isWarnEnabled()) { - LOG.warn(getTextMessage(action, "struts.messages.invalid.file", new String[]{inputName})); + LOG.warn(getTextMessage(action, STRUTS_MESSAGES_INVALID_FILE_KEY, new String[]{inputName})); } } } else { if (LOG.isWarnEnabled()) { - LOG.warn(getTextMessage(action, "struts.messages.invalid.content.type", new String[]{inputName})); + LOG.warn(getTextMessage(action, STRUTS_MESSAGES_INVALID_CONTENT_TYPE_KEY, new String[]{inputName})); } } }