Fixing broken FileUploadServletRequest VIVO-222

webapp/src/edu/cornell/mannlib/vitro/webapp/filestorage/uploadrequest/FileUploadServletRequest.java

@@ -3,8 +3,15 @@
 package edu.cornell.mannlib.vitro.webapp.filestorage.uploadrequest;
 
 import java.io.IOException;
+import java.io.UnsupportedEncodingException;
+import java.net.URLDecoder;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Enumeration;
+import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
 
 import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletRequestWrapper;
@@ -12,6 +19,8 @@ import javax.servlet.http.HttpServletRequestWrapper;
 import org.apache.commons.fileupload.FileItem;
 import org.apache.commons.fileupload.FileUploadException;
 import org.apache.commons.fileupload.servlet.ServletFileUpload;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
 
 /**
  * <p>
@@ -44,21 +53,39 @@ import org.apache.commons.fileupload.servlet.ServletFileUpload;
  */
 @SuppressWarnings("deprecation")
 public abstract class FileUploadServletRequest extends HttpServletRequestWrapper  {
+    
+    private static final Log log = LogFactory
+            .getLog(FileUploadServletRequest.class);
+    
 	public static final String FILE_ITEM_MAP = "file_item_map";
 	public static final String FILE_UPLOAD_EXCEPTION = "file_upload_exception";
 
+	private Map<String, List<String>> parameters;
+    private Map<String, List<FileItem>> files;
+    private FileUploadException fileUploadException;
+
+    private static final String[] EMPTY_ARRAY = new String[0];
+    
 	// ----------------------------------------------------------------------
 	// The factory method
 	// ----------------------------------------------------------------------
 
 	/**
 	 * Wrap this {@link HttpServletRequest} in an appropriate wrapper class.
+	 * set maxTempFileSize to 0 or -1 if streaming is desired.  Set it to > 0 if
+	 * you want any parts uploaded to a temporary directory 
 	 */
 	public static FileUploadServletRequest parseRequest(		
-			HttpServletRequest request, int maxFileSize) throws IOException {
+	        HttpServletRequest request, int maxTempFileSize) throws IOException {
+	    
 		boolean isMultipart = ServletFileUpload.isMultipartContent(request);
+		
 		if (isMultipart) {
-			return new MultipartHttpServletRequest(request, maxFileSize);
+		    if( maxTempFileSize <= 0 ){
+		        return new StreamingMultipartHttpServletRequest(request);
+		    }else{
+		        return new MultipartHttpServletRequest(request, maxTempFileSize);
+		    }
 		} else {
 			return new SimpleHttpServletRequestWrapper(request);
 		}
@@ -86,28 +113,183 @@ public abstract class FileUploadServletRequest extends HttpServletRequestWrapper
 	/** Was this a multipart HTTP request? */
 	public abstract boolean isMultipart();
 	
-	/**
+	protected void stashParametersInRequest(HttpServletRequest request, ServletFileUpload upload){
-	 * Get the map of file items, by name.
+	    Map<String, List<String>> parameters = new HashMap<String, List<String>>();
-	 */
+	    Map<String, List<FileItem>> files = new HashMap<String, List<FileItem>>();      
-	public abstract Map<String, List<FileItem>> getFiles();
+
+	    parseQueryString(request.getQueryString(), parameters);
+
+	    try {
+	        List<FileItem> items = upload.parseRequest( request );
+
+	        for (FileItem item : items) {
+	            // Process a regular form field
+	            if (item.isFormField()) {
+	                addToParameters(parameters, item.getFieldName(), item
+	                        .getString("UTF-8"));
+	                log.debug("Form field (parameter) " + item.getFieldName()
+	                        + "=" + item.getString());
+	            } else {
+	                addToFileItems(files, item);
+	                log.debug("File " + item.getFieldName() + ": "
+	                        + item.getName());
+	            }
+	        }
+	    } catch (FileUploadException e) {
+	        fileUploadException = e;
+	        request.setAttribute(
+	                FileUploadServletRequest.FILE_UPLOAD_EXCEPTION, e);
+	    } catch (UnsupportedEncodingException e) {
+	        log.error("could not convert to UTF-8",e);
+	    }
+
+	    this.parameters = Collections.unmodifiableMap(parameters);
+	    log.debug("Parameters are: " + this.parameters);
+	    this.files = Collections.unmodifiableMap(files);
+	    log.debug("Files are: " + this.files);
+	    request.setAttribute(FILE_ITEM_MAP, this.files);
+	}
+
+
 
 	/**
-	 * Find a non-empty file item with this name.
+	 * Pull any parameters out of the URL.
-	 * 
-	 * @return the first such file item, or <code>null</code> if no matching,
-	 *         non-empty items were found.
 	 */
-	public abstract FileItem getFileItem(String string);
+	private void parseQueryString(String queryString,
+	        Map<String, List<String>> parameters) {
+	    log.debug("Query string is : '" + queryString + "'");
+	    if (queryString != null) {
+	        String[] pieces = queryString.split("&");
+
+	        for (String piece : pieces) {
+	            int equalsHere = piece.indexOf('=');
+	            if (piece.trim().isEmpty()) {
+	                // Ignore an empty piece.
+	            } else if (equalsHere <= 0) {
+	                // A parameter without a value.
+	                addToParameters(parameters, decode(piece), "");
+	            } else {
+	                // A parameter with a value.
+	                String key = piece.substring(0, equalsHere);
+	                String value = piece.substring(equalsHere + 1);
+	                addToParameters(parameters, decode(key), decode(value));
+	            }
+	        }
+	    }
+	    log.debug("Parameters from query string are: " + parameters);
+	}
 
 	/**
-	 * Was there an exception when uploading the file items?
+	 * Remove any special URL-style encoding.
 	 */
-	public abstract boolean hasFileUploadException();
+	private String decode(String encoded) {
+	    try {
+	        return URLDecoder.decode(encoded, "UTF-8");
+	    } catch (UnsupportedEncodingException e) {
+	        log.error(e, e);
+	        return encoded;
+	    }
+	}
+
+
+	/** Either create a new List for the value, or add to an existing List. */
+	private void addToParameters(Map<String, List<String>> map, String name,
+	        String value) {
+	    if (!map.containsKey(name)) {
+	        map.put(name, new ArrayList<String>());
+	    }
+	    map.get(name).add(value);
+	}
+
+	/** Either create a new List for the file, or add to an existing List. */
+	private void addToFileItems(Map<String, List<FileItem>> map, FileItem file) {
+	    String name = file.getFieldName();
+	    if (!map.containsKey(name)) {
+	        map.put(name, new ArrayList<FileItem>());
+	    }
+	    map.get(name).add(file);
+	}
+
+	
+	public FileUploadException getFileUploadException() {
+	    return fileUploadException;
+	}
+
+	public boolean hasFileUploadException() {
+	    return fileUploadException != null;
+	}
+
+	// ----------------------------------------------------------------------
+	// Parameter-related methods won't find anything on the delegate request,
+	// since parsing consumed the parameters. So we need to look to the parsed
+	// info for the answers.
+	// ----------------------------------------------------------------------
+
+	@Override
+	public String getParameter(String name) {
+	    if (parameters.containsKey(name)) {
+	        return parameters.get(name).get(0);
+	    } else {
+	        return null;
+	    }
+	}
+
+	@Override
+	public Enumeration<?> getParameterNames() {
+	    return Collections.enumeration(parameters.keySet());
+	}
+
+	@Override
+	public String[] getParameterValues(String name) {
+	    if (parameters.containsKey(name)) {
+	        return parameters.get(name).toArray(EMPTY_ARRAY);
+	    } else {
+	        return null;
+	    }
+	}
+
+	@Override
+	public Map<String, String[]> getParameterMap() {
+	    Map<String, String[]> result = new HashMap<String, String[]>();
+	    for (Entry<String, List<String>> entry : parameters.entrySet()) {
+	        result.put(entry.getKey(), entry.getValue().toArray(EMPTY_ARRAY));
+	    }
+	    log.debug("resulting parameter map: " + result);
+	    return result;
+	}
+
 	
     /**
-	 * Get the exception that occurred when uploading the file items. If no such
+     * {@inheritDoc}
-	 * exception, return <code>null</code>.
+     * <p>
+     * There may be more than one file item with the given name. If the first
+     * one is empty (size is zero), keep looking for a non-empty one.
+     * </p>
      */    
-	public abstract FileUploadException getFileUploadException();
+    public FileItem getFileItem(String name) {
+        List<FileItem> items = files.get(name);
+        if (items == null) {
+            return null;
+        }
+
+        for (FileItem item : items) {
+            if (item.getSize() > 0L) {
+                return item;
+            }
+        }
+
+        return null;
+    }
+
+    /**
+     * Gets a map of parameter names to files.
+     * This will should return null.
+     */
+    public Map<String, List<FileItem>> getFiles() {
+        if( files == null )
+            return Collections.emptyMap();
+        else
+            return files;
+    }
 
 }

