Author: hermanns
Date: Fri Jun 13 14:14:44 2008
New Revision: 667656
URL: http://svn.apache.org/viewvc?rev=667656&view=rev
Log:
WW-2557 FileUploadInterceptor allows forbidden files when passed with allowed
files
o applied slightly modified patch submitted by Stephan Schroeder
Modified:
struts/struts2/trunk/core/src/main/java/org/apache/struts2/interceptor/FileUploadInterceptor.java
struts/struts2/trunk/core/src/test/java/org/apache/struts2/interceptor/FileUploadInterceptorTest.java
Modified:
struts/struts2/trunk/core/src/main/java/org/apache/struts2/interceptor/FileUploadInterceptor.java
URL:
http://svn.apache.org/viewvc/struts/struts2/trunk/core/src/main/java/org/apache/struts2/interceptor/FileUploadInterceptor.java?rev=667656&r1=667655&r2=667656&view=diff
==============================================================================
---
struts/struts2/trunk/core/src/main/java/org/apache/struts2/interceptor/FileUploadInterceptor.java
(original)
+++
struts/struts2/trunk/core/src/main/java/org/apache/struts2/interceptor/FileUploadInterceptor.java
Fri Jun 13 14:14:44 2008
@@ -22,15 +22,7 @@
package org.apache.struts2.interceptor;
import java.io.File;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.Enumeration;
-import java.util.HashSet;
-import java.util.Iterator;
-import java.util.Locale;
-import java.util.Map;
-import java.util.Set;
-import java.util.StringTokenizer;
+import java.util.*;
import javax.servlet.http.HttpServletRequest;
@@ -48,73 +40,73 @@
/**
* <!-- START SNIPPET: description -->
- *
+ * <p/>
* Interceptor that is based off of [EMAIL PROTECTED]
MultiPartRequestWrapper}, which is automatically applied for any request that
* includes a file. It adds the following parameters, where [File Name] is the
name given to the file uploaded by the
* HTML form:
- *
+ * <p/>
* <ul>
- *
+ * <p/>
* <li>[File Name] : File - the actual File</li>
- *
+ * <p/>
* <li>[File Name]ContentType : String - the content type of the file</li>
- *
+ * <p/>
* <li>[File Name]FileName : String - the actual name of the file uploaded
(not the HTML name)</li>
- *
+ * <p/>
* </ul>
- *
+ * <p/>
* <p/> You can get access to these files by merely providing setters in your
action that correspond to any of the three
* patterns above, such as setDocument(File document),
setDocumentContentType(String contentType), etc.
* <br/>See the example code section.
- *
+ * <p/>
* <p/> This interceptor will add several field errors, assuming that the
action implements [EMAIL PROTECTED] ValidationAware}.
* These error messages are based on several i18n values stored in
struts-messages.properties, a default i18n file
* processed for all i18n requests. You can override the text of these
messages by providing text for the following
* keys:
- *
+ * <p/>
* <ul>
- *
+ * <p/>
* <li>struts.messages.error.uploading - a general error that occurs when the
file could not be uploaded</li>
- *
+ * <p/>
* <li>struts.messages.error.file.too.large - occurs when the uploaded file is
too large</li>
- *
+ * <p/>
* <li>struts.messages.error.content.type.not.allowed - occurs when the
uploaded file does not match the expected
* content types specified</li>
- *
+ * <p/>
* </ul>
- *
+ * <p/>
* <!-- END SNIPPET: description -->
- *
+ * <p/>
* <p/> <u>Interceptor parameters:</u>
- *
+ * <p/>
* <!-- START SNIPPET: parameters -->
- *
+ * <p/>
* <ul>
- *
+ * <p/>
* <li>maximumSize (optional) - the maximum size (in bytes) that the
interceptor will allow a file reference to be set
* on the action. Note, this is <b>not</b> related to the various properties
found in struts.properties.
* Default to approximately 2MB.</li>
- *
+ * <p/>
* <li>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.</li>
- *
+ * <p/>
* </ul>
- *
+ * <p/>
* <!-- END SNIPPET: parameters -->
- *
+ * <p/>
* <p/> <u>Extending the interceptor:</u>
- *
* <p/>
- *
+ * <p/>
+ * <p/>
* <!-- START SNIPPET: extending -->
- *
+ * <p/>
* You can extend this interceptor and override the acceptFile method to
provide more control over which files
* are supported and which are not.
- *
+ * <p/>
* <!-- END SNIPPET: extending -->
- *
+ * <p/>
* <p/> <u>Example code:</u>
- *
+ * <p/>
* <pre>
* <!-- START SNIPPET: example-configuration -->
* <action name="doUpload" class="com.example.UploadAction">
@@ -124,13 +116,13 @@
* </action>
* <!-- END SNIPPET: example-configuration -->
* </pre>
- *
+ * <p/>
* <!-- START SNIPPET: multipart-note -->
- *
+ * <p/>
* You must set the encoding to <code>multipart/form-data</code> in the form
where the user selects the file to upload.
- *
+ * <p/>
* <!-- END SNIPPET: multipart-note -->
- *
+ * <p/>
* <pre>
* <!-- START SNIPPET: example-form -->
* <s:form action="doUpload" method="post"
enctype="multipart/form-data">
@@ -139,34 +131,34 @@
* </s:form>
* <!-- END SNIPPET: example-form -->
* </pre>
- *
+ * <p/>
* And then in your action code you'll have access to the File object if you
provide setters according to the
* naming convention documented in the start.
- *
+ * <p/>
* <pre>
* <!-- START SNIPPET: example-action -->
* package com.example;
- *
+ * <p/>
* import java.io.File;
* import com.opensymphony.xwork2.ActionSupport;
- *
+ * <p/>
* public UploadAction extends ActionSupport {
* private File file;
* private String contentType;
* private String filename;
- *
+ * <p/>
* public void setUpload(File file) {
* this.file = file;
* }
- *
+ * <p/>
* public void setUploadContentType(String contentType) {
* this.contentType = contentType;
* }
- *
+ * <p/>
* public void setUploadFileName(String filename) {
* this.filename = filename;
* }
- *
+ * <p/>
* public String execute() {
* //...
* return SUCCESS;
@@ -174,7 +166,6 @@
* }
* <!-- END SNIPPET: example-action -->
* </pre>
- *
*/
public class FileUploadInterceptor extends AbstractInterceptor {
@@ -264,15 +255,24 @@
if (isNonEmpty(fileName)) {
// Get a File object for the uploaded File
File[] files = multiWrapper.getFiles(inputName);
- if (files != null) {
+ if (files != null && files.length > 0) {
+ ArrayList<File> acceptedFiles = new
ArrayList<File>(files.length);
+ ArrayList<String> acceptedContentTypes = new
ArrayList<String>(files.length);
+ ArrayList<String> acceptedFileNames = new
ArrayList<String>(files.length);
+ String contentTypeName = inputName + "ContentType";
+ String fileNameName = inputName + "FileName";
for (int index = 0; index < files.length; index++) {
-
if (acceptFile(files[index], contentType[index],
inputName, validation, ac.getLocale())) {
- parameters.put(inputName, files);
- parameters.put(inputName + "ContentType",
contentType);
- parameters.put(inputName + "FileName",
fileName);
+ acceptedFiles.add(files[index]);
+ acceptedContentTypes.add(contentType[index]);
+ acceptedFileNames.add(fileName[index]);
}
}
+ if (acceptedFiles.size() != 0) {
+ parameters.put(inputName,
acceptedFiles.toArray(new File[acceptedFiles.size()]));
+ parameters.put(contentTypeName,
acceptedContentTypes.toArray(new String[acceptedContentTypes.size()]));
+ parameters.put(fileNameName,
acceptedFileNames.toArray(new String[acceptedFileNames.size()]));
+ }
}
} else {
LOG.error(getTextMessage("struts.messages.invalid.file",
new Object[]{inputName}, ActionContext.getContext().getLocale()));
@@ -292,8 +292,8 @@
File[] file = multiWrapper.getFiles(inputValue);
for (int index = 0; index < file.length; index++) {
File currentFile = file[index];
- if(LOG.isInfoEnabled()) {
-
LOG.info(getTextMessage("struts.messages.removing.file", new
Object[]{inputValue, currentFile}, ActionContext.getContext().getLocale()));
+ if (LOG.isInfoEnabled()) {
+ LOG.info(getTextMessage("struts.messages.removing.file",
new Object[]{inputValue, currentFile}, ActionContext.getContext().getLocale()));
}
if ((currentFile != null) && currentFile.isFile()) {
currentFile.delete();
@@ -333,7 +333,7 @@
}
LOG.error(errMsg);
- } else if ((! allowedTypesSet.isEmpty()) &&
(!containsItem(allowedTypesSet, contentType))) {
+ } else if ((!allowedTypesSet.isEmpty()) &&
(!containsItem(allowedTypesSet, contentType))) {
String errMsg =
getTextMessage("struts.messages.error.content.type.not.allowed", new
Object[]{inputName, file.getName(), contentType}, locale);
if (validation != null) {
validation.addFieldError(inputName, errMsg);
Modified:
struts/struts2/trunk/core/src/test/java/org/apache/struts2/interceptor/FileUploadInterceptorTest.java
URL:
http://svn.apache.org/viewvc/struts/struts2/trunk/core/src/test/java/org/apache/struts2/interceptor/FileUploadInterceptorTest.java?rev=667656&r1=667655&r2=667656&view=diff
==============================================================================
---
struts/struts2/trunk/core/src/test/java/org/apache/struts2/interceptor/FileUploadInterceptorTest.java
(original)
+++
struts/struts2/trunk/core/src/test/java/org/apache/struts2/interceptor/FileUploadInterceptorTest.java
Fri Jun 13 14:14:44 2008
@@ -37,6 +37,7 @@
import org.apache.struts2.dispatcher.multipart.JakartaMultiPartRequest;
import org.apache.struts2.dispatcher.multipart.MultiPartRequest;
import org.apache.struts2.dispatcher.multipart.MultiPartRequestWrapper;
+import org.apache.commons.fileupload.servlet.ServletFileUpload;
import org.springframework.mock.web.MockHttpServletRequest;
import com.opensymphony.xwork2.util.ClassLoaderUtil;
@@ -48,7 +49,6 @@
/**
* Test case for FileUploadInterceptor.
- *
*/
public class FileUploadInterceptorTest extends StrutsTestCase {
@@ -191,12 +191,12 @@
// inspired by the unit tests for jakarta commons fileupload
String content = ("-----1234\r\n" +
- "Content-Disposition: form-data; name=\"file\";
filename=\"deleteme.txt\"\r\n" +
- "Content-Type: text/html\r\n" +
- "\r\n" +
- "Unit test of FileUploadInterceptor" +
- "\r\n" +
- "-----1234--\r\n");
+ "Content-Disposition: form-data; name=\"file\";
filename=\"deleteme.txt\"\r\n" +
+ "Content-Type: text/html\r\n" +
+ "\r\n" +
+ "Unit test of FileUploadInterceptor" +
+ "\r\n" +
+ "-----1234--\r\n");
req.setContent(content.getBytes("US-ASCII"));
MyFileupAction action = new MyFileupAction();
@@ -205,15 +205,16 @@
mai.setAction(action);
mai.setResultCode("success");
mai.setInvocationContext(ActionContext.getContext());
- Map param = new HashMap();
+ Map<String, Object[]> param = new HashMap<String, Object[]>();
ActionContext.getContext().setParameters(param);
ActionContext.getContext().put(ServletActionContext.HTTP_REQUEST,
createMultipartRequest((HttpServletRequest) req, 2000));
interceptor.intercept(mai);
- assertTrue(! action.hasErrors());
+ assertTrue(!action.hasErrors());
assertTrue(param.size() == 3);
+ System.err.println("param.get(\"file\"): " +
param.get("file").getClass());
File[] files = (File[]) param.get("file");
String[] fileContentTypes = (String[]) param.get("fileContentType");
String[] fileRealFilenames = (String[]) param.get("fileFileName");
@@ -228,10 +229,89 @@
assertNotNull("deleteme.txt", fileRealFilenames[0]);
}
+ /**
+ * tests whether with multiple files sent with the same name, the ones
with forbiddenTypes (see
+ * FileUploadInterceptor.setAllowedTypes(...) ) are sorted out.
+ *
+ * @throws Exception
+ */
+ public void testMultipleAccept() throws Exception {
+ final String htmlContent = "<html><head></head><body>html
content</body></html>";
+ final String plainContent = "plain content";
+ final String bondary = "simple boundary";
+ final String endline = "\r\n";
+
+ MockHttpServletRequest req = new MockHttpServletRequest();
+ req.setCharacterEncoding("text/html");
+ req.setMethod("POST");
+ req.setContentType("multipart/form-data; boundary=" + bondary);
+ req.addHeader("Content-type", "multipart/form-data");
+ StringBuilder content = new StringBuilder(128);
+ content.append(encodeTextFile(bondary, endline, "file", "test.html",
"text/plain", plainContent));
+ content.append(encodeTextFile(bondary, endline, "file", "test1.html",
"text/html", htmlContent));
+ content.append(encodeTextFile(bondary, endline, "file", "test2.html",
"text/html", htmlContent));
+ content.append(endline);
+ content.append(endline);
+ content.append(endline);
+ content.append("--");
+ content.append(bondary);
+ content.append("--");
+ content.append(endline);
+ req.setContent(content.toString().getBytes());
+
+ assertTrue(ServletFileUpload.isMultipartContent(req));
+
+ MyFileupAction action = new MyFileupAction();
+ MockActionInvocation mai = new MockActionInvocation();
+ mai.setAction(action);
+ mai.setResultCode("success");
+ mai.setInvocationContext(ActionContext.getContext());
+ Map<String, Object[]> param = new HashMap<String, Object[]>();
+ ActionContext.getContext().setParameters(param);
+ ActionContext.getContext().put(ServletActionContext.HTTP_REQUEST,
createMultipartRequest(req, 2000));
+
+ interceptor.setAllowedTypes("text/html");
+ interceptor.intercept(mai);
+
+ assertEquals(3, param.size());
+ File[] files = (File[]) param.get("file");
+ String[] fileContentTypes = (String[]) param.get("fileContentType");
+ String[] fileRealFilenames = (String[]) param.get("fileFileName");
+
+ assertNotNull(files);
+ assertNotNull(fileContentTypes);
+ assertNotNull(fileRealFilenames);
+ assertEquals("files accepted ", 2, files.length);
+ assertEquals(2, fileContentTypes.length);
+ assertEquals(2, fileRealFilenames.length);
+ assertEquals("text/html", fileContentTypes[0]);
+ assertNotNull("test1.html", fileRealFilenames[0]);
+ }
+
+ private String encodeTextFile(String bondary, String endline, String name,
String filename, String contentType, String content) {
+ final StringBuilder sb = new StringBuilder(64);
+ sb.append(endline);
+ sb.append("--");
+ sb.append(bondary);
+ sb.append(endline);
+ sb.append("Content-Disposition: form-data; name=\"");
+ sb.append(name);
+ sb.append("\"; filename=\"");
+ sb.append(filename);
+ sb.append(endline);
+ sb.append("Content-Type: ");
+ sb.append(contentType);
+ sb.append(endline);
+ sb.append(endline);
+ sb.append(content);
+
+ return sb.toString();
+ }
+
private MultiPartRequestWrapper createMultipartRequest(HttpServletRequest
req, int maxsize) throws IOException {
- JakartaMultiPartRequest jak = new JakartaMultiPartRequest();
- jak.setMaxSize(String.valueOf(maxsize));
- return new MultiPartRequestWrapper(jak, req, tempDir.getAbsolutePath());
+ JakartaMultiPartRequest jak = new JakartaMultiPartRequest();
+ jak.setMaxSize(String.valueOf(maxsize));
+ return new MultiPartRequestWrapper(jak, req,
tempDir.getAbsolutePath());
}
protected void setUp() throws Exception {
