Minimize pollution of Log4j2's environment

Closes gh-40178
This commit is contained in:
Andy Wilkinson 2024-04-12 10:12:41 +01:00
parent 1ea6f2f51f
commit a946f66e7c
4 changed files with 45 additions and 20 deletions

View File

@ -88,6 +88,8 @@ public class Log4J2LoggingSystem extends AbstractLoggingSystem {
private static final String LOG4J_LOG_MANAGER = "org.apache.logging.log4j.jul.LogManager"; private static final String LOG4J_LOG_MANAGER = "org.apache.logging.log4j.jul.LogManager";
private static final SpringEnvironmentPropertySource propertySource = new SpringEnvironmentPropertySource();
static final String ENVIRONMENT_KEY = Conventions.getQualifiedAttributeName(Log4J2LoggingSystem.class, static final String ENVIRONMENT_KEY = Conventions.getQualifiedAttributeName(Log4J2LoggingSystem.class,
"environment"); "environment");
@ -214,8 +216,9 @@ public class Log4J2LoggingSystem extends AbstractLoggingSystem {
} }
Environment environment = initializationContext.getEnvironment(); Environment environment = initializationContext.getEnvironment();
if (environment != null) { if (environment != null) {
getLoggerContext().putObjectIfAbsent(ENVIRONMENT_KEY, environment); getLoggerContext().putObject(ENVIRONMENT_KEY, environment);
PropertiesUtil.getProperties().addPropertySource(new SpringEnvironmentPropertySource(environment)); Log4J2LoggingSystem.propertySource.setEnvironment(environment);
PropertiesUtil.getProperties().addPropertySource(Log4J2LoggingSystem.propertySource);
} }
loggerContext.getConfiguration().removeFilter(FILTER); loggerContext.getConfiguration().removeFilter(FILTER);
super.initialize(initializationContext, configLocation, logFile); super.initialize(initializationContext, configLocation, logFile);
@ -436,6 +439,8 @@ public class Log4J2LoggingSystem extends AbstractLoggingSystem {
LoggerContext loggerContext = getLoggerContext(); LoggerContext loggerContext = getLoggerContext();
markAsUninitialized(loggerContext); markAsUninitialized(loggerContext);
loggerContext.getConfiguration().removeFilter(FILTER); loggerContext.getConfiguration().removeFilter(FILTER);
Log4J2LoggingSystem.propertySource.setEnvironment(null);
getLoggerContext().removeObject(ENVIRONMENT_KEY);
} }
private LoggerConfig getLogger(String name) { private LoggerConfig getLogger(String name) {

View File

@ -19,7 +19,6 @@ package org.springframework.boot.logging.log4j2;
import org.apache.logging.log4j.util.PropertySource; import org.apache.logging.log4j.util.PropertySource;
import org.springframework.core.env.Environment; import org.springframework.core.env.Environment;
import org.springframework.util.Assert;
/** /**
* Returns properties from Spring. * Returns properties from Spring.
@ -33,12 +32,7 @@ class SpringEnvironmentPropertySource implements PropertySource {
*/ */
private static final int PRIORITY = -100; private static final int PRIORITY = -100;
private final Environment environment; private volatile Environment environment;
SpringEnvironmentPropertySource(Environment environment) {
Assert.notNull(environment, "Environment must not be null");
this.environment = environment;
}
@Override @Override
public int getPriority() { public int getPriority() {
@ -47,12 +41,18 @@ class SpringEnvironmentPropertySource implements PropertySource {
@Override @Override
public String getProperty(String key) { public String getProperty(String key) {
return this.environment.getProperty(key); Environment environment = this.environment;
return (environment != null) ? environment.getProperty(key) : null;
} }
@Override @Override
public boolean containsProperty(String key) { public boolean containsProperty(String key) {
return this.environment.containsProperty(key); Environment environment = this.environment;
return (environment != null) ? environment.containsProperty(key) : false;
}
void setEnvironment(Environment environment) {
this.environment = environment;
} }
} }

View File

@ -484,6 +484,20 @@ class Log4J2LoggingSystemTests extends AbstractLoggingSystemTests {
assertThat(properties.getStringProperty("spring")).isEqualTo("boot"); assertThat(properties.getStringProperty("spring")).isEqualTo("boot");
} }
@Test
void environmentIsUpdatedUponReinitialization() {
MockEnvironment environment = new MockEnvironment();
environment.setProperty("spring", "boot: one");
this.loggingSystem.beforeInitialize();
this.loggingSystem.initialize(new LoggingInitializationContext(environment), null, null);
assertThat(PropertiesUtil.getProperties().getStringProperty("spring")).isEqualTo("boot: one");
this.loggingSystem.cleanUp();
this.environment.setProperty("spring", "boot: two");
this.loggingSystem.beforeInitialize();
this.loggingSystem.initialize(this.initializationContext, null, null);
assertThat(PropertiesUtil.getProperties().getStringProperty("spring")).isEqualTo("boot: two");
}
@Test @Test
void nonFileUrlsAreResolvedUsingLog4J2UrlConnectionFactory() { void nonFileUrlsAreResolvedUsingLog4J2UrlConnectionFactory() {
this.loggingSystem.beforeInitialize(); this.loggingSystem.beforeInitialize();

View File

@ -1,5 +1,5 @@
/* /*
* Copyright 2012-2023 the original author or authors. * Copyright 2012-2024 the original author or authors.
* *
* Licensed under the Apache License, Version 2.0 (the "License"); * Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License. * you may not use this file except in compliance with the License.
@ -26,7 +26,6 @@ import org.junit.jupiter.api.Test;
import org.springframework.mock.env.MockEnvironment; import org.springframework.mock.env.MockEnvironment;
import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
/** /**
* Tests for {@link SpringEnvironmentPropertySource}. * Tests for {@link SpringEnvironmentPropertySource}.
@ -43,13 +42,8 @@ class SpringEnvironmentPropertySourceTests {
void setup() { void setup() {
this.environment = new MockEnvironment(); this.environment = new MockEnvironment();
this.environment.setProperty("spring", "boot"); this.environment.setProperty("spring", "boot");
this.propertySource = new SpringEnvironmentPropertySource(this.environment); this.propertySource = new SpringEnvironmentPropertySource();
} this.propertySource.setEnvironment(this.environment);
@Test
void createWhenEnvironmentIsNullThrowsException() {
assertThatIllegalArgumentException().isThrownBy(() -> new SpringEnvironmentPropertySource(null))
.withMessage("Environment must not be null");
} }
@Test @Test
@ -65,6 +59,12 @@ class SpringEnvironmentPropertySourceTests {
assertThat(this.propertySource.getProperty("spring")).isEqualTo("boot"); assertThat(this.propertySource.getProperty("spring")).isEqualTo("boot");
} }
@Test
void getPropertyWhenEnvironmentIsNullReturnsNull() {
this.propertySource.setEnvironment(null);
assertThat(this.propertySource.getProperty("spring")).isNull();
}
@Test @Test
void getPropertyWhenNotInEnvironmentReturnsNull() { void getPropertyWhenNotInEnvironmentReturnsNull() {
assertThat(this.propertySource.getProperty("nope")).isNull(); assertThat(this.propertySource.getProperty("nope")).isNull();
@ -75,6 +75,12 @@ class SpringEnvironmentPropertySourceTests {
assertThat(this.propertySource.containsProperty("spring")).isTrue(); assertThat(this.propertySource.containsProperty("spring")).isTrue();
} }
@Test
void containsPropertyWhenEnvironmentIsNullReturnsFalse() {
this.propertySource.setEnvironment(null);
assertThat(this.propertySource.containsProperty("spring")).isFalse();
}
@Test @Test
void containsPropertyWhenNotInEnvironmentReturnsFalse() { void containsPropertyWhenNotInEnvironmentReturnsFalse() {
assertThat(this.propertySource.containsProperty("nope")).isFalse(); assertThat(this.propertySource.containsProperty("nope")).isFalse();