[
https://issues.apache.org/jira/browse/WICKET-5924?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18105895#comment-18105895
]
ASF subversion and git services commented on WICKET-5924:
---------------------------------------------------------
Commit 33185909e0d2d4e8b88f5f67ddbf0ae0794b9fdb in wicket's branch
refs/heads/master from Emond Papegaaij
[ https://gitbox.apache.org/repos/asf?p=wicket.git;h=33185909e0 ]
Enforce the configured upload limits on the Servlet Part fallback
MultipartServletWebRequestImpl#parseFileParts() parses with commons-fileupload,
which newFileUpload() configures from the form's maxSize, fileMaxSize and
fileCountMax. When that parse yields no items it falls back to
readServlet3Parts(), which wrapped whatever HttpServletRequest#getParts()
returned in a ServletPartFileItem and applied none of those limits. Nothing
downstream re-checks - Form#handleMultiPart() only reacts to a
FileUploadException raised during parsing - so a quiet return left the limits
inert.
That fallback is not an error path. It is the normal one wherever something else
has already consumed the request body: a @MultipartConfig servlet, Spring Boot's
multipart resolver, or any filter calling getParameter() on a multipart request,
which is why it was added for WICKET-5924. It is also the only path while
uploadProgressUpdatesEnabled is left at its default of false.
The two per-item limits, fileMaxSize and fileCountMax, are the ones this
affected. Both are consulted per parsed item, and the fallback is taken
precisely when there are no items, so neither was ever applied to the parts the
container had already produced. An upload could therefore exceed the configured
size or count, up to whatever the component that parsed the request allowed.
The aggregate maxSize was applied on this path already.
FileItemInputIteratorImpl#init() compares Content-Length against sizeMax before
reading the body, and that header survives the body having been consumed, so a
request declaring an oversized length is rejected on the commons-fileupload path
and never reaches the fallback; confirmed by reading the library.
declaredContentLengthOverMaxSizeIsRejectedBeforeTheFallback pins it down,
because the other maxSize tests here would otherwise suggest the limit was not
applied at all: they only exercise the fallback because MockHttpServletRequest
reports the length of the near-empty body it builds for a parts-only request.
readServlet3Parts() now applies all three limits as it builds the item list,
mirroring commons-fileupload's own semantics so that both paths accept and
reject the same requests: fileMaxSize against every part, form fields included,
as FileItemInputImpl has no form-field guard; fileCountMax rejecting at
count == max, with a negative value meaning unlimited; and maxSize against a
running total of the part sizes. The aggregate check is for completeness rather
than a fix for anything observed - it can only matter for a request that
declares no length at all, which is conceivable through chunked transfer
encoding but has not been demonstrated to reach the fallback with parts
attached.
The checks read only Part#getSize(), so they complete before any part is opened.
That ordering is deliberate rather than incidental: parseFileParts() calls
getString() on every form field, which pulls the whole part into a single heap
array through IOUtils#toByteArray, so a check running after the parts had been
collected would allocate exactly what it was meant to refuse - for any part
arriving without a Content-Type, which is what ServletPartFileItem treats as a
form field. The test asserts the part's stream is never opened, so a later
reordering fails the build rather than quietly reintroducing it.
The exceptions are the types the upload entry points dispatch on, so the
existing feedback messages are unchanged: FileUploadByteCountLimitException for
a single file, FileUploadFileCountLimitException for the count, and
FileUploadSizeException for the total. Both Form#onFileUploadException() and
AbstractFileUploadResource#getFileUploadExceptionKey() map those three to the
UPLOAD_* resource keys; AjaxFileDropBehavior routes any FileUploadException to
onError(). Note the constructors do not agree on argument order - byte count
takes (actual, permitted), size takes (permitted, actual).
All three entry points that accept uploads this way - Form#handleMultiPart(),
AbstractFileUploadResource behind FileUploadToResourceField, and
AjaxFileDropBehavior#onEvent() - set the limits and then call parseFileParts(),
so enforcing in readServlet3Parts() covers each of them in one place.
Also removes the per-file loop in newMultipartWebRequest(). It could not fire on
the Form path: Form#handleMultiPart() reaches
ServletWebRequest#newMultipartWebRequest(), which constructs a fresh instance,
and even when the override was reached, files was empty and fileMaxSize unset,
both being set afterwards. Its FIXME asked why the sum of the file sizes is
never compared against maxSize; it is, by commons-fileupload, against the whole
request stream.
The tests assert the corrected behaviour. Five of the eleven fail without this
change. The other six - the commons-fileupload control, the Content-Length
behaviour above, delivery of files and of form fields within the limits, the
count boundary, and the unconfigured defaults of Bytes.MAX / null / -1 - hold
either way, and guard against the enforcement rejecting uploads it should not.
Reported and originally fixed by GitHub: @deprrous.
Co-Authored-By: Claude Opus 5 <[email protected]>
> FileUploadField does not work with Servlet 3.0 multipart config
> ---------------------------------------------------------------
>
> Key: WICKET-5924
> URL: https://issues.apache.org/jira/browse/WICKET-5924
> Project: Wicket
> Issue Type: Bug
> Components: wicket
> Affects Versions: 7.0.0-M5
> Environment: Win 7, Chrome, Firefox, Jetty 9.2, Spring Boot
> Reporter: Vojtěch Krása
> Assignee: Martin Tzvetanov Grigorov
> Priority: Major
> Fix For: 7.0.0
>
> Attachments: quickstartWithoutSpringBoot_WICKET-5924.rar
>
>
> quickstart: https://github.com/krasa/krasa-wicket-spring-boot-quickstart
> - run: krasa.wicket.Start
> - go to: http://localhost:8080/
> - upload a file
> - FileUploadField#getFileUploads will be empty
> I have tried the same with normal quickstart and the same Jetty version, it
> worked.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
