Author: jochen Date: Tue Jun 26 17:59:16 2007 New Revision: 551000 URL: http://svn.apache.org/viewvc?view=rev&rev=551000 Log: Short files could cause an unexpected end of the item stream. PR: FILEUPLOAD-135 Submitted-By: Alexander Sova <[EMAIL PROTECTED]>
Modified: jakarta/commons/proper/fileupload/trunk/pom.xml jakarta/commons/proper/fileupload/trunk/src/changes/changes.xml jakarta/commons/proper/fileupload/trunk/src/java/org/apache/commons/fileupload/FileUploadException.java jakarta/commons/proper/fileupload/trunk/src/java/org/apache/commons/fileupload/MultipartStream.java jakarta/commons/proper/fileupload/trunk/src/test/org/apache/commons/fileupload/MockHttpServletRequest.java jakarta/commons/proper/fileupload/trunk/src/test/org/apache/commons/fileupload/StreamingTest.java Modified: jakarta/commons/proper/fileupload/trunk/pom.xml URL: http://svn.apache.org/viewvc/jakarta/commons/proper/fileupload/trunk/pom.xml?view=diff&rev=551000&r1=550999&r2=551000 ============================================================================== --- jakarta/commons/proper/fileupload/trunk/pom.xml (original) +++ jakarta/commons/proper/fileupload/trunk/pom.xml Tue Jun 26 17:59:16 2007 @@ -105,6 +105,10 @@ <email>[EMAIL PROTECTED]</email> </contributor> <contributor> + <name>Alexander Sova</name> + <email>[EMAIL PROTECTED]</email> + </contributor> + <contributor> <name>Thomas Vandahl</name> <email>[EMAIL PROTECTED]</email> </contributor> Modified: jakarta/commons/proper/fileupload/trunk/src/changes/changes.xml URL: http://svn.apache.org/viewvc/jakarta/commons/proper/fileupload/trunk/src/changes/changes.xml?view=diff&rev=551000&r1=550999&r2=551000 ============================================================================== --- jakarta/commons/proper/fileupload/trunk/src/changes/changes.xml (original) +++ jakarta/commons/proper/fileupload/trunk/src/changes/changes.xml Tue Jun 26 17:59:16 2007 @@ -64,56 +64,50 @@ due-to="Thomas Vandahl" due-to-email="[EMAIL PROTECTED]"> DiskFileItem.toString() could throw an NPE. </action> + <action dev="jochen" type="fix" issue="FILEUPLOAD-135" + due-to="Alexander Sova" due-to-email="[EMAIL PROTECTED]"> + Short files could cause an unexpected end of the item stream. + </action> </release> <release version="1.2" date="2007-02-13"> - <action dev="jochen" type="fix" due-to="Aaron Freeman" due-to-email="[EMAIL PROTECTED]"> Made Streams.asString static. </action> - <action dev="jochen" type="update" issue="FILEUPLOAD-109"> Eliminated duplicate code. </action> - <action dev="jochen" type="add" issue="FILEUPLOAD-112"> Added a streaming API. </action> - <action dev="jochen" type="fix" issue="FILEUPLOAD-93"> Eliminated the necessity of a content-length header. </action> - <action dev="jochen" type="fix" issue="FILEUPLOAD-108" due-to="Amichai Rothman" due-to-email="[EMAIL PROTECTED]"> Eliminated the limitation of a maximum size for a single header line. (The total size of all headers is already limited, so there's no need for another limit.) </action> - <action dev="jochen" type="add" issue="FILEUPLOAD-87"> Added the ProgressListener, which allows to implement a progress bar. </action> - <action dev="jochen" type="add" issue="FILEUPLOAD-111" due-to="Amichai Rothman" due-to-email="[EMAIL PROTECTED]"> Added support for header continuation lines. </action> - <action dev="jochen" type="add" issue="FILEUPLOAD-88" due-to="Andrey Aristarkhov" due-to-email="[EMAIL PROTECTED]"> It is now possible to limit the actual file size and not the request size. </action> - <action dev="jochen" type="add" issue="FILEUPLOAD-120" due-to="Henry Yandell" due-to-email="[EMAIL PROTECTED]"> Added the FileCleanerCleanup as an example for how to close down the FileCleaner's reaper thread nicely. </action> - <action dev="jochen" type="fix" issue="FILEUPLOAD-123"> A descriptive NPE is now thrown, if the FileItemFactory has not been set. Modified: jakarta/commons/proper/fileupload/trunk/src/java/org/apache/commons/fileupload/FileUploadException.java URL: http://svn.apache.org/viewvc/jakarta/commons/proper/fileupload/trunk/src/java/org/apache/commons/fileupload/FileUploadException.java?view=diff&rev=551000&r1=550999&r2=551000 ============================================================================== --- jakarta/commons/proper/fileupload/trunk/src/java/org/apache/commons/fileupload/FileUploadException.java (original) +++ jakarta/commons/proper/fileupload/trunk/src/java/org/apache/commons/fileupload/FileUploadException.java Tue Jun 26 17:59:16 2007 @@ -92,4 +92,8 @@ cause.printStackTrace(writer); } } + + public Throwable getCause() { + return cause; + } } Modified: jakarta/commons/proper/fileupload/trunk/src/java/org/apache/commons/fileupload/MultipartStream.java URL: http://svn.apache.org/viewvc/jakarta/commons/proper/fileupload/trunk/src/java/org/apache/commons/fileupload/MultipartStream.java?view=diff&rev=551000&r1=550999&r2=551000 ============================================================================== --- jakarta/commons/proper/fileupload/trunk/src/java/org/apache/commons/fileupload/MultipartStream.java (original) +++ jakarta/commons/proper/fileupload/trunk/src/java/org/apache/commons/fileupload/MultipartStream.java Tue Jun 26 17:59:16 2007 @@ -21,6 +21,7 @@ import java.io.InputStream; import java.io.OutputStream; import java.io.UnsupportedEncodingException; +import java.util.Arrays; import org.apache.commons.fileupload.util.Closeable; import org.apache.commons.fileupload.util.Streams; @@ -202,13 +203,6 @@ CR, LF, DASH, DASH}; - /** - * The number of bytes, over and above the boundary size, to use for the - * keep region. - */ - private static final int KEEP_REGION_PAD = 3; - - // ----------------------------------------------------------- Data members @@ -341,7 +335,7 @@ // body-data tokens. this.boundary = new byte[boundary.length + BOUNDARY_PREFIX.length]; this.boundaryLength = boundary.length + BOUNDARY_PREFIX.length; - this.keepRegion = boundary.length + KEEP_REGION_PAD; + this.keepRegion = this.boundary.length; System.arraycopy(BOUNDARY_PREFIX, 0, this.boundary, 0, BOUNDARY_PREFIX.length); System.arraycopy(boundary, 0, this.boundary, BOUNDARY_PREFIX.length, @@ -421,8 +415,7 @@ * * @throws IOException if there is no more data available. */ - public byte readByte() - throws IOException { + public byte readByte() throws IOException { // Buffer depleted ? if (head == tail) { head = 0; @@ -432,7 +425,9 @@ // No more data available. throw new IOException("No more data is available"); } - notifier.noteBytesRead(tail); + if(notifier != null) { + notifier.noteBytesRead(tail); + } } return buffer[head++]; } @@ -855,7 +850,7 @@ throw new FileItemStream.ItemSkippedException(); } if (available() == 0) { - if (makeAvailable() == 0) { + if (makeAvailable() == 0) { return -1; } } @@ -957,18 +952,28 @@ // Refill buffer with new data. head = 0; - int bytesRead = input.read(buffer, pad, bufSize - pad); - if (bytesRead == -1) { - // The last pad amount is left in the buffer. - // Boundary can't be in there so signal an error - // condition. - throw new MalformedStreamException( - "Stream ended unexpectedly"); + tail = pad; + + for(;;) { + int bytesRead = input.read(buffer, tail, bufSize - tail); + if (bytesRead == -1) { + // The last pad amount is left in the buffer. + // Boundary can't be in there so signal an error + // condition. + throw new MalformedStreamException("Stream ended unexpectedly"); + } + if(notifier != null) { + notifier.noteBytesRead(bytesRead); + } + tail += bytesRead; + + findSeparator(); + int av = available(); + + if (av > 0 || pos != -1) { + return av; + } } - notifier.noteBytesRead(bytesRead); - tail = pad + bytesRead; - findSeparator(); - return available(); } /** Modified: jakarta/commons/proper/fileupload/trunk/src/test/org/apache/commons/fileupload/MockHttpServletRequest.java URL: http://svn.apache.org/viewvc/jakarta/commons/proper/fileupload/trunk/src/test/org/apache/commons/fileupload/MockHttpServletRequest.java?view=diff&rev=551000&r1=550999&r2=551000 ============================================================================== --- jakarta/commons/proper/fileupload/trunk/src/test/org/apache/commons/fileupload/MockHttpServletRequest.java (original) +++ jakarta/commons/proper/fileupload/trunk/src/test/org/apache/commons/fileupload/MockHttpServletRequest.java Tue Jun 26 17:59:16 2007 @@ -544,5 +544,10 @@ { return in.read(); } + + public int read(byte b[], int off, int len) throws IOException + { + return in.read(b, off, len); + } } } Modified: jakarta/commons/proper/fileupload/trunk/src/test/org/apache/commons/fileupload/StreamingTest.java URL: http://svn.apache.org/viewvc/jakarta/commons/proper/fileupload/trunk/src/test/org/apache/commons/fileupload/StreamingTest.java?view=diff&rev=551000&r1=550999&r2=551000 ============================================================================== --- jakarta/commons/proper/fileupload/trunk/src/test/org/apache/commons/fileupload/StreamingTest.java (original) +++ jakarta/commons/proper/fileupload/trunk/src/test/org/apache/commons/fileupload/StreamingTest.java Tue Jun 26 17:59:16 2007 @@ -21,11 +21,11 @@ import java.io.FilterInputStream; import java.io.IOException; import java.io.InputStream; +import java.io.OutputStreamWriter; 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.IOFileUploadException; import org.apache.commons.fileupload.disk.DiskFileItemFactory; import org.apache.commons.fileupload.servlet.ServletFileUpload; @@ -87,7 +87,7 @@ * Tests, whether an IOException is properly delegated. */ public void testIOException() - throws IOException, FileUploadException { + throws IOException { byte[] request = newRequest(); InputStream stream = new FilterInputStream(new ByteArrayInputStream(request)){ private int num; @@ -111,10 +111,43 @@ }; try { parseUpload(stream, request.length); - } catch (IOFileUploadException e) { + fail("Expected IOException"); + } catch (FileUploadException e) { assertTrue(e.getCause() instanceof IOException); assertEquals("123", e.getCause().getMessage()); - } + } + } + + /** + * Test for FILEUPLOAD-135 + */ + public void testFILEUPLOAD135() + throws IOException, FileUploadException + { + byte[] request = newShortRequest(); + final ByteArrayInputStream bais = new ByteArrayInputStream(request); + List fileItems = parseUpload(new InputStream() { + public int read() + throws IOException + { + return bais.read(); + } + public int read(byte b[], int off, int len) throws IOException + { + return bais.read(b, off, Math.min(len, 3)); + } + + }, request.length); + Iterator fileIter = fileItems.iterator(); + assertTrue(fileIter.hasNext()); + FileItem item = (FileItem) fileIter.next(); + assertEquals("field", item.getFieldName()); + byte[] bytes = item.get(); + assertEquals(3, bytes.length); + assertEquals((byte)'1', bytes[0]); + assertEquals((byte)'2', bytes[1]); + assertEquals((byte)'3', bytes[2]); + assertTrue(!fileIter.hasNext()); } private List parseUpload(byte[] bytes) throws FileUploadException { @@ -134,24 +167,46 @@ return fileItems; } + private String getHeader(String pField) { + return "-----1234\r\n" + + "Content-Disposition: form-data; name=\"" + pField + "\"\r\n" + + "\r\n"; + + } + + private String getFooter() { + return "-----1234--\r\n"; + } + + private byte[] newShortRequest() throws IOException { + final ByteArrayOutputStream baos = new ByteArrayOutputStream(); + final OutputStreamWriter osw = new OutputStreamWriter(baos, "US-ASCII"); + osw.write(getHeader("field")); + osw.write("123"); + osw.write("\r\n"); + osw.write(getFooter()); + osw.close(); + return baos.toByteArray(); + } + private byte[] newRequest() throws IOException { - ByteArrayOutputStream baos = new ByteArrayOutputStream(); + final ByteArrayOutputStream baos = new ByteArrayOutputStream(); + final OutputStreamWriter osw = new OutputStreamWriter(baos, "US-ASCII"); int add = 16; int num = 0; for (int i = 0; i < 16384; i += add) { if (++add == 32) { add = 16; } - String header = "-----1234\r\n" - + "Content-Disposition: form-data; name=\"field" + (num++) + "\"\r\n" - + "\r\n"; - baos.write(header.getBytes("US-ASCII")); + osw.write(getHeader("field" + (num++))); + osw.flush(); for (int j = 0; j < i; j++) { baos.write((byte) j); } - baos.write("\r\n".getBytes("US-ASCII")); + osw.write("\r\n"); } - baos.write("-----1234--\r\n".getBytes("US-ASCII")); + osw.write(getFooter()); + osw.close(); return baos.toByteArray(); } } --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]