From 688a69b9b12bd5cb9b85f9c302960f466e200049 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Wed, 27 Sep 2023 12:28:23 +0100 Subject: [PATCH] Prevent status messages from logging when Logback is being configured There's a window where the deny-all turbo filter has been removed but Logback has not yet been configured. If any logging that would have reached an appender is performed in this window, unwanted status messages will be logged. This window can be closed by ensuring that the turbo filter is in place while Logback is being configured. Closes gh-34505 --- .../logging/logback/LogbackLoggingSystem.java | 56 ++++++++++++------- .../logback/LogbackLoggingSystemTests.java | 14 ++++- 2 files changed, 49 insertions(+), 21 deletions(-) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/LogbackLoggingSystem.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/LogbackLoggingSystem.java index 335802a28c..9915b0a550 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/LogbackLoggingSystem.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/logging/logback/LogbackLoggingSystem.java @@ -30,6 +30,7 @@ import ch.qos.logback.classic.Level; import ch.qos.logback.classic.LoggerContext; import ch.qos.logback.classic.joran.JoranConfigurator; import ch.qos.logback.classic.jul.LevelChangePropagator; +import ch.qos.logback.classic.spi.TurboFilterList; import ch.qos.logback.classic.turbo.TurboFilter; import ch.qos.logback.core.joran.spi.JoranException; import ch.qos.logback.core.spi.FilterReply; @@ -218,33 +219,37 @@ public class LogbackLoggingSystem extends AbstractLoggingSystem implements BeanF protected void loadDefaults(LoggingInitializationContext initializationContext, LogFile logFile) { LoggerContext context = getLoggerContext(); stopAndReset(context); - boolean debug = Boolean.getBoolean("logback.debug"); - if (debug) { - StatusListenerConfigHelper.addOnConsoleListenerInstance(context, new OnConsoleStatusListener()); - } - Environment environment = initializationContext.getEnvironment(); - // Apply system properties directly in case the same JVM runs multiple apps - new LogbackLoggingSystemProperties(environment, context::putProperty).apply(logFile); - LogbackConfigurator configurator = debug ? new DebugLogbackConfigurator(context) - : new LogbackConfigurator(context); - new DefaultLogbackConfiguration(logFile).apply(configurator); - context.setPackagingDataEnabled(true); + withLoggingSuppressed(() -> { + boolean debug = Boolean.getBoolean("logback.debug"); + if (debug) { + StatusListenerConfigHelper.addOnConsoleListenerInstance(context, new OnConsoleStatusListener()); + } + Environment environment = initializationContext.getEnvironment(); + // Apply system properties directly in case the same JVM runs multiple apps + new LogbackLoggingSystemProperties(environment, context::putProperty).apply(logFile); + LogbackConfigurator configurator = debug ? new DebugLogbackConfigurator(context) + : new LogbackConfigurator(context); + new DefaultLogbackConfiguration(logFile).apply(configurator); + context.setPackagingDataEnabled(true); + }); } @Override protected void loadConfiguration(LoggingInitializationContext initializationContext, String location, LogFile logFile) { - if (initializationContext != null) { - applySystemProperties(initializationContext.getEnvironment(), logFile); - } LoggerContext loggerContext = getLoggerContext(); stopAndReset(loggerContext); - try { - configureByResourceUrl(initializationContext, loggerContext, ResourceUtils.getURL(location)); - } - catch (Exception ex) { - throw new IllegalStateException("Could not initialize Logback logging from " + location, ex); - } + withLoggingSuppressed(() -> { + if (initializationContext != null) { + applySystemProperties(initializationContext.getEnvironment(), logFile); + } + try { + configureByResourceUrl(initializationContext, loggerContext, ResourceUtils.getURL(location)); + } + catch (Exception ex) { + throw new IllegalStateException("Could not initialize Logback logging from " + location, ex); + } + }); reportConfigurationErrorsIfNecessary(loggerContext); } @@ -448,6 +453,17 @@ public class LogbackLoggingSystem extends AbstractLoggingSystem implements BeanF return contribution; } + private void withLoggingSuppressed(Runnable action) { + TurboFilterList turboFilters = getLoggerContext().getTurboFilterList(); + turboFilters.add(FILTER); + try { + action.run(); + } + finally { + turboFilters.remove(FILTER); + } + } + /** * {@link LoggingSystemFactory} that returns {@link LogbackLoggingSystem} if possible. */ 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 c9210207a8..b9d6dd41a8 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,12 +57,14 @@ import org.springframework.boot.logging.LoggerConfiguration; import org.springframework.boot.logging.LoggingInitializationContext; import org.springframework.boot.logging.LoggingSystem; import org.springframework.boot.logging.LoggingSystemProperties; +import org.springframework.boot.testsupport.classpath.ClassPathExclusions; import org.springframework.boot.testsupport.classpath.ClassPathOverrides; import org.springframework.boot.testsupport.system.CapturedOutput; import org.springframework.boot.testsupport.system.OutputCaptureExtension; import org.springframework.core.convert.ConversionService; import org.springframework.core.convert.support.ConfigurableConversionService; import org.springframework.core.env.ConfigurableEnvironment; +import org.springframework.core.env.MapPropertySource; import org.springframework.mock.env.MockEnvironment; import org.springframework.test.util.ReflectionTestUtils; import org.springframework.util.ReflectionUtils; @@ -89,6 +91,7 @@ import static org.mockito.Mockito.times; * @author Scott Frederick */ @ExtendWith(OutputCaptureExtension.class) +@ClassPathExclusions({ "log4j-core-*.jar", "log4j-api-*.jar" }) class LogbackLoggingSystemTests extends AbstractLoggingSystemTests { private final LogbackLoggingSystem loggingSystem = new LogbackLoggingSystem(getClass().getClassLoader()); @@ -123,7 +126,7 @@ class LogbackLoggingSystemTests extends AbstractLoggingSystemTests { } @Test - @ClassPathOverrides("org.jboss.logging:jboss-logging:3.5.0.Final") + @ClassPathOverrides({ "org.jboss.logging:jboss-logging:3.5.0.Final", "org.apache.logging.log4j:log4j-core:2.19.0" }) void jbossLoggingRoutesThroughLog4j2ByDefault() { System.getProperties().remove("org.jboss.logging.provider"); org.jboss.logging.Logger jbossLogger = org.jboss.logging.Logger.getLogger(getClass()); @@ -706,6 +709,15 @@ class LogbackLoggingSystemTests extends AbstractLoggingSystemTests { .satisfies((ex) -> assertThat(ex.getCause()).isNotInstanceOf(IllegalArgumentException.class)); } + @Test + void applyingSystemPropertiesDoesNotCauseUnwantedStatusWarnings(CapturedOutput output) { + this.loggingSystem.beforeInitialize(); + this.environment.getPropertySources() + .addFirst(new MapPropertySource("test", Map.of("logging.pattern.console", "[CONSOLE]%m"))); + this.loggingSystem.initialize(this.initializationContext, "classpath:logback-nondefault.xml", null); + assertThat(output).doesNotContain("WARN"); + } + private void initialize(LoggingInitializationContext context, String configLocation, LogFile logFile) { this.loggingSystem.getSystemProperties((ConfigurableEnvironment) context.getEnvironment()).apply(logFile); this.loggingSystem.initialize(context, configLocation, logFile);