diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/DefaultLogbackConfiguration.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/DefaultLogbackConfiguration.java index bf1efdc00593..9bd91169f676 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/DefaultLogbackConfiguration.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/DefaultLogbackConfiguration.java @@ -17,6 +17,9 @@ package org.springframework.boot.logging.logback; import java.lang.reflect.Method; +import java.util.Objects; +import java.util.regex.Matcher; +import java.util.regex.Pattern; import ch.qos.logback.classic.Level; import ch.qos.logback.classic.encoder.PatternLayoutEncoder; @@ -35,7 +38,10 @@ import org.springframework.core.env.Environment; import org.springframework.core.env.PropertyResolver; import org.springframework.core.env.PropertySourcesPropertyResolver; +import org.springframework.util.Assert; import org.springframework.util.ReflectionUtils; +import org.springframework.util.StringUtils; +import org.springframework.util.unit.DataSize; /** * Default logback configuration used by Spring Boot. Uses {@link LogbackConfigurator} to @@ -58,6 +64,9 @@ class DefaultLogbackConfiguration { private static final String FILE_LOG_PATTERN = "%d{${LOG_DATEFORMAT_PATTERN:-yyyy-MM-dd HH:mm:ss.SSS}} " + "${LOG_LEVEL_PATTERN:-%5p} ${PID:- } --- [%t] %-40.40logger{39} : %m%n${LOG_EXCEPTION_CONVERSION_WORD:-%wEx}"; + private static final Pattern SIZE_PATTERN = Pattern + .compile("^([+\\-]?\\d+)\\s*([A-Z]{0,2})S?$", Pattern.CASE_INSENSITIVE); + private static final String MAX_FILE_SIZE = "10MB"; private final PropertyResolver patterns; @@ -144,30 +153,51 @@ private void setRollingPolicy(RollingFileAppender appender, rollingPolicy.setCleanHistoryOnStart(this.patterns.getProperty( "logging.file.clean-history-on-start", Boolean.class, false)); rollingPolicy.setFileNamePattern(logFile + ".%d{yyyy-MM-dd}.%i.gz"); - setMaxFileSize(rollingPolicy, - this.patterns.getProperty("logging.file.max-size", MAX_FILE_SIZE)); + setMaxFileSize(rollingPolicy, getDataSize( + this.patterns.getProperty("logging.file.max-size", MAX_FILE_SIZE))); + setTotalSizeCap(rollingPolicy, + getDataSize(this.patterns.getProperty("logging.file.total-size-cap", + String.valueOf(CoreConstants.UNBOUNDED_TOTAL_SIZE_CAP)))); rollingPolicy.setMaxHistory(this.patterns.getProperty("logging.file.max-history", Integer.class, CoreConstants.UNBOUND_HISTORY)); - rollingPolicy.setTotalSizeCap( - FileSize.valueOf(this.patterns.getProperty("logging.file.total-size-cap", - String.valueOf(CoreConstants.UNBOUNDED_TOTAL_SIZE_CAP)))); appender.setRollingPolicy(rollingPolicy); rollingPolicy.setParent(appender); config.start(rollingPolicy); } + private void setTotalSizeCap( + SizeAndTimeBasedRollingPolicy rollingPolicy, + DataSize totalSizeCap) { + Assert.isTrue(!totalSizeCap.isNegative(), "TotalSizeCap must not be a negative"); + rollingPolicy.setTotalSizeCap(new FileSize(totalSizeCap.toBytes())); + } + private void setMaxFileSize( SizeAndTimeBasedRollingPolicy rollingPolicy, - String maxFileSize) { + DataSize maxFileSize) { + Assert.isTrue(!maxFileSize.isNegative(), "MaxFileSize must not be a negative"); try { - rollingPolicy.setMaxFileSize(FileSize.valueOf(maxFileSize)); + rollingPolicy.setMaxFileSize(new FileSize(maxFileSize.toBytes())); } catch (NoSuchMethodError ex) { // Logback < 1.1.8 used String configuration Method method = ReflectionUtils.findMethod( SizeAndTimeBasedRollingPolicy.class, "setMaxFileSize", String.class); - ReflectionUtils.invokeMethod(method, rollingPolicy, maxFileSize); + ReflectionUtils.invokeMethod(method, rollingPolicy, + Objects.toString(maxFileSize.toBytes())); + } + } + + private static DataSize getDataSize(String value) { + Matcher matcher = SIZE_PATTERN.matcher(value); + if (matcher.matches()) { + String size = matcher.group(1); + String unit = matcher.group(2); + return StringUtils.hasText(unit) ? DataSize.parse(size + unit.toUpperCase()) + : DataSize.parse(size); } + throw new IllegalArgumentException( + "'" + value + "' does not match data size pattern"); } } diff --git a/spring-boot-project/spring-boot/src/main/resources/META-INF/additional-spring-configuration-metadata.json b/spring-boot-project/spring-boot/src/main/resources/META-INF/additional-spring-configuration-metadata.json index 68498b6ded50..b17f85788787 100644 --- a/spring-boot-project/spring-boot/src/main/resources/META-INF/additional-spring-configuration-metadata.json +++ b/spring-boot-project/spring-boot/src/main/resources/META-INF/additional-spring-configuration-metadata.json @@ -97,7 +97,7 @@ }, { "name": "logging.file.max-size", - "type": "java.lang.String", + "type": "org.springframework.util.unit.DataSize", "description": "Maximum log file size. Only supported with the default logback setup.", "sourceType": "org.springframework.boot.context.logging.LoggingApplicationListener", "defaultValue": "10MB" @@ -111,7 +111,7 @@ }, { "name": "logging.file.total-size-cap", - "type": "java.lang.String", + "type": "org.springframework.util.unit.DataSize", "description": "Total size of log backups to be kept. Only supported with the default logback setup.", "sourceType": "org.springframework.boot.context.logging.LoggingApplicationListener", "defaultValue": "0" diff --git a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/logback/LogbackLoggingSystemTests.java b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/logback/LogbackLoggingSystemTests.java index 617e1a30c706..6443755930e3 100644 --- a/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/logback/LogbackLoggingSystemTests.java +++ b/spring-boot-project/spring-boot/src/test/java/org/springframework/boot/logging/logback/LogbackLoggingSystemTests.java @@ -57,6 +57,7 @@ import org.springframework.util.StringUtils; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.assertj.core.api.Assertions.contentOf; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.not; @@ -381,10 +382,53 @@ public void testCleanHistoryOnStartPropertyWithXmlConfiguration() { assertThat(getRollingPolicy().isCleanHistoryOnStart()).isTrue(); } + @Test + public void testMaxFileSizePropertyNegativeValue() { + assertThatThrownBy(() -> { + MockEnvironment environment = new MockEnvironment(); + environment.setProperty("logging.file.max-size", "-100MB"); + LoggingInitializationContext loggingInitializationContext = new LoggingInitializationContext( + environment); + File file = new File(tmpDir(), "logback-test.log"); + LogFile logFile = getLogFile(file.getPath(), null); + this.loggingSystem.initialize(loggingInitializationContext, null, logFile); + }).isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("MaxFileSize must not be a negative"); + } + + @Test + public void testMaxFileSizePropertyInvalidFormat() { + assertThatThrownBy(() -> { + MockEnvironment environment = new MockEnvironment(); + environment.setProperty("logging.file.max-size", "100 :)"); + LoggingInitializationContext loggingInitializationContext = new LoggingInitializationContext( + environment); + File file = new File(tmpDir(), "logback-test.log"); + LogFile logFile = getLogFile(file.getPath(), null); + this.loggingSystem.initialize(loggingInitializationContext, null, logFile); + }).isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("does not match data size pattern"); + } + + @Test + public void testMaxFileSizePropertyLogbackFormat() { + MockEnvironment environment = new MockEnvironment(); + environment.setProperty("logging.file.max-size", "100 kbs"); + LoggingInitializationContext loggingInitializationContext = new LoggingInitializationContext( + environment); + File file = new File(tmpDir(), "logback-test.log"); + LogFile logFile = getLogFile(file.getPath(), null); + this.loggingSystem.initialize(loggingInitializationContext, null, logFile); + this.logger.info("Hello world"); + assertThat(getLineWithText(file, "Hello world")).contains("INFO"); + assertThat(ReflectionTestUtils.getField(getRollingPolicy(), "maxFileSize") + .toString()).isEqualTo("100 KB"); + } + @Test public void testMaxFileSizeProperty() { MockEnvironment environment = new MockEnvironment(); - environment.setProperty("logging.file.max-size", "100MB"); + environment.setProperty("logging.file.max-size", "+10TB"); LoggingInitializationContext loggingInitializationContext = new LoggingInitializationContext( environment); File file = new File(tmpDir(), "logback-test.log"); @@ -393,7 +437,22 @@ public void testMaxFileSizeProperty() { this.logger.info("Hello world"); assertThat(getLineWithText(file, "Hello world")).contains("INFO"); assertThat(ReflectionTestUtils.getField(getRollingPolicy(), "maxFileSize") - .toString()).isEqualTo("100 MB"); + .toString()).isEqualTo("10240 GB"); + } + + @Test + public void testMaxFileSizePropertyRawBytes() { + MockEnvironment environment = new MockEnvironment(); + environment.setProperty("logging.file.max-size", "1024"); + LoggingInitializationContext loggingInitializationContext = new LoggingInitializationContext( + environment); + File file = new File(tmpDir(), "logback-test.log"); + LogFile logFile = getLogFile(file.getPath(), null); + this.loggingSystem.initialize(loggingInitializationContext, null, logFile); + this.logger.info("Hello world"); + assertThat(getLineWithText(file, "Hello world")).contains("INFO"); + assertThat(ReflectionTestUtils.getField(getRollingPolicy(), "maxFileSize") + .toString()).isEqualTo("1 KB"); } @Test @@ -427,7 +486,7 @@ public void testMaxHistoryProperty() { } @Test - public void testMaxHistoryPropertyWithXmlConfiguration() throws Exception { + public void testMaxHistoryPropertyWithXmlConfiguration() { MockEnvironment environment = new MockEnvironment(); environment.setProperty("logging.file.max-history", "30"); LoggingInitializationContext loggingInitializationContext = new LoggingInitializationContext( @@ -441,11 +500,53 @@ public void testMaxHistoryPropertyWithXmlConfiguration() throws Exception { assertThat(getRollingPolicy().getMaxHistory()).isEqualTo(30); } + @Test + public void testTotalSizeCapPropertyNegativeValue() { + assertThatThrownBy(() -> { + MockEnvironment environment = new MockEnvironment(); + environment.setProperty("logging.file.total-size-cap", "-101 Mb"); + LoggingInitializationContext loggingInitializationContext = new LoggingInitializationContext( + environment); + File file = new File(tmpDir(), "logback-test.log"); + LogFile logFile = getLogFile(file.getPath(), null); + this.loggingSystem.initialize(loggingInitializationContext, null, logFile); + }).isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("TotalSizeCap must not be a negative"); + } + + @Test + public void testTotalSizeCapPropertyInvalidFormat() { + assertThatThrownBy(() -> { + MockEnvironment environment = new MockEnvironment(); + environment.setProperty("logging.file.total-size-cap", "101 :)"); + LoggingInitializationContext loggingInitializationContext = new LoggingInitializationContext( + environment); + File file = new File(tmpDir(), "logback-test.log"); + LogFile logFile = getLogFile(file.getPath(), null); + this.loggingSystem.initialize(loggingInitializationContext, null, logFile); + }).isInstanceOf(IllegalArgumentException.class) + .hasMessageContaining("does not match data size pattern"); + } + + @Test + public void testTotalSizeCapPropertyLogbackFormat() { + MockEnvironment environment = new MockEnvironment(); + environment.setProperty("logging.file.total-size-cap", "101 mbs"); + LoggingInitializationContext loggingInitializationContext = new LoggingInitializationContext( + environment); + File file = new File(tmpDir(), "logback-test.log"); + LogFile logFile = getLogFile(file.getPath(), null); + this.loggingSystem.initialize(loggingInitializationContext, null, logFile); + this.logger.info("Hello world"); + assertThat(getLineWithText(file, "Hello world")).contains("INFO"); + assertThat(ReflectionTestUtils.getField(getRollingPolicy(), "totalSizeCap") + .toString()).isEqualTo("101 MB"); + } + @Test public void testTotalSizeCapProperty() { - String expectedSize = "101 MB"; MockEnvironment environment = new MockEnvironment(); - environment.setProperty("logging.file.total-size-cap", expectedSize); + environment.setProperty("logging.file.total-size-cap", "101MB"); LoggingInitializationContext loggingInitializationContext = new LoggingInitializationContext( environment); File file = new File(tmpDir(), "logback-test.log"); @@ -454,14 +555,28 @@ public void testTotalSizeCapProperty() { this.logger.info("Hello world"); assertThat(getLineWithText(file, "Hello world")).contains("INFO"); assertThat(ReflectionTestUtils.getField(getRollingPolicy(), "totalSizeCap") - .toString()).isEqualTo(expectedSize); + .toString()).isEqualTo("101 MB"); + } + + @Test + public void testTotalSizeCapPropertyRawBytes() { + MockEnvironment environment = new MockEnvironment(); + environment.setProperty("logging.file.total-size-cap", "10485760"); + LoggingInitializationContext loggingInitializationContext = new LoggingInitializationContext( + environment); + File file = new File(tmpDir(), "logback-test.log"); + LogFile logFile = getLogFile(file.getPath(), null); + this.loggingSystem.initialize(loggingInitializationContext, null, logFile); + this.logger.info("Hello world"); + assertThat(getLineWithText(file, "Hello world")).contains("INFO"); + assertThat(ReflectionTestUtils.getField(getRollingPolicy(), "totalSizeCap") + .toString()).isEqualTo("10 MB"); } @Test public void testTotalSizeCapPropertyWithXmlConfiguration() { - String expectedSize = "101 MB"; MockEnvironment environment = new MockEnvironment(); - environment.setProperty("logging.file.total-size-cap", expectedSize); + environment.setProperty("logging.file.total-size-cap", "101MB"); LoggingInitializationContext loggingInitializationContext = new LoggingInitializationContext( environment); File file = new File(tmpDir(), "logback-test.log"); @@ -471,7 +586,7 @@ public void testTotalSizeCapPropertyWithXmlConfiguration() { this.logger.info("Hello world"); assertThat(getLineWithText(file, "Hello world")).contains("INFO"); assertThat(ReflectionTestUtils.getField(getRollingPolicy(), "totalSizeCap") - .toString()).isEqualTo(expectedSize); + .toString()).isEqualTo("101 MB"); } @Test