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/11 13:56:14 UTC
[commons-fileupload] branch master updated: Make DiskFileItemFactory fluent and immutable
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
The following commit(s) were added to refs/heads/master by this push:
new 8a0c4e1 Make DiskFileItemFactory fluent and immutable
8a0c4e1 is described below
commit 8a0c4e1a01eaf153a740ab6b240ebc0bd51f0e91
Author: Gary Gregory <ga...@gmail.com>
AuthorDate: Sun Jun 11 09:56:10 2023 -0400
Make DiskFileItemFactory fluent and immutable
Use the builder.
---
.../commons/fileupload2/disk/DiskFileItem.java | 16 ++--
.../fileupload2/disk/DiskFileItemFactory.java | 103 +++++++++------------
.../jakarta/JakartaServletFileUploadTest.java | 10 +-
.../javax/JavaxServletFileUploadTest.java | 10 +-
4 files changed, 63 insertions(+), 76 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 ee4e628..9f6439f 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
@@ -289,9 +289,9 @@ public final class DiskFileItem implements FileItem {
private FileItemHeaders fileItemHeaders;
/**
- * Default content charset to be used when no explicit charset parameter is provided by the sender.
+ * Default content Charset to be used when no explicit Charset parameter is provided by the sender.
*/
- private Charset defaultCharset = DEFAULT_CHARSET;
+ private Charset charsetDefault = DEFAULT_CHARSET;
/**
* Constructs a new {@code DiskFileItem} instance.
@@ -309,7 +309,7 @@ public final class DiskFileItem implements FileItem {
final Path repository, final FileItemHeaders fileItemHeaders, final Charset defaultCharset) {
this.fieldName = fieldName;
this.contentType = contentType;
- this.defaultCharset = defaultCharset;
+ this.charsetDefault = defaultCharset;
this.isFormField = isFormField;
this.fileName = fileName;
this.fileItemHeaders = fileItemHeaders;
@@ -364,7 +364,7 @@ public final class DiskFileItem implements FileItem {
parser.setLowerCaseNames(true);
// Parameter parser can handle null input
final Map<String, String> params = parser.parse(getContentType(), ';');
- return Charsets.toCharset(params.get("charset"), defaultCharset);
+ return Charsets.toCharset(params.get("charset"), charsetDefault);
}
/**
@@ -382,8 +382,8 @@ public final class DiskFileItem implements FileItem {
*
* @return the default charset
*/
- public Charset getDefaultCharset() {
- return defaultCharset;
+ public Charset getCharsetDefault() {
+ return charsetDefault;
}
/**
@@ -560,8 +560,8 @@ public final class DiskFileItem implements FileItem {
*
* @param charset the default charset
*/
- public void setDefaultCharset(final Charset charset) {
- defaultCharset = charset;
+ public void setCharsetDefault(final Charset charset) {
+ charsetDefault = charset;
}
/**
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 ac82897..a7ce5df 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
@@ -30,9 +30,8 @@ import org.apache.commons.io.file.PathUtils;
/**
* The default {@link FileItemFactory} implementation.
* <p>
- * This implementation creates {@link FileItem} instances which keep their content either in memory, for smaller items, or in a
- * temporary file on disk, for larger items. The size threshold, above which content will be stored on disk, is configurable, as is the directory in which
- * temporary files will be created.
+ * This implementation creates {@link FileItem} instances which keep their content either in memory, for smaller items, or in a temporary file on disk, for
+ * larger items. The size threshold, above which content will be stored on disk, is configurable, as is the directory in which temporary files will be created.
* </p>
* <p>
* If not otherwise configured, the default configuration values are as follows:
@@ -44,7 +43,7 @@ import org.apache.commons.io.file.PathUtils;
* <p>
* <b>NOTE</b>: Files are created in the system default temporary 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(Path)}
+ * uploaded file is used but could be significant. When using this implementation in an environment with local, untrusted users, {@link Builder#setPath(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>
@@ -55,6 +54,7 @@ import org.apache.commons.io.file.PathUtils;
* 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.
* </p>
+ *
* @see Builder
* @see Builder#get()
*/
@@ -67,18 +67,25 @@ public final class DiskFileItemFactory implements FileItemFactory {
* </p>
*
* <pre>{@code
- * DiskFileItemFactory factory = DiskFileItemFactory.builder()
- * .setPath(path)
- * .setBufferSize(DEFAULT_THRESHOLD)
- * .get();
+ * DiskFileItemFactory factory = DiskFileItemFactory.builder().setPath(path).setBufferSize(DEFAULT_THRESHOLD).get();
* }
* </pre>
*/
public static class Builder extends AbstractStreamBuilder<DiskFileItemFactory, Builder> {
+ /**
+ * The instance of {@link FileCleaningTracker}, which is responsible for deleting temporary files.
+ * <p>
+ * May be null, if tracking files is not required.
+ * </p>
+ */
+ private FileCleaningTracker fileCleaningTracker;
+
public Builder() {
setBufferSize(DEFAULT_THRESHOLD);
setPath(PathUtils.getTempDirectory());
+ setCharset(DiskFileItem.DEFAULT_CHARSET);
+ setCharsetDefault(DiskFileItem.DEFAULT_CHARSET);
}
/**
@@ -97,7 +104,18 @@ public final class DiskFileItemFactory implements FileItemFactory {
*/
@Override
public DiskFileItemFactory get() {
- return new DiskFileItemFactory(getOrigin().getPath(), getBufferSize());
+ return new DiskFileItemFactory(getOrigin().getPath(), getBufferSize(), getCharset(), fileCleaningTracker);
+ }
+
+ /**
+ * Sets the tracker, which is responsible for deleting temporary files.
+ *
+ * @param fileCleaningTracker Callback to track files created, or null (default) to disable tracking.
+ * @return this
+ */
+ public Builder setFileCleaningTracker(final FileCleaningTracker fileCleaningTracker) {
+ this.fileCleaningTracker = fileCleaningTracker;
+ return this;
}
}
@@ -119,12 +137,12 @@ public final class DiskFileItemFactory implements FileItemFactory {
/**
* The directory in which uploaded files will be stored, if stored on disk.
*/
- private Path repository;
+ private final Path repository;
/**
* The threshold above which uploads will be stored on disk.
*/
- private int threshold = DEFAULT_THRESHOLD;
+ private final int threshold;
/**
* The instance of {@link FileCleaningTracker}, which is responsible for deleting temporary files.
@@ -132,21 +150,26 @@ public final class DiskFileItemFactory implements FileItemFactory {
* May be null, if tracking files is not required.
* </p>
*/
- private FileCleaningTracker fileCleaningTracker;
+ private final FileCleaningTracker fileCleaningTracker;
/**
- * Default content charset to be used when no explicit charset parameter is provided by the sender.
+ * Default content Charset to be used when no explicit Charset parameter is provided by the sender.
*/
- private Charset defaultCharset = DiskFileItem.DEFAULT_CHARSET;
+ private final Charset charsetDefault;
/**
* Constructs a preconfigured instance of this class.
- * @param repository The data repository, which is the directory in which files will be created, should the item size exceed the threshold.
- * @param threshold 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.
+ * @param threshold The threshold, in bytes, below which items will be retained in memory and above which they will be stored as a file.
+ * @param charsetDefault Sets the default charset for use when no explicit charset parameter is provided by the sender.
+ * @param fileCleaningTracker Callback to track files created, or null (default) to disable tracking.
*/
- private DiskFileItemFactory(final Path repository, final int threshold) {
+ private DiskFileItemFactory(final Path repository, final int threshold, final Charset charsetDefault, final FileCleaningTracker fileCleaningTracker) {
this.threshold = threshold;
this.repository = repository;
+ this.charsetDefault = charsetDefault;
+ this.fileCleaningTracker = fileCleaningTracker;
}
/**
@@ -165,7 +188,7 @@ public final class DiskFileItemFactory implements FileItemFactory {
final DiskFileItem result = DiskFileItem.builder()
.setBufferSize(threshold)
.setContentType(contentType)
- .setCharset(defaultCharset)
+ .setCharset(charsetDefault)
.setFieldName(fieldName)
.setFileItemHeaders(fileItemHeaders)
.setFileName(fileName)
@@ -185,8 +208,8 @@ public final class DiskFileItemFactory implements FileItemFactory {
*
* @return the default charset
*/
- public Charset getDefaultCharset() {
- return defaultCharset;
+ public Charset getCharsetDefault() {
+ return charsetDefault;
}
/**
@@ -202,7 +225,6 @@ public final 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(Path)
*/
public Path getRepository() {
return repository;
@@ -212,47 +234,8 @@ public final class DiskFileItemFactory implements FileItemFactory {
* Gets the size threshold beyond which files are written directly to disk. The default value is {@value #DEFAULT_THRESHOLD} bytes.
*
* @return The size threshold in bytes.
- * @see #setThreshold(int)
*/
public int getThreshold() {
return threshold;
}
-
- /**
- * Sets the default charset for use when no explicit charset parameter is provided by the sender.
- *
- * @param charset the default charset
- */
- public void setDefaultCharset(final Charset charset) {
- defaultCharset = charset;
- }
-
- /**
- * Sets the tracker, which is responsible for deleting temporary files.
- *
- * @param tracker An instance of {@link FileCleaningTracker}, which will from now on track the created files, or null (default), to disable tracking.
- */
- public void setFileCleaningTracker(final FileCleaningTracker tracker) {
- fileCleaningTracker = tracker;
- }
-
- /**
- * Sets the directory used to temporarily store files that are larger than the configured size threshold.
- *
- * @param repository The directory in which temporary files will be located.
- * @see #getRepository()
- */
- public void setRepository(final Path repository) {
- this.repository = repository;
- }
-
- /**
- * Sets the size threshold beyond which files are written directly to disk. The default value is {@value #DEFAULT_THRESHOLD} bytes.
- *
- * @param threshold The size threshold in bytes.
- * @see #getThreshold()
- */
- public void setThreshold(final int threshold) {
- this.threshold = threshold;
- }
}
diff --git a/commons-fileupload2-jakarta/src/test/java/org/apache/commons/fileupload2/jakarta/JakartaServletFileUploadTest.java b/commons-fileupload2-jakarta/src/test/java/org/apache/commons/fileupload2/jakarta/JakartaServletFileUploadTest.java
index 9f65a2e..ae143a9 100644
--- a/commons-fileupload2-jakarta/src/test/java/org/apache/commons/fileupload2/jakarta/JakartaServletFileUploadTest.java
+++ b/commons-fileupload2-jakarta/src/test/java/org/apache/commons/fileupload2/jakarta/JakartaServletFileUploadTest.java
@@ -57,16 +57,18 @@ public class JakartaServletFileUploadTest extends AbstractFileUploadTest<Jakarta
final byte[] bytes = text.getBytes(StandardCharsets.UTF_8);
final HttpServletRequest request = new JakartaMockServletHttpRequest(bytes, Constants.CONTENT_TYPE);
-
- final DiskFileItemFactory fileItemFactory = DiskFileItemFactory.builder().get();
- fileItemFactory.setDefaultCharset(StandardCharsets.UTF_8);
+ // @formatter:off
+ final DiskFileItemFactory fileItemFactory = DiskFileItemFactory.builder()
+ .setCharset(StandardCharsets.UTF_8)
+ .get();
+ // @formatter:on
final JakartaServletFileUpload upload = new JakartaServletFileUpload(fileItemFactory);
final List<FileItem> fileItems = upload.parseRequest(request);
final FileItem fileItem = fileItems.get(0);
assertTrue(fileItem.getString().contains("coñteñt"), fileItem.getString());
}
- /**
+ /*
* Test case for <a href="https://issues.apache.org/jira/browse/FILEUPLOAD-210">
*/
@Test
diff --git a/commons-fileupload2-javax/src/test/java/org/apache/commons/fileupload2/javax/JavaxServletFileUploadTest.java b/commons-fileupload2-javax/src/test/java/org/apache/commons/fileupload2/javax/JavaxServletFileUploadTest.java
index 79b0205..a6d2b03 100644
--- a/commons-fileupload2-javax/src/test/java/org/apache/commons/fileupload2/javax/JavaxServletFileUploadTest.java
+++ b/commons-fileupload2-javax/src/test/java/org/apache/commons/fileupload2/javax/JavaxServletFileUploadTest.java
@@ -61,16 +61,18 @@ public class JavaxServletFileUploadTest extends AbstractFileUploadTest<JavaxServ
final byte[] bytes = text.getBytes(StandardCharsets.UTF_8);
final HttpServletRequest request = new JavaxMockHttpServletRequest(bytes, Constants.CONTENT_TYPE);
-
- final DiskFileItemFactory fileItemFactory = DiskFileItemFactory.builder().get();
- fileItemFactory.setDefaultCharset(StandardCharsets.UTF_8);
+ // @formatter:off
+ final DiskFileItemFactory fileItemFactory = DiskFileItemFactory.builder()
+ .setCharset(StandardCharsets.UTF_8)
+ .get();
+ // @formatter:on
final JavaxServletFileUpload upload = new JavaxServletFileUpload(fileItemFactory);
final List<FileItem> fileItems = upload.parseRequest(request);
final FileItem fileItem = fileItems.get(0);
assertTrue(fileItem.getString().contains("coñteñt"), fileItem.getString());
}
- /**
+ /*
* Test case for <a href="https://issues.apache.org/jira/browse/FILEUPLOAD-210">
*/
@Test