You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@commons.apache.org by gg...@apache.org on 2023/06/06 15:58:43 UTC
[commons-fileupload] 01/02: Switch some code from IO to NIO
This is an automated email from the ASF dual-hosted git repository.
ggregory pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/commons-fileupload.git
commit e7bb78aeda998a39216b3e73edeb8e2a41065b1a
Author: Gary Gregory <ga...@gmail.com>
AuthorDate: Tue Jun 6 11:08:20 2023 -0400
Switch some code from IO to NIO
---
.../commons/fileupload2/disk/DiskFileItem.java | 20 +++++--------
.../fileupload2/disk/DiskFileItemFactory.java | 18 +++++------
.../fileupload2/DiskFileItemSerializeTest.java | 35 +++++++++-------------
3 files changed, 31 insertions(+), 42 deletions(-)
diff --git a/commons-fileupload2-core/src/main/java/org/apache/commons/fileupload2/disk/DiskFileItem.java b/commons-fileupload2-core/src/main/java/org/apache/commons/fileupload2/disk/DiskFileItem.java
index dad510a..e3a13b5 100644
--- a/commons-fileupload2-core/src/main/java/org/apache/commons/fileupload2/disk/DiskFileItem.java
+++ b/commons-fileupload2-core/src/main/java/org/apache/commons/fileupload2/disk/DiskFileItem.java
@@ -39,6 +39,7 @@ import org.apache.commons.fileupload2.InvalidFileNameException;
import org.apache.commons.fileupload2.ParameterParser;
import org.apache.commons.io.Charsets;
import org.apache.commons.io.FileUtils;
+import org.apache.commons.io.file.PathUtils;
import org.apache.commons.io.function.Uncheck;
import org.apache.commons.io.output.DeferredFileOutputStream;
@@ -157,7 +158,7 @@ public class DiskFileItem implements FileItem {
/**
* The directory in which uploaded files will be stored, if stored on disk.
*/
- private final File repository;
+ private final Path repository;
/**
* Cached contents of the file.
@@ -172,7 +173,7 @@ public class DiskFileItem implements FileItem {
/**
* The temporary file to use.
*/
- private transient File tempFile;
+ private transient Path tempFile;
/**
* The file items headers.
@@ -195,13 +196,13 @@ public class DiskFileItem implements FileItem {
* @param repository The data repository, which is the directory in which files will be created, should the item size exceed the threshold.
*/
public DiskFileItem(final String fieldName, final String contentType, final boolean isFormField, final String fileName, final int sizeThreshold,
- final File repository) {
+ final Path repository) {
this.fieldName = fieldName;
this.contentType = contentType;
this.isFormField = isFormField;
this.fileName = fileName;
this.sizeThreshold = sizeThreshold;
- this.repository = repository;
+ this.repository = repository != null ? repository : PathUtils.getTempDirectory();
}
/**
@@ -330,7 +331,7 @@ public class DiskFileItem implements FileItem {
@Override
public OutputStream getOutputStream() {
if (dfos == null) {
- dfos = DeferredFileOutputStream.builder().setThreshold(sizeThreshold).setOutputFile(getTempFile()).get();
+ dfos = DeferredFileOutputStream.builder().setThreshold(sizeThreshold).setOutputFile(getTempFile().toFile()).get();
}
return dfos;
}
@@ -409,14 +410,9 @@ public class DiskFileItem implements FileItem {
*
* @return The {@link java.io.File File} to be used for temporary storage.
*/
- protected File getTempFile() {
+ protected Path getTempFile() {
if (tempFile == null) {
- File tempDir = repository;
- if (tempDir == null) {
- tempDir = FileUtils.getTempDirectory();
- }
- final String tempFileName = String.format("upload_%s_%s.tmp", UID, getUniqueId());
- tempFile = new File(tempDir, tempFileName);
+ tempFile = repository.resolve(String.format("upload_%s_%s.tmp", UID, getUniqueId()));
}
return tempFile;
}
diff --git a/commons-fileupload2-core/src/main/java/org/apache/commons/fileupload2/disk/DiskFileItemFactory.java b/commons-fileupload2-core/src/main/java/org/apache/commons/fileupload2/disk/DiskFileItemFactory.java
index 92a13a7..bf881f0 100644
--- a/commons-fileupload2-core/src/main/java/org/apache/commons/fileupload2/disk/DiskFileItemFactory.java
+++ b/commons-fileupload2-core/src/main/java/org/apache/commons/fileupload2/disk/DiskFileItemFactory.java
@@ -16,8 +16,8 @@
*/
package org.apache.commons.fileupload2.disk;
-import java.io.File;
import java.nio.charset.Charset;
+import java.nio.file.Path;
import org.apache.commons.fileupload2.FileItem;
import org.apache.commons.fileupload2.FileItemFactory;
@@ -40,7 +40,7 @@ import org.apache.commons.io.FileCleaningTracker;
* <p>
* <b>NOTE</b>: Files are created in the system default temp directory with predictable names. This means that a local attacker with write access to that
* directory can perform a TOUTOC attack to replace any uploaded file with a file of the attackers choice. The implications of this will depend on how the
- * uploaded file is used but could be significant. When using this implementation in an environment with local, untrusted users, {@link #setRepository(File)}
+ * uploaded file is used but could be significant. When using this implementation in an environment with local, untrusted users, {@link #setRepository(Path)}
* MUST be used to configure a repository location that is not publicly writable. In a Servlet container the location identified by the ServletContext attribute
* {@code javax.servlet.context.tempdir} may be used.
* </p>
@@ -49,7 +49,7 @@ import org.apache.commons.io.FileCleaningTracker;
* set on the {@link DiskFileItemFactory}. However, if you do use such a tracker, then you must consider the following: Temporary files are automatically
* deleted as soon as they are no longer needed. (More precisely, when the corresponding instance of {@link java.io.File} is garbage collected.) This is done by
* the so-called reaper thread, which is started and stopped automatically by the {@link FileCleaningTracker} when there are files to be tracked. It might make
- * sense to terminate that thread, for example, if your web application ends. See the section on "Resource cleanup" in the users guide of commons-fileupload.
+ * sense to terminate that thread, for example, if your web application ends. See the section on "Resource cleanup" in the users guide of Commons FileUpload.
* </p>
*/
public class DiskFileItemFactory implements FileItemFactory {
@@ -62,7 +62,7 @@ public class DiskFileItemFactory implements FileItemFactory {
/**
* The directory in which uploaded files will be stored, if stored on disk.
*/
- private File repository;
+ private Path repository;
/**
* The threshold above which uploads will be stored on disk.
@@ -95,7 +95,7 @@ public class DiskFileItemFactory implements FileItemFactory {
* @param sizeThreshold The threshold, in bytes, below which items will be retained in memory and above which they will be stored as a file.
* @param repository The data repository, which is the directory in which files will be created, should the item size exceed the threshold.
*/
- public DiskFileItemFactory(final int sizeThreshold, final File repository) {
+ public DiskFileItemFactory(final int sizeThreshold, final Path repository) {
this.sizeThreshold = sizeThreshold;
this.repository = repository;
}
@@ -115,7 +115,7 @@ public class DiskFileItemFactory implements FileItemFactory {
result.setDefaultCharset(defaultCharset);
final FileCleaningTracker tracker = getFileCleaningTracker();
if (tracker != null) {
- tracker.track(result.getTempFile(), result);
+ tracker.track(result.getTempFile().toFile(), result);
}
return result;
}
@@ -142,9 +142,9 @@ public class DiskFileItemFactory implements FileItemFactory {
* Gets the directory used to temporarily store files that are larger than the configured size threshold.
*
* @return The directory in which temporary files will be located.
- * @see #setRepository(File)
+ * @see #setRepository(Path)
*/
- public File getRepository() {
+ public Path getRepository() {
return repository;
}
@@ -182,7 +182,7 @@ public class DiskFileItemFactory implements FileItemFactory {
* @param repository The directory in which temporary files will be located.
* @see #getRepository()
*/
- public void setRepository(final File repository) {
+ public void setRepository(final Path repository) {
this.repository = repository;
}
diff --git a/commons-fileupload2-core/src/test/java/org/apache/commons/fileupload2/DiskFileItemSerializeTest.java b/commons-fileupload2-core/src/test/java/org/apache/commons/fileupload2/DiskFileItemSerializeTest.java
index dad40e7..313a7cf 100644
--- a/commons-fileupload2-core/src/test/java/org/apache/commons/fileupload2/DiskFileItemSerializeTest.java
+++ b/commons-fileupload2-core/src/test/java/org/apache/commons/fileupload2/DiskFileItemSerializeTest.java
@@ -27,10 +27,12 @@ import java.io.File;
import java.io.IOException;
import java.io.ObjectOutputStream;
import java.io.OutputStream;
-import java.nio.file.InvalidPathException;
+import java.nio.file.Files;
+import java.nio.file.Path;
import org.apache.commons.fileupload2.disk.DiskFileItemFactory;
import org.apache.commons.io.FileUtils;
+import org.apache.commons.io.file.PathUtils;
import org.apache.commons.lang3.SerializationUtils;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
@@ -42,7 +44,7 @@ import org.junit.jupiter.api.Test;
public class DiskFileItemSerializeTest {
// Use a private repository to catch any files left over by tests
- private static final File REPOSITORY = new File(FileUtils.getTempDirectory(), "DiskFileItemRepo");
+ private static final Path REPOSITORY = PathUtils.getTempDirectory().resolve("DiskFileItemRepo");
/**
* Content type for regular form items.
@@ -92,7 +94,7 @@ public class DiskFileItemSerializeTest {
/**
* Create a FileItem with the specfied content bytes and repository.
*/
- private FileItem createFileItem(final byte[] contentBytes, final File repository) throws IOException {
+ private FileItem createFileItem(final byte[] contentBytes, final Path repository) throws IOException {
final FileItemFactory factory = new DiskFileItemFactory(THRESHOLD, repository);
final String textFieldName = "textField";
@@ -124,18 +126,20 @@ public class DiskFileItemSerializeTest {
@BeforeEach
public void setUp() throws IOException {
- if (REPOSITORY.exists()) {
- FileUtils.deleteDirectory(REPOSITORY);
+ if (Files.exists(REPOSITORY)) {
+ PathUtils.deleteDirectory(REPOSITORY);
+ } else {
+ Files.createDirectories(REPOSITORY);
}
- FileUtils.forceMkdir(REPOSITORY);
}
@AfterEach
public void tearDown() throws IOException {
- for (final File file : FileUtils.listFiles(REPOSITORY, null, true)) {
+
+ for (final File file : FileUtils.listFiles(REPOSITORY.toFile(), null, true)) {
System.out.println("Found leftover file " + file);
}
- FileUtils.deleteDirectory(REPOSITORY);
+ PathUtils.deleteDirectory(REPOSITORY);
}
/**
@@ -176,7 +180,7 @@ public class DiskFileItemSerializeTest {
/**
* Helper method to test creation of a field when a repository is used.
*/
- public void testInMemoryObject(final byte[] testFieldValueBytes, final File repository) throws IOException {
+ public void testInMemoryObject(final byte[] testFieldValueBytes, final Path repository) throws IOException {
final FileItem item = createFileItem(testFieldValueBytes, repository);
// Check state is as expected
@@ -194,22 +198,11 @@ public class DiskFileItemSerializeTest {
public void testInvalidRepository() throws IOException {
// Create the FileItem
final byte[] testFieldValueBytes = createContentBytes(THRESHOLD);
- final File repository = new File(FileUtils.getTempDirectory(), "file");
+ final Path repository = PathUtils.getTempDirectory().resolve("file");
final FileItem item = createFileItem(testFieldValueBytes, repository);
assertThrows(IOException.class, () -> deserialize(serialize(item)));
}
- /**
- * Fails when repository contains a null character.
- */
- @Test
- public void testInvalidRepositoryWithNullChar() {
- // Create the FileItem
- final byte[] testFieldValueBytes = createContentBytes(THRESHOLD);
- final File repository = new File(FileUtils.getTempDirectory(), "\0");
- assertThrows(InvalidPathException.class, () -> createFileItem(testFieldValueBytes, repository));
- }
-
/**
* Test creation of a field for which the amount of data equals the configured threshold.
*/