This is an automated email from the ASF dual-hosted git repository. reiern70 pushed a commit to branch upload-to-resource in repository https://gitbox.apache.org/repos/asf/wicket.git
commit 0aa724c3b235fc67f9d78cc8c08f8d9d2d886622 Author: reiern70 <[email protected]> AuthorDate: Thu Oct 15 10:50:51 2026 -0500 Make uploads to a resource fail cleanly and keep files of one name apart GitHub issue #1670 FileUploadToResourceField and the resource it uploads to had several problems: - A failed upload ended on the error page instead of in onUploadFailure(). For a failure other than a limit the resource answers the key uploadFailed, whose text expects an exception in ${exception.localizedMessage}; the field put the key, a String, in its place, and the interpolation threw. Such failures now have a text of their own, uploadToResourceFailed ("The upload failed."). - The field took whatever the browser reported as a resource key. The status text of an HTTP error, or the exception message the resource sent with debug logging on, threw MissingResourceException, and an empty one (HTTP/2, a network failure) read "File must be less than ...". The field now accepts the keys of the limits, of no files selected and of a lost connection, and reads anything else as a failed upload. The script reports HTTP errors, answers that are not JSON and requests that time out as uploadFailed, and a request that got no answer as uploadConnectionLost, which the field words "The upload failed: the connection to the server was lost.". The check is the protected getUploadErrorMessage(String), so a subclass whose own script reports other keys can word them; the field still never looks up a key it does not know, since the browser can send any. The resource no longer sends exception messages to the browser; it logs the exception as an error, which it did not do at all before. - The script checks the limits of the field before sending: files beyond them are reported with the key of the limit and not sent. Before, the server refused them, often by resetting the connection, which the browser saw as a network failure. - A second upload started while one ran sent a second request, which shared the progress information of the first in the session. Whichever ended first removed it, and the other failed with an IllegalStateException. The script now refuses to start an upload while one of the field runs. - The script reported the received files to the field by GET, with their JSON in the URL. Beyond some 65 files that exceeded the 8 KB request line of Jetty and Tomcat: the server answered 414 and onUploadSuccess() was never called, although the files were stored. They are reported by POST now. - FolderUploadsFileManager stored each file under its client file name, so two files of one name, in one upload or in two, overwrote each other while the field was told of both. Files are now stored under a random name, which the resource sends to the browser with the client file name and UploadInfo carries back to the field. - cancel() after an upload had ended still claimed to cancel it, the return value of the before-send callback was ignored, and an answer that was not JSON left the field waiting, as did a timeout the before-send callback set on the request. Now cancel() does nothing once the upload has ended, false from the before-send callback keeps the upload from being sent and calls onUploadCanceled(), and an answer that is not JSON or a timeout fails the upload. The Javadoc of AbstractFileUploadResource said the field uploads a single file with jQuery; it uploads all the files of its input with a plain XMLHttpRequest. Keeping files of one name apart needs a name for each stored file that travels from the file manager through the browser back to the field, so the API changes: - IUploadsFileManager gets store(FileUpload, String), which returns the name the file was stored under, and getFile(String, String) takes that name. save() is deprecated, and Wicket no longer calls it. The default store() calls save() and returns the client file name, so an implementation written for save() keeps working, with the old behaviour for files of one name. The default save() throws an UnsupportedOperationException saying to implement store(): save() used to be abstract, and with two defaults calling each other an implementation overriding neither would compile and then fail with a StackOverflowError. - AbstractFileUploadResource.saveFiles() returns the stored names, and prepareResponse() and generateJsonResponse() receive them. These are hooks for subclasses: an old signature kept as deprecated would no longer be called, so the signatures change and an override fails to compile rather than being skipped silently. - FileUploadResourceReference.generateFileInfo() receives the stored name and puts it into the JSON as fileName. - UploadInfo carries the stored name (getFileName() and a new constructor), and equals() and hashCode() use it instead of the client file name. The old constructor is deprecated. A JSON answer without fileName, from a custom resource, falls back to the client file name. - FileUploadToResourceField gets the constants UPLOAD_FAILED_RESOURCE_KEY and CONNECTION_LOST_RESOURCE_KEY, and getUploadErrorMessage(String) becomes protected. Both are additions. What an application sees after the change: files stored by FolderUploadsFileManager no longer have their client file name on disk, so code looking them up with getFile(fieldId, clientFileName) has to use UploadInfo#getFile() or #getFileName() instead. A failed upload calls onUploadFailure() with "The upload failed."; an application that had changed the text of uploadFailed to work around the error page changes uploadToResourceFailed instead. An upload whose connection breaks off reads "The upload failed: the connection to the server was lost.", and so does one the server refuses by resetting the connection, for a limit the field does not check before sending. The client-side error callback receives only resource keys as errorMessage, uploadConnectionLost among them. A before-send callback returning false now keeps the upload from being sent, and an upload that times out calls the error callback and onUploadFailure() with uploadFailed. Code calling the deprecated save() on a manager that implements only store() gets an UnsupportedOperationException, and calls store() instead. migration-to-11.md describes the migration in detail: for each change, what it was before, what it is now, who has to change something and how. The comment of the MigrateToWicket11 recipe has the short version, as for the other changes of Wicket 11 that need manual migration. Draft for the migration guide on the wiki: Uploads to a resource (FileUploadToResourceField): implement IUploadsFileManager#store(FileUpload, String) instead of the deprecated save(). It stores the file under a name of its own and returns that name, which getFile(uploadFieldId, fileName) then receives; the name goes to the browser and back, so getFile() must not return a file of another upload field. FolderUploadsFileManager stores files under random names: find them with UploadInfo#getFile() or UploadInfo#getFileName(), not by client file name. Subclasses of AbstractFileUploadResource receive the stored names in saveFiles(), prepareResponse() and generateJsonResponse(), subclasses of FileUploadResourceReference in generateFileInfo(). A failed upload reads "The upload failed." (resource key uploadToResourceFailed) instead of ending on the error page. --- migration-to-11.md | 218 ++++++++++++ testing/wicket-js-tests/Gruntfile.js | 3 +- .../FileUploadToResourceFieldSecurityTest.java | 8 +- .../FileUploadToResourceFieldTest$UploadPage.html | 5 + .../resource/FileUploadToResourceFieldTest.java | 380 +++++++++++++++++++++ .../resource/FolderUploadsFileManagerTest.java | 38 +++ .../upload/resource/IUploadsFileManagerTest.java | 100 ++++++ .../resource/AbstractFileUploadResource.java | 97 ++---- .../resource/FileUploadResourceReference.java | 22 +- .../upload/resource/FileUploadToResourceField.java | 143 ++++++-- .../upload/resource/FileUploadToResourceField.js | 142 ++++++-- .../resource/FileUploadToResourceField.properties | 4 +- .../upload/resource/FolderUploadsFileManager.java | 37 +- .../form/upload/resource/IUploadsFileManager.java | 56 ++- wicket-core/src/test/js/all.html | 7 + wicket-core/src/test/js/upload-to-resource.js | 240 +++++++++++++ .../src/main/resources/META-INF/rewrite/wicket.yml | 11 + 17 files changed, 1353 insertions(+), 158 deletions(-) diff --git a/migration-to-11.md b/migration-to-11.md new file mode 100644 index 0000000000..d80ab57180 --- /dev/null +++ b/migration-to-11.md @@ -0,0 +1,218 @@ +# Migrating to Wicket 11 + +This file describes changes in Wicket 11 that break existing code or change behaviour, and what +an application has to do about them. The official guide is going to be the wiki page +[Migration to Wicket 11.0](https://cwiki.apache.org/confluence/display/WICKET/Migration+to+Wicket+11.0), +which does not exist yet. + +The OpenRewrite recipe `org.apache.wicket.MigrateToWicket11` +(`wicket-migration/src/main/resources/META-INF/rewrite/wicket.yml`) handles the mechanical part +of the migration. Its comment lists the other Wicket 11 changes that need manual migration. + +## Uploads to a resource (`FileUploadToResourceField`) + +Pull request [#1671](https://github.com/apache/wicket/pull/1671), issue +[#1670](https://github.com/apache/wicket/issues/1670). All classes are in +`org.apache.wicket.markup.html.form.upload.resource`. None of these changes is mechanical, so +there is no OpenRewrite recipe for them. + +### Who has to change something + +You have nothing to do if you use `FileUploadToResourceField` with the default +`FileUploadResourceReference`, and reach the uploaded files only through `UploadInfo#getFile()`. + +You do have to act if you: + +- implement `IUploadsFileManager`; +- find files stored by `FolderUploadsFileManager` by their client file name; +- extend `AbstractFileUploadResource` or `FileUploadResourceReference`; +- rely on `UploadInfo#equals()`, on the error texts of the field, or on what the browser-side + callbacks receive. + +### Why the API changed + +`FolderUploadsFileManager` stored each file under its client file name, so two files of the same +name overwrote each other, in one upload or in two, while the field was told of both. Keeping +them apart needs a name for each stored file that the file manager chooses, and that name has to +travel from the file manager through the browser back to the field. That is why the file manager, +the resource and `UploadInfo` all gained a file name. + +### `IUploadsFileManager` + +Before: + +```java +void save(FileUpload fileItem, String uploadFieldId); + +File getFile(String uploadFieldId, String clientFileName); +``` + +After: + +```java +// new, @since 11.0.0 +default String store(FileUpload fileItem, String uploadFieldId) +{ + save(fileItem, uploadFieldId); + return fileItem.getClientFileName(); +} + +@Deprecated(since = "11.0.0", forRemoval = true) +default void save(FileUpload fileItem, String uploadFieldId) +{ + throw new UnsupportedOperationException(...); +} + +File getFile(String uploadFieldId, String fileName); +``` + +To migrate, implement `store()` instead of `save()`: + +- Store the file under a name of your own choice and return that name. It must be unique among + the files of the upload field. +- `getFile()` then receives that name. +- The name goes to the browser and comes back from it, so it must not reveal anything the user + is not supposed to know. +- Since the browser can change the name, `getFile()` must never resolve it to a file of another + upload field. + +An existing implementation of `save()` still compiles, with a removal warning, and keeps working +unchanged. The default `store()` calls it and returns the client file name, which means files of +the same name still overwrite each other until you implement `store()`. + +Watch out: + +- `save()` used to be abstract, and the compiler made you implement it. Now it has a default, so + an implementation that overrides neither method compiles. At its first upload it fails with an + `UnsupportedOperationException` that says to implement `store()`. +- Wicket no longer calls `save()`. If your own code calls it, call `store()` instead. On a manager + that implements only `store()`, the deprecated `save()` throws an + `UnsupportedOperationException`. `FolderUploadsFileManager` still implements both. + +### `FolderUploadsFileManager` + +| | Before | After | +|---|---|---| +| Stores with | `save()` | `store()`; the deprecated `save()` still works as before | +| File on disk | `<folder>/<uploadFieldId>/<client file name>` | `<folder>/<uploadFieldId>/<random UUID>` | +| `getFile(uploadFieldId, name)` | `name` is the client file name | `name` is the stored name | + +To migrate, find the files with `UploadInfo#getFile()` or `UploadInfo#getFileName()`, not by +their client file name. Files stored before the upgrade keep their client file name on disk, and +`getFile(uploadFieldId, clientFileName)` still finds them. The checks against path traversal are +unchanged. + +### `AbstractFileUploadResource` + +Before: + +```java +protected void saveFiles(List<FileUpload> fileItems, String uploadId) + +protected void prepareResponse(ResourceResponse resourceResponse, ServletWebRequest webRequest, + List<FileUpload> fileItems) + +protected abstract String generateJsonResponse(ResourceResponse resourceResponse, + ServletWebRequest webRequest, List<FileUpload> files); +``` + +After: + +```java +protected List<String> saveFiles(List<FileUpload> fileItems, String uploadId) + +protected void prepareResponse(ResourceResponse resourceResponse, ServletWebRequest webRequest, + List<FileUpload> fileItems, List<String> fileNames) + +protected abstract String generateJsonResponse(ResourceResponse resourceResponse, + ServletWebRequest webRequest, List<FileUpload> files, List<String> fileNames); +``` + +`fileNames` holds the stored name of each file, in the order of the files. There are no +deprecated overloads with the old signatures, on purpose: these methods are hooks that the +resource calls, so an override of an old signature would compile and then never be called. With +`@Override` it now fails to compile, which tells you to migrate. + +To migrate: + +- Add the `List<String> fileNames` parameter to your overrides. +- In `generateJsonResponse()`, write each name into the JSON of its file as `fileName`. +- An override of `saveFiles()` returns the names that `IUploadsFileManager#store()` returned. + +How the resource answers errors has changed as well: + +| | Before | After | +|---|---|---| +| Answer to a failure | `{"error": true, "errorMessage": "<key>"}` | Unchanged | +| Exception other than a `FileUploadException` (the file manager failing, for example) | Not logged. With debug logging on, the exception message was sent as `errorMessage` | Logged at ERROR. The key `uploadFailed` is sent, never the exception message | + +### `FileUploadResourceReference` + +Before: + +```java +protected void generateFileInfo(JSONObject fileJson, FileUpload fileItem) +``` + +After: + +```java +protected void generateFileInfo(JSONObject fileJson, FileUpload fileItem, String fileName) +``` + +The JSON of each file gets `fileName` next to `clientFileName`, `size` and `contentType`. To +migrate, add the parameter to your override, and keep calling `super` or put `fileName` in +yourself. + +### `FileUploadToResourceField.UploadInfo` + +| | Before | After | +|---|---|---| +| Constructor | `UploadInfo(String clientFileName, long size, String contentType)` | `UploadInfo(String fileName, String clientFileName, long size, String contentType)`. The old one is deprecated and uses the client file name as the stored name | +| Stored name | None | `getFileName()` | +| `equals()` and `hashCode()` | By client file name | By stored name: two files of the same client name are no longer equal | +| `fromJson()` | Reads `clientFileName`, `size`, `contentType` | Also reads `fileName`, and falls back to `clientFileName` when it is missing, as in the answer of a resource written for Wicket 10 | + +### `FileUploadToResourceField`: errors + +| | Before | After | +|---|---|---| +| Failure other than a limit (`uploadFailed`) | The text interpolates `${exception.localizedMessage}`, where the field had put a String, so the Ajax request failed and the user landed on the error page | `onUploadFailure()` receives "The upload failed." (key `uploadToResourceFailed`) | +| HTTP error | The status text was looked up as a resource key: `MissingResourceException`, and the error page | Read as a failed upload | +| Empty key (HTTP/2, network failure) | "File must be less than …" (`uploadTooLarge`) | Network failure: "The upload failed: the connection to the server was lost." (key `uploadConnectionLost`). HTTP/2 error: read as a failed upload | +| Unknown key | `MissingResourceException`, and the error page | Read as a failed upload | +| Choosing the text for a key | Private | `protected String getUploadErrorMessage(String errorKey)` | + +To migrate: + +- If you changed the text of `uploadFailed` to work around the error page, change + `uploadToResourceFailed` instead. +- To word errors that your own script reports, override `getUploadErrorMessage()`. Answer the + keys you know and leave the others to `super`. Never look up a key you were given as it is: it + comes from the browser. + +New public constants: `UPLOAD_FAILED_RESOURCE_KEY` (`"uploadToResourceFailed"`) and +`CONNECTION_LOST_RESOURCE_KEY` (`"uploadConnectionLost"`). + +New resource keys, in `FileUploadToResourceField.properties`: + +| Key | Text | +|---|---| +| `uploadToResourceFailed` | The upload failed. | +| `uploadConnectionLost` | The upload failed: the connection to the server was lost. | + +### `FileUploadToResourceField`: browser side + +| | Before | After | +|---|---|---| +| `getClientBeforeSendCallBack()` default | `function (xhr, settings) { return true; }`. The return value was ignored and `settings` never passed | `function (xhr) { return true; }`. Returning `false` keeps the upload from being sent and calls `onUploadCanceled()` | +| `errorMessage` given to `getClientSideUploadErrorCallBack()` | A resource key, an HTTP status text, or an exception message | Only keys: `uploadTooLarge`, `uploadSingleFileTooLarge`, `uploadTooManyFiles`, `wicket.no.files.selected`, `uploadFailed`, `uploadConnectionLost` | +| Upload started while one runs | A second request was sent, and the two broke each other | Refused: `upload()` returns `false` and logs it | +| Files beyond a limit of the field | Sent, then refused by the server, often with a reset connection | Not sent. The error callback gets the key of the limit, `onUploadFailure()` its text | +| Telling the field which files arrived | `GET` with the files in the URL. Beyond about 65 files the server answered 414, and `onUploadSuccess()` was never called | `POST` | +| `cancel()` after the upload ended | Called the cancel callback and logged "canceled" | Only logs that there is nothing to cancel | +| Answer that is not JSON | The field waited forever | The upload fails with `uploadFailed` | +| Timeout, set on the request by the before-send callback | Neither the error callback nor `onUploadFailure()` was called | The upload fails with `uploadFailed` | + +To migrate, check a before-send callback that returns `false`: it used to be ignored, and now +cancels the upload. Also check an error callback that expects HTTP status texts. diff --git a/testing/wicket-js-tests/Gruntfile.js b/testing/wicket-js-tests/Gruntfile.js index 848722e083..e9cfb0d78e 100644 --- a/testing/wicket-js-tests/Gruntfile.js +++ b/testing/wicket-js-tests/Gruntfile.js @@ -46,7 +46,8 @@ module.exports = function(grunt) { "../../wicket-core/src/test/js/dom.js", "../../wicket-core/src/test/js/channels.js", "../../wicket-core/src/test/js/event.js", - "../../wicket-core/src/test/js/timer.js" + "../../wicket-core/src/test/js/timer.js", + "../../wicket-core/src/test/js/upload-to-resource.js" ], extensionsTestsJs = [ "../../wicket-extensions/src/test/js/palette-test.js", diff --git a/wicket-core-tests/src/test/java/org/apache/wicket/markup/html/form/upload/resource/FileUploadToResourceFieldSecurityTest.java b/wicket-core-tests/src/test/java/org/apache/wicket/markup/html/form/upload/resource/FileUploadToResourceFieldSecurityTest.java index 0251ba071a..cc1dc7b01c 100644 --- a/wicket-core-tests/src/test/java/org/apache/wicket/markup/html/form/upload/resource/FileUploadToResourceFieldSecurityTest.java +++ b/wicket-core-tests/src/test/java/org/apache/wicket/markup/html/form/upload/resource/FileUploadToResourceFieldSecurityTest.java @@ -33,6 +33,7 @@ import org.apache.wicket.util.file.File; import org.apache.wicket.util.tester.WicketTestCase; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; +import com.github.openjson.JSONArray; class FileUploadToResourceFieldSecurityTest extends WicketTestCase { @@ -70,7 +71,9 @@ class FileUploadToResourceFieldSecurityTest extends WicketTestCase File upload = createUploadFile("allowed.txt", 256); submitUpload(page.field.buildUploadUrl(), upload); - File storedFile = fileManager.getFile(page.field.getMarkupId(), upload.getName()); + String fileName = new JSONArray(tester.getLastResponseAsString()).getJSONObject(0) + .getString("fileName"); + File storedFile = fileManager.getFile(page.field.getMarkupId(), fileName); assertTrue(storedFile.exists()); assertEquals(upload.length(), storedFile.length()); } @@ -93,8 +96,7 @@ class FileUploadToResourceFieldSecurityTest extends WicketTestCase submitUpload(tamperedUrl, upload); - File storedFile = fileManager.getFile(page.field.getMarkupId(), upload.getName()); - assertFalse(storedFile.exists()); + assertFalse(Files.exists(tempDir.resolve(page.field.getMarkupId()))); assertTrue(tester.getLastResponseAsString().contains("uploadFailed")); } diff --git a/wicket-core-tests/src/test/java/org/apache/wicket/markup/html/form/upload/resource/FileUploadToResourceFieldTest$UploadPage.html b/wicket-core-tests/src/test/java/org/apache/wicket/markup/html/form/upload/resource/FileUploadToResourceFieldTest$UploadPage.html new file mode 100644 index 0000000000..2658b563d3 --- /dev/null +++ b/wicket-core-tests/src/test/java/org/apache/wicket/markup/html/form/upload/resource/FileUploadToResourceFieldTest$UploadPage.html @@ -0,0 +1,5 @@ +<html xmlns:wicket="http://wicket.apache.org";> +<body> + <input type="file" wicket:id="upload"/> +</body> +</html> diff --git a/wicket-core-tests/src/test/java/org/apache/wicket/markup/html/form/upload/resource/FileUploadToResourceFieldTest.java b/wicket-core-tests/src/test/java/org/apache/wicket/markup/html/form/upload/resource/FileUploadToResourceFieldTest.java new file mode 100644 index 0000000000..da0f6739ce --- /dev/null +++ b/wicket-core-tests/src/test/java/org/apache/wicket/markup/html/form/upload/resource/FileUploadToResourceFieldTest.java @@ -0,0 +1,380 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.wicket.markup.html.form.upload.resource; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import java.io.IOException; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; +import java.util.ArrayList; +import java.util.List; +import org.apache.wicket.ajax.AbstractDefaultAjaxBehavior; +import org.apache.wicket.ajax.AjaxRequestTarget; +import org.apache.wicket.markup.html.WebPage; +import org.apache.wicket.markup.html.form.upload.FileUpload; +import org.apache.wicket.request.mapper.parameter.PageParameters; +import org.apache.wicket.util.file.File; +import org.apache.wicket.util.lang.Bytes; +import org.apache.wicket.util.tester.WicketTestCase; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; +import com.github.openjson.JSONArray; +import com.github.openjson.JSONObject; + +/** + * Tests the upload of files to a resource by {@link FileUploadToResourceField}, and how the field + * takes what the browser reports of it. + */ +class FileUploadToResourceFieldTest extends WicketTestCase +{ + @TempDir + Path tempDir; + + private UploadPage page; + + @Test + void storesFilesOfTheSameClientFileNameApart() throws IOException + { + FolderUploadsFileManager fileManager = startPage(); + + submitUpload(createUploadFile("first", "same.txt", "first content"), + createUploadFile("second", "same.txt", "second content")); + + JSONArray filesInfo = new JSONArray(tester.getLastResponseAsString()); + assertEquals(2, filesInfo.length()); + JSONObject first = filesInfo.getJSONObject(0); + JSONObject second = filesInfo.getJSONObject(1); + assertEquals("same.txt", first.getString("clientFileName")); + assertEquals("same.txt", second.getString("clientFileName")); + assertNotEquals(first.getString("fileName"), second.getString("fileName")); + assertEquals("first content", readStoredFile(fileManager, first.getString("fileName"))); + assertEquals("second content", readStoredFile(fileManager, second.getString("fileName"))); + } + + @Test + void givesTheStoredFilesToOnUploadSuccess() throws IOException + { + startPage(); + submitUpload(createUploadFile("first", "same.txt", "first content"), + createUploadFile("second", "same.txt", "second content")); + + report("error", "false", "filesInfo", tester.getLastResponseAsString()); + + List<FileUploadToResourceField.UploadInfo> uploadInfos = page.field.uploadInfos; + assertEquals(2, uploadInfos.size()); + assertNotEquals(uploadInfos.get(0), uploadInfos.get(1)); + assertEquals("first content", read(uploadInfos.get(0).getFile())); + assertEquals("second content", read(uploadInfos.get(1).getFile())); + } + + @Test + void answersAFailureOfTheFileManagerWithTheKeyOfAFailedUpload() throws IOException + { + startPage(new FolderUploadsFileManager(new File(tempDir.resolve("uploads").toFile())) + { + @Override + public String store(FileUpload fileItem, String uploadFieldId) + { + throw new IllegalStateException("disk full"); + } + }); + + submitUpload(createUploadFile("first", "file.txt", "content")); + + JSONObject answer = new JSONObject(tester.getLastResponseAsString()); + assertTrue(answer.getBoolean("error")); + assertEquals("uploadFailed", answer.getString("errorMessage")); + } + + @Test + void answersAnUploadWithoutFilesWithTheKeyOfNoFilesSelected() + { + startPage(); + + tester.getRequest().setMethod("POST"); + tester.getRequest().setUseMultiPartContentType(true); + tester.getRequest().getPostParameters().setParameterValue("other", "value"); + tester.executeUrl(page.field.buildUploadUrl()); + + JSONObject answer = new JSONObject(tester.getLastResponseAsString()); + assertTrue(answer.getBoolean("error")); + assertEquals(AbstractFileUploadResource.NO_FILE_SELECTED, answer.getString("errorMessage")); + } + + @Test + void answersFilesBeyondALimitWithTheKeyOfTheLimit() throws IOException + { + startPage(); + + submitUpload(createUploadFile("1", "file.txt", "1"), createUploadFile("2", "file.txt", "2"), + createUploadFile("3", "file.txt", "3"), createUploadFile("4", "file.txt", "4")); + + JSONObject answer = new JSONObject(tester.getLastResponseAsString()); + assertTrue(answer.getBoolean("error")); + assertEquals("uploadTooManyFiles", answer.getString("errorMessage")); + } + + @Test + void callsOnUploadFailureForAFailedUpload() + { + startPage(); + + report("error", "true", "errorMessage", "uploadFailed"); + + assertEquals(List.of("The upload failed."), page.field.failures); + } + + @Test + void callsOnUploadFailureForAnErrorItHasNoTextFor() + { + startPage(); + + report("error", "true", "errorMessage", "Internal Server Error"); + + assertEquals(List.of("The upload failed."), page.field.failures); + } + + @Test + void callsOnUploadFailureForAnErrorWithoutKey() + { + startPage(); + + report("error", "true"); + + assertEquals(List.of("The upload failed."), page.field.failures); + } + + @Test + void callsOnUploadFailureWithTheTextOfABrokenLimit() + { + startPage(); + + report("error", "true", "errorMessage", "uploadTooManyFiles"); + + assertEquals(List.of("There should be no more than 3 files."), page.field.failures); + } + + @Test + void callsOnUploadFailureWithTheTextOfALostConnection() + { + startPage(); + + report("error", "true", "errorMessage", + FileUploadToResourceField.CONNECTION_LOST_RESOURCE_KEY); + + assertEquals(List.of("The upload failed: the connection to the server was lost."), + page.field.failures); + } + + @Test + void callsOnUploadFailureWithTheTextOfAnErrorASubclassWords() + { + startPage(); + + report("error", "true", "errorMessage", TestField.PAUSED_KEY); + report("error", "true", "errorMessage", "uploadTooManyFiles"); + + assertEquals(List.of("The upload was paused.", "There should be no more than 3 files."), + page.field.failures); + } + + @Test + void callsOnUploadCanceledForACanceledUpload() + { + startPage(); + + report("error", "true", "errorMessage", FileUploadToResourceField.UPLOAD_CANCELED); + + assertTrue(page.field.canceled); + assertTrue(page.field.failures.isEmpty()); + } + + @SuppressWarnings("removal") + @Test + void anUploadInfoWithoutStoredNameIsStoredUnderItsClientFileName() + { + FileUploadToResourceField.UploadInfo info = + new FileUploadToResourceField.UploadInfo("file.txt", 3, "text/plain"); + + assertEquals("file.txt", info.getFileName()); + assertEquals("file.txt", info.getClientFileName()); + } + + @Test + void uploadInfosAreEqualByTheirStoredName() + { + FileUploadToResourceField.UploadInfo info = new FileUploadToResourceField.UploadInfo( + "stored-1", "same.txt", 3, "text/plain"); + FileUploadToResourceField.UploadInfo sameStoredName = + new FileUploadToResourceField.UploadInfo("stored-1", "other.txt", 5, "text/html"); + FileUploadToResourceField.UploadInfo otherStoredName = + new FileUploadToResourceField.UploadInfo("stored-2", "same.txt", 3, "text/plain"); + + assertEquals(info, sameStoredName); + assertEquals(info.hashCode(), sameStoredName.hashCode()); + assertNotEquals(info, otherStoredName); + } + + private FolderUploadsFileManager startPage() + { + FolderUploadsFileManager fileManager = new FolderUploadsFileManager( + new File(tempDir.resolve("uploads").toFile())); + startPage(fileManager); + return fileManager; + } + + private void startPage(IUploadsFileManager fileManager) + { + page = new UploadPage(new TestFileUploadResourceReference(fileManager)); + tester.startPage(page); + } + + private File createUploadFile(String folder, String fileName, String content) + throws IOException + { + Path filePath = Files.createDirectories(tempDir.resolve(folder)).resolve(fileName); + Files.writeString(filePath, content, StandardCharsets.UTF_8); + return new File(filePath.toFile()); + } + + private void submitUpload(File... uploads) + { + tester.getRequest().setMethod("POST"); + for (File upload : uploads) + { + tester.getRequest().addFile(AbstractFileUploadResource.PARAM_NAME, upload, "text/plain"); + } + tester.executeUrl(page.field.buildUploadUrl()); + } + + /** + * Reports to the field how the upload ended, as its script does. + */ + private void report(String... parameters) + { + tester.getRequest().setMethod("POST"); + for (int i = 0; i < parameters.length; i += 2) + { + tester.getRequest().getPostParameters().setParameterValue(parameters[i], + parameters[i + 1]); + } + tester.executeBehavior(page.field.getBehaviors(AbstractDefaultAjaxBehavior.class).get(0)); + } + + private String readStoredFile(FolderUploadsFileManager fileManager, String fileName) + throws IOException + { + return read(fileManager.getFile(page.field.getMarkupId(), fileName)); + } + + private static String read(java.io.File file) throws IOException + { + return Files.readString(file.toPath(), StandardCharsets.UTF_8); + } + + static class UploadPage extends WebPage + { + private static final long serialVersionUID = 1L; + + final TestField field; + + UploadPage(TestFileUploadResourceReference resourceReference) + { + field = new TestField("upload", resourceReference); + field.setMaxSize(Bytes.kilobytes(1)); + field.setFileCountMax(3); + add(field); + } + } + + static class TestField extends FileUploadToResourceField + { + private static final long serialVersionUID = 1L; + + static final String PAUSED_KEY = "uploadPaused"; + + private final TestFileUploadResourceReference resourceReference; + + List<UploadInfo> uploadInfos; + + final List<String> failures = new ArrayList<>(); + + boolean canceled; + + TestField(String id, TestFileUploadResourceReference resourceReference) + { + super(id); + this.resourceReference = resourceReference; + } + + @Override + protected FileUploadResourceReference getFileUploadResourceReference() + { + return resourceReference; + } + + @Override + protected void onUploadSuccess(AjaxRequestTarget target, List<UploadInfo> fileInfos) + { + uploadInfos = fileInfos; + } + + @Override + protected void onUploadFailure(AjaxRequestTarget target, String errorInfo) + { + failures.add(errorInfo); + } + + @Override + protected void onUploadCanceled(AjaxRequestTarget target) + { + canceled = true; + } + + @Override + protected String getUploadErrorMessage(String errorKey) + { + return PAUSED_KEY.equals(errorKey) ? "The upload was paused." + : super.getUploadErrorMessage(errorKey); + } + + String buildUploadUrl() + { + return urlFor(resourceReference, new PageParameters()).toString() + '?' + + AbstractFileUploadResource.UPLOAD_ID + '=' + getMarkupId() + '&' + + AbstractFileUploadResource.MAX_SIZE + '=' + getMaxSize().bytes() + '&' + + AbstractFileUploadResource.FILE_COUNT_MAX + '=' + getFileCountMax() + '&' + + AbstractFileUploadResource.UPLOAD_TOKEN + '=' + + AbstractFileUploadResource.createUploadToken(getMarkupId(), getMaxSize(), + getFileMaxSize(), getFileCountMax()); + } + } + + static class TestFileUploadResourceReference extends FileUploadResourceReference + { + private static final long serialVersionUID = 1L; + + TestFileUploadResourceReference(IUploadsFileManager uploadFileManager) + { + super(uploadFileManager); + } + } +} diff --git a/wicket-core-tests/src/test/java/org/apache/wicket/markup/html/form/upload/resource/FolderUploadsFileManagerTest.java b/wicket-core-tests/src/test/java/org/apache/wicket/markup/html/form/upload/resource/FolderUploadsFileManagerTest.java index 4930aed2d7..92b4e4d526 100644 --- a/wicket-core-tests/src/test/java/org/apache/wicket/markup/html/form/upload/resource/FolderUploadsFileManagerTest.java +++ b/wicket-core-tests/src/test/java/org/apache/wicket/markup/html/form/upload/resource/FolderUploadsFileManagerTest.java @@ -17,6 +17,7 @@ package org.apache.wicket.markup.html.form.upload.resource; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.Mockito.mock; @@ -54,6 +55,31 @@ class FolderUploadsFileManagerTest assertThrows(SecurityException.class, () -> manager.getFile("uploadField", "../../etc/passwd")); } + @Test + void storeRejectsTraversalInUploadFieldId() throws IOException + { + FolderUploadsFileManager manager = new FolderUploadsFileManager(new File(tempDir.toFile())); + FileUpload fileUpload = newFileUpload("safe.txt", "content"); + + assertThrows(SecurityException.class, () -> manager.store(fileUpload, "../escaped")); + } + + @Test + void storeKeepsFilesOfTheSameClientFileNameApart() throws IOException + { + FolderUploadsFileManager manager = new FolderUploadsFileManager(new File(tempDir.toFile())); + + String first = manager.store(newFileUpload("same.txt", "first"), "uploadField"); + String second = manager.store(newFileUpload("same.txt", "second"), "uploadField"); + + assertNotEquals(first, second); + assertEquals("first", + Files.readString(manager.getFile("uploadField", first).toPath(), StandardCharsets.UTF_8)); + assertEquals("second", + Files.readString(manager.getFile("uploadField", second).toPath(), StandardCharsets.UTF_8)); + } + + @SuppressWarnings("removal") @Test void saveRejectsTraversalInUploadFieldId() throws IOException { @@ -66,6 +92,7 @@ class FolderUploadsFileManagerTest assertThrows(SecurityException.class, () -> manager.save(fileUpload, "../escaped")); } + @SuppressWarnings("removal") @Test void saveRejectsTraversalInClientFileName() throws IOException { @@ -78,6 +105,7 @@ class FolderUploadsFileManagerTest assertThrows(SecurityException.class, () -> manager.save(fileUpload, "uploadField")); } + @SuppressWarnings("removal") @Test void saveSucceedsWithValidUploadFieldIdAndClientFileName() throws IOException { @@ -93,4 +121,14 @@ class FolderUploadsFileManagerTest assertTrue(Files.exists(savedFile)); assertEquals("content", Files.readString(savedFile, StandardCharsets.UTF_8)); } + + private static FileUpload newFileUpload(String clientFileName, String content) + throws IOException + { + FileUpload fileUpload = mock(FileUpload.class); + when(fileUpload.getClientFileName()).thenReturn(clientFileName); + when(fileUpload.getInputStream()) + .thenReturn(new ByteArrayInputStream(content.getBytes(StandardCharsets.UTF_8))); + return fileUpload; + } } diff --git a/wicket-core-tests/src/test/java/org/apache/wicket/markup/html/form/upload/resource/IUploadsFileManagerTest.java b/wicket-core-tests/src/test/java/org/apache/wicket/markup/html/form/upload/resource/IUploadsFileManagerTest.java new file mode 100644 index 0000000000..e936815f6f --- /dev/null +++ b/wicket-core-tests/src/test/java/org/apache/wicket/markup/html/form/upload/resource/IUploadsFileManagerTest.java @@ -0,0 +1,100 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.wicket.markup.html.form.upload.resource; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.util.ArrayList; +import java.util.List; + +import org.apache.wicket.markup.html.form.upload.FileUpload; +import org.apache.wicket.util.file.File; +import org.junit.jupiter.api.Test; + +class IUploadsFileManagerTest +{ + @Test + void storeOfAManagerWrittenForSaveSavesTheFileUnderItsClientFileName() + { + List<String> saved = new ArrayList<>(); + IUploadsFileManager manager = new IUploadsFileManager() + { + @SuppressWarnings("removal") + @Override + public void save(FileUpload fileItem, String uploadFieldId) + { + saved.add(uploadFieldId + '/' + fileItem.getClientFileName()); + } + + @Override + public File getFile(String uploadFieldId, String fileName) + { + return null; + } + }; + + assertEquals("file.txt", manager.store(newFileUpload("file.txt"), "field")); + assertEquals(List.of("field/file.txt"), saved); + } + + @SuppressWarnings("removal") + @Test + void saveOfAManagerWrittenForStoreSaysToCallStore() + { + IUploadsFileManager manager = new IUploadsFileManager() + { + @Override + public String store(FileUpload fileItem, String uploadFieldId) + { + return "stored-name"; + } + + @Override + public File getFile(String uploadFieldId, String fileName) + { + return null; + } + }; + + UnsupportedOperationException exception = assertThrows( + UnsupportedOperationException.class, + () -> manager.save(newFileUpload("file.txt"), "field")); + assertTrue(exception.getMessage().contains("store(FileUpload, String)")); + } + + @Test + void storeOfAManagerImplementingNeitherMethodFailsSayingWhichToImplement() + { + IUploadsFileManager manager = (uploadFieldId, fileName) -> null; + + UnsupportedOperationException exception = assertThrows( + UnsupportedOperationException.class, + () -> manager.store(newFileUpload("file.txt"), "field")); + assertTrue(exception.getMessage().contains("store(FileUpload, String)")); + } + + private static FileUpload newFileUpload(String clientFileName) + { + FileUpload fileUpload = mock(FileUpload.class); + when(fileUpload.getClientFileName()).thenReturn(clientFileName); + return fileUpload; + } +} diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/upload/resource/AbstractFileUploadResource.java b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/upload/resource/AbstractFileUploadResource.java index c20e7a5ce8..e71f2d5cfa 100644 --- a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/upload/resource/AbstractFileUploadResource.java +++ b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/upload/resource/AbstractFileUploadResource.java @@ -55,7 +55,8 @@ import jakarta.servlet.http.HttpServletResponse; * <p> * The main difference is that there some JQuery plugin is used at client side (and it supports multiple uploads + * some UI allowing to delete/preview files and so on). - * Here we are just using plain jQuery code at client side to upload a single file. + * Here {@link FileUploadToResourceField} sends all the files of its input in one request, with a + * plain {@code XMLHttpRequest}. */ public abstract class AbstractFileUploadResource extends AbstractResource { @@ -134,76 +135,28 @@ public abstract class AbstractFileUploadResource extends AbstractResource Map<String, List<FileItem>> files = multiPartRequest.getFiles(); List<FileItem> fileItems = files.get(PARAM_NAME); - if (fileItems != null) + if (fileItems == null) { - List<FileUpload> fileUploads = new ArrayList<>(); - for (FileItem fileItem : fileItems) - { - fileUploads.add(new FileUpload(fileItem)); - } - saveFiles(fileUploads, uploadId); - prepareResponse(resourceResponse, webRequest, fileUploads); + return createErrorResponse(resourceResponse, NO_FILE_SELECTED); } - else + List<FileUpload> fileUploads = new ArrayList<>(); + for (FileItem fileItem : fileItems) { - resourceResponse.setContentType("application/json"); - resourceResponse.setWriteCallback(new WriteCallback() - { - @Override - public void writeData(Attributes attributes) throws IOException - { - JSONObject json = new JSONObject(); - json.put("error", true); - json.put("errorMessage", NO_FILE_SELECTED); - String error = json.toString(); - attributes.getResponse().write(error); - } - }); + fileUploads.add(new FileUpload(fileItem)); } + List<String> fileNames = saveFiles(fileUploads, uploadId); + prepareResponse(resourceResponse, webRequest, fileUploads, fileNames); return resourceResponse; - } catch (FileUploadException fux) { - resourceResponse.setContentType("application/json"); - // even when this is an exceptional situation we want the request to be successful - // as we are just sending back to the client some JSON with error messages, - // which will in turn be sent to wicket component (who will handle the errors) - resourceResponse.setStatusCode(HttpServletResponse.SC_OK); - JSONObject json = new JSONObject(); - json.put("error", true); - String errorMessage = getFileUploadExceptionKey(fux); - json.put("errorMessage", errorMessage); - String error = json.toString(); - resourceResponse.setWriteCallback(new WriteCallback() - { - @Override - public void writeData(Attributes attributes) throws IOException - { - attributes.getResponse().write(error); - } - }); - return resourceResponse; + return createErrorResponse(resourceResponse, getFileUploadExceptionKey(fux)); } - catch (Exception fux) + catch (Exception e) { - resourceResponse.setContentType("application/json"); - JSONObject json = new JSONObject(); - json.put("error", true); - String errorMessage = LOG.isDebugEnabled() ? fux.getMessage() : Form.UPLOAD_FAILED_RESOURCE_KEY; - json.put("errorMessage", errorMessage); - String error = json.toString(); - resourceResponse.setWriteCallback(new WriteCallback() - { - @Override - public void writeData(Attributes attributes) throws IOException - { - attributes.getResponse().write(error); - } - }); + LOG.error("Upload to '{}' failed", uploadId, e); + return createErrorResponse(resourceResponse, Form.UPLOAD_FAILED_RESOURCE_KEY); } - - return resourceResponse; } private static String serializeUploadSettings(String uploadId, Bytes maxSize, Bytes fileMaxSize, @@ -247,6 +200,7 @@ public abstract class AbstractFileUploadResource extends AbstractResource private ResourceResponse createErrorResponse(ResourceResponse resourceResponse, String errorMessage) { resourceResponse.setContentType("application/json"); + // the error goes to the field in the JSON, which it reads from successful answers only resourceResponse.setStatusCode(HttpServletResponse.SC_OK); JSONObject json = new JSONObject(); json.put("error", true); @@ -293,11 +247,15 @@ public abstract class AbstractFileUploadResource extends AbstractResource * ServletWebRequest * @param fileItems * List<FileUpload> + * @param fileNames + * the names the files were stored under, in the order of {@code fileItems} */ - protected void prepareResponse(ResourceResponse resourceResponse, ServletWebRequest webRequest, List<FileUpload> fileItems) + protected void prepareResponse(ResourceResponse resourceResponse, ServletWebRequest webRequest, + List<FileUpload> fileItems, List<String> fileNames) { resourceResponse.setContentType("application/json"); - final String responseContent = generateJsonResponse(resourceResponse, webRequest, fileItems); + final String responseContent = generateJsonResponse(resourceResponse, webRequest, fileItems, + fileNames); resourceResponse.setWriteCallback(new WriteCallback() { @@ -314,13 +272,19 @@ public abstract class AbstractFileUploadResource extends AbstractResource * * @param fileItems * List<FileUpload> + * @param uploadId + * the unique ID of the upload field + * @return the names the files were stored under, in the order of {@code fileItems} + * @see IUploadsFileManager#store(FileUpload, String) */ - protected void saveFiles(List<FileUpload> fileItems, String uploadId) + protected List<String> saveFiles(List<FileUpload> fileItems, String uploadId) { + List<String> fileNames = new ArrayList<>(); for (FileUpload fileItem : fileItems) { - fileManager.save(fileItem, uploadId); + fileNames.add(fileManager.store(fileItem, uploadId)); } + return fileNames; } // Use getQueryParameters() instead of getRequestParameters() to avoid reading POST parameters @@ -389,9 +353,12 @@ public abstract class AbstractFileUploadResource extends AbstractResource * ServletWebRequest * @param files * List<FileUpload> + * @param fileNames + * the names the files were stored under, in the order of {@code files}; the browser + * sends them back for {@link FileUploadToResourceField} to find the files * @return The generated JSON */ protected abstract String generateJsonResponse(ResourceResponse resourceResponse, - ServletWebRequest webRequest, List<FileUpload> files); + ServletWebRequest webRequest, List<FileUpload> files, List<String> fileNames); } diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/upload/resource/FileUploadResourceReference.java b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/upload/resource/FileUploadResourceReference.java index b57644503c..1a4db65e90 100644 --- a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/upload/resource/FileUploadResourceReference.java +++ b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/upload/resource/FileUploadResourceReference.java @@ -83,17 +83,18 @@ public class FileUploadResourceReference extends ResourceReference { @Override - protected String generateJsonResponse(ResourceResponse resourceResponse, ServletWebRequest webRequest, List<FileUpload> files) + protected String generateJsonResponse(ResourceResponse resourceResponse, + ServletWebRequest webRequest, List<FileUpload> files, List<String> fileNames) { JSONArray json = new JSONArray(); - for (FileUpload fileItem : files) + for (int i = 0; i < files.size(); i++) { JSONObject fileJson = new JSONObject(); try { - generateFileInfo(fileJson, fileItem); + generateFileInfo(fileJson, files.get(i), fileNames.get(i)); json.put(fileJson); } catch (JSONException e) @@ -111,8 +112,21 @@ public class FileUploadResourceReference extends ResourceReference return uploadFileManager; } - protected void generateFileInfo(JSONObject fileJson, FileUpload fileItem) + /** + * Puts the information about an uploaded file the field receives into the JSON answering the + * upload. + * + * @param fileJson + * the JSON of the file + * @param fileItem + * the uploaded file + * @param fileName + * the name the file was stored under, see + * {@link IUploadsFileManager#store(FileUpload, String)} + */ + protected void generateFileInfo(JSONObject fileJson, FileUpload fileItem, String fileName) { + fileJson.put("fileName", fileName); fileJson.put("clientFileName", fileItem.getClientFileName()); fileJson.put("size", fileItem.getSize()); fileJson.put("contentType", fileItem.getContentType()); diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/upload/resource/FileUploadToResourceField.java b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/upload/resource/FileUploadToResourceField.java index 47d10fae0e..5ce7eec0cd 100644 --- a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/upload/resource/FileUploadToResourceField.java +++ b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/upload/resource/FileUploadToResourceField.java @@ -25,6 +25,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Set; import java.util.UUID; import org.apache.commons.io.IOUtils; import org.apache.wicket.ajax.AbstractDefaultAjaxBehavior; @@ -34,17 +35,17 @@ import org.apache.wicket.core.request.handler.IPartialPageRequestHandler; import org.apache.wicket.markup.head.IHeaderResponse; import org.apache.wicket.markup.head.JavaScriptHeaderItem; import org.apache.wicket.markup.head.OnDomReadyHeaderItem; +import org.apache.wicket.markup.html.form.Form; import org.apache.wicket.markup.html.form.upload.FileUpload; import org.apache.wicket.markup.html.form.upload.FileUploadField; import org.apache.wicket.model.IModel; import org.apache.wicket.model.Model; -import org.apache.wicket.request.Request; +import org.apache.wicket.request.IRequestParameters; import org.apache.wicket.request.cycle.RequestCycle; import org.apache.wicket.request.mapper.parameter.PageParameters; import org.apache.wicket.request.resource.JavaScriptResourceReference; import org.apache.wicket.resource.CoreLibrariesContributor; import org.apache.wicket.util.lang.Bytes; -import org.apache.wicket.util.string.Strings; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import com.github.openjson.JSONArray; @@ -65,19 +66,42 @@ public abstract class FileUploadToResourceField extends FileUploadField public static final class UploadInfo { private File file; + private final String fileName; private final String clientFileName; private final long size; private final String contentType; - public UploadInfo(String clientFileName, long size, String contentType) + /** + * @param fileName + * the name the {@link IUploadsFileManager} stored the file under + * @param clientFileName + * the name of the file on the client + * @param size + * the size of the file in bytes + * @param contentType + * the content type of the file + * @since 11.0.0 + */ + public UploadInfo(String fileName, String clientFileName, long size, String contentType) { + this.fileName = fileName; this.clientFileName = clientFileName; this.size = size; this.contentType = contentType; } + /** + * @deprecated the file is then looked up by its client file name, which several files can + * share. Use {@link #UploadInfo(String, String, long, String)} instead. + */ + @Deprecated(since = "11.0.0", forRemoval = true) + public UploadInfo(String clientFileName, long size, String contentType) + { + this(clientFileName, clientFileName, size, contentType); + } + public File getFile() { return file; @@ -88,6 +112,15 @@ public abstract class FileUploadToResourceField extends FileUploadField this.file = file; } + /** + * @return the name the {@link IUploadsFileManager} stored the file under + * @since 11.0.0 + */ + public String getFileName() + { + return fileName; + } + public String getClientFileName() { return clientFileName; @@ -109,7 +142,7 @@ public abstract class FileUploadToResourceField extends FileUploadField if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; UploadInfo fileInfo = (UploadInfo) o; - return Objects.equals(clientFileName, fileInfo.clientFileName); + return Objects.equals(fileName, fileInfo.fileName); } /** @@ -147,7 +180,7 @@ public abstract class FileUploadToResourceField extends FileUploadField @Override public int hashCode() { - return Objects.hash(clientFileName); + return Objects.hash(fileName); } public static List<UploadInfo> fromJson(String json) @@ -157,7 +190,10 @@ public abstract class FileUploadToResourceField extends FileUploadField for (int i = 0; i < jsonArray.length(); i++) { JSONObject jsonObject = jsonArray.getJSONObject(i); - infos.add(new UploadInfo(jsonObject.getString("clientFileName"), jsonObject.getLong("size"), jsonObject.getString("contentType"))); + String clientFileName = jsonObject.getString("clientFileName"); + // a resource written before files had names of their own stores them by client name + infos.add(new UploadInfo(jsonObject.optString("fileName", clientFileName), + clientFileName, jsonObject.getLong("size"), jsonObject.getString("contentType"))); } return infos; } @@ -167,6 +203,26 @@ public abstract class FileUploadToResourceField extends FileUploadField public static String UPLOAD_CANCELED = "upload.canceled"; + /** + * The resource key of the text of an upload that failed for a reason other than a limit. + * + * @since 11.0.0 + */ + public static final String UPLOAD_FAILED_RESOURCE_KEY = "uploadToResourceFailed"; + + /** + * The resource key of the text of an upload whose connection broke off before the server + * answered, which the browser reports under this key. + * + * @since 11.0.0 + */ + public static final String CONNECTION_LOST_RESOURCE_KEY = "uploadConnectionLost"; + + private static final Set<String> ERROR_RESOURCE_KEYS = Set.of( + AbstractFileUploadResource.NO_FILE_SELECTED, CONNECTION_LOST_RESOURCE_KEY, + Form.UPLOAD_TOO_LARGE_RESOURCE_KEY, Form.UPLOAD_SINGLE_FILE_TOO_LARGE_RESOURCE_KEY, + Form.UPLOAD_TOO_MANY_FILES_RESOURCE_KEY); + private static abstract class FileModel implements IModel<List<UploadInfo>> { @@ -180,7 +236,7 @@ public abstract class FileUploadToResourceField extends FileUploadField // at this point files were stored at the server side by resource // UploadFieldId acts as a discriminator at application level // so that uploaded files are isolated. - uploadInfo.setFile(fileManager().getFile(getUploadFieldId(), uploadInfo.clientFileName)); + uploadInfo.setFile(fileManager().getFile(getUploadFieldId(), uploadInfo.getFileName())); } return fileInfos; } @@ -251,38 +307,23 @@ public abstract class FileUploadToResourceField extends FileUploadField @Override protected void respond(AjaxRequestTarget target) { - Request request = RequestCycle.get().getRequest(); - boolean error = request.getRequestParameters().getParameterValue("error").toBoolean(true); + IRequestParameters parameters = RequestCycle.get().getRequest().getRequestParameters(); + boolean error = parameters.getParameterValue("error").toBoolean(true); if (!error) { - String filesIfo = request.getRequestParameters().getParameterValue("filesInfo").toString(); + String filesIfo = parameters.getParameterValue("filesInfo").toString(); fileUploadInfos = UploadInfo.fromJson(filesIfo); onUploadSuccess(target, getFileUploadInfos()); } else { - String errorMessage = request.getRequestParameters().getParameterValue("errorMessage").toString(null); - if (UPLOAD_CANCELED.equals(errorMessage)) + String errorKey = parameters.getParameterValue("errorMessage").toString(null); + if (UPLOAD_CANCELED.equals(errorKey)) { onUploadCanceled(target); } else { - if (AbstractFileUploadResource.NO_FILE_SELECTED.equals(errorMessage)) - { - errorMessage = getString(AbstractFileUploadResource.NO_FILE_SELECTED); - } - else - { - final Map<String, Object> model = new HashMap<>(); - if (Strings.isEmpty(errorMessage)) { - errorMessage = "uploadTooLarge"; - } - model.put("exception", errorMessage); - model.put("maxSize", getMaxSize()); - model.put("fileMaxSize", getFileMaxSize()); - model.put("fileCountMax", getFileCountMax()); - errorMessage = getString(errorMessage, Model.ofMap(model)); - } + String errorMessage = getUploadErrorMessage(errorKey); error(errorMessage); onUploadFailure(target, errorMessage); } @@ -292,6 +333,30 @@ public abstract class FileUploadToResourceField extends FileUploadField add(ajaxBehavior); } + /** + * Words the error of a failed upload, which {@link #onUploadFailure(AjaxRequestTarget, String)} + * then receives. The key comes from the browser, which can send any, so it is never looked up + * as given: a key this method does not know gets the text of + * {@link #UPLOAD_FAILED_RESOURCE_KEY}. An override words the keys it knows, such as those its + * own script reports, and leaves the others to this method. + * + * @param errorKey + * the key of the error the browser reported, or {@code null} + * @return the text of a limit, of no files selected or of + * {@link #CONNECTION_LOST_RESOURCE_KEY}, else that of {@link #UPLOAD_FAILED_RESOURCE_KEY} + * @since 11.0.0 + */ + protected String getUploadErrorMessage(String errorKey) + { + final Map<String, Object> model = new HashMap<>(); + model.put("maxSize", getMaxSize()); + model.put("fileMaxSize", getFileMaxSize()); + model.put("fileCountMax", getFileCountMax()); + String key = errorKey != null && ERROR_RESOURCE_KEYS.contains(errorKey) ? errorKey + : UPLOAD_FAILED_RESOURCE_KEY; + return getString(key, Model.ofMap(model)); + } + private List<UploadInfo> getFileUploadInfos() { // mind that this makes files to be added. @@ -332,7 +397,7 @@ public abstract class FileUploadToResourceField extends FileUploadField * Override to do something on a non successful upload * * @param target The {@link AjaxRequestTarget} - * @param errorInfo The cause of the failure + * @param errorInfo The cause of the failure, as {@link #getUploadErrorMessage(String)} words it */ protected void onUploadFailure(AjaxRequestTarget target, String errorInfo) { @@ -469,7 +534,10 @@ public abstract class FileUploadToResourceField extends FileUploadField } /** - * Starts the upload via an AJAX request. + * Starts the upload via an AJAX request. Only one upload of the field runs at a time: while one + * runs, starting another one does nothing. Files beyond the limits of the field are not sent: + * {@link #onUploadFailure(AjaxRequestTarget, String)} is called with the text of the limit + * instead. * * @param target The {@link AjaxRequestTarget} */ @@ -496,13 +564,13 @@ public abstract class FileUploadToResourceField extends FileUploadField } /** - * See jQuery.ajax documentation. - * - * @return A JavaScript function to be executed on beforeSend the upload request. + * @return A JavaScript function called with the {@code XMLHttpRequest} of the upload before it + * is sent. If it returns {@code false}, the upload is not sent, and + * {@link #onUploadCanceled(AjaxRequestTarget)} is called. */ protected CharSequence getClientBeforeSendCallBack() { - return "function (xhr, settings) { return true; }"; + return "function (xhr) { return true; }"; } /** @@ -516,7 +584,7 @@ public abstract class FileUploadToResourceField extends FileUploadField /** * @return A JavaScript function to be executed on upload canceled. This is, besides the normal wicket - * AJAX request (see {@link #onUploadFailure(AjaxRequestTarget, String)}). + * AJAX request (see {@link #onUploadCanceled(AjaxRequestTarget)}). */ protected CharSequence getClientSideCancelCallBack() { @@ -524,9 +592,10 @@ public abstract class FileUploadToResourceField extends FileUploadField } /** - * @return A JavaScript function to be executed on upload canceled. This is, besides the normal wicket + * @return A JavaScript function to be executed on upload failed. This is, besides the normal wicket * AJAX request (see {@link #onUploadFailure(AjaxRequestTarget, String)}). It receives as parameter a JSON object - * like <code>{'error': true, errorMessage: 'xxx'}</code>. + * like <code>{'error': true, errorMessage: 'uploadTooLarge'}</code>, where {@code errorMessage} is the + * resource key of the error. */ protected CharSequence getClientSideUploadErrorCallBack() { diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/upload/resource/FileUploadToResourceField.js b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/upload/resource/FileUploadToResourceField.js index 5d79c59f0e..4ae7b50823 100644 --- a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/upload/resource/FileUploadToResourceField.js +++ b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/upload/resource/FileUploadToResourceField.js @@ -43,65 +43,141 @@ this.uploadErrorCallBack = uploadErrorCallBack; }; + // the keys of the errors, as the field knows them + const UPLOAD_TOO_LARGE = 'uploadTooLarge'; + const UPLOAD_SINGLE_FILE_TOO_LARGE = 'uploadSingleFileTooLarge'; + const UPLOAD_TOO_MANY_FILES = 'uploadTooManyFiles'; + const UPLOAD_FAILED = 'uploadFailed'; + const CONNECTION_LOST = 'uploadConnectionLost'; + const UPLOAD_CANCELED = 'upload.canceled'; + + /** + * Uploads the files of the input, unless an upload of the field is running already, or the + * files break a limit of the field: the server is told of those as of an upload it refused, + * without the files being sent. + * + * @returns {boolean} whether the upload was sent + */ Wicket.FileUploadToResourceField.prototype.upload = function() { + if (this.xhr) { + Wicket.Log.log("An upload of field '" + this.inputName + "' is running: it has to end before another one starts."); + return false; + } // get a fresh reference to input this.input = document.getElementById(this.inputName); + const files = this.input.files; + const exceededLimit = this.exceededLimit(files); + if (exceededLimit) { + this.failed(exceededLimit); + return false; + } // we add the files to a FormData object. const formData = new FormData(); - const totalfiles = this.input.files.length; - for (let index = 0; index < totalfiles; index++) { - formData.append("WICKET-FILE-UPLOAD",this.input.files[index]); + for (let index = 0; index < files.length; index++) { + formData.append("WICKET-FILE-UPLOAD", files[index]); } const self = this; // we post the files to the resource (this.resourceUrl) with a plain XMLHttpRequest // and keep a reference to it in order to be able to cancel the upload const xhr = new XMLHttpRequest(); - this.xhr = xhr; xhr.open("POST", this.resourceUrl); xhr.onload = function () { - let res, ep; - if (xhr.status >= 200 && xhr.status < 300) { - try { - res = xhr.responseText ? JSON.parse(xhr.responseText) : {}; - } catch (parseError) { - // this error will only happen is generated JSON at server side is faulty - Wicket.Log.log(xhr.responseText); - return; - } - // do clean up on success - if (res.error) { - self.uploadErrorCallBack(res); - Wicket.Ajax.get({"u": self.ajaxCallBackUrl, "ep": res}); - } else { - self.clientSideSuccessCallBack(); - ep = {'error': false, 'filesInfo': JSON.stringify(res)}; - Wicket.Ajax.get({"u": self.ajaxCallBackUrl, "ep": ep}); - } + if (xhr.status < 200 || xhr.status >= 300) { + self.failed(UPLOAD_FAILED); + return; + } + let res; + try { + res = xhr.responseText ? JSON.parse(xhr.responseText) : {}; + } catch (parseError) { + Wicket.Log.error("The answer to the upload of field '" + self.inputName + "' is not JSON: " + xhr.responseText); + self.failed(UPLOAD_FAILED); + return; + } + if (res.error) { + self.failed(res.errorMessage); } else { - ep = {'error': true, "errorMessage": xhr.statusText}; - self.uploadErrorCallBack(ep); - Wicket.Ajax.get({"u": self.ajaxCallBackUrl, "ep": ep}); + delete self.xhr; + self.clientSideSuccessCallBack(); + self.report({'error': false, 'filesInfo': JSON.stringify(res)}); } }; xhr.onerror = function () { - const ep = {'error': true, "errorMessage": xhr.statusText}; - self.uploadErrorCallBack(ep); - Wicket.Ajax.get({"u": self.ajaxCallBackUrl, "ep": ep}); + self.failed(CONNECTION_LOST); + }; + + xhr.ontimeout = function () { + self.failed(UPLOAD_FAILED); }; xhr.onabort = function () { // user aborted the upload. - const ep = {'error': true, 'errorMessage': 'upload.canceled'}; - Wicket.Ajax.get({"u": self.ajaxCallBackUrl, "ep": ep}); + delete self.xhr; + self.report({'error': true, 'errorMessage': UPLOAD_CANCELED}); }; - self.clientBeforeSendCallBack(xhr); + if (this.clientBeforeSendCallBack(xhr) === false) { + Wicket.Log.log("The upload of field '" + this.inputName + "' was not sent: the before-send callback returned false."); + this.report({'error': true, 'errorMessage': UPLOAD_CANCELED}); + return false; + } + this.xhr = xhr; xhr.send(formData); + return true; + }; + + /** + * @param files the files to upload + * @returns {string|null} the key of the first limit of the field the files break, if any + */ + Wicket.FileUploadToResourceField.prototype.exceededLimit = function (files) + { + const settings = this.settings; + let totalSize = 0; + let largestSize = 0; + for (let index = 0; index < files.length; index++) { + totalSize += files[index].size; + largestSize = Math.max(largestSize, files[index].size); + } + if (totalSize > settings.maxSize) { + return UPLOAD_TOO_LARGE; + } + if (settings.fileCountMax >= 0 && files.length > settings.fileCountMax) { + return UPLOAD_TOO_MANY_FILES; + } + if (settings.fileMaxSize != null && largestSize > settings.fileMaxSize) { + return UPLOAD_SINGLE_FILE_TOO_LARGE; + } + return null; + }; + + /** + * Ends an upload that failed, and tells the page and the server why. + * + * @param errorMessage the key of the error + */ + Wicket.FileUploadToResourceField.prototype.failed = function (errorMessage) + { + delete this.xhr; + const ep = {'error': true, 'errorMessage': errorMessage}; + this.uploadErrorCallBack(ep); + this.report(ep); + }; + + /** + * Tells the server how an upload ended. A POST carries the information about the files in its + * body, which would make the URL of a GET too long for the server with many files. + * + * @param ep the parameters of the request + */ + Wicket.FileUploadToResourceField.prototype.report = function (ep) + { + Wicket.Ajax.post({"u": this.ajaxCallBackUrl, "ep": ep}); }; // cancel the upload @@ -109,11 +185,11 @@ // we have a reference to the request we can cancel it. if (this.xhr) { this.xhr.abort(); + delete (this.xhr); this.clientSideCancelCallBack(); Wicket.Log.log("The upload associated with field '" + this.inputName + "' has been canceled!"); - delete (this.xhr); } else { - Wicket.Log.log("Too late to cancel upload for field '" + this.inputName + "': the upload has already finished."); + Wicket.Log.log("No upload of field '" + this.inputName + "' to cancel: it has already ended, or never started."); } }; })(); diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/upload/resource/FileUploadToResourceField.properties b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/upload/resource/FileUploadToResourceField.properties index 5c115a887c..c614ece583 100644 --- a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/upload/resource/FileUploadToResourceField.properties +++ b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/upload/resource/FileUploadToResourceField.properties @@ -12,4 +12,6 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. -wicket.no.files.selected = Nothing to upload: no files were selected. \ No newline at end of file +wicket.no.files.selected = Nothing to upload: no files were selected. +uploadToResourceFailed = The upload failed. +uploadConnectionLost = The upload failed: the connection to the server was lost. diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/upload/resource/FolderUploadsFileManager.java b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/upload/resource/FolderUploadsFileManager.java index f877acc9a6..90414da153 100644 --- a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/upload/resource/FolderUploadsFileManager.java +++ b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/upload/resource/FolderUploadsFileManager.java @@ -22,6 +22,7 @@ import java.io.InputStream; import java.nio.file.Files; import java.nio.file.NoSuchFileException; import java.nio.file.Path; +import java.util.UUID; import org.apache.wicket.WicketRuntimeException; import org.apache.wicket.markup.html.form.upload.FileUpload; import org.apache.wicket.util.file.File; @@ -31,7 +32,8 @@ import org.apache.wicket.util.lang.Args; /** * Implementation of {@link IUploadsFileManager} that stores files in sub-folders of a given folder. * Sub-folders are named using the input field ID (and those are unique per application). Thus, there - * is no possibility that uploads get mixed up. + * is no possibility that uploads get mixed up. In its sub-folder, each file gets a random name of its + * own, so files of the same client file name do not replace each other. */ public class FolderUploadsFileManager implements IUploadsFileManager { @@ -84,12 +86,12 @@ public class FolderUploadsFileManager implements IUploadsFileManager } /** - * Validates {@code uploadFieldId} and {@code clientFileName} against the base folder to + * Validates {@code uploadFieldId} and {@code fileName} against the base folder to * prevent path traversal, and returns the fully resolved, canonical target file. * * @throws SecurityException if either component would escape the base folder */ - private java.io.File resolveTargetFile(String uploadFieldId, String clientFileName) + private java.io.File resolveTargetFile(String uploadFieldId, String fileName) throws IOException { Path baseFolderPath = getFolder().toPath().toRealPath(); @@ -98,21 +100,40 @@ public class FolderUploadsFileManager implements IUploadsFileManager { throw new SecurityException("Path traversal detected in uploadFieldId"); } - java.io.File target = new File(uploadFieldFolder, clientFileName).getCanonicalFile(); + java.io.File target = new File(uploadFieldFolder, fileName).getCanonicalFile(); Path uploadFieldFolderPath = getPathForComparison(uploadFieldFolder); if (!target.toPath().startsWith(uploadFieldFolderPath)) { - throw new SecurityException("Path traversal detected in client filename"); + throw new SecurityException("Path traversal detected in file name"); } return target; } + @Override + public String store(FileUpload fileItem, String uploadFieldId) + { + String fileName = UUID.randomUUID().toString(); + write(fileItem, uploadFieldId, fileName); + return fileName; + } + + /** + * @deprecated files saved under their client file name replace each other when they share it. + * Use {@link #store(FileUpload, String)} instead. + */ + @Deprecated(since = "11.0.0", forRemoval = true) + @SuppressWarnings("removal") @Override public void save(FileUpload fileItem, String uploadFieldId) + { + write(fileItem, uploadFieldId, fileItem.getClientFileName()); + } + + private void write(FileUpload fileItem, String uploadFieldId, String fileName) { try { - java.io.File target = resolveTargetFile(uploadFieldId, fileItem.getClientFileName()); + java.io.File target = resolveTargetFile(uploadFieldId, fileName); Files.createDirectories(target.toPath().getParent()); try (InputStream in = fileItem.getInputStream(); FileOutputStream out = new FileOutputStream(target)) @@ -127,11 +148,11 @@ public class FolderUploadsFileManager implements IUploadsFileManager } @Override - public File getFile(String uploadFieldId, String clientFileName) + public File getFile(String uploadFieldId, String fileName) { try { - return new File(resolveTargetFile(uploadFieldId, clientFileName)); + return new File(resolveTargetFile(uploadFieldId, fileName)); } catch (IOException e) { diff --git a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/upload/resource/IUploadsFileManager.java b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/upload/resource/IUploadsFileManager.java index 78a1208503..790f1ebfd5 100644 --- a/wicket-core/src/main/java/org/apache/wicket/markup/html/form/upload/resource/IUploadsFileManager.java +++ b/wicket-core/src/main/java/org/apache/wicket/markup/html/form/upload/resource/IUploadsFileManager.java @@ -25,28 +25,72 @@ import org.apache.wicket.util.file.File; * how to store file. Same identifier is used to retrieve the file, once uploaded, from component in page. * Mind that uploader resource is a singleton => identifier needs to be unique among different sessions * (and pages in a session). + * <p> + * An implementation implements {@link #store(FileUpload, String)} and + * {@link #getFile(String, String)}. */ public interface IUploadsFileManager { /** - * Saves an uploaded files into some persistent storage (e,g, disk). + * Saves an uploaded file into some persistent storage (e.g. disk), under a name of its own: + * files of the same client file name, sent in one upload or in several, must not replace each + * other. + * <p> + * The name goes to the browser, which sends it back for {@link #getFile(String, String)} to + * find the file. It must not reveal anything the user is not supposed to know. + * <p> + * The default implementation saves the file with {@link #save(FileUpload, String)} and returns + * its client file name, for implementations written before this method existed. An + * implementation has to implement one of the two methods. * * @param fileItem * The {@link FileUpload} * @param uploadFieldId * The unique ID of the upload field. + * @return the name the file was saved under, unique among the files of the upload field + * @since 11.0.0 */ - void save(FileUpload fileItem, String uploadFieldId); + default String store(FileUpload fileItem, String uploadFieldId) + { + save(fileItem, uploadFieldId); + return fileItem.getClientFileName(); + } /** - * Retrieves the file based on an uploadFieldId and clientFileName. + * Saves an uploaded file into some persistent storage (e.g. disk), under its client file name. + * <p> + * Wicket no longer calls this method, only the default {@link #store(FileUpload, String)} does. + * The default implementation throws an {@link UnsupportedOperationException}: an + * implementation that implements neither method fails at its first upload with an exception + * saying so. + * + * @param fileItem + * The {@link FileUpload} + * @param uploadFieldId + * The unique ID of the upload field. + * @deprecated files saved under their client file name replace each other when they share it. + * Implement and call {@link #store(FileUpload, String)} instead. + */ + @Deprecated(since = "11.0.0", forRemoval = true) + default void save(FileUpload fileItem, String uploadFieldId) + { + throw new UnsupportedOperationException(getClass().getName() + + " does not implement the deprecated save(FileUpload, String): it has to implement " + + "store(FileUpload, String), which is to be called instead"); + } + + /** + * Retrieves the file saved under a name by {@link #store(FileUpload, String)}. + * <p> + * The name comes back from the browser, which can change it: an implementation must never + * return a file that does not belong to the upload field. * * @param uploadFieldId * The unique ID of the upload field. - * @param clientFileName - * The client file name + * @param fileName + * The name {@link #store(FileUpload, String)} returned for the file * @return File the file */ - File getFile(String uploadFieldId, String clientFileName); + File getFile(String uploadFieldId, String fileName); } diff --git a/wicket-core/src/test/js/all.html b/wicket-core/src/test/js/all.html index ae6c778aad..83f4085202 100644 --- a/wicket-core/src/test/js/all.html +++ b/wicket-core/src/test/js/all.html @@ -156,6 +156,11 @@ <div id="usedAsContextWicket5025"></div> + <!-- + FileUploadToResourceField + --> + <input type="file" id="uploadToResourceInput" multiple="multiple"/> + <div id="ajaxIndicator"></div> </div> @@ -179,6 +184,7 @@ document.write("<scr"+"ipt src='../../main/java/org/apache/wicket/ajax/res/js/wicket-ajax.js'></scr"+"ipt>"); } </script> + <script type="text/javascript" src="../../main/java/org/apache/wicket/markup/html/form/upload/resource/FileUploadToResourceField.js"></script> <script type="text/javascript" src="qunit/qunit.js"></script> <script type="text/javascript" src="qunit/blanket.min.js"></script> @@ -194,5 +200,6 @@ <script type="text/javascript" src="head.js"></script> <script type="text/javascript" src="ajax.js"></script> <script type="text/javascript" src="timer.js"></script> + <script type="text/javascript" src="upload-to-resource.js"></script> </body> </html> diff --git a/wicket-core/src/test/js/upload-to-resource.js b/wicket-core/src/test/js/upload-to-resource.js new file mode 100644 index 0000000000..baac2feb0e --- /dev/null +++ b/wicket-core/src/test/js/upload-to-resource.js @@ -0,0 +1,240 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/*global QUnit: true, DataTransfer: true */ + +;(function (undefined) { + "use strict"; + + Wicket.Event.add(window, 'domready', function() { + const { module, test } = QUnit; + + const INPUT_ID = 'uploadToResourceInput'; + + // stands in for the XMLHttpRequest the field uploads with + function FakeXhr() { + FakeXhr.instances.push(this); + this.status = 0; + this.responseText = ''; + } + FakeXhr.prototype.open = function (method, url) { + this.method = method; + this.url = url; + }; + FakeXhr.prototype.send = function (body) { + this.body = body; + }; + FakeXhr.prototype.abort = function () { + this.aborted = true; + this.onabort(); + }; + FakeXhr.prototype.respond = function (status, responseText) { + this.status = status; + this.responseText = responseText; + this.onload(); + }; + FakeXhr.prototype.timeOut = function () { + this.ontimeout(); + }; + + let originalXhr, originalPost, reports, calls; + + function chooseFiles(files) { + const dataTransfer = new DataTransfer(); + files.forEach(file => dataTransfer.items.add(file)); + document.getElementById(INPUT_ID).files = dataTransfer.files; + } + + function newFile(name, size) { + return new File([new Uint8Array(size)], name, {type: 'text/plain'}); + } + + function newField(settings, beforeSend) { + return new Wicket.FileUploadToResourceField(Object.assign({ + inputName: INPUT_ID, + resourceUrl: 'resource', + ajaxCallBackUrl: 'callback', + maxSize: 100, + fileCountMax: -1, + uploadToken: 'token' + }, settings), + beforeSend || function () { return true; }, + function () { calls.push('success'); }, + function () { calls.push('cancel'); }, + function (res) { calls.push('error:' + res.errorMessage); }); + } + + module('Wicket.FileUploadToResourceField', { + beforeEach: function () { + originalXhr = window.XMLHttpRequest; + originalPost = Wicket.Ajax.post; + FakeXhr.instances = []; + window.XMLHttpRequest = FakeXhr; + reports = []; + calls = []; + Wicket.Ajax.post = function (attrs) { + reports.push(attrs); + }; + }, + afterEach: function () { + window.XMLHttpRequest = originalXhr; + Wicket.Ajax.post = originalPost; + } + }); + + test('sends all the files of the input in one request', assert => { + chooseFiles([newFile('a.txt', 3), newFile('b.txt', 4)]); + + assert.true(newField().upload()); + + assert.equal(FakeXhr.instances.length, 1); + const xhr = FakeXhr.instances[0]; + assert.equal(xhr.method, 'POST'); + assert.equal(xhr.body.getAll('WICKET-FILE-UPLOAD').length, 2); + }); + + test('reports the files received by POST', assert => { + chooseFiles([newFile('a.txt', 3)]); + newField().upload(); + + const filesInfo = '[{"fileName":"f1","clientFileName":"a.txt","size":3,"contentType":"text/plain"}]'; + FakeXhr.instances[0].respond(200, filesInfo); + + assert.deepEqual(calls, ['success']); + assert.equal(reports.length, 1); + assert.equal(reports[0].u, 'callback'); + assert.deepEqual(reports[0].ep, {'error': false, 'filesInfo': JSON.stringify(JSON.parse(filesInfo))}); + }); + + test('reports the error the resource answered', assert => { + chooseFiles([newFile('a.txt', 3)]); + newField().upload(); + + FakeXhr.instances[0].respond(200, '{"error":true,"errorMessage":"uploadTooManyFiles"}'); + + assert.deepEqual(calls, ['error:uploadTooManyFiles']); + assert.deepEqual(reports[0].ep, {'error': true, 'errorMessage': 'uploadTooManyFiles'}); + }); + + test('reports an HTTP error as a failed upload', assert => { + chooseFiles([newFile('a.txt', 3)]); + newField().upload(); + + FakeXhr.instances[0].respond(500, 'Internal Server Error'); + + assert.deepEqual(calls, ['error:uploadFailed']); + assert.deepEqual(reports[0].ep, {'error': true, 'errorMessage': 'uploadFailed'}); + }); + + test('reports a network failure as a lost connection', assert => { + chooseFiles([newFile('a.txt', 3)]); + newField().upload(); + + FakeXhr.instances[0].onerror(); + + assert.deepEqual(calls, ['error:uploadConnectionLost']); + assert.deepEqual(reports[0].ep, {'error': true, 'errorMessage': 'uploadConnectionLost'}); + }); + + test('reports a timed-out upload as a failed upload, and another one can start', assert => { + chooseFiles([newFile('a.txt', 3)]); + const field = newField(); + field.upload(); + + FakeXhr.instances[0].timeOut(); + + assert.deepEqual(calls, ['error:uploadFailed']); + assert.deepEqual(reports.map(report => report.ep), [{'error': true, 'errorMessage': 'uploadFailed'}]); + assert.true(field.upload(), 'sent again'); + assert.equal(FakeXhr.instances.length, 2); + }); + + test('reports an answer that is not JSON as a failed upload', assert => { + chooseFiles([newFile('a.txt', 3)]); + newField().upload(); + + FakeXhr.instances[0].respond(200, '<html>not JSON</html>'); + + assert.deepEqual(calls, ['error:uploadFailed']); + assert.deepEqual(reports[0].ep, {'error': true, 'errorMessage': 'uploadFailed'}); + }); + + test('does not send files beyond the limits of the field', assert => { + const field = newField({maxSize: 10, fileMaxSize: 4, fileCountMax: 2}); + + chooseFiles([newFile('a.txt', 6), newFile('b.txt', 6)]); + assert.false(field.upload(), 'beyond maxSize'); + chooseFiles([newFile('a.txt', 1), newFile('b.txt', 1), newFile('c.txt', 1)]); + assert.false(field.upload(), 'beyond fileCountMax'); + chooseFiles([newFile('a.txt', 5)]); + assert.false(field.upload(), 'beyond fileMaxSize'); + + assert.equal(FakeXhr.instances.length, 0); + assert.deepEqual(reports.map(report => report.ep.errorMessage), + ['uploadTooLarge', 'uploadTooManyFiles', 'uploadSingleFileTooLarge']); + }); + + test('refuses a second upload while one is running', assert => { + chooseFiles([newFile('a.txt', 3)]); + const field = newField(); + + assert.true(field.upload()); + assert.false(field.upload(), 'refused while the first one runs'); + assert.equal(FakeXhr.instances.length, 1); + + FakeXhr.instances[0].respond(200, '[]'); + assert.true(field.upload(), 'sent once the first one ended'); + assert.equal(FakeXhr.instances.length, 2); + }); + + test('cancels the running upload', assert => { + chooseFiles([newFile('a.txt', 3)]); + const field = newField(); + field.upload(); + + field.cancel(); + + assert.true(FakeXhr.instances[0].aborted); + assert.deepEqual(calls, ['cancel']); + assert.deepEqual(reports[0].ep, {'error': true, 'errorMessage': 'upload.canceled'}); + }); + + test('does not cancel an upload that has ended', assert => { + chooseFiles([newFile('a.txt', 3)]); + const field = newField(); + field.upload(); + FakeXhr.instances[0].respond(200, '[]'); + + field.cancel(); + + assert.notOk(FakeXhr.instances[0].aborted); + assert.deepEqual(calls, ['success']); + }); + + test('does not send the upload if the before-send callback returns false', assert => { + chooseFiles([newFile('a.txt', 3)]); + const field = newField({}, function () { return false; }); + + assert.false(field.upload()); + + assert.notOk(FakeXhr.instances[0].body, 'not sent'); + assert.deepEqual(reports[0].ep, {'error': true, 'errorMessage': 'upload.canceled'}); + field.upload(); + assert.equal(FakeXhr.instances.length, 2, 'the field is not left busy'); + }); + }); +})(); diff --git a/wicket-migration/src/main/resources/META-INF/rewrite/wicket.yml b/wicket-migration/src/main/resources/META-INF/rewrite/wicket.yml index 7bd0e14305..66cdaaa26f 100644 --- a/wicket-migration/src/main/resources/META-INF/rewrite/wicket.yml +++ b/wicket-migration/src/main/resources/META-INF/rewrite/wicket.yml @@ -89,6 +89,17 @@ recipeList: # handed the object the model was holding before that object is discarded; the default onDetach() # delegates to it, so an override of onDetach() that does not call super suppresses it. These # require manual migration. +# +# org.apache.wicket.markup.html.form.upload.resource.IUploadsFileManager#save() is deprecated in +# favour of store(), which stores the file under a name of its own and returns it; getFile() then +# receives that name. Wicket no longer calls save(), and its default throws: code calling it calls +# store() instead. The name travels through the browser, so getFile() must not resolve it to a +# file of another upload field. FolderUploadsFileManager stores files under random names, so code +# finding them by client file name uses UploadInfo#getFile() or #getFileName() instead. Subclasses +# of AbstractFileUploadResource receive the stored names in saveFiles(), prepareResponse() and +# generateJsonResponse(), subclasses of FileUploadResourceReference in generateFileInfo(). A +# failed upload reads "The upload failed." (resource key uploadToResourceFailed) instead of +# ending on the error page. These require manual migration. type: specs.openrewrite.org/v1beta/recipe name: org.apache.wicket.MigrateToWicket11 displayName: Migrate to Wicket 11.x
