From a43cd18ac9969e814615789505e901fdac9bc358 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Wed, 22 Jul 2015 17:37:08 +0100 Subject: [PATCH] Avoid changing root logger level when setting level of unconfigured logger MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously getLoggerConfig(loggerName) was used to retrieve the LoggerConfig object on which the level was to be set. As described in the method’s javadoc it will “remove tokens from the package name as necessary or return the root LoggerConfig if no other matches were found”. This is problematic as, if there’s no configuration for the logger whose level is being configured, the level will be applied to a logger from an outer package or to the root logger. This commit updates Log4J2LoggingSystem to use the configuration’s map of LoggerConfigs, rather than calling getLoggerConfig. In the event of the level being set on an unconfigured logger, this will produce a null LoggerConfig. When a null LoggerConfig is encountered, a new one is created with the appropriate level. If the config already exists, its level is set as it was before. The code that was accessing the root logger using a magic null value (which was then coerced into the root logger’s name (an empty string)) has also been updated to make it clearer that they are purposefully dealing with the root logger. Closes gh-3550 --- .../logging/log4j2/Log4J2LoggingSystem.java | 24 +++++++++++++------ .../log4j2/Log4J2LoggingSystemTests.java | 13 ++++++++++ 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/spring-boot/src/main/java/org/springframework/boot/logging/log4j2/Log4J2LoggingSystem.java b/spring-boot/src/main/java/org/springframework/boot/logging/log4j2/Log4J2LoggingSystem.java index 18780942da..56b16710df 100644 --- a/spring-boot/src/main/java/org/springframework/boot/logging/log4j2/Log4J2LoggingSystem.java +++ b/spring-boot/src/main/java/org/springframework/boot/logging/log4j2/Log4J2LoggingSystem.java @@ -121,12 +121,12 @@ public class Log4J2LoggingSystem extends Slf4JLoggingSystem { @Override public void beforeInitialize() { super.beforeInitialize(); - getLoggerConfig(null).addFilter(FILTER); + getRootLoggerConfig().addFilter(FILTER); } @Override public void initialize(String configLocation, LogFile logFile) { - getLoggerConfig(null).removeFilter(FILTER); + getRootLoggerConfig().removeFilter(FILTER); super.initialize(configLocation, logFile); } @@ -164,15 +164,25 @@ public class Log4J2LoggingSystem extends Slf4JLoggingSystem { } @Override - public void setLogLevel(String loggerName, LogLevel level) { - getLoggerConfig(loggerName).setLevel(LEVELS.get(level)); + public void setLogLevel(String loggerName, LogLevel logLevel) { + Level level = LEVELS.get(logLevel); + LoggerConfig loggerConfig = getLoggerConfig(loggerName); + if (loggerConfig == null) { + loggerConfig = new LoggerConfig(loggerName, level, true); + getLoggerContext().getConfiguration().addLogger(loggerName, loggerConfig); + } + else { + loggerConfig.setLevel(level); + } getLoggerContext().updateLoggers(); } + private LoggerConfig getRootLoggerConfig() { + return getLoggerContext().getConfiguration().getLoggerConfig(""); + } + private LoggerConfig getLoggerConfig(String loggerName) { - LoggerConfig loggerConfig = getLoggerContext().getConfiguration() - .getLoggerConfig(loggerName == null ? "" : loggerName); - return loggerConfig; + return getLoggerContext().getConfiguration().getLoggers().get(loggerName); } private LoggerContext getLoggerContext() { diff --git a/spring-boot/src/test/java/org/springframework/boot/logging/log4j2/Log4J2LoggingSystemTests.java b/spring-boot/src/test/java/org/springframework/boot/logging/log4j2/Log4J2LoggingSystemTests.java index d28b24a4e4..cac50ae690 100644 --- a/spring-boot/src/test/java/org/springframework/boot/logging/log4j2/Log4J2LoggingSystemTests.java +++ b/spring-boot/src/test/java/org/springframework/boot/logging/log4j2/Log4J2LoggingSystemTests.java @@ -34,9 +34,11 @@ import org.springframework.util.StringUtils; import com.fasterxml.jackson.databind.ObjectMapper; +import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.Matchers.arrayContaining; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.not; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; @@ -115,6 +117,17 @@ public class Log4J2LoggingSystemTests extends AbstractLoggingSystemTests { equalTo(1)); } + @Test + public void setLevelOfUnconfiguredLoggerDoesNotAffectRootConfiguration() + throws Exception { + this.loggingSystem.beforeInitialize(); + this.loggingSystem.initialize(null, null); + LogManager.getRootLogger().debug("Hello"); + this.loggingSystem.setLogLevel("foo.bar.baz", LogLevel.DEBUG); + LogManager.getRootLogger().debug("Hello"); + assertThat(this.output.toString(), not(containsString("Hello"))); + } + @Test @Ignore("Fails on Bamboo") public void loggingThatUsesJulIsCaptured() {