mschachter 01/02/14 13:43:10 Modified: src/share/org/apache/struts/upload DiskFile.java DiskMultipartRequestHandler.java FormFile.java MultipartElement.java MultipartIterator.java web/upload/WEB-INF/classes/org/apache/struts/example/upload UploadAction.java Added: src/share/org/apache/struts/upload MultipartValueStream.java Log: Made changes to classes in upload package to address problems with storing files in memory. This commit will break any code that implements the org.apache.struts.upload.FormFile interface, because a new method was added. Changed MultipartIterator to use the new MultipartValueStream class from Jimmy Larsson for going through multipart elements. This will fix a bug with null content types for files. Also updated the upload example to use the new recommended method for retrieving file content. Submitted By: Jimmy Larsson Revision Changes Path 1.2 +14 -5 jakarta-struts/src/share/org/apache/struts/upload/DiskFile.java Index: DiskFile.java =================================================================== RCS file: /home/cvs/jakarta-struts/src/share/org/apache/struts/upload/DiskFile.java,v retrieving revision 1.1 retrieving revision 1.2 diff -u -r1.1 -r1.2 --- DiskFile.java 2000/11/09 20:08:56 1.1 +++ DiskFile.java 2001/02/14 21:43:05 1.2 @@ -1,6 +1,7 @@ package org.apache.struts.upload; import java.io.File; +import java.io.InputStream; import java.io.IOException; import java.io.FileInputStream; import java.io.ByteArrayOutputStream; @@ -26,10 +27,8 @@ /** * The name of the file */ - protected String fileName; + protected String fileName; - - public DiskFile(String filePath) { this.filePath = filePath; } @@ -39,6 +38,9 @@ * array form. Tries to read the entire file (using a byte array * the size of getFileSize()) at once, in one call to FileInputStream.read(byte[]). * For buffered reading, see {@link #getFileData(int) getFileData(int)}. + * Note that this method can be dangerous, and that the size of a file + * can cause an OutOfMemoryError quite easily. You should use + * {@link #getInputStream() getInputStream} and do your own thing. * * @exception ServletException If the temp file no longer exists, or if there is * some sort of IOException @@ -56,6 +58,9 @@ /** * Attempts to read a file n bytes at a time, n being equal to "bufferSize". + * Note that this method can be dangerous, and that the size of a file + * can cause an OutOfMemoryError quite easily. You should use + * {@link #getInputStream() getInputStream} and do your own thing. * * @param bufferSize The size in bytes that are read from the file at a time * @exception FileNotFoundException If the temp file no longer exists @@ -150,7 +155,11 @@ public int getFileSize() { return fileSize; } - - + /** + * Returns a FileInputStream to the file + */ + public InputStream getInputStream() throws FileNotFoundException, IOException { + return new FileInputStream(filePath); + } } 1.7 +55 -97 jakarta-struts/src/share/org/apache/struts/upload/DiskMultipartRequestHandler.java Index: DiskMultipartRequestHandler.java =================================================================== RCS file: /home/cvs/jakarta-struts/src/share/org/apache/struts/upload/DiskMultipartRequestHandler.java,v retrieving revision 1.6 retrieving revision 1.7 diff -u -r1.6 -r1.7 --- DiskMultipartRequestHandler.java 2000/11/28 07:17:17 1.6 +++ DiskMultipartRequestHandler.java 2001/02/14 21:43:05 1.7 @@ -47,11 +47,19 @@ */ protected Hashtable allElements; + /** + * The temporary directory + */ + protected String tempDir; + public void handleRequest(HttpServletRequest request) throws ServletException { + retrieveTempDir(); + MultipartIterator iterator = new MultipartIterator(request, servlet.getBufferSize(), - getMaxSizeFromServlet()); + getMaxSizeFromServlet(), + tempDir); MultipartElement element; textElements = new Hashtable(); @@ -60,29 +68,21 @@ try { while ((element = iterator.getNextElement()) != null) { - if ((element.getFileName() == null) && (element.getContentType() == null)) { - String textData; - try { - textData = new String(element.getData(), "ISO-8859-1"); - } - catch (UnsupportedEncodingException uee) { - textData = new String(element.getData()); - } - - textElements.put(element.getName(), textData); - allElements.put(element.getName(), textData); + if (!element.isFile()) { + textElements.put(element.getName(), element.getValue()); + allElements.put(element.getName(), element.getValue()); } else { - try { - DiskFile theFile = writeFile(element); - fileElements.put(element.getName(), theFile); - allElements.put(element.getName(), theFile); - } - catch (IOException ioe) { - throw new ServletException("DiskMultipartRequestHandler." + - "handleRequest(), IOException: " + - ioe.getMessage()); - } + + File tempFile = element.getFile(); + if (tempFile.exists()) { + DiskFile theFile = new DiskFile(tempFile.getAbsolutePath()); + theFile.setContentType(element.getContentType()); + theFile.setFileName(element.getFileName()); + theFile.setFileSize((int) tempFile.length()); + fileElements.put(element.getName(), theFile); + allElements.put(element.getName(), theFile); + } } } } @@ -92,81 +92,6 @@ } - - protected DiskFile writeFile(MultipartElement element) throws IOException, - ServletException { - DiskFile theFile = null; - - //get a handle to some temporary file and open - //a stream to it - String tempDir = servlet.getTempDir(); - if (tempDir == null) { - //attempt to retrieve the servlet container's temporary directory - ServletContext context = servlet.getServletConfig().getServletContext(); - - try { - tempDir = (String) context.getAttribute("javax.servlet.context.tempdir"); - } - catch (ClassCastException cce) { - tempDir = ((File) context.getAttribute("javax.servlet.context.tempdir")).getAbsolutePath(); - } - - - if (tempDir == null) { - //default to system-wide tempdir - tempDir = System.getProperty("java.io.tmpdir"); - - if (servlet.getDebug() > 1) { - servlet.log("DiskMultipartRequestHandler.handleRequest(): " + - "defaulting to java.io.tmpdir directory \"" + - tempDir); - } - } - } - - File tempDirectory = new File(tempDir); - - if ((!tempDirectory.exists()) || (!tempDirectory.isDirectory())) { - throw new ServletException("DiskMultipartRequestHandler: no " + - "temporary directory specified for disk write"); - } - - File tempFile = File.createTempFile("strts", null, tempDirectory); - FileOutputStream fos = new FileOutputStream(tempFile); - - //int bufferSize = servlet.getBufferSize(); - int bufferSize = -1; - if (bufferSize > 0) { - //buffer the write if servlet.getBufferSize() > 0 - ByteArrayInputStream byteArray = new ByteArrayInputStream(element.getData()); - byte[] bufferBytes = new byte[bufferSize]; - int bytesWritten = 0; - int bytesRead = 0; - int offset = 0; - - while ((bytesRead = byteArray.read(bufferBytes, - offset, bufferSize)) != -1) { - fos.write(bufferBytes, offset, bytesRead); - bytesWritten += bytesRead; - offset += bytesRead; - } - byteArray.close(); - } - else { - //write in one big chunk - fos.write(element.getData()); - } - - theFile = new DiskFile(tempFile.getAbsolutePath()); - theFile.setContentType(element.getContentType()); - theFile.setFileName(element.getFileName()); - theFile.setFileSize(element.getData().length); - - fos.close(); - - return theFile; - } - public Hashtable getAllElements() { return allElements; } @@ -248,4 +173,37 @@ return (size * multiplier); } + + /** + * Retrieves the temporary directory from either ActionServlet, a context + * property, or a system property, in that order + */ + protected void retrieveTempDir() { + //get a handle to some temporary file and open + //a stream to it + tempDir = servlet.getTempDir(); + if (tempDir == null) { + //attempt to retrieve the servlet container's temporary directory + ServletContext context = servlet.getServletConfig().getServletContext(); + + try { + tempDir = (String) context.getAttribute("javax.servlet.context.tempdir"); + } + catch (ClassCastException cce) { + tempDir = ((File) context.getAttribute("javax.servlet.context.tempdir")).getAbsolutePath(); + } + + + if (tempDir == null) { + //default to system-wide tempdir + tempDir = System.getProperty("java.io.tmpdir"); + + if (servlet.getDebug() > 1) { + servlet.log("DiskMultipartRequestHandler.handleRequest(): " + + "defaulting to java.io.tmpdir directory \"" + + tempDir); + } + } + } + } } 1.2 +15 -4 jakarta-struts/src/share/org/apache/struts/upload/FormFile.java Index: FormFile.java =================================================================== RCS file: /home/cvs/jakarta-struts/src/share/org/apache/struts/upload/FormFile.java,v retrieving revision 1.1 retrieving revision 1.2 diff -u -r1.1 -r1.2 --- FormFile.java 2000/11/09 20:08:56 1.1 +++ FormFile.java 2001/02/14 21:43:05 1.2 @@ -1,5 +1,6 @@ package org.apache.struts.upload; +import java.io.InputStream; import java.io.IOException; import java.io.FileNotFoundException; @@ -45,18 +46,28 @@ public void setFileName(String fileName); /** - * Get the data in byte array for - * for this file + * Get the data in byte array for for this file. Note that this can be + * a very hazardous method, files can be large enough to cause + * OutOfMemoryErrors. Short of being deprecated, it's strongly recommended + * that you use {@link #getInputStream() getInputStream} to get the file + * data. * * @exception FileNotFoundException If some sort of file representation * cannot be found for the FormFile - * * @exception IOException If there is some sort of IOException - * * @return An array of bytes representing the data contained * in the form file */ public byte[] getFileData() throws FileNotFoundException, IOException; + + /** + * Get an InputStream that represents this file. This is the preferred + * method of getting file data. + * @exception FileNotFoundException If some sort of file representation + * cannot be found for the FormFile + * @exception IOException If there is some sort of IOException + */ + public InputStream getInputStream() throws FileNotFoundException, IOException; /** * Destroy all content for this form file. 1.2 +98 -0 jakarta-struts/src/share/org/apache/struts/upload/MultipartElement.java Index: MultipartElement.java =================================================================== RCS file: /home/cvs/jakarta-struts/src/share/org/apache/struts/upload/MultipartElement.java,v retrieving revision 1.1 retrieving revision 1.2 diff -u -r1.1 -r1.2 --- MultipartElement.java 2000/11/09 20:08:56 1.1 +++ MultipartElement.java 2001/02/14 21:43:06 1.2 @@ -1,5 +1,7 @@ package org.apache.struts.upload; +import java.io.File; + /** * This class represents an element in a multipart request. * It has a few methods for determining @@ -20,10 +22,16 @@ /** * The element data + * @deprecated This should never be used. */ protected byte[] data; /** + * The element's data represented in a (possibly temporary) file + */ + protected File file; + + /** * The element name */ protected String name; @@ -33,15 +41,65 @@ */ protected String fileName; + /** + * The element's text value, null for file elements + */ + protected String value; + + /** + * Whether or not this element is a file + */ + protected boolean isFile = false; + /** + * @deprecated Use the constructor that takes an File as an argument + * as opposed to a byte array argument, which can cause + * memory problems + */ public MultipartElement(String name, String fileName, String contentType, byte[] data) { this.name = name; this.fileName = fileName; this.contentType = contentType; this.data = data; + + if (fileName != null) { + isFile = true; + } } /** + * Constructor for a file element + * @param name The form name of the element + * @param fileName The file name of the element if this element is a file + * @param contentType The content type of the element if a file + * @param file The (possibly temporary) file representing this element if + * it's a file + */ + public MultipartElement(String name, + String fileName, + String contentType, + File file) { + + this.name = name; + this.fileName = fileName; + this.contentType = contentType; + this.file = file; + this.isFile = true; + } + + /** + * Constructor for a text element + * @param name The name of the element + * @param value The value of the element + */ + public MultipartElement(String name, String value) { + this.name = name; + this.value = value; + this.isFile = false; + } + + + /** * Retrieve the content type */ public String getContentType() { @@ -51,12 +109,21 @@ /** * Retrieve the data + * @deprecated Use the getFile method to get a File representing the + * data for this element */ public byte[] getData() { return data; } /** + * Get the File that holds the data for this element. + */ + public File getFile() { + return file; + } + + /** * Retrieve the name */ public String getName() { @@ -71,6 +138,22 @@ return fileName; } + /** + * Returns the value of this multipart element + * @return A String if the element is a text element, <code>null</code> + * otherwise + */ + public String getValue() { + return value; + } + + /** + * Set the file that represents this element + */ + public void setFile(File file) { + this.file = file; + } + /** * Set the file name for this element */ @@ -92,9 +175,24 @@ this.contentType = contentType; } + /** + * Is this element a file? + */ + public boolean isFile() { + if (file == null) { + return false; + } + return true; + } + + public void setValue(String value) { + this.value = value; + } /** * Set the data + * @deprecated Use the setFile method to set the file + * that represents the data of this element */ public void setData(byte[] data) { this.data = data; 1.9 +111 -32 jakarta-struts/src/share/org/apache/struts/upload/MultipartIterator.java Index: MultipartIterator.java =================================================================== RCS file: /home/cvs/jakarta-struts/src/share/org/apache/struts/upload/MultipartIterator.java,v retrieving revision 1.8 retrieving revision 1.9 diff -u -r1.8 -r1.9 --- MultipartIterator.java 2001/01/05 21:51:35 1.8 +++ MultipartIterator.java 2001/02/14 21:43:06 1.9 @@ -1,7 +1,10 @@ package org.apache.struts.upload; import java.io.File; +import java.io.InputStream; import java.io.IOException; +import java.io.OutputStream; +import java.io.FileOutputStream; import java.io.UnsupportedEncodingException; import javax.servlet.ServletException; import javax.servlet.ServletInputStream; @@ -72,6 +75,11 @@ * Defaults to 4 * 1024 (4 KB) */ protected int bufferSize = 4 * 1024; + + /** + * The temporary directory to store files + */ + protected String tempDir; /** * Constructs a MultipartIterator with a default buffer size and no file size @@ -106,11 +114,28 @@ */ public MultipartIterator(HttpServletRequest request, int bufferSize, long maxSize) throws ServletException { + + this(request, bufferSize, maxSize, null); + + } + + public MultipartIterator(HttpServletRequest request, + int bufferSize, + long maxSize, + String tempDir) throws ServletException { + this.request = request; this.maxSize = maxSize; if (bufferSize > -1) { this.bufferSize = bufferSize; } + if (tempDir != null) { + this.tempDir = tempDir; + } + else { + //default to system-wide tempdir + tempDir = System.getProperty("java.io.tmpdir"); + } parseRequest(); } @@ -134,8 +159,9 @@ String filename = parseDispositionFilename(disposition); String contentType = null; - - if (filename != null) { + boolean isFile = (filename != null); + + if (isFile) { filename = new File(filename).getName(); //check for windows filenames, @@ -158,38 +184,55 @@ contentType = parseContentType(contentType); } - //read data into String form, then convert to bytes - //for both normal text and file - StringBuffer textData = new StringBuffer(); - String line; + - //ignore next line (whitespace) - readLine(); + //ignore next line (whitespace) (unless it's a file + //without content-type) + if (! ((isFile) && contentType == null)) { + readLine(); + } + + MultipartElement element = null; - //parse for text data - line = readLine(); - - while ((line != null) && (!line.startsWith(boundary))) { - textData.append(line); + //process a file element + if (isFile) { + try { + //create a local file on disk representing the element + File elementFile = createLocalFile(); + + element = new MultipartElement(name, filename, contentType, elementFile); + } catch (IOException ioe) { + throw new ServletException("IOException while reading file element: ioe.getMessage()", ioe); + } + } + else { + //read data into String form, then convert to bytes + //for text + StringBuffer textData = new StringBuffer(); + String line; + //parse for text data line = readLine(); - - if (maxSize > -1) { - if (totalLength > maxSize) { - throw new ServletException("Multipart data size exceeds the maximum " + - "allowed post size"); + + while ((line != null) && (!line.startsWith(boundary))) { + textData.append(line); + line = readLine(); + + if (maxSize > -1) { + if (totalLength > maxSize) { + throw new ServletException("Multipart data size exceeds the maximum " + + "allowed post size"); + } } } + + if (textData.length() > 1) { + //cut off "\r\n" from the end + textData.setLength(textData.length()-2); + } + + //create the element + element = new MultipartElement(name, textData.toString()); } - - if (textData.length() > 1) { - //cut off "\r\n" from the end - textData.setLength(textData.length()-2); - } - - MultipartElement element = new MultipartElement(name, - filename, - contentType, - textData.toString().getBytes("ISO-8859-1")); return element; } //reset stream @@ -331,10 +374,6 @@ } return null; } - - - - /** * Retrieves the "name" attribute from a content disposition line @@ -416,4 +455,44 @@ totalLength += bytesRead; return new String(bufferByte, 0, bytesRead, "ISO-8859-1"); } + + /** + * Creates a file on disk from the current mulitpart element + * @param fileName the name of the multipart file + */ + protected File createLocalFile() throws IOException,ServletException { + + File tempFile = File.createTempFile("strts", null, new File(tempDir)); + OutputStream fos = new FileOutputStream(tempFile); + + byte[] bufferBytes = new byte[bufferSize]; + InputStream requestIn = new MultipartValueStream(inputStream, boundary); + + int bytesRead = 0; + while (bytesRead != -1) { + + bytesRead = requestIn.read(bufferBytes); + + if (bytesRead > 0) { + totalLength += bytesRead; + } + + //check to make sure the read doesn't exceed the bounds + if (bytesRead > 0) { + if (maxSize > -1) { + if (totalLength > maxSize) { + throw new ServletException("Multipart data size exceeds the maximum " + + "allowed post size"); + } + } + } + if (bytesRead > 0) { + fos.write(bufferBytes, 0, bytesRead); + } + } + + fos.close(); + return tempFile; + } + } 1.1 jakarta-struts/src/share/org/apache/struts/upload/MultipartValueStream.java Index: MultipartValueStream.java =================================================================== package org.apache.struts.upload; import java.io.*; import java.util.*; import javax.servlet.ServletException; /** * This class implements an inputStream that reads another stream until * a multipart boundary is found. The class reports eof when boundary found. * The undelying stream is not closed. * * <p> * See RFC 1867 (http://info.internet.isi.edu:80/in-notes/rfc/files/rfc1867.txt) * for details about the protocol. * <p> * * @author Jimmy Larsson */ class MultipartValueStream extends InputStream { public static final String HEADER_ENCODING = "iso-8859-1"; /** the underlying stream */ private InputStream in; /** byte buffer with the boundary */ private byte boundaryBytes[]; /** how many curretly matched boundary bytes? */ private int matchedBoundaryBytes; /** the read ahead buffer (cyclic) */ private byte readAheadBytes[]; /** The start index for the read ahead cyclic buffer (points to the first byte) */ private int readAheadBufferStartI; /** The end index for the read ahead cyclic buffer (points to the last byte) */ private int readAheadBufferEndI; /** have we reached the boundary? */ private boolean boundaryReached = false; /** is the boundary found a final boundary? */ private boolean finalBoundaryReached = false; /** * Create a stream that stops reading at the boundary * * NOTE: the boundary parameter is without the trailing dashes "--". */ public MultipartValueStream(InputStream in, String boundary) throws IOException { this.in = in; this.boundaryBytes = ("\r\n" + boundary).getBytes(HEADER_ENCODING); this.matchedBoundaryBytes = 0; this.readAheadBytes = new byte[this.boundaryBytes.length]; /* Fill read ahead buffer */ if (in.read(readAheadBytes, 0, readAheadBytes.length) != readAheadBytes.length) { throw new IOException("end of stream before boundary found!"); } /* Count the number of matched chars */ for (int i = 0; i < readAheadBytes.length; i++) { if (readAheadBytes[i] == boundaryBytes[matchedBoundaryBytes]) { matchedBoundaryBytes++; } else { matchedBoundaryBytes = 0; if (readAheadBytes[i] == boundaryBytes[0]) { matchedBoundaryBytes = 1; } } } readAheadBufferStartI = 0; readAheadBufferEndI = readAheadBytes.length - 1; } /** * Read the next byte * * @return -1 on boundary reached * @exception IOException if the ending boundary is never found * */ public int read() throws IOException { if (boundaryReached) { return -1; } if (matchedBoundaryBytes == boundaryBytes.length) { boundaryReached = true; /* * Boundary found... * * Read two more bytes: * 1. the bytes are "--": this is the last parameter value (then read "\r\n" too) * 2. the bytes are "\r\n": this is not the last value * 3. the bytes are somthing else: Exception */ byte buf[] = new byte[2]; if (in.read(buf) != 2) { throw new IOException("end of stream before boundary found!"); } String readStr = new String(buf, HEADER_ENCODING); if (readStr.equals("--")) { if (in.read(buf) != 2) { throw new IOException("invalid end of final boundary found!"); } readStr = new String(buf, HEADER_ENCODING); if (!readStr.equals("\r\n")) { throw new IOException("invalid end of final boundary found!"); } finalBoundaryReached = true; } else if (readStr.equals("\r\n")) { finalBoundaryReached = false; } else { throw new IOException("invalid end of boundary found!"); } return -1; } /* * Might seem odd, but we are supposed to return * a byte as an int in range 0 - 255, and the byte type * is signed (-128 to 127) * */ int returnByte = (int)(char) readAheadBytes[readAheadBufferStartI]; /* Move cyclic-buffers start pointer */ readAheadBufferStartI++; if (readAheadBufferStartI == readAheadBytes.length) { readAheadBufferStartI = 0; } /* read from the underlying stream */ int underlyingRead = in.read(); if (underlyingRead == -1) { throw new IOException("end of stream before boundary found!"); } /* Move cyclic-buffers end pointer */ readAheadBufferEndI++; if (readAheadBufferEndI == readAheadBytes.length) { readAheadBufferEndI = 0; } readAheadBytes[readAheadBufferEndI] = (byte) underlyingRead; if (readAheadBytes[readAheadBufferEndI] == boundaryBytes[matchedBoundaryBytes]) { matchedBoundaryBytes++; } else { matchedBoundaryBytes = 0; if (readAheadBytes[readAheadBufferEndI] == boundaryBytes[0]) { matchedBoundaryBytes = 1; } } return returnByte; } /** * @return true if we are the last stream, ie. we encountered a final boundary * @return false otherwise * * @exception ParameterException if the boundary has not yet been reached */ public boolean encounteredFinalBoundary() throws ServletException { if (!boundaryReached) { throw new ServletException("have not reached boundary yet!"); } return finalBoundaryReached; } } 1.2 +12 -4 jakarta-struts/web/upload/WEB-INF/classes/org/apache/struts/example/upload/UploadAction.java Index: UploadAction.java =================================================================== RCS file: /home/cvs/jakarta-struts/web/upload/WEB-INF/classes/org/apache/struts/example/upload/UploadAction.java,v retrieving revision 1.1 retrieving revision 1.2 diff -u -r1.1 -r1.2 --- UploadAction.java 2000/12/19 19:23:07 1.1 +++ UploadAction.java 2001/02/14 21:43:09 1.2 @@ -1,6 +1,8 @@ package org.apache.struts.example.upload; +import java.io.InputStream; import java.io.IOException; +import java.io.ByteArrayOutputStream; import java.io.FileNotFoundException; import javax.servlet.http.HttpServletRequest; @@ -27,9 +29,9 @@ public class UploadAction extends Action { public ActionForward perform(ActionMapping mapping, - ActionForm form, - HttpServletRequest request, - HttpServletResponse response) { + ActionForm form, + HttpServletRequest request, + HttpServletResponse response) { if (form instanceof UploadForm) { @@ -54,7 +56,13 @@ try { //retrieve the file data - data = new String(file.getFileData()); + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + + InputStream stream = file.getInputStream(); + byte[] buffer = new byte[file.getFileSize()]; + stream.read(buffer); + baos.write(buffer); + data = new String(baos.toByteArray()); } catch (FileNotFoundException fnfe) { return null;