webapp/src/edu/cornell/mannlib/vitro/webapp/filestorage/uploadrequest/MultipartHttpServletRequest.java

@@ -33,33 +33,9 @@ public class MultipartHttpServletRequest extends FileUploadServletRequest {
 	private static final Log log = LogFactory
 			.getLog(MultipartHttpServletRequest.class);
 
-	private static final String[] EMPTY_ARRAY = new String[0];
-
-	private Map<String, List<String>> parameters;
-	private Map<String, List<FileItem>> files;
-	private FileUploadException fileUploadException;
-
-    private boolean storeFilesToTempDir = false;
     private int maxFileSize = 0;
     private File tempDir = null; 
             
-    /** 
-     * Parse the multipart request. Store the info about the request parameters.
-     * Don't store the uploaded files to a temporary directory to allow streaming.
-     *
-     * Only use this constructor if you plan to consume the FileItems using streaming
-     * to deal with inputs of very large sizes.
-     *
-     * In all other case you should use the maxFileSize constructor to deal with
-     * the size of the uploaded file in a safe way.
-     */
-    public MultipartHttpServletRequest(HttpServletRequest request)
-        throws IOException{
-        super(request);
-        storeFilesToTempDir = false;
-        
-    }
-            
 	/**
 	 * Parse the multipart request. Store the info about the request parameters
 	 * and the uploaded files to a temporary directory.
@@ -70,91 +46,15 @@ public class MultipartHttpServletRequest extends FileUploadServletRequest {
 	public MultipartHttpServletRequest(HttpServletRequest request,
 			int maxFileSize) throws IOException {
 		super(request);
-        storeFilesToTempDir = true;
+		
         this.maxFileSize = maxFileSize;
         this.tempDir = figureTemporaryDirectory(request);
+
+        //use an upload handler that will stash the file items in a temporary directory.
+        ServletFileUpload upload = createUploadHandler( this.maxFileSize, this.tempDir );
+        stashParametersInRequest( request , upload );
 	}
 
-    private void setup(HttpServletRequest request){
-		Map<String, List<String>> parameters = new HashMap<String, List<String>>();
-		Map<String, List<FileItem>> files = new HashMap<String, List<FileItem>>();
-
-		ServletFileUpload upload = createUploadHandler();
-        
-		//File tempDir = figureTemporaryDirectory(request);
-		//ServletFileUpload upload = createUploadHandler(maxFileSize, tempDir);
-
-		parseQueryString(request.getQueryString(), parameters);
-
-		try {
-			List<FileItem> items = parseRequestIntoFileItems(request, upload);
-
-			for (FileItem item : items) {
-				// Process a regular form field
-				if (item.isFormField()) {
-					addToParameters(parameters, item.getFieldName(), item
-							.getString("UTF-8"));
-					log.debug("Form field (parameter) " + item.getFieldName()
-							+ "=" + item.getString());
-				} else {
-					addToFileItems(files, item);
-					log.debug("File " + item.getFieldName() + ": "
-							+ item.getName());
-				}
-			}
-		} catch (FileUploadException e) {
-			fileUploadException = e;
-			request.setAttribute(
-					FileUploadServletRequest.FILE_UPLOAD_EXCEPTION, e);
-		} catch (UnsupportedEncodingException e) {
-		    log.error("could not convert to UTF-8",e);
-        }
-
-		this.parameters = Collections.unmodifiableMap(parameters);
-		log.debug("Parameters are: " + this.parameters);
-		this.files = Collections.unmodifiableMap(files);
-		log.debug("Files are: " + this.files);
-		request.setAttribute(FILE_ITEM_MAP, this.files);
-    }
-
-	/**
-	 * Pull any parameters out of the URL.
-	 */
-	private void parseQueryString(String queryString,
-			Map<String, List<String>> parameters) {
-		log.debug("Query string is : '" + queryString + "'");
-		if (queryString != null) {
-			String[] pieces = queryString.split("&");
-
-			for (String piece : pieces) {
-				int equalsHere = piece.indexOf('=');
-				if (piece.trim().isEmpty()) {
-					// Ignore an empty piece.
-				} else if (equalsHere <= 0) {
-					// A parameter without a value.
-					addToParameters(parameters, decode(piece), "");
-				} else {
-					// A parameter with a value.
-					String key = piece.substring(0, equalsHere);
-					String value = piece.substring(equalsHere + 1);
-					addToParameters(parameters, decode(key), decode(value));
-				}
-			}
-		}
-		log.debug("Parameters from query string are: " + parameters);
-	}
-
-	/**
-	 * Remove any special URL-style encoding.
-	 */
-	private String decode(String encoded) {
-		try {
-			return URLDecoder.decode(encoded, "UTF-8");
-		} catch (UnsupportedEncodingException e) {
-			log.error(e, e);
-			return encoded;
-		}
-	}
  
 	/**
 	 * Find the temporary storage directory for this webapp.
@@ -164,18 +64,6 @@ public class MultipartHttpServletRequest extends FileUploadServletRequest {
 				"javax.servlet.context.tempdir");
 	}
     
-    
-    /**
-     * Create an upload handler based on this.storeFilesToTempDir.
-     */
-    private ServletFileUpload createUploadHandler(){
-        if( storeFilesToTempDir ){
-            return createUploadHandler( this.maxFileSize, this.tempDir );
-        }else{
-            return new ServletFileUpload();
-        }
-    }
-
 	/**
 	 * Create an upload handler that will throw an exception if the file is too
 	 * large.
@@ -191,116 +79,10 @@ public class MultipartHttpServletRequest extends FileUploadServletRequest {
 		return upload;
 	}
 
-	/** Either create a new List for the value, or add to an existing List. */
-	private void addToParameters(Map<String, List<String>> map, String name,
-			String value) {
-		if (!map.containsKey(name)) {
-			map.put(name, new ArrayList<String>());
-		}
-		map.get(name).add(value);
-	}
-
-	/** Either create a new List for the file, or add to an existing List. */
-	private void addToFileItems(Map<String, List<FileItem>> map, FileItem file) {
-		String name = file.getFieldName();
-		if (!map.containsKey(name)) {
-			map.put(name, new ArrayList<FileItem>());
-		}
-		map.get(name).add(file);
-	}
-
-	/** Minimize the code that uses the unchecked cast. */
-	@SuppressWarnings("unchecked")
-	private List<FileItem> parseRequestIntoFileItems(HttpServletRequest req,
-			ServletFileUpload upload) throws FileUploadException {
-		return upload.parseRequest(req);
-	}
-
-	// ----------------------------------------------------------------------
-	// This is a multipart request, so make the file info available. If there
-	// was an exception during parsing, make that available too.
-	// ----------------------------------------------------------------------
-
     @Override
     public boolean isMultipart() {
         return true;
     }
 
-	@Override
-	public Map<String, List<FileItem>> getFiles() {
-		return files;
-	}
-
-	/**
-	 * {@inheritDoc}
-	 * <p>
-	 * There may be more than one file item with the given name. If the first
-	 * one is empty (size is zero), keep looking for a non-empty one.
-	 * </p>
-	 */
-	@Override
-	public FileItem getFileItem(String name) {
-		List<FileItem> items = files.get(name);
-		if (items == null) {
-			return null;
-		}
-
-		for (FileItem item : items) {
-			if (item.getSize() > 0L) {
-				return item;
-			}
-		}
-
-		return null;
-	}
-
-	@Override
-	public FileUploadException getFileUploadException() {
-		return fileUploadException;
-	}
-
-	@Override
-	public boolean hasFileUploadException() {
-		return fileUploadException != null;
-	}
-
-	// ----------------------------------------------------------------------
-	// Parameter-related methods won't find anything on the delegate request,
-	// since parsing consumed the parameters. So we need to look to the parsed
-	// info for the answers.
-	// ----------------------------------------------------------------------
-
-	@Override
-	public String getParameter(String name) {
-		if (parameters.containsKey(name)) {
-			return parameters.get(name).get(0);
-		} else {
-			return null;
-		}
-	}
-
-	@Override
-	public Enumeration<?> getParameterNames() {
-		return Collections.enumeration(parameters.keySet());
-	}
-
-	@Override
-	public String[] getParameterValues(String name) {
-		if (parameters.containsKey(name)) {
-			return parameters.get(name).toArray(EMPTY_ARRAY);
-		} else {
-			return null;
-		}
-	}
-
-	@Override
-	public Map<String, String[]> getParameterMap() {
-		Map<String, String[]> result = new HashMap<String, String[]>();
-		for (Entry<String, List<String>> entry : parameters.entrySet()) {
-			result.put(entry.getKey(), entry.getValue().toArray(EMPTY_ARRAY));
-		}
-		log.debug("resulting parameter map: " + result);
-		return result;
-	}
 	
 }

