From 6cffc40c33f9b61cb6825d1d0aa3d8e9418a7b79 Mon Sep 17 00:00:00 2001 From: jeb228 Date: Tue, 13 Jul 2010 14:25:54 +0000 Subject: [PATCH] NIHVIVO-161 - Improve the exception handling for files that exceed the maximum size. --- .../FedoraDatastreamController.java | 10 ++-- .../controller/edit/N3MultiPartUpload.java | 12 ++--- .../controller/jena/JenaXMLFileUpload.java | 12 ++--- .../controller/jena/RDFUploadController.java | 11 ++--- .../FileUploadServletRequest.java | 32 ++++++++++--- .../MultipartHttpServletRequest.java | 48 +++++++++++++------ .../SimpleHttpServletRequestWrapper.java | 13 ++++- 7 files changed, 86 insertions(+), 52 deletions(-) diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/FedoraDatastreamController.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/FedoraDatastreamController.java index 34e33a36f..c1221087b 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/FedoraDatastreamController.java +++ b/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; - try { - req = FileUploadServletRequest - .parseRequest(rawRequest, maxFileSize); - } catch (FileUploadException e) { - throw new FdcException("Size limit exceeded: " + e.getLocalizedMessage()); + FileUploadServletRequest req = FileUploadServletRequest.parseRequest(rawRequest, maxFileSize); + if (req.hasFileUploadException()) { + throw new FdcException("Size limit exceeded: " + req.getFileUploadException().getLocalizedMessage()); } if (!req.isMultipart()) { throw new FdcException("Must POST a multipart encoded request"); diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/edit/N3MultiPartUpload.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/edit/N3MultiPartUpload.java index e6abc28ec..babaf6e1f 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/edit/N3MultiPartUpload.java +++ b/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; - try { - request = FileUploadServletRequest - .parseRequest(rawRequest, maxFileSize); - } catch (FileUploadException e) { + FileUploadServletRequest request = FileUploadServletRequest.parseRequest(rawRequest, maxFileSize); + if (request.hasFileUploadException()) { // 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"); diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/jena/JenaXMLFileUpload.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/jena/JenaXMLFileUpload.java index dbfb47ce5..0a3f0b17b 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/jena/JenaXMLFileUpload.java +++ b/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; - try { - request = FileUploadServletRequest - .parseRequest(rawRequest, maxFileSize); - } catch (FileUploadException e) { - // TODO: forward to error message - throw new ServletException("Size limit exceeded: " + e.getLocalizedMessage()); + FileUploadServletRequest request = FileUploadServletRequest.parseRequest(rawRequest, maxFileSize); + if (request.hasFileUploadException()) { + throw new ServletException("Size limit exceeded: " + + request.getFileUploadException().getLocalizedMessage()); } if (request.isMultipart()) { log.debug("multipart content detected"); diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/jena/RDFUploadController.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/jena/RDFUploadController.java index c359050d9..bf293e89d 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/controller/jena/RDFUploadController.java +++ b/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; - try { - req = FileUploadServletRequest.parseRequest(rawRequest, - maxFileSizeInBytes); - } catch (FileUploadException e) { - forwardToFileUploadError(e.getLocalizedMessage(), req, response); + FileUploadServletRequest req = FileUploadServletRequest.parseRequest(rawRequest, + maxFileSizeInBytes); + if (req.hasFileUploadException()) { + forwardToFileUploadError(req.getFileUploadException().getLocalizedMessage(), req, response); return; } diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/filestorage/uploadrequest/FileUploadServletRequest.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/filestorage/uploadrequest/FileUploadServletRequest.java index 2e3be3253..499744f9c 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/filestorage/uploadrequest/FileUploadServletRequest.java +++ b/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. *

*

- * Most methods are declared here, and simply delegate to the wrapped request. - * Methods that have to do with parameters or files are handled differently for - * simple requests and multipart request, and are implemented in the - * sub-classes. + * If the uploaded file(s) would be larger than the maxFileSize, + * {@link #parseRequest(HttpServletRequest, int)} does not throw an Exception. + * Instead, it records the exception in a request attribute named by + * {@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. + *

+ *

+ * 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. *

*/ @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, - FileUploadException { + HttpServletRequest request, int maxFileSize) throws IOException { 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 null. + */ + public abstract FileUploadException getFileUploadException(); + // ---------------------------------------------------------------------- // Delegated methods. // ---------------------------------------------------------------------- diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/filestorage/uploadrequest/MultipartHttpServletRequest.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/filestorage/uploadrequest/MultipartHttpServletRequest.java index aa9f78582..305fd6a43 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/filestorage/uploadrequest/MultipartHttpServletRequest.java +++ b/webapp/src/edu/cornell/mannlib/vitro/webapp/filestorage/uploadrequest/MultipartHttpServletRequest.java @@ -37,13 +37,14 @@ class MultipartHttpServletRequest extends FileUploadServletRequest { private final Map> parameters; private final Map> 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> parameters = new HashMap>(); @@ -54,20 +55,26 @@ class MultipartHttpServletRequest extends FileUploadServletRequest { parseQueryString(request.getQueryString(), parameters); - List 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()); + try { + List 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); } 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 diff --git a/webapp/src/edu/cornell/mannlib/vitro/webapp/filestorage/uploadrequest/SimpleHttpServletRequestWrapper.java b/webapp/src/edu/cornell/mannlib/vitro/webapp/filestorage/uploadrequest/SimpleHttpServletRequestWrapper.java index 11f90360d..d7ffc1274 100644 --- a/webapp/src/edu/cornell/mannlib/vitro/webapp/filestorage/uploadrequest/SimpleHttpServletRequestWrapper.java +++ b/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.