Author: tn
Date: Tue Mar 12 22:01:02 2013
New Revision: 1455729
URL: http://svn.apache.org/r1455729
Log:
[FILEUPLOAD-212] Add more unit tests for sizeMax setting; propagate correctly
any SizeException to the caller.
Modified:
commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/FileUploadBase.java
commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/MultipartStream.java
commons/proper/fileupload/trunk/src/test/java/org/apache/commons/fileupload/MockHttpServletRequest.java
commons/proper/fileupload/trunk/src/test/java/org/apache/commons/fileupload/SizesTest.java
Modified:
commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/FileUploadBase.java
URL:
http://svn.apache.org/viewvc/commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/FileUploadBase.java?rev=1455729&r1=1455728&r2=1455729&view=diff
==============================================================================
---
commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/FileUploadBase.java
(original)
+++
commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/FileUploadBase.java
Tue Mar 12 22:01:02 2013
@@ -305,7 +305,12 @@ public abstract class FileUploadBase {
*/
public FileItemIterator getItemIterator(RequestContext ctx)
throws FileUploadException, IOException {
- return new FileItemIteratorImpl(ctx);
+ try {
+ return new FileItemIteratorImpl(ctx);
+ } catch (FileUploadIOException e) {
+ // unwrap encapsulated SizeException
+ throw (FileUploadException) e.getCause();
+ }
}
/**
@@ -328,20 +333,17 @@ public abstract class FileUploadBase {
FileItemIterator iter = getItemIterator(ctx);
FileItemFactory fac = getFileItemFactory();
if (fac == null) {
- throw new NullPointerException(
- "No FileItemFactory has been set.");
+ throw new NullPointerException("No FileItemFactory has been
set.");
}
while (iter.hasNext()) {
final FileItemStream item = iter.next();
// Don't use getName() here to prevent an
InvalidFileNameException.
final String fileName =
((FileItemIteratorImpl.FileItemStreamImpl) item).name;
- FileItem fileItem = fac.createItem(item.getFieldName(),
- item.getContentType(), item.isFormField(),
- fileName);
+ FileItem fileItem = fac.createItem(item.getFieldName(),
item.getContentType(),
+ item.isFormField(),
fileName);
items.add(fileItem);
try {
- Streams.copy(item.openStream(), fileItem.getOutputStream(),
- true);
+ Streams.copy(item.openStream(),
fileItem.getOutputStream(), true);
} catch (FileUploadIOException e) {
throw (FileUploadException) e.getCause();
} catch (IOException e) {
@@ -1091,7 +1093,12 @@ public abstract class FileUploadBase {
if (itemValid) {
return true;
}
- return findNextItem();
+ try {
+ return findNextItem();
+ } catch (FileUploadIOException e) {
+ // unwrap encapsulated SizeException
+ throw (FileUploadException) e.getCause();
+ }
}
/**
Modified:
commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/MultipartStream.java
URL:
http://svn.apache.org/viewvc/commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/MultipartStream.java?rev=1455729&r1=1455728&r2=1455729&view=diff
==============================================================================
---
commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/MultipartStream.java
(original)
+++
commons/proper/fileupload/trunk/src/main/java/org/apache/commons/fileupload/MultipartStream.java
Tue Mar 12 22:01:02 2013
@@ -24,6 +24,7 @@ import java.io.InputStream;
import java.io.OutputStream;
import java.io.UnsupportedEncodingException;
+import org.apache.commons.fileupload.FileUploadBase.FileUploadIOException;
import org.apache.commons.fileupload.util.Closeable;
import org.apache.commons.fileupload.util.Streams;
@@ -438,7 +439,7 @@ public class MultipartStream {
* fails to follow required syntax.
*/
public boolean readBoundary()
- throws MalformedStreamException {
+ throws FileUploadIOException, MalformedStreamException {
byte[] marker = new byte[2];
boolean nextChunk = false;
@@ -464,6 +465,9 @@ public class MultipartStream {
throw new MalformedStreamException(
"Unexpected characters follow a boundary");
}
+ } catch (FileUploadIOException e) {
+ // wraps a SizeException, re-throw as it will be unwrapped later
+ throw e;
} catch (IOException e) {
throw new MalformedStreamException("Stream ended unexpectedly");
}
@@ -514,7 +518,7 @@ public class MultipartStream {
*
* @throws MalformedStreamException if the stream ends unexpecetedly.
*/
- public String readHeaders() throws MalformedStreamException {
+ public String readHeaders() throws FileUploadIOException,
MalformedStreamException {
int i = 0;
byte b;
// to support multi-byte characters
@@ -523,6 +527,9 @@ public class MultipartStream {
while (i < HEADER_SEPARATOR.length) {
try {
b = readByte();
+ } catch (FileUploadIOException e) {
+ // wraps a SizeException, re-throw as it will be unwrapped
later
+ throw e;
} catch (IOException e) {
throw new MalformedStreamException("Stream ended
unexpectedly");
}
Modified:
commons/proper/fileupload/trunk/src/test/java/org/apache/commons/fileupload/MockHttpServletRequest.java
URL:
http://svn.apache.org/viewvc/commons/proper/fileupload/trunk/src/test/java/org/apache/commons/fileupload/MockHttpServletRequest.java?rev=1455729&r1=1455728&r2=1455729&view=diff
==============================================================================
---
commons/proper/fileupload/trunk/src/test/java/org/apache/commons/fileupload/MockHttpServletRequest.java
(original)
+++
commons/proper/fileupload/trunk/src/test/java/org/apache/commons/fileupload/MockHttpServletRequest.java
Tue Mar 12 22:01:02 2013
@@ -39,9 +39,11 @@ class MockHttpServletRequest implements
private final InputStream m_requestData;
- private final long length;
+ private long length;
private String m_strContentType;
+
+ private int readLimit = -1;
private final Map<String, String> m_headers = new
java.util.HashMap<String, String>();
@@ -295,6 +297,13 @@ class MockHttpServletRequest implements
}
/**
+ * For testing attack scenarios in SizesTest.
+ */
+ public void setContentLength(long length) {
+ this.length = length;
+ }
+
+ /**
* @see javax.servlet.ServletRequest#getContentType()
*/
public String getContentType() {
@@ -305,11 +314,20 @@ class MockHttpServletRequest implements
* @see javax.servlet.ServletRequest#getInputStream()
*/
public ServletInputStream getInputStream() throws IOException {
- ServletInputStream sis = new MyServletInputStream(m_requestData);
+ ServletInputStream sis = new MyServletInputStream(m_requestData,
readLimit);
return sis;
}
/**
+ * Sets the read limit. This can be used to limit the number of bytes to
read ahead.
+ *
+ * @param readLimit the read limit to use
+ */
+ public void setReadLimit(int readLimit) {
+ this.readLimit = readLimit;
+ }
+
+ /**
* @see javax.servlet.ServletRequest#getParameter(String)
*/
public String getParameter(String arg0) {
@@ -463,23 +481,19 @@ class MockHttpServletRequest implements
return null;
}
- /**
- *
- *
- *
- *
- */
private static class MyServletInputStream
extends javax.servlet.ServletInputStream {
private final InputStream in;
+ private final int readLimit;
/**
* Creates a new instance, which returns the given
* streams data.
*/
- public MyServletInputStream(InputStream pStream) {
+ public MyServletInputStream(InputStream pStream, int readLimit) {
in = pStream;
+ this.readLimit = readLimit;
}
@Override
@@ -489,7 +503,11 @@ class MockHttpServletRequest implements
@Override
public int read(byte b[], int off, int len) throws IOException {
- return in.read(b, off, len);
+ if (readLimit > 0) {
+ return in.read(b, off, Math.min(readLimit, len));
+ } else {
+ return in.read(b, off, len);
+ }
}
}
Modified:
commons/proper/fileupload/trunk/src/test/java/org/apache/commons/fileupload/SizesTest.java
URL:
http://svn.apache.org/viewvc/commons/proper/fileupload/trunk/src/test/java/org/apache/commons/fileupload/SizesTest.java?rev=1455729&r1=1455728&r2=1455729&view=diff
==============================================================================
---
commons/proper/fileupload/trunk/src/test/java/org/apache/commons/fileupload/SizesTest.java
(original)
+++
commons/proper/fileupload/trunk/src/test/java/org/apache/commons/fileupload/SizesTest.java
Tue Mar 12 22:01:02 2013
@@ -17,18 +17,23 @@
package org.apache.commons.fileupload;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import java.io.ByteArrayOutputStream;
import java.io.IOException;
+import java.io.InputStream;
import java.util.Iterator;
import java.util.List;
import javax.servlet.http.HttpServletRequest;
+import org.apache.commons.fileupload.FileUploadBase.FileUploadIOException;
+import org.apache.commons.fileupload.FileUploadBase.SizeException;
import org.apache.commons.fileupload.disk.DiskFileItemFactory;
import org.apache.commons.fileupload.servlet.ServletFileUpload;
+import org.apache.commons.fileupload.util.Streams;
import org.junit.Test;
/**
@@ -122,4 +127,161 @@ public class SizesTest extends FileUploa
}
}
+ /** Checks, whether a faked Content-Length header is detected.
+ */
+ @Test
+ public void testFileSizeLimitWithFakedContentLength()
+ throws IOException, FileUploadException {
+ final String request =
+ "-----1234\r\n" +
+ "Content-Disposition: form-data; name=\"file\";
filename=\"foo.tab\"\r\n" +
+ "Content-Type: text/whatever\r\n" +
+ "Content-Length: 10\r\n" +
+ "\r\n" +
+ "This is the content of the file\n" +
+ "\r\n" +
+ "-----1234--\r\n";
+
+ ServletFileUpload upload = new ServletFileUpload(new
DiskFileItemFactory());
+ upload.setFileSizeMax(-1);
+ HttpServletRequest req = new
MockHttpServletRequest(request.getBytes("US-ASCII"), CONTENT_TYPE);
+ List<FileItem> fileItems = upload.parseRequest(req);
+ assertEquals(1, fileItems.size());
+ FileItem item = fileItems.get(0);
+ assertEquals("This is the content of the file\n", new
String(item.get()));
+
+ upload = new ServletFileUpload(new DiskFileItemFactory());
+ upload.setFileSizeMax(40);
+ req = new MockHttpServletRequest(request.getBytes("US-ASCII"),
CONTENT_TYPE);
+ fileItems = upload.parseRequest(req);
+ assertEquals(1, fileItems.size());
+ item = fileItems.get(0);
+ assertEquals("This is the content of the file\n", new
String(item.get()));
+
+ // provided Content-Length is larger than the FileSizeMax -> handled
by ctor
+ upload = new ServletFileUpload(new DiskFileItemFactory());
+ upload.setFileSizeMax(5);
+ req = new MockHttpServletRequest(request.getBytes("US-ASCII"),
CONTENT_TYPE);
+ try {
+ upload.parseRequest(req);
+ fail("Expected exception.");
+ } catch (FileUploadBase.FileSizeLimitExceededException e) {
+ assertEquals(5, e.getPermittedSize());
+ }
+
+ // provided Content-Length is wrong, actual content is larger ->
handled by LimitedInputStream
+ upload = new ServletFileUpload(new DiskFileItemFactory());
+ upload.setFileSizeMax(15);
+ req = new MockHttpServletRequest(request.getBytes("US-ASCII"),
CONTENT_TYPE);
+ try {
+ upload.parseRequest(req);
+ fail("Expected exception.");
+ } catch (FileUploadBase.FileSizeLimitExceededException e) {
+ assertEquals(15, e.getPermittedSize());
+ }
+ }
+
+ /** Checks, whether the maxSize works.
+ */
+ @Test
+ public void testMaxSizeLimit()
+ throws IOException, FileUploadException {
+ final String request =
+ "-----1234\r\n" +
+ "Content-Disposition: form-data; name=\"file1\";
filename=\"foo1.tab\"\r\n" +
+ "Content-Type: text/whatever\r\n" +
+ "Content-Length: 10\r\n" +
+ "\r\n" +
+ "This is the content of the file\n" +
+ "\r\n" +
+ "-----1234\r\n" +
+ "Content-Disposition: form-data; name=\"file2\";
filename=\"foo2.tab\"\r\n" +
+ "Content-Type: text/whatever\r\n" +
+ "\r\n" +
+ "This is the content of the file\n" +
+ "\r\n" +
+ "-----1234--\r\n";
+
+ ServletFileUpload upload = new ServletFileUpload(new
DiskFileItemFactory());
+ upload.setFileSizeMax(-1);
+ upload.setSizeMax(200);
+
+ MockHttpServletRequest req = new
MockHttpServletRequest(request.getBytes("US-ASCII"), CONTENT_TYPE);
+ try {
+ upload.parseRequest(req);
+ fail("Expected exception.");
+ } catch (FileUploadBase.SizeLimitExceededException e) {
+ assertEquals(200, e.getPermittedSize());
+ }
+
+ }
+
+ @Test
+ public void testMaxSizeLimitUnknownContentLength()
+ throws IOException, FileUploadException {
+ final String request =
+ "-----1234\r\n" +
+ "Content-Disposition: form-data; name=\"file1\";
filename=\"foo1.tab\"\r\n" +
+ "Content-Type: text/whatever\r\n" +
+ "Content-Length: 10\r\n" +
+ "\r\n" +
+ "This is the content of the file\n" +
+ "\r\n" +
+ "-----1234\r\n" +
+ "Content-Disposition: form-data; name=\"file2\";
filename=\"foo2.tab\"\r\n" +
+ "Content-Type: text/whatever\r\n" +
+ "\r\n" +
+ "This is the content of the file\n" +
+ "\r\n" +
+ "-----1234--\r\n";
+
+ ServletFileUpload upload = new ServletFileUpload(new
DiskFileItemFactory());
+ upload.setFileSizeMax(-1);
+ upload.setSizeMax(300);
+
+ // the first item should be within the max size limit
+ // set the read limit to 10 to simulate a "real" stream
+ // otherwise the buffer would be immediately filled
+
+ MockHttpServletRequest req = new
MockHttpServletRequest(request.getBytes("US-ASCII"), CONTENT_TYPE);
+ req.setContentLength(-1);
+ req.setReadLimit(10);
+
+ FileItemIterator it = upload.getItemIterator(req);
+ assertTrue(it.hasNext());
+
+ FileItemStream item = it.next();
+ assertFalse(item.isFormField());
+ assertEquals("file1", item.getFieldName());
+ assertEquals("foo1.tab", item.getName());
+
+ try {
+ InputStream stream = item.openStream();
+ ByteArrayOutputStream baos = new ByteArrayOutputStream();
+ Streams.copy(stream, baos, true);
+ } catch (FileUploadIOException e) {
+ fail(e.getMessage());
+ }
+
+ // the second item is over the size max, thus we expect an error
+ try {
+ // the header is still within size max -> this shall still succeed
+ assertTrue(it.hasNext());
+ } catch (SizeException e) {
+ fail();
+ }
+
+ item = it.next();
+
+ try {
+ InputStream stream = item.openStream();
+ ByteArrayOutputStream baos = new ByteArrayOutputStream();
+ Streams.copy(stream, baos, true);
+ fail();
+ } catch (FileUploadIOException e) {
+ // expected
+ }
+
+ }
+
}
