From ae2f937761de7678b6abe586499fcf466dad93d8 Mon Sep 17 00:00:00 2001 From: Musachy Barroso Date: Wed, 3 Dec 2008 16:23:43 +0000 Subject: [PATCH] WW-2580 Add filename argument to acceptFile method in FileUploadInterceptor. Unused overload of acceptFile was also removed. Thanks to Jelmer Kuperus for the patch (applied with some modifications). git-svn-id: https://svn.apache.org/repos/asf/struts/struts2/trunk@722956 13f79535-47bb-0310-9956-ffa450edef68 --- .../interceptor/FileUploadInterceptor.java | 67 +++++++++++++------ .../apache/struts2/struts-messages.properties | 5 +- .../struts2/struts-messages_de.properties | 5 +- .../struts2/struts-messages_pl.properties | 5 +- .../FileUploadInterceptorTest.java | 41 ++++++++++-- 5 files changed, 91 insertions(+), 32 deletions(-) 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 5d0985773..16239f6f9 100644 --- a/core/src/main/java/org/apache/struts2/interceptor/FileUploadInterceptor.java +++ b/core/src/main/java/org/apache/struts2/interceptor/FileUploadInterceptor.java @@ -74,6 +74,9 @@ import com.opensymphony.xwork2.util.logging.LoggerFactory; *
  • struts.messages.error.content.type.not.allowed - occurs when the uploaded file does not match the expected * content types specified
    • *

    + *

  • struts.messages.error.file.extension.not.allowed - occurs when the uploaded file does not match the expected + * file extensions specified
    • + *

    * *

    * @@ -91,8 +94,11 @@ import com.opensymphony.xwork2.util.logging.LoggerFactory; *

  • allowedTypes (optional) - a comma separated list of content types (ie: text/html) that the interceptor will allow * a file reference to be set on the action. If none is specified allow all types to be uploaded.
    • *

    + *

  • allowedExtensions (optional) - a comma separated list of file extensions (ie: .html) that the interceptor will allow + * a file reference to be set on the action. If none is specified allow all extensions to be uploaded.
    • * *

    + *

    * *

    *

    Extending the interceptor: @@ -179,11 +185,21 @@ public class FileUploadInterceptor extends AbstractInterceptor { protected Long maximumSize; protected Set allowedTypesSet = Collections.emptySet(); + protected Set allowedExtensionsSet = Collections.emptySet(); public void setUseActionMessageBundle(String value) { this.useActionMessageBundle = Boolean.valueOf(value); } + /** + * Sets the allowed extensions + * + * @param allowedExtensions A comma-delimited list of extensions + */ + public void setAllowedExtensions(String allowedExtensions) { + allowedExtensionsSet = TextParseUtil.commaDelimitedStringToSet(allowedExtensions); + } + /** * Sets the allowed mimetypes * @@ -263,7 +279,7 @@ public class FileUploadInterceptor extends AbstractInterceptor { String fileNameName = inputName + "FileName"; for (int index = 0; index < files.length; index++) { - if (acceptFile(action, files[index], contentType[index], inputName, validation, ac.getLocale())) { + if (acceptFile(action, files[index], fileName[index], contentType[index], inputName, validation, ac.getLocale())) { acceptedFiles.add(files[index]); acceptedContentTypes.add(contentType[index]); acceptedFileNames.add(fileName[index]); @@ -309,22 +325,6 @@ public class FileUploadInterceptor extends AbstractInterceptor { return result; } - /** - * Override for added functionality. Checks if the proposed file is acceptable based on contentType and size. - * - * @param file - proposed upload file. - * @param contentType - contentType of the file. - * @param inputName - inputName of the file. - * @param validation - Non-null ValidationAware if the action implements ValidationAware, allowing for better - * logging. - * @param locale - * @return true if the proposed file is acceptable by contentType and size. - */ - protected boolean acceptFile(File file, String contentType, String inputName, ValidationAware validation, Locale locale) { - return acceptFile(null, file, contentType, inputName, validation, locale); - - } - /** * Override for added functionality. Checks if the proposed file is acceptable based on contentType and size. * @@ -337,7 +337,7 @@ public class FileUploadInterceptor extends AbstractInterceptor { * @param locale * @return true if the proposed file is acceptable by contentType and size. */ - protected boolean acceptFile(Object action, File file, String contentType, String inputName, ValidationAware validation, Locale locale) { + protected boolean acceptFile(Object action, File file, String filename, String contentType, String inputName, ValidationAware validation, Locale locale) { boolean fileIsAcceptable = false; // If it's null the upload failed @@ -349,14 +349,21 @@ public class FileUploadInterceptor extends AbstractInterceptor { LOG.error(errMsg); } else if (maximumSize != null && maximumSize < file.length()) { - String errMsg = getTextMessage(action, "struts.messages.error.file.too.large", new Object[]{inputName, file.getName(), "" + file.length()}, locale); + String errMsg = getTextMessage(action, "struts.messages.error.file.too.large", new Object[]{inputName, filename, file.getName(), "" + file.length()}, locale); if (validation != null) { validation.addFieldError(inputName, errMsg); } LOG.error(errMsg); } else if ((!allowedTypesSet.isEmpty()) && (!containsItem(allowedTypesSet, contentType))) { - String errMsg = getTextMessage(action, "struts.messages.error.content.type.not.allowed", new Object[]{inputName, file.getName(), contentType}, locale); + String errMsg = getTextMessage(action, "struts.messages.error.content.type.not.allowed", new Object[]{inputName, filename, file.getName(), contentType}, locale); + if (validation != null) { + validation.addFieldError(inputName, errMsg); + } + + LOG.error(errMsg); + } else if ((! allowedExtensionsSet.isEmpty()) && (!hasAllowedExtension(allowedExtensionsSet, filename))) { + String errMsg = getTextMessage(action, "struts.messages.error.file.extension.not.allowed", new Object[]{inputName, filename, file.getName(), contentType}, locale); if (validation != null) { validation.addFieldError(inputName, errMsg); } @@ -369,6 +376,26 @@ public class FileUploadInterceptor extends AbstractInterceptor { return fileIsAcceptable; } + /** + * @param extensionCollection - Collection of extensions (all lowercase). + * @param filename - filename to check. + * @return true if the filename has an allowed extension, false otherwise. + */ + private static boolean hasAllowedExtension(Collection extensionCollection, String filename) { + if (filename == null) { + return false; + } + + String lowercaseFilename = filename.toLowerCase(); + for (String extension : extensionCollection) { + if (lowercaseFilename.endsWith(extension)) { + return true; + } + } + + return false; + } + /** * @param itemCollection - Collection of string items (all lowercase). * @param item - Item to search for. diff --git a/core/src/main/resources/org/apache/struts2/struts-messages.properties b/core/src/main/resources/org/apache/struts2/struts-messages.properties index 1510f2e8d..2882ec5dc 100644 --- a/core/src/main/resources/org/apache/struts2/struts-messages.properties +++ b/core/src/main/resources/org/apache/struts2/struts-messages.properties @@ -27,7 +27,8 @@ struts.messages.invalid.file=Could not find a Filename for {0}. Verify that a va struts.messages.invalid.content.type=Could not find a Content-Type for {0}. Verify that a valid file was submitted. struts.messages.removing.file=Removing file {0} {1} struts.messages.error.uploading=Error uploading: {0} -struts.messages.error.file.too.large=File too large: {0} "{1}" {2} -struts.messages.error.content.type.not.allowed=Content-Type not allowed: {0} "{1}" {2} +struts.messages.error.file.too.large=File too large: {0} "{1}" "{2}" {3} +struts.messages.error.content.type.not.allowed=Content-Type not allowed: {0} "{1}" "{2}" {3} +struts.messages.error.file.extension.not.allowed=File extension not allowed: {0} "{1}" "{2}" {3} devmode.notification=Developer Notification (set struts.devMode to false to disable this message):\n{0} diff --git a/core/src/main/resources/org/apache/struts2/struts-messages_de.properties b/core/src/main/resources/org/apache/struts2/struts-messages_de.properties index 725c92aca..dec04bf84 100644 --- a/core/src/main/resources/org/apache/struts2/struts-messages_de.properties +++ b/core/src/main/resources/org/apache/struts2/struts-messages_de.properties @@ -27,7 +27,8 @@ struts.messages.invalid.file=Es konnte kein Dateiname f\u00fcr {0} ermittelt wer struts.messages.invalid.content.type=Es konnte kein Content-Type f\u00fcr {0} ermittelt werden. \u00dcberpr\u00fcfen Sie ob eine g\u00fcltige Datei \u00fcbermittelt wurde. struts.messages.removing.file=Entferne Datei {0} {1} struts.messages.error.uploading=Fehler beim Upload: {0} -struts.messages.error.file.too.large=Datei zu gross: {0} "{1}" {2} -struts.messages.error.content.type.not.allowed=Content-Type nicht erlaubt: {0} "{1}" {2} +struts.messages.error.file.too.large=Datei zu gross: {0} "{1}" "{2}" {3} +struts.messages.error.content.type.not.allowed=Content-Type nicht erlaubt: {0} "{1}" "{2}" {3} +struts.messages.error.file.extension.not.allowed=File extension nicht erlaubt: {0} "{1}" "{2}" {3} devmode.notification=Entwickler Hinweis (Setzen Sie struts.devMode auf false um diese Nachricht zu deaktivieren):\n{0} diff --git a/core/src/main/resources/org/apache/struts2/struts-messages_pl.properties b/core/src/main/resources/org/apache/struts2/struts-messages_pl.properties index 2463e50bb..41d500b2b 100644 --- a/core/src/main/resources/org/apache/struts2/struts-messages_pl.properties +++ b/core/src/main/resources/org/apache/struts2/struts-messages_pl.properties @@ -27,7 +27,8 @@ struts.messages.invalid.file=Nie odnaleziono nazwy pliku dla {0}. Upewnij si\u01 struts.messages.invalid.content.type=Nie odnaleziono Content-Type dla {0}. Upewnij si\u0119 \u017ce wys\u0142ano w\u0142a\u015bciwy plik. struts.messages.removing.file=Usuwanie pliku {0} {1} struts.messages.error.uploading=B\u0142\u0105d podczas wysy\u0142ania pliku: {0} -struts.messages.error.file.too.large=Plik jest za du\u017cy: {0} "{1}" {2} -struts.messages.error.content.type.not.allowed=Niedozwolony Content-Type: {0} "{1}" {2} +struts.messages.error.file.too.large=Plik jest za du\u017cy: {0} "{1}" "{2}" {3} +struts.messages.error.content.type.not.allowed=Niedozwolony Content-Type: {0} "{1}" "{2}" {3} +struts.messages.error.file.extension.not.allowed=Niedozwolony File extension: {0} "{1}" "{2}" {3} devmode.notification=Powiadmienie Developera (ustaw struts.devMode na false by wy\u0142\u0105czy\u0107 t\u0119 wiadomo\u015b\u0107):\n{0} diff --git a/core/src/test/java/org/apache/struts2/interceptor/FileUploadInterceptorTest.java b/core/src/test/java/org/apache/struts2/interceptor/FileUploadInterceptorTest.java index 695e28fff..d8bd7f061 100644 --- a/core/src/test/java/org/apache/struts2/interceptor/FileUploadInterceptorTest.java +++ b/core/src/test/java/org/apache/struts2/interceptor/FileUploadInterceptorTest.java @@ -55,10 +55,10 @@ public class FileUploadInterceptorTest extends StrutsTestCase { private FileUploadInterceptor interceptor; private File tempDir; - public void testAcceptFileWithEmptyAllowedTypes() throws Exception { + public void testAcceptFileWithEmptyAllowedTypesAndExtensions() throws Exception { // when allowed type is empty ValidationAwareSupport validation = new ValidationAwareSupport(); - boolean ok = interceptor.acceptFile(new File(""), "text/plain", "inputName", validation, Locale.getDefault()); + boolean ok = interceptor.acceptFile(null, new File(""), "filename", "text/plain", "inputName", validation, Locale.getDefault()); assertTrue(ok); assertTrue(validation.getFieldErrors().isEmpty()); @@ -70,7 +70,7 @@ public class FileUploadInterceptorTest extends StrutsTestCase { // when file is of allowed types ValidationAwareSupport validation = new ValidationAwareSupport(); - boolean ok = interceptor.acceptFile(new File(""), "text/plain", "inputName", validation, Locale.getDefault()); + boolean ok = interceptor.acceptFile(null, new File(""), "filename.txt", "text/plain", "inputName", validation, Locale.getDefault()); assertTrue(ok); assertTrue(validation.getFieldErrors().isEmpty()); @@ -78,20 +78,49 @@ public class FileUploadInterceptorTest extends StrutsTestCase { // when file is not of allowed types validation = new ValidationAwareSupport(); - boolean notOk = interceptor.acceptFile(new File(""), "text/html", "inputName", validation, Locale.getDefault()); + boolean notOk = interceptor.acceptFile(null, new File(""), "filename.html", "text/html", "inputName", validation, Locale.getDefault()); assertFalse(notOk); assertFalse(validation.getFieldErrors().isEmpty()); assertTrue(validation.hasErrors()); } + public void testAcceptFileWithoutEmptyExtensions() throws Exception { + interceptor.setAllowedExtensions(".txt"); + + // when file is of allowed extensions + ValidationAwareSupport validation = new ValidationAwareSupport(); + boolean ok = interceptor.acceptFile(null, new File(""), "filename.txt", "text/plain", "inputName", validation, Locale.getDefault()); + + assertTrue(ok); + assertTrue(validation.getFieldErrors().isEmpty()); + assertFalse(validation.hasErrors()); + + // when file is not of allowed extensions + validation = new ValidationAwareSupport(); + boolean notOk = interceptor.acceptFile(null, new File(""), "filename.html", "text/html", "inputName", validation, Locale.getDefault()); + + assertFalse(notOk); + assertFalse(validation.getFieldErrors().isEmpty()); + assertTrue(validation.hasErrors()); + + //test with multiple extensions + interceptor.setAllowedExtensions(".txt,.lol"); + validation = new ValidationAwareSupport(); + ok = interceptor.acceptFile(null, new File(""), "filename.lol", "text/plain", "inputName", validation, Locale.getDefault()); + + assertTrue(ok); + assertTrue(validation.getFieldErrors().isEmpty()); + assertFalse(validation.hasErrors()); + } + public void testAcceptFileWithNoFile() throws Exception { FileUploadInterceptor interceptor = new FileUploadInterceptor(); interceptor.setAllowedTypes("text/plain"); // when file is not of allowed types ValidationAwareSupport validation = new ValidationAwareSupport(); - boolean notOk = interceptor.acceptFile(null, "text/html", "inputName", validation, Locale.getDefault()); + boolean notOk = interceptor.acceptFile(null, null, "filename.html", "text/html", "inputName", validation, Locale.getDefault()); assertFalse(notOk); assertFalse(validation.getFieldErrors().isEmpty()); @@ -113,7 +142,7 @@ public class FileUploadInterceptorTest extends StrutsTestCase { URL url = ClassLoaderUtil.getResource("log4j.properties", FileUploadInterceptorTest.class); File file = new File(new URI(url.toString())); assertTrue("log4j.properties should be in src/test folder", file.exists()); - boolean notOk = interceptor.acceptFile(file, "text/html", "inputName", validation, Locale.getDefault()); + boolean notOk = interceptor.acceptFile(null, file, "filename", "text/html", "inputName", validation, Locale.getDefault()); assertFalse(notOk); assertFalse(validation.getFieldErrors().isEmpty());