webapp/src/edu/cornell/mannlib/vitro/webapp/filestorage/uploadrequest/SimpleHttpServletRequestWrapper.java

@@ -33,25 +33,6 @@ class SimpleHttpServletRequestWrapper extends FileUploadServletRequest {
 		return false;
 	}
 
-	@Override
-	public Map<String, List<FileItem>> getFiles() {
-		return Collections.emptyMap();
-	}
-
-	@Override
-	public FileItem getFileItem(String string) {
-		return null;
-	}
-
-	@Override
-	public FileUploadException getFileUploadException() {
-		return null;
-	}
-
-	@Override
-	public boolean hasFileUploadException() {
-		return false;
-	}
 	
 	// ----------------------------------------------------------------------
 	// Since this is not a multipart request, the parameter methods can be
@@ -64,7 +45,7 @@ class SimpleHttpServletRequestWrapper extends FileUploadServletRequest {
 	}
 
 	@Override
-	public Map<?, ?> getParameterMap() {
+	public Map<String, String[]> getParameterMap() {
 		return getDelegate().getParameterMap();
 	}
 

webapp/src/edu/cornell/mannlib/vitro/webapp/filestorage/uploadrequest/StreamingMultipartHttpServletRequest.java

@@ -0,0 +1,43 @@
+package edu.cornell.mannlib.vitro.webapp.filestorage.uploadrequest;
+
+import java.io.IOException;
+import java.util.List;
+import java.util.Map;
+
+import javax.servlet.http.HttpServletRequest;
+
+import org.apache.commons.fileupload.FileItem;
+import org.apache.commons.fileupload.FileUploadException;
+import org.apache.commons.fileupload.servlet.ServletFileUpload;
+
+/**
+ * Wrapping ServletRequest that does multipart.  In order to allow
+ * streaming, this class does NOT save the parts to a temporary directory. 
+ * 
+ *
+ */
+public class StreamingMultipartHttpServletRequest extends
+        FileUploadServletRequest {    
+    /** 
+     * Parse the multipart request. Store the info about the request parameters.
+     * Don't store the uploaded files to a temporary directory to allow streaming.
+     *
+     * Only use this if you plan to consume the FileItems using streaming
+     * to deal with inputs of very large sizes.
+     * 
+     */
+    public StreamingMultipartHttpServletRequest(HttpServletRequest request)
+        throws IOException{
+        super(request);        
+        
+        //use a file uploader that does not save the files to a temporary directory.
+        stashParametersInRequest( request , new ServletFileUpload());
+    }
+
+    @Override
+    public boolean isMultipart() { 
+        return true;
+    }
+
+
+}

webapp/src/edu/cornell/mannlib/vitro/webapp/search/controller/SearchServiceController.java

@@ -25,6 +25,7 @@ import edu.cornell.mannlib.vitro.webapp.controller.freemarker.FreemarkerHttpServ
 import edu.cornell.mannlib.vitro.webapp.controller.freemarker.responsevalues.ExceptionResponseValues;
 import edu.cornell.mannlib.vitro.webapp.controller.freemarker.responsevalues.ResponseValues;
 import edu.cornell.mannlib.vitro.webapp.controller.freemarker.responsevalues.TemplateResponseValues;
+import edu.cornell.mannlib.vitro.webapp.filestorage.uploadrequest.FileUploadServletRequest;
 import edu.cornell.mannlib.vitro.webapp.filestorage.uploadrequest.MultipartHttpServletRequest;
 import edu.cornell.mannlib.vitro.webapp.search.indexing.IndexBuilder;
 
@@ -45,7 +46,7 @@ public class SearchServiceController extends FreemarkerHttpServlet {
 	protected Actions requiredActions(VitroRequest vreq) {
         try{
 			// Works by side effect: parse the multi-part request and stash FileItems in request			
-			new MultipartHttpServletRequest( vreq );
+			FileUploadServletRequest.parseRequest(vreq, 0);
 
             //first figure out if the password and email login to an account with a password
             String pw = vreq.getParameter("password");