From 6734e1122201ba7bf8698a73c7fec3f9e0f9a834 Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Sun, 12 Aug 2018 09:31:08 +0200 Subject: [PATCH] Adapt MultipartConfig to use DataSize support See gh-13974 --- .../web/servlet/MultipartProperties.java | 56 +++++----- ...spatcherServletAutoConfigurationTests.java | 5 +- .../appendix-application-properties.adoc | 6 +- .../web/servlet/MultipartConfigFactory.java | 104 +++++++++++------- .../servlet/MultipartConfigFactoryTests.java | 35 +++++- 5 files changed, 130 insertions(+), 76 deletions(-) diff --git a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/MultipartProperties.java b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/MultipartProperties.java index e7b1649249..8331ff0bf6 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/MultipartProperties.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/main/java/org/springframework/boot/autoconfigure/web/servlet/MultipartProperties.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2017 the original author or authors. + * Copyright 2012-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -19,8 +19,11 @@ package org.springframework.boot.autoconfigure.web.servlet; import javax.servlet.MultipartConfigElement; import org.springframework.boot.context.properties.ConfigurationProperties; +import org.springframework.boot.context.properties.PropertyMapper; +import org.springframework.boot.convert.DataSizeUnit; import org.springframework.boot.web.servlet.MultipartConfigFactory; -import org.springframework.util.StringUtils; +import org.springframework.util.unit.DataSize; +import org.springframework.util.unit.DataUnit; /** * Properties to be used in configuring a {@link MultipartConfigElement}. @@ -37,10 +40,11 @@ import org.springframework.util.StringUtils; *

* These properties are ultimately passed to {@link MultipartConfigFactory} which means * you may specify numeric values using {@literal long} values or using more readable - * {@literal String} variants that accept {@literal KB} or {@literal MB} suffixes. + * {@link DataSize} variants. * * @author Josh Long * @author Toshiaki Maki + * @author Stephane Nicoll * @since 1.1.0 */ @ConfigurationProperties(prefix = "spring.servlet.multipart", ignoreUnknownFields = false) @@ -57,22 +61,21 @@ public class MultipartProperties { private String location; /** - * Max file size. Values can use the suffixes "MB" or "KB" to indicate megabytes or - * kilobytes, respectively. + * Max file size. */ - private String maxFileSize = "1MB"; + @DataSizeUnit(DataUnit.MEGABYTES) + private DataSize maxFileSize = DataSize.ofMegaBytes(1); /** - * Max request size. Values can use the suffixes "MB" or "KB" to indicate megabytes or - * kilobytes, respectively. + * Max request size. */ - private String maxRequestSize = "10MB"; + @DataSizeUnit(DataUnit.MEGABYTES) + private DataSize maxRequestSize = DataSize.ofMegaBytes(10); /** - * Threshold after which files are written to disk. Values can use the suffixes "MB" - * or "KB" to indicate megabytes or kilobytes, respectively. + * Threshold after which files are written to disk. */ - private String fileSizeThreshold = "0"; + private DataSize fileSizeThreshold = DataSize.ofBytes(0); /** * Whether to resolve the multipart request lazily at the time of file or parameter @@ -96,27 +99,27 @@ public class MultipartProperties { this.location = location; } - public String getMaxFileSize() { + public DataSize getMaxFileSize() { return this.maxFileSize; } - public void setMaxFileSize(String maxFileSize) { + public void setMaxFileSize(DataSize maxFileSize) { this.maxFileSize = maxFileSize; } - public String getMaxRequestSize() { + public DataSize getMaxRequestSize() { return this.maxRequestSize; } - public void setMaxRequestSize(String maxRequestSize) { + public void setMaxRequestSize(DataSize maxRequestSize) { this.maxRequestSize = maxRequestSize; } - public String getFileSizeThreshold() { + public DataSize getFileSizeThreshold() { return this.fileSizeThreshold; } - public void setFileSizeThreshold(String fileSizeThreshold) { + public void setFileSizeThreshold(DataSize fileSizeThreshold) { this.fileSizeThreshold = fileSizeThreshold; } @@ -134,18 +137,11 @@ public class MultipartProperties { */ public MultipartConfigElement createMultipartConfig() { MultipartConfigFactory factory = new MultipartConfigFactory(); - if (StringUtils.hasText(this.fileSizeThreshold)) { - factory.setFileSizeThreshold(this.fileSizeThreshold); - } - if (StringUtils.hasText(this.location)) { - factory.setLocation(this.location); - } - if (StringUtils.hasText(this.maxRequestSize)) { - factory.setMaxRequestSize(this.maxRequestSize); - } - if (StringUtils.hasText(this.maxFileSize)) { - factory.setMaxFileSize(this.maxFileSize); - } + PropertyMapper map = PropertyMapper.get().alwaysApplyingWhenNonNull(); + map.from(() -> this.fileSizeThreshold).to(factory::setFileSizeThreshold); + map.from(() -> this.location).whenHasText().to(factory::setLocation); + map.from(() -> this.maxRequestSize).to(factory::setMaxRequestSize); + map.from(() -> this.maxFileSize).to(factory::setMaxFileSize); return factory.createMultipartConfig(); } diff --git a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/DispatcherServletAutoConfigurationTests.java b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/DispatcherServletAutoConfigurationTests.java index 9ad1d12203..a36a5c9ed6 100644 --- a/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/DispatcherServletAutoConfigurationTests.java +++ b/spring-boot-project/spring-boot-autoconfigure/src/test/java/org/springframework/boot/autoconfigure/web/servlet/DispatcherServletAutoConfigurationTests.java @@ -29,6 +29,7 @@ import org.springframework.boot.web.servlet.ServletRegistrationBean; import org.springframework.context.annotation.Bean; import org.springframework.context.annotation.Configuration; import org.springframework.context.event.ContextRefreshedEvent; +import org.springframework.util.unit.DataSize; import org.springframework.web.multipart.MultipartException; import org.springframework.web.multipart.MultipartHttpServletRequest; import org.springframework.web.multipart.MultipartResolver; @@ -210,8 +211,8 @@ public class DispatcherServletAutoConfigurationTests { @Bean public MultipartConfigElement multipartConfig() { MultipartConfigFactory factory = new MultipartConfigFactory(); - factory.setMaxFileSize("128KB"); - factory.setMaxRequestSize("128KB"); + factory.setMaxFileSize(DataSize.ofKiloBytes(128)); + factory.setMaxRequestSize(DataSize.ofKiloBytes(128)); return factory.createMultipartConfig(); } diff --git a/spring-boot-project/spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc b/spring-boot-project/spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc index bfa76439a6..e19e1b9823 100644 --- a/spring-boot-project/spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc +++ b/spring-boot-project/spring-boot-docs/src/main/asciidoc/appendix-application-properties.adoc @@ -338,10 +338,10 @@ content into your application. Rather, pick only the properties that you need. # MULTIPART ({sc-spring-boot-autoconfigure}/web/servlet/MultipartProperties.{sc-ext}[MultipartProperties]) spring.servlet.multipart.enabled=true # Whether to enable support of multipart uploads. - spring.servlet.multipart.file-size-threshold=0 # Threshold after which files are written to disk. Values can use the suffixes "MB" or "KB" to indicate megabytes or kilobytes, respectively. + spring.servlet.multipart.file-size-threshold=0 # Threshold after which files are written to disk. spring.servlet.multipart.location= # Intermediate location of uploaded files. - spring.servlet.multipart.max-file-size=1MB # Max file size. Values can use the suffixes "MB" or "KB" to indicate megabytes or kilobytes, respectively. - spring.servlet.multipart.max-request-size=10MB # Max request size. Values can use the suffixes "MB" or "KB" to indicate megabytes or kilobytes, respectively. + spring.servlet.multipart.max-file-size=1MB # Max file size. + spring.servlet.multipart.max-request-size=10MB # Max request size. spring.servlet.multipart.resolve-lazily=false # Whether to resolve the multipart request lazily at the time of file or parameter access. # JACKSON ({sc-spring-boot-autoconfigure}/jackson/JacksonProperties.{sc-ext}[JacksonProperties]) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/MultipartConfigFactory.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/MultipartConfigFactory.java index 28476c0734..d9adc25764 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/MultipartConfigFactory.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/web/servlet/MultipartConfigFactory.java @@ -16,21 +16,14 @@ package org.springframework.boot.web.servlet; -import java.util.Locale; - import javax.servlet.MultipartConfigElement; -import org.springframework.util.Assert; +import org.springframework.util.unit.DataSize; /** * Factory that can be used to create a {@link MultipartConfigElement}. Size values can be * set using traditional {@literal long} values which are set in bytes or using more - * readable {@literal String} variants that accept KB or MB suffixes, for example: - * - * 

- * factory.setMaxFileSize("10MB");
- * factory.setMaxRequestSize("100KB");
- *
+ * convenient {@link DataSize} variants. * * @author Phillip Webb * @since 1.4.0 @@ -39,11 +32,11 @@ public class MultipartConfigFactory { private String location; - private long maxFileSize = -1; + private DataSize maxFileSize; - private long maxRequestSize = -1; + private DataSize maxRequestSize; - private int fileSizeThreshold = 0; + private DataSize fileSizeThreshold; /** * Sets the directory location where files will be stored. @@ -53,73 +46,91 @@ public class MultipartConfigFactory { this.location = location; } + /** + * Sets the maximum {@link DataSize size} allowed for uploaded files. + * @param maxFileSize the maximum file size + */ + public void setMaxFileSize(DataSize maxFileSize) { + this.maxFileSize = maxFileSize; + } + /** * Sets the maximum size in bytes allowed for uploaded files. * @param maxFileSize the maximum file size - * @see #setMaxFileSize(String) + * @deprecated since 2.1.0 in favour of {@link #setMaxFileSize(DataSize)} */ + @Deprecated public void setMaxFileSize(long maxFileSize) { - this.maxFileSize = maxFileSize; + setMaxFileSize(DataSize.ofBytes(maxFileSize)); } /** * Sets the maximum size allowed for uploaded files. Values can use the suffixed "MB" * or "KB" to indicate a Megabyte or Kilobyte size. * @param maxFileSize the maximum file size - * @see #setMaxFileSize(long) + * @deprecated since 2.1.0 in favour of {@link #setMaxFileSize(DataSize)} */ + @Deprecated public void setMaxFileSize(String maxFileSize) { - this.maxFileSize = parseSize(maxFileSize); + setMaxFileSize(DataSize.parse(maxFileSize)); + } + + /** + * Sets the maximum {@link DataSize} allowed for multipart/form-data requests. + * @param maxRequestSize the maximum request size + */ + public void setMaxRequestSize(DataSize maxRequestSize) { + this.maxRequestSize = maxRequestSize; } /** * Sets the maximum size allowed in bytes for multipart/form-data requests. * @param maxRequestSize the maximum request size - * @see #setMaxRequestSize(String) + * @deprecated since 2.1.0 in favour of {@link #setMaxRequestSize(DataSize)} */ + @Deprecated public void setMaxRequestSize(long maxRequestSize) { - this.maxRequestSize = maxRequestSize; + setMaxRequestSize(DataSize.ofBytes(maxRequestSize)); } /** * Sets the maximum size allowed for multipart/form-data requests. Values can use the * suffixed "MB" or "KB" to indicate a Megabyte or Kilobyte size. * @param maxRequestSize the maximum request size - * @see #setMaxRequestSize(long) + * @deprecated since 2.1.0 in favour of {@link #setMaxRequestSize(DataSize)} */ + @Deprecated public void setMaxRequestSize(String maxRequestSize) { - this.maxRequestSize = parseSize(maxRequestSize); + setMaxRequestSize(DataSize.parse(maxRequestSize)); + } + + /** + * Sets the {@link DataSize size} threshold after which files will be written to disk. + * @param fileSizeThreshold the file size threshold + */ + public void setFileSizeThreshold(DataSize fileSizeThreshold) { + this.fileSizeThreshold = fileSizeThreshold; } /** * Sets the size threshold in bytes after which files will be written to disk. * @param fileSizeThreshold the file size threshold - * @see #setFileSizeThreshold(String) + * @deprecated since 2.1.0 in favour of {@link #setFileSizeThreshold(DataSize)} */ + @Deprecated public void setFileSizeThreshold(int fileSizeThreshold) { - this.fileSizeThreshold = fileSizeThreshold; + setFileSizeThreshold(DataSize.ofBytes(fileSizeThreshold)); } /** * Sets the size threshold after which files will be written to disk. Values can use * the suffixed "MB" or "KB" to indicate a Megabyte or Kilobyte size. * @param fileSizeThreshold the file size threshold - * @see #setFileSizeThreshold(int) + * @deprecated since 2.1.0 in favour of {@link #setFileSizeThreshold(DataSize)} */ + @Deprecated public void setFileSizeThreshold(String fileSizeThreshold) { - this.fileSizeThreshold = (int) parseSize(fileSizeThreshold); - } - - private long parseSize(String size) { - Assert.hasLength(size, "Size must not be empty"); - size = size.toUpperCase(Locale.ENGLISH); - if (size.endsWith("KB")) { - return Long.valueOf(size.substring(0, size.length() - 2)) * 1024; - } - if (size.endsWith("MB")) { - return Long.valueOf(size.substring(0, size.length() - 2)) * 1024 * 1024; - } - return Long.valueOf(size); + setFileSizeThreshold(DataSize.parse(fileSizeThreshold)); } /** @@ -127,8 +138,25 @@ public class MultipartConfigFactory { * @return the multipart config element */ public MultipartConfigElement createMultipartConfig() { - return new MultipartConfigElement(this.location, this.maxFileSize, - this.maxRequestSize, this.fileSizeThreshold); + long maxFileSizeBytes = convertToBytes(this.maxFileSize, -1); + long maxRequestSizeBytes = convertToBytes(this.maxRequestSize, -1); + long fileSizeThresholdBytes = convertToBytes(this.fileSizeThreshold, 0); + return new MultipartConfigElement(this.location, maxFileSizeBytes, + maxRequestSizeBytes, (int) fileSizeThresholdBytes); + } + + /** + * Return the amount of bytes from the specified {@link DataSize size}. If the size is + * {@code null} or negative, returns {@code defaultValue}. + * @param size the data size to handle + * @param defaultValue the default value if the size is {@code null} or negative + * @return the amount of bytes to use + */ + private long convertToBytes(DataSize size, int defaultValue) { + if (size != null && !size.isNegative()) { + return size.toBytes(); + } + return defaultValue; } } diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/MultipartConfigFactoryTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/MultipartConfigFactoryTests.java index 4b1c07bed4..fbad220d15 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/MultipartConfigFactoryTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/web/servlet/MultipartConfigFactoryTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2017 the original author or authors. + * Copyright 2012-2018 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -20,12 +20,15 @@ import javax.servlet.MultipartConfigElement; import org.junit.Test; +import org.springframework.util.unit.DataSize; + import static org.assertj.core.api.Assertions.assertThat; /** * Tests for {@link MultipartConfigFactory}. * * @author Phillip Webb + * @author Stephane Nicoll */ public class MultipartConfigFactoryTests { @@ -40,6 +43,7 @@ public class MultipartConfigFactoryTests { } @Test + @Deprecated public void create() { MultipartConfigFactory factory = new MultipartConfigFactory(); factory.setLocation("loc"); @@ -54,15 +58,40 @@ public class MultipartConfigFactoryTests { } @Test + @Deprecated public void createWithStringSizes() { MultipartConfigFactory factory = new MultipartConfigFactory(); factory.setMaxFileSize("1"); - factory.setMaxRequestSize("2kB"); - factory.setFileSizeThreshold("3Mb"); + factory.setMaxRequestSize("2KB"); + factory.setFileSizeThreshold("3MB"); MultipartConfigElement config = factory.createMultipartConfig(); assertThat(config.getMaxFileSize()).isEqualTo(1L); assertThat(config.getMaxRequestSize()).isEqualTo(2 * 1024L); assertThat(config.getFileSizeThreshold()).isEqualTo(3 * 1024 * 1024); } + @Test + public void createWithDataSizes() { + MultipartConfigFactory factory = new MultipartConfigFactory(); + factory.setMaxFileSize(DataSize.ofBytes(1)); + factory.setMaxRequestSize(DataSize.ofKiloBytes(2)); + factory.setFileSizeThreshold(DataSize.ofMegaBytes(3)); + MultipartConfigElement config = factory.createMultipartConfig(); + assertThat(config.getMaxFileSize()).isEqualTo(1L); + assertThat(config.getMaxRequestSize()).isEqualTo(2 * 1024L); + assertThat(config.getFileSizeThreshold()).isEqualTo(3 * 1024 * 1024); + } + + @Test + public void createWithNegativeDataSizes() { + MultipartConfigFactory factory = new MultipartConfigFactory(); + factory.setMaxFileSize(DataSize.ofBytes(-1)); + factory.setMaxRequestSize(DataSize.ofKiloBytes(-2)); + factory.setFileSizeThreshold(DataSize.ofMegaBytes(-3)); + MultipartConfigElement config = factory.createMultipartConfig(); + assertThat(config.getMaxFileSize()).isEqualTo(-1L); + assertThat(config.getMaxRequestSize()).isEqualTo(-1); + assertThat(config.getFileSizeThreshold()).isEqualTo(0); + } + }