NIHVIVO-161 - Improve the exception handling for files that exceed the maximum size.

webapp/src/edu/cornell/mannlib/vitro/webapp/controller/FedoraDatastreamController.java

@@ -20,7 +20,6 @@ import javax.servlet.http.HttpServletResponse;
 import javax.servlet.http.HttpSession;
 
 import org.apache.commons.fileupload.FileItem;
-import org.apache.commons.fileupload.FileUploadException;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.joda.time.DateTime;
@@ -221,12 +220,9 @@ public class FedoraDatastreamController extends VitroHttpServlet implements Cons
     public void doPost(HttpServletRequest rawRequest, HttpServletResponse res)
     throws ServletException,IOException {
         try{          
-        	FileUploadServletRequest req = null;
+        	FileUploadServletRequest req = FileUploadServletRequest.parseRequest(rawRequest, maxFileSize);
-        	try {
+       		if (req.hasFileUploadException()) {
-        		req = FileUploadServletRequest
+        		throw new FdcException("Size limit exceeded: " + req.getFileUploadException().getLocalizedMessage());
-        		.parseRequest(rawRequest, maxFileSize);
-        	} catch (FileUploadException e) {
-        		throw new FdcException("Size limit exceeded: " + e.getLocalizedMessage());
         	}
         	if (!req.isMultipart()) {
         		throw new FdcException("Must POST a multipart encoded request");

webapp/src/edu/cornell/mannlib/vitro/webapp/controller/edit/N3MultiPartUpload.java

@@ -19,7 +19,6 @@ import javax.servlet.http.HttpServletResponse;
 import javax.servlet.http.HttpSession;
 
 import org.apache.commons.fileupload.FileItem;
-import org.apache.commons.fileupload.FileUploadException;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
@@ -115,14 +114,11 @@ public class N3MultiPartUpload extends VitroHttpServlet {
             throws ServletException, IOException {
         log.debug("N3MultiPartProcess 0.01");
         
-		FileUploadServletRequest request = null;
+		FileUploadServletRequest request = FileUploadServletRequest.parseRequest(rawRequest, maxFileSize);
-		try {
+   		if (request.hasFileUploadException()) {
-			request = FileUploadServletRequest
-					.parseRequest(rawRequest, maxFileSize);
-		} catch (FileUploadException e) {
             // TODO: forward to error message
-            throw new ServletException("Size limit exceeded: " + e.getLocalizedMessage());
+            throw new ServletException("Size limit exceeded: " + request.getFileUploadException().getLocalizedMessage());
-		}
+       	}
         if (!request.isMultipart()) {
             // TODO: forward to error message
             throw new ServletException("Must POST a multipart encoded request");

webapp/src/edu/cornell/mannlib/vitro/webapp/controller/jena/JenaXMLFileUpload.java

@@ -24,7 +24,6 @@ import net.sf.saxon.s9api.XsltExecutable;
 import net.sf.saxon.s9api.XsltTransformer;
 
 import org.apache.commons.fileupload.FileItem;
-import org.apache.commons.fileupload.FileUploadException;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
@@ -90,13 +89,10 @@ public class JenaXMLFileUpload  extends BaseEditController  {
 	 */
 	public void doPost(HttpServletRequest rawRequest, HttpServletResponse resp)
 	throws ServletException, IOException {
-		FileUploadServletRequest request = null;
+		FileUploadServletRequest request = FileUploadServletRequest.parseRequest(rawRequest, maxFileSize);
-		try {
+		if (request.hasFileUploadException()) {
-			request = FileUploadServletRequest
+			throw new ServletException("Size limit exceeded: "
-					.parseRequest(rawRequest, maxFileSize);
+					+ request.getFileUploadException().getLocalizedMessage());
-		} catch (FileUploadException e) {
-            // TODO: forward to error message
-            throw new ServletException("Size limit exceeded: " + e.getLocalizedMessage());
 		}
         if (request.isMultipart()) {
         	log.debug("multipart content detected");

webapp/src/edu/cornell/mannlib/vitro/webapp/controller/jena/RDFUploadController.java

@@ -15,7 +15,6 @@ import javax.servlet.http.HttpServletRequest;
 import javax.servlet.http.HttpServletResponse;
 
 import org.apache.commons.fileupload.FileItem;
-import org.apache.commons.fileupload.FileUploadException;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
@@ -45,12 +44,10 @@ public class RDFUploadController extends BaseEditController {
 	
 	public void doPost(HttpServletRequest rawRequest,
 			HttpServletResponse response) throws ServletException, IOException {
-		FileUploadServletRequest req = null;
+		FileUploadServletRequest req = FileUploadServletRequest.parseRequest(rawRequest,
-		try {
+				maxFileSizeInBytes);
-			req = FileUploadServletRequest.parseRequest(rawRequest,
+		if (req.hasFileUploadException()) {
-					maxFileSizeInBytes);
+			forwardToFileUploadError(req.getFileUploadException().getLocalizedMessage(), req, response);
-		} catch (FileUploadException e) {
-			forwardToFileUploadError(e.getLocalizedMessage(), req, response);
 			return;
 		}
 

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

@@ -35,15 +35,25 @@ import org.apache.commons.fileupload.servlet.ServletFileUpload;
  * uploaded in a given field, the length of the file will be 0.
  * </p>
  * <p>
- * Most methods are declared here, and simply delegate to the wrapped request.
+ * If the uploaded file(s) would be larger than the <code>maxFileSize</code>,
- * Methods that have to do with parameters or files are handled differently for
+ * {@link #parseRequest(HttpServletRequest, int)} does not throw an Exception.
- * simple requests and multipart request, and are implemented in the
+ * Instead, it records the exception in a request attribute named by
- * sub-classes.
+ * {@link #FILE_UPLOAD_EXCEPTION}. This attribute can be accessed directly, or
+ * indirectly via the methods {@link #hasFileUploadException()} and
+ * {@link #getFileUploadException()}. If there is an exception, the file item
+ * map (see above) will still be non-null, but it will be empty.
+ * </p>
+ * <p>
+ * Most methods are declared here simply delegate to the wrapped request.
+ * Methods that have to do with parameters, files, or parsing exceptions, are
+ * handled differently for simple requests and multipart request, and are
+ * implemented in the sub-classes.
  * </p>
  */
 @SuppressWarnings("deprecation")
 public abstract class FileUploadServletRequest implements HttpServletRequest {
 	public static final String FILE_ITEM_MAP = "file_item_map";
+	public static final String FILE_UPLOAD_EXCEPTION = "file_upload_exception";
 
 	// ----------------------------------------------------------------------
 	// The factory method
@@ -53,8 +63,7 @@ public abstract class FileUploadServletRequest implements HttpServletRequest {
 	 * Wrap this {@link HttpServletRequest} in an appropriate wrapper class.
 	 */
 	public static FileUploadServletRequest parseRequest(
-			HttpServletRequest request, int maxFileSize) throws IOException,
+			HttpServletRequest request, int maxFileSize) throws IOException {
-			FileUploadException {
 		boolean isMultipart = ServletFileUpload.isMultipartContent(request);
 		if (isMultipart) {
 			return new MultipartHttpServletRequest(request, maxFileSize);
@@ -97,6 +106,17 @@ public abstract class FileUploadServletRequest implements HttpServletRequest {
 	 */
 	public abstract FileItem getFileItem(String string);
 
+	/**
+	 * Was there an exception when uploading the file items?
+	 */
+	public abstract boolean hasFileUploadException();
+
+	/**
+	 * Get the exception that occurred when uploading the file items. If no such
+	 * exception, return <code>null</code>.
+	 */
+	public abstract FileUploadException getFileUploadException();
+
 	// ----------------------------------------------------------------------
 	// Delegated methods.
 	// ----------------------------------------------------------------------

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

@@ -37,13 +37,14 @@ class MultipartHttpServletRequest extends FileUploadServletRequest {
 
 	private final Map<String, List<String>> parameters;
 	private final Map<String, List<FileItem>> files;
+	private FileUploadException fileUploadException;
 
 	/**
 	 * Parse the multipart request. Store the info about the request parameters
 	 * and the uploaded files.
 	 */
 	public MultipartHttpServletRequest(HttpServletRequest request,
-			int maxFileSize) throws IOException, FileUploadException {
+			int maxFileSize) throws IOException {
 		super(request);
 
 		Map<String, List<String>> parameters = new HashMap<String, List<String>>();
@@ -54,20 +55,26 @@ class MultipartHttpServletRequest extends FileUploadServletRequest {
 
 		parseQueryString(request.getQueryString(), parameters);
 
-		List<FileItem> items = parseRequestIntoFileItems(request, upload);
+		try {
-		for (FileItem item : items) {
+			List<FileItem> items = parseRequestIntoFileItems(request, upload);
-			// Process a regular form field
+
-			if (item.isFormField()) {
+			for (FileItem item : items) {
-				addToParameters(parameters, item.getFieldName(), item
+				// Process a regular form field
-						.getString("UTF-8"));
+				if (item.isFormField()) {
-				log.debug("Form field (parameter) " + item.getFieldName() + "="
+					addToParameters(parameters, item.getFieldName(), item
-						+ item.getString());
+							.getString("UTF-8"));
-			} else {
+					log.debug("Form field (parameter) " + item.getFieldName()
-				addToFileItems(files, item);
+							+ "=" + item.getString());
-				log
+				} else {
-						.debug("File " + item.getFieldName() + ": "
+					addToFileItems(files, item);
-								+ item.getName());
+					log.debug("File " + item.getFieldName() + ": "
+							+ item.getName());
+				}
 			}
+		} catch (FileUploadException e) {
+			fileUploadException = e;
+			request.setAttribute(
+					FileUploadServletRequest.FILE_UPLOAD_EXCEPTION, e);
 		}
 
 		this.parameters = Collections.unmodifiableMap(parameters);
@@ -165,7 +172,8 @@ class MultipartHttpServletRequest extends FileUploadServletRequest {
 	}
 
 	// ----------------------------------------------------------------------
-	// This is a multipart request, so make the file info available.
+	// This is a multipart request, so make the file info available. If there
+	// was an exception during parsing, make that available too.
 	// ----------------------------------------------------------------------
 
 	@Override
@@ -201,6 +209,16 @@ class MultipartHttpServletRequest extends FileUploadServletRequest {
 		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

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

@@ -10,6 +10,7 @@ import java.util.Map;
 import javax.servlet.http.HttpServletRequest;
 
 import org.apache.commons.fileupload.FileItem;
+import org.apache.commons.fileupload.FileUploadException;
 
 /**
  * A wrapper for a servlet request that does not hold multipart content. Pass
@@ -24,7 +25,7 @@ class SimpleHttpServletRequestWrapper extends FileUploadServletRequest {
 	}
 
 	// ----------------------------------------------------------------------
-	// Not a multipart request, so there are no files.
+	// Not a multipart request, so there are no files or upload exceptions.
 	// ----------------------------------------------------------------------
 
 	@Override
@@ -42,6 +43,16 @@ class SimpleHttpServletRequestWrapper extends FileUploadServletRequest {
 		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
 	// delegated.