[I] problems with upload to resource [wicket]

["reiern70 (via GitHub)" <gi...@apache.org>]

reiern70 opened a new issue, #1670:
URL: https://github.com/apache/wicket/issues/1670

   Bugs found in Wicket's upload to a resource while working on #36, to report to Wicket. Found with Wicket **11.0.0-SNAPSHOT** (wicket-core built 2026-10-06), Jetty, Chrome and Firefox. Classes are in `org.apache.wicket.markup.html.form.upload.resource` unless said otherwise; line numbers are those of that snapshot.
   
   Our code works around each bug (see the end). Once Wicket fixes them, the work-arounds can go.
   
   ---
   
   ### 1. A failed upload ends on the error page instead of `onUploadFailure`
   
   **Where:** `FileUploadToResourceField`, the Ajax behavior's `respond()` (lines 276–284), with Wicket's text `uploadFailed = File failed to upload: ${exception.localizedMessage}` (`org/apache/wicket/Application.utf8.properties`, line 66).
   
   **What happens:** for any error other than "no file selected" or "canceled", the field builds the model of the message like this:
   
   ```java
   model.put("exception", errorMessage);   // errorMessage is a String: the key the resource sent
   ...
   errorMessage = getString(errorMessage, Model.ofMap(model));
   ```
   
   `AbstractFileUploadResource` sends the key `uploadFailed` whenever the upload fails for a reason other than a size or count limit: the upload token is invalid (line 122), the file manager throws while saving (disk full, permissions, ...), or any other exception (line 188), such as the one of bug 3. `Form` puts the `FileUploadException` itself into `exception`, but here it's a String, so the interpolation of `${exception.localizedMessage}` fails:
   
   ```
   WicketRuntimeException: Property could not be resolved for class: class java.lang.String expression: localizedMessage
   ```
   
   The Ajax request fails, and the user lands on Wicket's error page. `onUploadFailure` is never called.
   
   **To reproduce:** a `FileUploadToResourceField` whose `IUploadsFileManager.save()` throws a `RuntimeException`; upload any file.
   
   **Expected:** `onUploadFailure(target, "File failed to upload…")`. Fix: give the resource's failures a text of their own without `${exception…}` (such as `uploadToResourceFailed = The file failed to upload.`), or put a real exception, or an object with a `localizedMessage`, into the model.
   
   ### 2. Any error key the field doesn't know ends on the error page too, and an HTTP error or a lost connection reads "File must be less than …"
   
   **Where:** `FileUploadToResourceField.respond()` (lines 263–286) and `FileUploadToResourceField.js` (lines 83–94).
   
   **What happens:** the field uses whatever `errorMessage` the browser sends as a resource key: `getString(errorMessage, …)`. The script sends:
   
   - `xhr.statusText` when the resource answers with a status other than 2xx (line 84) or the request fails (line 91);
   - what the resource answered, which with debug logging on is the raw `fux.getMessage()` (`AbstractFileUploadResource`, line 193).
   
   So:
   
   - **Over HTTP/1.1,** a status text such as `Internal Server Error`, `Not Found` or `URI Too Long` is not a resource key. `getString` throws `MissingResourceException` (the default `throwExceptionOnMissingResource`), and the user lands on the error page.
   - **Over HTTP/2, and for every network failure,** `statusText` is empty, and the field turns an empty message into `uploadTooLarge` (lines 277–279). A 500, a server that is down, or a connection lost all read *"File must be less than 20 MB"*. That mapping is right only when the server resets the connection because the `Content-Length` is beyond `maxSize`.
   - **With debug logging on,** the server's exception message is sent to the browser, then used as a key: `MissingResourceException` again. If the message is null, it reads "too large".
   
   **Expected:** the field accepts only the keys it knows (`uploadTooLarge`, `uploadSingleFileTooLarge`, `uploadTooManyFiles`, `uploadFailed`, `wicket.no.files.selected`, `upload.canceled`) and takes anything else as `uploadFailed`. The script tells a network failure apart from an HTTP status (send the status code), so that only a reset upload beyond `maxSize` reads "too large".
   
   ### 3. A second upload of the same field while the first runs breaks the first
   
   **Where:** `MultipartServletWebRequestImpl` (`org.apache.wicket.protocol.http.servlet`, lines 441–473) and `FileUploadToResourceField.js` `upload()` (lines 46–105).
   
   **What happens:** the progress of an upload is kept in the HTTP session under a key made of the field's id (`SESSION_KEY + ":" + uploadId`), so two uploads of one field share it. Calling `upload()` again while an upload runs (a second drop, a second click) starts a second request: the script doesn't check `this.xhr`. When one of them completes, `onUploadCompleted()` removes the shared `UploadInfo`. The next `onUploadUpdate()` of the other one throws
   
   ```
   IllegalStateException: could not find UploadInfo object in session which should have been set when uploaded started
   ```
   
   `AbstractFileUploadResource` catches that as `uploadFailed`, and bug 1 lands the user on the error page. Also, `this.xhr` now holds only the second request, so `cancel()` can no longer cancel the first.
   
   **To reproduce:** a slow upload (a big file, or a throttled network) and `Wicket.Timer[id].upload()` called twice.
   
   **Expected:** `upload()` refuses, or queues, while an upload of the field runs; or each upload gets an id of its own for its progress.
   
   ### 4. Beyond about 65 files, the page never hears of the upload (HTTP 414)
   
   **Where:** `FileUploadToResourceField.js`, lines 80–81.
   
   **What happens:** after a successful upload, the script reports the received files to the field with `Wicket.Ajax.get`, and the JSON of every file (`filesInfo`) goes into the URL. With 80 short file names the URL is about 10,000 characters, beyond the 8 KB limit of the request line in Jetty (Tomcat has the same default). The server answers `414 URI Too Long`, and `onUploadSuccess` is never called, although the files are stored on the server. 40 files (about 5,000 characters) still go. Longer names fail with fewer files. The field sets no limit to the number of files by default (`fileCountMax = -1`).
   
   **Expected:** report the files with `Wicket.Ajax.post` (we changed `attrs.m` to `POST` for these requests, and the 80-file report then got 200 with all 80 listed).
   
   ### 5. Two files of the same name overwrite each other
   
   **Where:** `FolderUploadsFileManager.save()` / `getFile()` (lines 92–134), and `FileUploadToResourceField.UploadInfo.equals()` / `hashCode()` (lines 106–151).
   
   **What happens:** the folder file manager stores each file under its client file name, in the folder of the field. Two files of the same name in one upload are stored at one path: the second one overwrites the first. The field is still told about both. `FileModel.getObject()` gives both `UploadInfo` the same file, and `UploadInfo` is equal by client name only. The page thinks it received two files and has one, twice. Same names come up with `webkitdirectory` inputs (sub-folders), with drag & drop, or when a script puts files from two picks into the input.
   
   **Expected:** store each file of an upload under a name of its own (an index or a UUID, keeping the client name in the `UploadInfo`), or refuse the upload with a message.
   
   ### 6. Smaller problems of `FileUploadToResourceField.js`
   
   Found by reading the script; unlike 1–5, not reproduced in a browser yet.
   
   - **`cancel()` after an upload has ended says it canceled it.** `this.xhr` is deleted only by `cancel()`, never when the upload ends, so the branch *"Too late to cancel upload"* (line 116) is never reached after a first upload. `abort()` on a finished XHR does nothing and fires no `abort` event, so the server isn't told. But `clientSideCancelCallBack()` runs, and the log says *"has been canceled!"*.
   - **The return value of the before-send callback is ignored.** `getClientBeforeSendCallBack()` documents *"See jQuery.ajax documentation"* and defaults to `function (xhr, settings) { return true; }`. But the script calls it as `self.clientBeforeSendCallBack(xhr)` (line 102), without `settings`, and sends the upload even when it returns `false`.
   - **A response that isn't JSON leaves the field waiting.** If `JSON.parse` fails (line 67), the script only logs it and returns: no error callback runs, and the server isn't told.
   
   ### 7. The Javadoc of `AbstractFileUploadResource` says it uploads a single file
   
   Its class Javadoc (line 58) says *"Here we are just using plain jQuery code at client side to upload a single file."* The script uses plain `XMLHttpRequest`, not jQuery, and uploads every file of the input in one request (`formData.append` in a loop, lines 51–55). We uploaded 5 files at once, and 10 in two picks, and all of them arrived.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@wicket.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [I] problems with upload to resource [wicket]

["reiern70 (via GitHub)" <gi...@apache.org>]

reiern70 closed issue #1670: problems with upload to resource
URL: https://github.com/apache/wicket/issues/1670


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@wicket.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org