From a0f9d5a0b0ed16f5c1adc52f2f85b6b9cdaa383b Mon Sep 17 00:00:00 2001 From: Roman Grigoriadi Date: Tue, 28 Oct 2014 07:19:15 +0100 Subject: [PATCH 1/2] WICKET-5735 maxFileSize per file in upload request --- .../org/apache/wicket/Application.properties | 1 + .../apache/wicket/markup/html/form/Form.java | 62 +++++++------ .../MultipartServletWebRequestImpl.java | 86 ++++++++++--------- .../http/servlet/ServletWebRequest.java | 29 +++---- .../examples/upload/MultiUploadPage.java | 1 + .../examples/upload/UploadPage.properties | 1 + .../atmosphere/AtmosphereWebRequest.java | 23 +++-- 7 files changed, 109 insertions(+), 94 deletions(-) diff --git a/wicket-core/src/main/java/org/apache/wicket/Application.properties b/wicket-core/src/main/java/org/apache/wicket/Application.properties index 1408c9ad8fc..0998fbde5d5 100644 --- a/wicket-core/src/main/java/org/apache/wicket/Application.properties +++ b/wicket-core/src/main/java/org/apache/wicket/Application.properties @@ -61,6 +61,7 @@ org.apache.wicket.mfu.caption.limited=Files (maximum ${max}): org.apache.wicket.mfu.delete=Delete uploadTooLarge = File must be less than ${maxSize}. +uploadSingleFileTooLarge = Each of files must be less than ${maxSize}. uploadFailed = File failed to upload: ${exception.localizedMessage} FeedbackMessage.CSS.undefined=feedbackPanelUNDEFINED diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/Form.java b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/Form.java index dbede58fb0e..fae27e792cf 100644 --- a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/Form.java +++ b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/Form.java @@ -16,16 +16,6 @@ */ package org.apache.wicket.markup.html.form; -import java.util.ArrayList; -import java.util.Collection; -import java.util.Collections; -import java.util.HashMap; -import java.util.List; -import java.util.Locale; -import java.util.Map; - -import javax.servlet.http.HttpServletRequest; - import org.apache.commons.fileupload.FileUploadBase; import org.apache.commons.fileupload.FileUploadException; import org.apache.wicket.Component; @@ -45,13 +35,7 @@ import org.apache.wicket.model.IModel; import org.apache.wicket.model.Model; import org.apache.wicket.protocol.http.servlet.ServletWebRequest; -import org.apache.wicket.request.IRequestHandler; -import org.apache.wicket.request.IRequestMapper; -import org.apache.wicket.request.IRequestParameters; -import org.apache.wicket.request.Request; -import org.apache.wicket.request.Response; -import org.apache.wicket.request.Url; -import org.apache.wicket.request.UrlRenderer; +import org.apache.wicket.request.*; import org.apache.wicket.request.http.WebRequest; import org.apache.wicket.request.mapper.parameter.PageParameters; import org.apache.wicket.util.encoding.UrlDecoder; @@ -70,6 +54,9 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import javax.servlet.http.HttpServletRequest; +import java.util.*; + /** * Container for {@link FormComponent}s (such as {@link CheckBox}es, {@link ListChoice}s or @@ -254,6 +241,7 @@ public void component(final Component component, final IVisit visit) private static final String UPLOAD_FAILED_RESOURCE_KEY = "uploadFailed"; private static final String UPLOAD_TOO_LARGE_RESOURCE_KEY = "uploadTooLarge"; + private static final String UPLOAD_SINGLE_FILE_TOO_LARGE_RESOURCE_KEY = "uploadSingleFileTooLarge"; /** * Any default IFormSubmittingComponent. If set, a hidden submit component will be rendered @@ -274,6 +262,11 @@ public void component(final Component component, final IVisit visit) */ private Bytes maxSize = null; + /** + * Maximum size of file of upload in bytes (if there are more than one) in request. + */ + private Bytes fileMaxSize; + /** True if the form has enctype of multipart/form-data */ private short multiPart = 0; @@ -574,7 +567,15 @@ public void component(Form component, IVisit visit) return maxSize[0]; } - /** + /** + * Gets maximum size for each file of an upload. + * @return + */ + public Bytes getFileMaxSize() { + return fileMaxSize; + } + + /** * Returns the root form or this, if this is the root form. * * @return root form or this form @@ -1041,7 +1042,15 @@ public final void setMaxSize(final Bytes maxSize) this.maxSize = maxSize; } - /** + /** + * Sets maximum size of each file in upload request. + * @param fileMaxSize + */ + public void setFileMaxSize(Bytes fileMaxSize) { + this.fileMaxSize = fileMaxSize; + } + + /** * Set to true to use enctype='multipart/form-data', and to process file uploads by default * multiPart = false * @@ -1380,7 +1389,7 @@ protected boolean handleMultiPart() { ServletWebRequest request = (ServletWebRequest)getRequest(); final WebRequest multipartWebRequest = request.newMultipartWebRequest(getMaxSize(), - getPage().getId()); + fileMaxSize, getPage().getId()); // TODO: Can't this be detected from header? getRequestCycle().setRequest(multipartWebRequest); } @@ -1418,10 +1427,15 @@ protected void onFileUploadException(final FileUploadException e, String msg = getString(UPLOAD_TOO_LARGE_RESOURCE_KEY, Model.ofMap(model)); error(msg); } - else - { - String msg = getString(UPLOAD_FAILED_RESOURCE_KEY, Model.ofMap(model)); - error(msg); + else if (e instanceof FileUploadBase.FileSizeLimitExceededException) + { + String msg = getString(UPLOAD_SINGLE_FILE_TOO_LARGE_RESOURCE_KEY, Model.ofMap(model)); + error(msg); + } + else + { + String msg = getString(UPLOAD_FAILED_RESOURCE_KEY, Model.ofMap(model)); + error(msg); log.warn(msg, e); } diff --git a/wicket-core/src/main/java/org/apache/wicket/protocol/http/servlet/MultipartServletWebRequestImpl.java b/wicket-core/src/main/java/org/apache/wicket/protocol/http/servlet/MultipartServletWebRequestImpl.java index 75712bc9603..550b2b1147a 100644 --- a/wicket-core/src/main/java/org/apache/wicket/protocol/http/servlet/MultipartServletWebRequestImpl.java +++ b/wicket-core/src/main/java/org/apache/wicket/protocol/http/servlet/MultipartServletWebRequestImpl.java @@ -16,16 +16,6 @@ */ package org.apache.wicket.protocol.http.servlet; -import java.io.IOException; -import java.io.InputStream; -import java.io.UnsupportedEncodingException; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; -import java.util.Map; - -import javax.servlet.http.HttpServletRequest; - import org.apache.commons.fileupload.FileItem; import org.apache.commons.fileupload.FileItemFactory; import org.apache.commons.fileupload.FileUploadBase; @@ -43,6 +33,15 @@ import org.apache.wicket.util.string.StringValue; import org.apache.wicket.util.value.ValueMap; +import javax.servlet.http.HttpServletRequest; +import java.io.IOException; +import java.io.InputStream; +import java.io.UnsupportedEncodingException; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + /** * Servlet specific WebRequest subclass for multipart content uploads. @@ -88,9 +87,9 @@ public class MultipartServletWebRequestImpl extends MultipartServletWebRequest * Thrown if something goes wrong with upload */ public MultipartServletWebRequestImpl(HttpServletRequest request, String filterPrefix, - Bytes maxSize, String upload) throws FileUploadException + Bytes maxSize, Bytes fileMaxSize, String upload) throws FileUploadException { - this(request, filterPrefix, maxSize, upload, new DiskFileItemFactory() + this(request, filterPrefix, maxSize, fileMaxSize, upload, new DiskFileItemFactory() { @Override public FileCleaningTracker getFileCleaningTracker() @@ -121,7 +120,7 @@ public FileCleaningTracker getFileCleaningTracker() * Thrown if something goes wrong with upload */ public MultipartServletWebRequestImpl(HttpServletRequest request, String filterPrefix, - Bytes maxSize, String upload, FileItemFactory factory) throws FileUploadException + Bytes maxSize, Bytes fileMaxSize, String upload, FileItemFactory factory) throws FileUploadException { super(request, filterPrefix); @@ -139,31 +138,23 @@ public MultipartServletWebRequestImpl(HttpServletRequest request, String filterP "ServletRequest does not contain multipart content. One possible solution is to explicitly call Form.setMultipart(true), Wicket tries its best to auto-detect multipart forms but there are certain situation where it cannot."); } - // Configure the factory here, if desired. - ServletFileUpload fileUpload = new ServletFileUpload(factory); - - // The encoding that will be used to decode the string parameters - // It should NOT be null at this point, but it may be - // especially if the older Servlet API 2.2 is used - String encoding = request.getCharacterEncoding(); + // The encoding that will be used to decode the string parameters + // It should NOT be null at this point, but it may be + // especially if the older Servlet API 2.2 is used + String encoding = request.getCharacterEncoding(); - // The encoding can also be null when using multipart/form-data encoded forms. - // In that case we use the [application-encoding] which we always demand using - // the attribute 'accept-encoding' in wicket forms. - if (encoding == null) - { - encoding = Application.get().getRequestCycleSettings().getResponseRequestEncoding(); - } + // The encoding can also be null when using multipart/form-data encoded forms. + // In that case we use the [application-encoding] which we always demand using + // the attribute 'accept-encoding' in wicket forms. + if (encoding == null) + { + encoding = Application.get().getRequestCycleSettings().getResponseRequestEncoding(); + } - // set encoding specifically when we found it - if (encoding != null) - { - fileUpload.setHeaderEncoding(encoding); - } + FileUploadBase fileUpload = newFileUpload(maxSize, fileMaxSize, factory, encoding); - fileUpload.setSizeMax(maxSize.bytes()); - final List items; + final List items; if (wantUploadProgressUpdates()) { @@ -233,7 +224,24 @@ public InputStream getInputStream() throws IOException } } - /** + protected FileUploadBase newFileUpload(Bytes maxSize, Bytes fileMaxSize, FileItemFactory factory, String encoding) { + // Configure the factory here, if desired. + ServletFileUpload fileUpload = new ServletFileUpload(factory); + + // set encoding specifically when we found it + if (encoding != null) + { + fileUpload.setHeaderEncoding(encoding); + } + + fileUpload.setSizeMax(maxSize.bytes()); + if (fileMaxSize != null) { + fileUpload.setFileSizeMax(fileMaxSize.bytes()); + } + return fileUpload; + } + + /** * Adds a parameter to the parameters value map * * @param name @@ -415,7 +423,7 @@ public int read(byte[] b, int off, int len) throws IOException } @Override - public MultipartServletWebRequest newMultipartWebRequest(Bytes maxSize, String upload) + public MultipartServletWebRequest newMultipartWebRequest(Bytes maxSize, Bytes maxFileSize, String upload) throws FileUploadException { for (Map.Entry> entry : files.entrySet()) @@ -423,12 +431,12 @@ public MultipartServletWebRequest newMultipartWebRequest(Bytes maxSize, String u List fileItems = entry.getValue(); for (FileItem fileItem : fileItems) { - if (fileItem.getSize() > maxSize.bytes()) + if (fileItem.getSize() > maxFileSize.bytes()) { String fieldName = entry.getKey(); FileUploadException fslex = new FileUploadBase.FileSizeLimitExceededException("The field " + fieldName + " exceeds its maximum permitted " + " size of " + - maxSize + " characters.", fileItem.getSize(), maxSize.bytes()); + maxSize + " characters.", fileItem.getSize(), maxFileSize.bytes()); throw fslex; } } @@ -437,7 +445,7 @@ public MultipartServletWebRequest newMultipartWebRequest(Bytes maxSize, String u } @Override - public MultipartServletWebRequest newMultipartWebRequest(Bytes maxSize, String upload, + public MultipartServletWebRequest newMultipartWebRequest(Bytes maxSize, Bytes fileMaxSize, String upload, FileItemFactory factory) throws FileUploadException { return this; diff --git a/wicket-core/src/main/java/org/apache/wicket/protocol/http/servlet/ServletWebRequest.java b/wicket-core/src/main/java/org/apache/wicket/protocol/http/servlet/ServletWebRequest.java index a4ecc20b6c7..6f2cb5b2a83 100644 --- a/wicket-core/src/main/java/org/apache/wicket/protocol/http/servlet/ServletWebRequest.java +++ b/wicket-core/src/main/java/org/apache/wicket/protocol/http/servlet/ServletWebRequest.java @@ -16,21 +16,6 @@ */ package org.apache.wicket.protocol.http.servlet; -import java.nio.charset.Charset; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; -import java.util.Enumeration; -import java.util.HashMap; -import java.util.List; -import java.util.Locale; -import java.util.Map; -import java.util.Set; - -import javax.servlet.http.Cookie; -import javax.servlet.http.HttpServletRequest; -import javax.servlet.http.HttpServletResponse; - import org.apache.commons.fileupload.FileItemFactory; import org.apache.commons.fileupload.FileUploadException; import org.apache.wicket.protocol.http.RequestUtils; @@ -49,6 +34,12 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import javax.servlet.http.Cookie; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; +import java.nio.charset.Charset; +import java.util.*; + /** * {@link WebRequest} subclass that wraps a {@link HttpServletRequest} object. * @@ -425,11 +416,11 @@ public IRequestParameters getPostParameters() * @return multipart request * @throws FileUploadException */ - public MultipartServletWebRequest newMultipartWebRequest(Bytes maxSize, String upload) + public MultipartServletWebRequest newMultipartWebRequest(Bytes maxSize, Bytes fileMaxSize, String upload) throws FileUploadException { return new MultipartServletWebRequestImpl(getContainerRequest(), filterPrefix, maxSize, - upload); + fileMaxSize, upload); } /** @@ -442,11 +433,11 @@ public MultipartServletWebRequest newMultipartWebRequest(Bytes maxSize, String u * @return multipart request * @throws FileUploadException */ - public MultipartServletWebRequest newMultipartWebRequest(Bytes maxSize, String upload, + public MultipartServletWebRequest newMultipartWebRequest(Bytes maxSize, Bytes fileMaxSize, String upload, FileItemFactory factory) throws FileUploadException { return new MultipartServletWebRequestImpl(getContainerRequest(), filterPrefix, maxSize, - upload, factory); + fileMaxSize, upload, factory); } private static final Logger logger = LoggerFactory.getLogger(ServletWebRequest.class); diff --git a/wicket-examples/src/main/java/org/apache/wicket/examples/upload/MultiUploadPage.java b/wicket-examples/src/main/java/org/apache/wicket/examples/upload/MultiUploadPage.java index dbac3c34359..cbb39ea1017 100644 --- a/wicket-examples/src/main/java/org/apache/wicket/examples/upload/MultiUploadPage.java +++ b/wicket-examples/src/main/java/org/apache/wicket/examples/upload/MultiUploadPage.java @@ -125,6 +125,7 @@ public FileUploadForm(String name) // Set maximum size to 100K for demo purposes setMaxSize(Bytes.kilobytes(100)); + setFileMaxSize(Bytes.kilobytes(30)); } /** diff --git a/wicket-examples/src/main/java/org/apache/wicket/examples/upload/UploadPage.properties b/wicket-examples/src/main/java/org/apache/wicket/examples/upload/UploadPage.properties index 7ebae3af5f7..0618306bc24 100644 --- a/wicket-examples/src/main/java/org/apache/wicket/examples/upload/UploadPage.properties +++ b/wicket-examples/src/main/java/org/apache/wicket/examples/upload/UploadPage.properties @@ -16,4 +16,5 @@ simpleUpload.uploadTooLarge = Simple - File must be less than ${maxSize}. simpleUpload.uploadFailed = Simple - File failed to upload: ${exception.localizedMessage} progressUpload.uploadTooLarge = Progress - File must be less than ${maxSize}. +progressUpload.uploadSingleFileTooLarge = Progress - Each of files must be less than ${maxSize}. progressUpload.uploadFailed = Progress - File failed to upload: ${exception.localizedMessage} diff --git a/wicket-experimental/wicket-atmosphere/src/main/java/org/apache/wicket/atmosphere/AtmosphereWebRequest.java b/wicket-experimental/wicket-atmosphere/src/main/java/org/apache/wicket/atmosphere/AtmosphereWebRequest.java index 592227018ca..d4c68b01ca5 100644 --- a/wicket-experimental/wicket-atmosphere/src/main/java/org/apache/wicket/atmosphere/AtmosphereWebRequest.java +++ b/wicket-experimental/wicket-atmosphere/src/main/java/org/apache/wicket/atmosphere/AtmosphereWebRequest.java @@ -16,14 +16,6 @@ */ package org.apache.wicket.atmosphere; -import java.nio.charset.Charset; -import java.util.Iterator; -import java.util.List; -import java.util.Locale; - -import javax.servlet.http.Cookie; -import javax.servlet.http.HttpServletRequest; - import org.apache.commons.fileupload.FileItemFactory; import org.apache.commons.fileupload.FileUploadException; import org.apache.wicket.protocol.http.RequestUtils; @@ -34,6 +26,13 @@ import org.apache.wicket.util.lang.Bytes; import org.apache.wicket.util.time.Time; +import javax.servlet.http.Cookie; +import javax.servlet.http.HttpServletRequest; +import java.nio.charset.Charset; +import java.util.Iterator; +import java.util.List; +import java.util.Locale; + /** * Internal request to signal the processing of an event. This request will be mapped by * {@link AtmosphereRequestMapper} to an {@link AtmosphereRequestHandler}. The response will be @@ -188,17 +187,17 @@ public ServletWebRequest cloneWithUrl(Url url) } @Override - public MultipartServletWebRequest newMultipartWebRequest(Bytes maxSize, String upload) + public MultipartServletWebRequest newMultipartWebRequest(Bytes maxSize, Bytes fileMaxSize, String upload) throws FileUploadException { - return wrappedRequest.newMultipartWebRequest(maxSize, upload); + return wrappedRequest.newMultipartWebRequest(maxSize, fileMaxSize, upload); } @Override - public MultipartServletWebRequest newMultipartWebRequest(Bytes maxSize, String upload, + public MultipartServletWebRequest newMultipartWebRequest(Bytes maxSize, Bytes fileMaxSize, String upload, FileItemFactory factory) throws FileUploadException { - return wrappedRequest.newMultipartWebRequest(maxSize, upload, factory); + return wrappedRequest.newMultipartWebRequest(maxSize, fileMaxSize, upload, factory); } @Override From 46922fea222e8e4caf74c94d781c63e12fdeec19 Mon Sep 17 00:00:00 2001 From: Roman Grigoriadi Date: Tue, 28 Oct 2014 18:50:02 +0100 Subject: [PATCH 2/2] WICKET-5735 maxFileSize code review fixes --- .../MultipartServletWebRequestImpl.java | 49 +++++++++++++++++++ .../http/servlet/ServletWebRequest.java | 8 +-- 2 files changed, 54 insertions(+), 3 deletions(-) diff --git a/wicket-core/src/main/java/org/apache/wicket/protocol/http/servlet/MultipartServletWebRequestImpl.java b/wicket-core/src/main/java/org/apache/wicket/protocol/http/servlet/MultipartServletWebRequestImpl.java index 550b2b1147a..3e62ca32ee8 100644 --- a/wicket-core/src/main/java/org/apache/wicket/protocol/http/servlet/MultipartServletWebRequestImpl.java +++ b/wicket-core/src/main/java/org/apache/wicket/protocol/http/servlet/MultipartServletWebRequestImpl.java @@ -86,6 +86,30 @@ public class MultipartServletWebRequestImpl extends MultipartServletWebRequest * @throws FileUploadException * Thrown if something goes wrong with upload */ + public MultipartServletWebRequestImpl(HttpServletRequest request, String filterPrefix, + Bytes maxSize, String upload) throws FileUploadException + { + this(request, filterPrefix, maxSize, null, upload); + } + + /** + * Constructor. + * + * This constructor will use {@link DiskFileItemFactory} to store uploads. + * + * @param request + * the servlet request + * @param filterPrefix + * prefix to wicket filter mapping + * @param maxSize + * the maximum size allowed for this request + * @param fileMaxSize + * the maximum size allowed for each uploaded file for this request + * @param upload + * upload identifier for {@link UploadInfo} + * @throws FileUploadException + * Thrown if something goes wrong with upload + */ public MultipartServletWebRequestImpl(HttpServletRequest request, String filterPrefix, Bytes maxSize, Bytes fileMaxSize, String upload) throws FileUploadException { @@ -119,6 +143,31 @@ public FileCleaningTracker getFileCleaningTracker() * @throws FileUploadException * Thrown if something goes wrong with upload */ + public MultipartServletWebRequestImpl(HttpServletRequest request, String filterPrefix, + Bytes maxSize, String upload, FileItemFactory factory) throws FileUploadException + { + this(request, filterPrefix, maxSize, null, upload, factory); + } + + /** + * Constructor + * + * @param request + * the servlet request + * @param filterPrefix + * prefix to wicket filter mapping + * @param maxSize + * the maximum size allowed for this request + * @param fileMaxSize + * the maximum size allowed for each uploaded file for this request + * @param upload + * upload identifier for {@link UploadInfo} + * @param factory + * {@link DiskFileItemFactory} to use when creating file items used to represent + * uploaded files + * @throws FileUploadException + * Thrown if something goes wrong with upload + */ public MultipartServletWebRequestImpl(HttpServletRequest request, String filterPrefix, Bytes maxSize, Bytes fileMaxSize, String upload, FileItemFactory factory) throws FileUploadException { diff --git a/wicket-core/src/main/java/org/apache/wicket/protocol/http/servlet/ServletWebRequest.java b/wicket-core/src/main/java/org/apache/wicket/protocol/http/servlet/ServletWebRequest.java index 6f2cb5b2a83..3b7d826d1b0 100644 --- a/wicket-core/src/main/java/org/apache/wicket/protocol/http/servlet/ServletWebRequest.java +++ b/wicket-core/src/main/java/org/apache/wicket/protocol/http/servlet/ServletWebRequest.java @@ -410,7 +410,8 @@ public IRequestParameters getPostParameters() /** * Creates multipart web request from this request. * - * @param maxSize + * @param maxSize max allowed size of request + * @param fileMaxSize max allowed size of each file in request * @param upload * upload identifier for {@link UploadInfo} * @return multipart request @@ -425,8 +426,9 @@ public MultipartServletWebRequest newMultipartWebRequest(Bytes maxSize, Bytes fi /** * Creates multipart web request from this request. - * - * @param maxSize + * + * @param maxSize max allowed size of request + * @param fileMaxSize max allowed size of each file in request * @param upload * upload identifier for {@link UploadInfo} * @param factory