You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@logging.apache.org by pk...@apache.org on 2024/12/09 08:01:32 UTC
(logging-log4j2) branch 2.24.x updated: Hardens `PropertiesUtil` against recursive property sources (#3263)
This is an automated email from the ASF dual-hosted git repository.
pkarwasz pushed a commit to branch 2.24.x
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
The following commit(s) were added to refs/heads/2.24.x by this push:
new 18a1debd11 Hardens `PropertiesUtil` against recursive property sources (#3263)
18a1debd11 is described below
commit 18a1debd11a92d2e898a8e816db810a1777dfa39
Author: Piotr P. Karwasz <pi...@karwasz.org>
AuthorDate: Mon Dec 9 09:01:27 2024 +0100
Hardens `PropertiesUtil` against recursive property sources (#3263)
As showed in #3252, Spring's `JndiPropertySource` not only can throw exceptions, but can also perform logging calls.
Such a call causes a recursive call to `PropertiesUtil.getProperty("log4j2.flowMessageFactory"`) and a `StackOverflowException` in the best scenario. The worst scenario includes a deadlock.
This PR:
- Moves the creation of the default `MessageFactory` and `FlowMessageFactory` to the static initializer of `LoggerContext`. This should be close enough to the pre-2.23.0 location in `AbstractLogger`. The `LoggerContext` class is usually initialized, before Spring Boot adds its property sources to `PropertiesUtil`.
- Adds a check to `PropertiesUtil` to ignore recursive calls.
Closes #3252.
---
.../logging/log4j/util/PropertiesUtilTest.java | 69 +++++++++++++++++++-
.../apache/logging/log4j/util/PropertiesUtil.java | 35 +++++++---
.../LoggerMessageFactoryCustomizationTest.java | 59 +++++++++--------
...oggerMessageFactoryDefaultsTlaDisabledTest.java | 15 +++--
...LoggerMessageFactoryDefaultsTlaEnabledTest.java | 18 +++---
.../core/impl/ThreadContextDataInjectorTest.java | 2 -
.../java/org/apache/logging/log4j/core/Logger.java | 75 ++++------------------
.../apache/logging/log4j/core/LoggerContext.java | 62 +++++++++++++++---
.../logging/log4j/core/async/AsyncLogger.java | 25 ++++----
9 files changed, 223 insertions(+), 137 deletions(-)
diff --git a/log4j-api-test/src/test/java/org/apache/logging/log4j/util/PropertiesUtilTest.java b/log4j-api-test/src/test/java/org/apache/logging/log4j/util/PropertiesUtilTest.java
index a655959765..5f85d13256 100644
--- a/log4j-api-test/src/test/java/org/apache/logging/log4j/util/PropertiesUtilTest.java
+++ b/log4j-api-test/src/test/java/org/apache/logging/log4j/util/PropertiesUtilTest.java
@@ -38,6 +38,9 @@ import java.util.List;
import java.util.Map;
import java.util.Properties;
import java.util.stream.Stream;
+import org.apache.logging.log4j.Level;
+import org.apache.logging.log4j.test.ListStatusListener;
+import org.apache.logging.log4j.test.junit.UsingStatusListener;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.parallel.ResourceAccessMode;
@@ -193,7 +196,8 @@ class PropertiesUtilTest {
@Test
@ResourceLock(value = Resources.SYSTEM_PROPERTIES, mode = ResourceAccessMode.READ)
@Issue("https://github.com/spring-projects/spring-boot/issues/33450")
- void testBadPropertySource() {
+ @UsingStatusListener
+ void testErrorPropertySource(ListStatusListener statusListener) {
final String key = "testKey";
final Properties props = new Properties();
props.put(key, "test");
@@ -201,8 +205,47 @@ class PropertiesUtilTest {
final ErrorPropertySource source = new ErrorPropertySource();
util.addPropertySource(source);
try {
+ statusListener.clear();
assertEquals("test", util.getStringProperty(key));
assertTrue(source.exceptionThrown);
+ assertThat(statusListener.findStatusData(Level.WARN))
+ .anySatisfy(data ->
+ assertThat(data.getMessage().getFormattedMessage()).contains("Failed"));
+ } finally {
+ util.removePropertySource(source);
+ }
+ }
+
+ @Test
+ @ResourceLock(value = Resources.SYSTEM_PROPERTIES, mode = ResourceAccessMode.READ)
+ @Issue("https://github.com/apache/logging-log4j2/issues/3252")
+ @UsingStatusListener
+ void testRecursivePropertySource(ListStatusListener statusListener) {
+ final String key = "testKey";
+ final Properties props = new Properties();
+ props.put(key, "test");
+ final PropertiesUtil util = new PropertiesUtil(props);
+ final PropertySource source = new RecursivePropertySource(util);
+ util.addPropertySource(source);
+ try {
+ // We ignore the recursive source
+ statusListener.clear();
+ assertThat(util.getStringProperty(key)).isEqualTo("test");
+ assertThat(statusListener.findStatusData(Level.WARN))
+ .anySatisfy(data -> assertThat(data.getMessage().getFormattedMessage())
+ .contains("Recursive call", "getProperty"));
+
+ statusListener.clear();
+ // To check for existence, the sources are looked up in a random order.
+ assertThat(util.hasProperty(key)).isTrue();
+ // To find a missing key, all the sources must be used.
+ assertThat(util.hasProperty("noSuchKey")).isFalse();
+ assertThat(statusListener.findStatusData(Level.WARN))
+ .anySatisfy(data -> assertThat(data.getMessage().getFormattedMessage())
+ .contains("Recursive call", "containsProperty"));
+ // We check that the source is recursive
+ assertThat(source.getProperty(key)).isEqualTo("test");
+ assertThat(source.containsProperty(key)).isTrue();
} finally {
util.removePropertySource(source);
}
@@ -289,4 +332,28 @@ class PropertiesUtilTest {
throw new IllegalStateException("Test");
}
}
+
+ private static class RecursivePropertySource implements PropertySource {
+
+ private final PropertiesUtil propertiesUtil;
+
+ private RecursivePropertySource(PropertiesUtil propertiesUtil) {
+ this.propertiesUtil = propertiesUtil;
+ }
+
+ @Override
+ public int getPriority() {
+ return Integer.MIN_VALUE;
+ }
+
+ @Override
+ public String getProperty(String key) {
+ return propertiesUtil.getStringProperty(key);
+ }
+
+ @Override
+ public boolean containsProperty(String key) {
+ return propertiesUtil.hasProperty(key);
+ }
+ }
}
diff --git a/log4j-api/src/main/java/org/apache/logging/log4j/util/PropertiesUtil.java b/log4j-api/src/main/java/org/apache/logging/log4j/util/PropertiesUtil.java
index 8b4baf4e53..30b92854b0 100644
--- a/log4j-api/src/main/java/org/apache/logging/log4j/util/PropertiesUtil.java
+++ b/log4j-api/src/main/java/org/apache/logging/log4j/util/PropertiesUtil.java
@@ -496,6 +496,7 @@ public final class PropertiesUtil {
private static final class Environment {
private final Set<PropertySource> sources = ConcurrentHashMap.newKeySet();
+ private final ThreadLocal<PropertySource> CURRENT_PROPERTY_SOURCE = new ThreadLocal<>();
private Environment(final PropertySource propertySource) {
final PropertySource sysProps = new PropertyFilePropertySource(LOG4J_SYSTEM_PROPERTIES_FILE_NAME, false);
@@ -547,21 +548,35 @@ public final class PropertiesUtil {
}
private boolean sourceContainsProperty(final PropertySource source, final String key) {
- try {
- return source.containsProperty(key);
- } catch (final Exception e) {
- LOGGER.warn("Failed to retrieve Log4j property {} from property source {}.", key, source, e);
- return false;
+ PropertySource recursiveSource = CURRENT_PROPERTY_SOURCE.get();
+ if (recursiveSource == null) {
+ CURRENT_PROPERTY_SOURCE.set(source);
+ try {
+ return source.containsProperty(key);
+ } catch (final Exception e) {
+ LOGGER.warn("Failed to retrieve Log4j property {} from property source {}.", key, source, e);
+ } finally {
+ CURRENT_PROPERTY_SOURCE.remove();
+ }
}
+ LOGGER.warn("Recursive call to `containsProperty()` from property source {}.", recursiveSource);
+ return false;
}
private String sourceGetProperty(final PropertySource source, final String key) {
- try {
- return source.getProperty(key);
- } catch (final Exception e) {
- LOGGER.warn("Failed to retrieve Log4j property {} from property source {}.", key, source, e);
- return null;
+ PropertySource recursiveSource = CURRENT_PROPERTY_SOURCE.get();
+ if (recursiveSource == null) {
+ CURRENT_PROPERTY_SOURCE.set(source);
+ try {
+ return source.getProperty(key);
+ } catch (final Exception e) {
+ LOGGER.warn("Failed to retrieve Log4j property {} from property source {}.", key, source, e);
+ } finally {
+ CURRENT_PROPERTY_SOURCE.remove();
+ }
}
+ LOGGER.warn("Recursive call to `getProperty()` from property source {}.", recursiveSource);
+ return null;
}
private boolean containsKey(final String key) {
diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/LoggerMessageFactoryCustomizationTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/LoggerMessageFactoryCustomizationTest.java
index 9043c47b1c..e303bfdea2 100644
--- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/LoggerMessageFactoryCustomizationTest.java
+++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/LoggerMessageFactoryCustomizationTest.java
@@ -20,46 +20,51 @@ import static org.assertj.core.api.Assertions.assertThat;
import org.apache.logging.log4j.message.AbstractMessageFactory;
import org.apache.logging.log4j.message.DefaultFlowMessageFactory;
+import org.apache.logging.log4j.message.FlowMessageFactory;
import org.apache.logging.log4j.message.Message;
import org.apache.logging.log4j.message.MessageFactory;
import org.apache.logging.log4j.message.ParameterizedMessageFactory;
import org.junit.jupiter.api.Test;
-import org.junitpioneer.jupiter.ClearSystemProperty;
+import org.junit.jupiter.api.TestInfo;
import org.junitpioneer.jupiter.SetSystemProperty;
+@SetSystemProperty(
+ key = "log4j2.messageFactory",
+ value = "org.apache.logging.log4j.core.LoggerMessageFactoryCustomizationTest$AlternativeTestMessageFactory")
+@SetSystemProperty(
+ key = "log4j2.flowMessageFactory",
+ value = "org.apache.logging.log4j.core.LoggerMessageFactoryCustomizationTest$AlternativeTestFlowMessageFactory")
class LoggerMessageFactoryCustomizationTest {
@Test
- @ClearSystemProperty(key = "log4j2.messageFactory")
- @ClearSystemProperty(key = "log4j2.flowMessageFactory")
- void arguments_should_be_honored() {
- final LoggerContext loggerContext =
- new LoggerContext(LoggerMessageFactoryCustomizationTest.class.getSimpleName());
- final Logger logger = new Logger(
- loggerContext, "arguments_should_be_honored", new TestMessageFactory(), new TestFlowMessageFactory());
- assertTestMessageFactories(logger);
+ void arguments_should_be_honored(TestInfo testInfo) {
+ try (LoggerContext loggerContext =
+ new LoggerContext(LoggerMessageFactoryCustomizationTest.class.getSimpleName())) {
+ Logger logger = new Logger(
+ loggerContext, testInfo.getDisplayName(), new TestMessageFactory(), new TestFlowMessageFactory());
+ assertTestMessageFactories(logger, TestMessageFactory.class, TestFlowMessageFactory.class);
+ }
}
@Test
- @SetSystemProperty(
- key = "log4j2.messageFactory",
- value = "org.apache.logging.log4j.core.LoggerMessageFactoryCustomizationTest$TestMessageFactory")
- @SetSystemProperty(
- key = "log4j2.flowMessageFactory",
- value = "org.apache.logging.log4j.core.LoggerMessageFactoryCustomizationTest$TestFlowMessageFactory")
- void properties_should_be_honored() {
- final LoggerContext loggerContext =
- new LoggerContext(LoggerMessageFactoryCustomizationTest.class.getSimpleName());
- final Logger logger = new Logger(loggerContext, "properties_should_be_honored", null, null);
- assertTestMessageFactories(logger);
+ void properties_should_be_honored(TestInfo testInfo) {
+ try (LoggerContext loggerContext =
+ new LoggerContext(LoggerMessageFactoryCustomizationTest.class.getSimpleName())) {
+ Logger logger = loggerContext.getLogger(testInfo.getDisplayName());
+ assertTestMessageFactories(
+ logger, AlternativeTestMessageFactory.class, AlternativeTestFlowMessageFactory.class);
+ }
}
- private static void assertTestMessageFactories(Logger logger) {
- assertThat((MessageFactory) logger.getMessageFactory()).isInstanceOf(TestMessageFactory.class);
- assertThat(logger.getFlowMessageFactory()).isInstanceOf(TestFlowMessageFactory.class);
+ private static void assertTestMessageFactories(
+ Logger logger,
+ Class<? extends MessageFactory> messageFactoryClass,
+ Class<? extends FlowMessageFactory> flowMessageFactoryClass) {
+ assertThat((MessageFactory) logger.getMessageFactory()).isInstanceOf(messageFactoryClass);
+ assertThat(logger.getFlowMessageFactory()).isInstanceOf(flowMessageFactoryClass);
}
- public static final class TestMessageFactory extends AbstractMessageFactory {
+ public static class TestMessageFactory extends AbstractMessageFactory {
@Override
public Message newMessage(final String message, final Object... params) {
@@ -67,5 +72,9 @@ class LoggerMessageFactoryCustomizationTest {
}
}
- public static final class TestFlowMessageFactory extends DefaultFlowMessageFactory {}
+ public static class AlternativeTestMessageFactory extends TestMessageFactory {}
+
+ public static class TestFlowMessageFactory extends DefaultFlowMessageFactory {}
+
+ public static class AlternativeTestFlowMessageFactory extends TestFlowMessageFactory {}
}
diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/LoggerMessageFactoryDefaultsTlaDisabledTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/LoggerMessageFactoryDefaultsTlaDisabledTest.java
index af3c862464..28d39c30f7 100644
--- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/LoggerMessageFactoryDefaultsTlaDisabledTest.java
+++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/LoggerMessageFactoryDefaultsTlaDisabledTest.java
@@ -23,19 +23,20 @@ import org.apache.logging.log4j.message.DefaultFlowMessageFactory;
import org.apache.logging.log4j.message.MessageFactory;
import org.apache.logging.log4j.message.ParameterizedMessageFactory;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInfo;
import org.junitpioneer.jupiter.SetSystemProperty;
class LoggerMessageFactoryDefaultsTlaDisabledTest {
@Test
@SetSystemProperty(key = "log4j2.enableThreadLocals", value = "false")
- void defaults_should_match_when_thread_locals_disabled() {
+ void defaults_should_match_when_thread_locals_disabled(TestInfo testInfo) {
assertThat(Constants.ENABLE_THREADLOCALS).isFalse();
- final LoggerContext loggerContext =
- new LoggerContext(LoggerMessageFactoryDefaultsTlaDisabledTest.class.getSimpleName());
- final Logger logger =
- new Logger(loggerContext, "defaults_should_match_when_thread_locals_disabled", null, null);
- assertThat((MessageFactory) logger.getMessageFactory()).isSameAs(ParameterizedMessageFactory.INSTANCE);
- assertThat(logger.getFlowMessageFactory()).isSameAs(DefaultFlowMessageFactory.INSTANCE);
+ try (LoggerContext loggerContext =
+ new LoggerContext(LoggerMessageFactoryDefaultsTlaDisabledTest.class.getSimpleName())) {
+ final Logger logger = loggerContext.getLogger(testInfo.getDisplayName());
+ assertThat((MessageFactory) logger.getMessageFactory()).isSameAs(ParameterizedMessageFactory.INSTANCE);
+ assertThat(logger.getFlowMessageFactory()).isSameAs(DefaultFlowMessageFactory.INSTANCE);
+ }
}
}
diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/LoggerMessageFactoryDefaultsTlaEnabledTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/LoggerMessageFactoryDefaultsTlaEnabledTest.java
index 3255566710..3b8f7ba339 100644
--- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/LoggerMessageFactoryDefaultsTlaEnabledTest.java
+++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/LoggerMessageFactoryDefaultsTlaEnabledTest.java
@@ -23,19 +23,21 @@ import org.apache.logging.log4j.message.DefaultFlowMessageFactory;
import org.apache.logging.log4j.message.MessageFactory;
import org.apache.logging.log4j.message.ReusableMessageFactory;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInfo;
import org.junitpioneer.jupiter.SetSystemProperty;
class LoggerMessageFactoryDefaultsTlaEnabledTest {
@Test
- @SetSystemProperty(key = "log4j2.is.webapp", value = "false")
- @SetSystemProperty(key = "log4j2.enableThreadLocals", value = "true")
- void defaults_should_match_when_thread_locals_enabled() {
+ @SetSystemProperty(key = "log4j2.isWebapp", value = "false")
+ @SetSystemProperty(key = "log4j2.enableThreadlocals", value = "true")
+ void defaults_should_match_when_thread_locals_enabled(TestInfo testInfo) {
assertThat(Constants.ENABLE_THREADLOCALS).isTrue();
- final LoggerContext loggerContext =
- new LoggerContext(LoggerMessageFactoryDefaultsTlaEnabledTest.class.getSimpleName());
- final Logger logger = new Logger(loggerContext, "defaults_should_match_when_thread_locals_enabled", null, null);
- assertThat((MessageFactory) logger.getMessageFactory()).isSameAs(ReusableMessageFactory.INSTANCE);
- assertThat(logger.getFlowMessageFactory()).isSameAs(DefaultFlowMessageFactory.INSTANCE);
+ try (LoggerContext loggerContext =
+ new LoggerContext(LoggerMessageFactoryDefaultsTlaEnabledTest.class.getSimpleName())) {
+ Logger logger = loggerContext.getLogger(testInfo.getDisplayName());
+ assertThat((MessageFactory) logger.getMessageFactory()).isSameAs(ReusableMessageFactory.INSTANCE);
+ assertThat(logger.getFlowMessageFactory()).isSameAs(DefaultFlowMessageFactory.INSTANCE);
+ }
}
}
diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/impl/ThreadContextDataInjectorTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/impl/ThreadContextDataInjectorTest.java
index 896d0d2d8b..135ea4162a 100644
--- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/impl/ThreadContextDataInjectorTest.java
+++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/impl/ThreadContextDataInjectorTest.java
@@ -33,7 +33,6 @@ import java.util.concurrent.ExecutionException;
import org.apache.logging.log4j.ThreadContext;
import org.apache.logging.log4j.core.ContextDataInjector;
import org.apache.logging.log4j.spi.ThreadContextMap;
-import org.apache.logging.log4j.util.PropertiesUtil;
import org.apache.logging.log4j.util.ProviderUtil;
import org.apache.logging.log4j.util.SortedArrayStringMap;
import org.apache.logging.log4j.util.StringMap;
@@ -59,7 +58,6 @@ public class ThreadContextDataInjectorTest {
public String threadContextMapClassName;
private static void resetThreadContextMap() {
- PropertiesUtil.getProperties().reload();
final Log4jProvider provider = (Log4jProvider) ProviderUtil.getProvider();
provider.resetThreadContextMap();
ThreadContext.init();
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/Logger.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/Logger.java
index 169c39162d..fdd2465099 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/Logger.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/Logger.java
@@ -33,16 +33,11 @@ import org.apache.logging.log4j.core.config.LocationAwareReliabilityStrategy;
import org.apache.logging.log4j.core.config.LoggerConfig;
import org.apache.logging.log4j.core.config.ReliabilityStrategy;
import org.apache.logging.log4j.core.filter.CompositeFilter;
-import org.apache.logging.log4j.core.util.Constants;
-import org.apache.logging.log4j.message.DefaultFlowMessageFactory;
import org.apache.logging.log4j.message.FlowMessageFactory;
import org.apache.logging.log4j.message.Message;
import org.apache.logging.log4j.message.MessageFactory;
-import org.apache.logging.log4j.message.ParameterizedMessageFactory;
-import org.apache.logging.log4j.message.ReusableMessageFactory;
import org.apache.logging.log4j.message.SimpleMessage;
import org.apache.logging.log4j.spi.AbstractLogger;
-import org.apache.logging.log4j.util.LoaderUtil;
import org.apache.logging.log4j.util.Strings;
import org.apache.logging.log4j.util.Supplier;
@@ -62,90 +57,42 @@ public class Logger extends AbstractLogger implements Supplier<LoggerConfig> {
private static final long serialVersionUID = 1L;
- private static final String MESSAGE_FACTORY_PROPERTY_NAME = "log4j2.messageFactory";
-
- private static final String FLOW_MESSAGE_FACTORY_PROPERTY_NAME = "log4j2.flowMessageFactory";
-
/**
* Config should be consistent across threads.
*/
protected volatile PrivateConfig privateConfig;
- // FIXME: ditto to the above
private final LoggerContext context;
/**
* Constructs an instance using the given {@link LoggerContext}, logger name, and {@link MessageFactory}.
*
- * @param context the {@link LoggerContext} this logger is associated with
+ * @param context The {@link LoggerContext} this logger is associated with, never {@code null}.
+ * @param name The logger name, never {@code null}.
* @param messageFactory The message factory to be used.
- * If null, first the {@value #MESSAGE_FACTORY_PROPERTY_NAME} property will be used to instantiate the message factory.
- * If the property is missing and the {@code log4j2.enableThreadLocals} property is not {@code false}, {@link ReusableMessageFactory} will be used.
- * Otherwise, we will fall back to {@link ParameterizedMessageFactory}.
- * @param name the logger name
+ * Passing a {@code null} value is deprecated, but supported for backward compatibility.
*/
- protected Logger(final LoggerContext context, final String name, final MessageFactory messageFactory) {
- this(context, name, messageFactory, null);
+ protected Logger(LoggerContext context, String name, MessageFactory messageFactory) {
+ this(context, name, messageFactory, LoggerContext.DEFAULT_FLOW_MESSAGE_FACTORY);
}
/**
* The canonical constructor.
*
- * @param context the {@link LoggerContext} this logger is associated with
+ * @param context The {@link LoggerContext} this logger is associated with, never {@code null}.
+ * @param name The logger name, never {@code null}.
* @param messageFactory The message factory to be used.
- * If null, first the {@value #MESSAGE_FACTORY_PROPERTY_NAME} property will be used to instantiate the message factory.
- * If the property is missing and the {@code log4j2.enableThreadLocals} property is not {@code false}, {@link ReusableMessageFactory} will be used.
- * Otherwise, we will fall back to {@link ParameterizedMessageFactory}.
+ * Passing a {@code null} value is deprecated, but supported for backward compatibility.
* @param flowMessageFactory The flow message factory to be used.
- * If null, first the {@value #FLOW_MESSAGE_FACTORY_PROPERTY_NAME} property will be used to instantiate the flow message factory.
- * If the property is missing, {@link DefaultFlowMessageFactory} will be used.
- * @param name the logger name
+ * Passing a {@code null} value is deprecated, but supported for backward compatibility.
*/
protected Logger(
- final LoggerContext context,
- final String name,
- final MessageFactory messageFactory,
- final FlowMessageFactory flowMessageFactory) {
- super(name, getEffectiveMessageFactory(messageFactory), getEffectiveFlowMessageFactory(flowMessageFactory));
+ LoggerContext context, String name, MessageFactory messageFactory, FlowMessageFactory flowMessageFactory) {
+ super(name, messageFactory, flowMessageFactory);
this.context = requireNonNull(context, "context");
this.privateConfig = new PrivateConfig(context.getConfiguration(), this);
}
- static MessageFactory getEffectiveMessageFactory(final MessageFactory messageFactory) {
- return createInstanceFromFactoryProperty(
- MessageFactory.class,
- messageFactory,
- MESSAGE_FACTORY_PROPERTY_NAME,
- () -> Constants.ENABLE_THREADLOCALS
- ? ReusableMessageFactory.INSTANCE
- : ParameterizedMessageFactory.INSTANCE);
- }
-
- private static FlowMessageFactory getEffectiveFlowMessageFactory(final FlowMessageFactory flowMessageFactory) {
- return createInstanceFromFactoryProperty(
- FlowMessageFactory.class,
- flowMessageFactory,
- FLOW_MESSAGE_FACTORY_PROPERTY_NAME,
- () -> DefaultFlowMessageFactory.INSTANCE);
- }
-
- private static <V> V createInstanceFromFactoryProperty(
- final Class<V> instanceType,
- final V providedInstance,
- final String propertyName,
- final java.util.function.Supplier<V> fallbackInstanceSupplier) {
- if (providedInstance != null) {
- return providedInstance;
- }
- try {
- return LoaderUtil.newCheckedInstanceOfProperty(propertyName, instanceType, fallbackInstanceSupplier);
- } catch (final Exception error) {
- final String message =
- String.format("failed instantiating the class pointed by the `%s` property", propertyName);
- throw new RuntimeException(message, error);
- }
- }
-
protected Object writeReplace() throws ObjectStreamException {
return new LoggerProxy(getName(), getMessageFactory());
}
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java
index e0b88a75d6..d73e4c1460 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/LoggerContext.java
@@ -44,16 +44,22 @@ import org.apache.logging.log4j.core.config.Reconfigurable;
import org.apache.logging.log4j.core.impl.Log4jLogEvent;
import org.apache.logging.log4j.core.jmx.Server;
import org.apache.logging.log4j.core.util.Cancellable;
+import org.apache.logging.log4j.core.util.Constants;
import org.apache.logging.log4j.core.util.ExecutorServices;
import org.apache.logging.log4j.core.util.NetUtils;
import org.apache.logging.log4j.core.util.ShutdownCallbackRegistry;
import org.apache.logging.log4j.core.util.internal.InternalLoggerRegistry;
+import org.apache.logging.log4j.message.DefaultFlowMessageFactory;
+import org.apache.logging.log4j.message.FlowMessageFactory;
import org.apache.logging.log4j.message.MessageFactory;
+import org.apache.logging.log4j.message.ParameterizedMessageFactory;
+import org.apache.logging.log4j.message.ReusableMessageFactory;
import org.apache.logging.log4j.spi.LoggerContextFactory;
import org.apache.logging.log4j.spi.LoggerContextShutdownAware;
import org.apache.logging.log4j.spi.LoggerContextShutdownEnabled;
import org.apache.logging.log4j.spi.Terminable;
import org.apache.logging.log4j.spi.ThreadContextMapFactory;
+import org.apache.logging.log4j.util.LoaderUtil;
import org.apache.logging.log4j.util.PropertiesUtil;
/**
@@ -73,14 +79,46 @@ public class LoggerContext extends AbstractLifeCycle
*/
public static final String PROPERTY_CONFIG = "config";
- private static final Configuration NULL_CONFIGURATION = new NullConfiguration();
+ private static final String EXTERNAL_CONTEXT_KEY = "__EXTERNAL_CONTEXT_KEY__";
+ private static final String MESSAGE_FACTORY_PROPERTY_NAME = "log4j2.messageFactory";
/**
- * The default message factory to use while creating loggers, if none is provided.
- *
- * @see <a href="https://github.com/apache/logging-log4j2/pull/2936">#2936</a> for the discussion on why we leak the message factory of the default logger and hardcode it here.
+ * The default message factory to use while creating loggers if the user provides none.
+ * <p>
+ * To mitigate initialization problems as the one described in
+ * <a href="https://github.com/apache/logging-log4j2/issues/3252>#3252</a>,
+ * this field should be initialized as soon as possible.
+ * </p>
+ */
+ private static final MessageFactory DEFAULT_MESSAGE_FACTORY = createInstanceFromFactoryProperty(
+ MessageFactory.class,
+ MESSAGE_FACTORY_PROPERTY_NAME,
+ Constants.ENABLE_THREADLOCALS ? ReusableMessageFactory.INSTANCE : ParameterizedMessageFactory.INSTANCE);
+
+ private static final String FLOW_MESSAGE_FACTORY_PROPERTY_NAME = "log4j2.flowMessageFactory";
+ /**
+ * The default flow message factory to use while creating loggers.
+ * <p>
+ * To mitigate initialization problems as the one described in
+ * <a href="https://github.com/apache/logging-log4j2/issues/3252>#3252</a>,
+ * this field should be initialized as soon as possible.
+ * </p>
*/
- private static final MessageFactory DEFAULT_MESSAGE_FACTORY = Logger.getEffectiveMessageFactory(null);
+ static final FlowMessageFactory DEFAULT_FLOW_MESSAGE_FACTORY = createInstanceFromFactoryProperty(
+ FlowMessageFactory.class, FLOW_MESSAGE_FACTORY_PROPERTY_NAME, DefaultFlowMessageFactory.INSTANCE);
+
+ private static final Configuration NULL_CONFIGURATION = new NullConfiguration();
+
+ private static <V> V createInstanceFromFactoryProperty(
+ final Class<V> instanceType, final String propertyName, final V fallbackInstance) {
+ try {
+ return LoaderUtil.newCheckedInstanceOfProperty(propertyName, instanceType, () -> fallbackInstance);
+ } catch (final Exception error) {
+ final String message =
+ String.format("failed instantiating the class pointed by the `%s` property", propertyName);
+ throw new RuntimeException(message, error);
+ }
+ }
private final InternalLoggerRegistry loggerRegistry = new InternalLoggerRegistry();
private final CopyOnWriteArrayList<PropertyChangeListener> propertyChangeListeners = new CopyOnWriteArrayList<>();
@@ -92,7 +130,6 @@ public class LoggerContext extends AbstractLifeCycle
*/
private volatile Configuration configuration = new DefaultConfiguration();
- private static final String EXTERNAL_CONTEXT_KEY = "__EXTERNAL_CONTEXT_KEY__";
private final ConcurrentMap<String, Object> externalMap = new ConcurrentHashMap<>();
private String contextName;
private volatile URI configLocation;
@@ -812,8 +849,15 @@ public class LoggerContext extends AbstractLifeCycle
return newInstance(this, name, messageFactory);
}
- // LOG4J2-151: changed visibility from private to protected
- protected Logger newInstance(final LoggerContext ctx, final String name, final MessageFactory messageFactory) {
- return new Logger(ctx, name, messageFactory);
+ /**
+ * Callback to create a new logger.
+ *
+ * @param context The {@link LoggerContext} this logger is associated with, never {@code null}.
+ * @param messageFactory The message factory to be used, never {@code null}.
+ * @param name The logger name, never {@code null}.
+ * @return A new logger instance.
+ */
+ protected Logger newInstance(LoggerContext context, String name, MessageFactory messageFactory) {
+ return new Logger(context, name, messageFactory);
}
}
diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLogger.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLogger.java
index 0378d010cc..a4cdb55eb9 100644
--- a/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLogger.java
+++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncLogger.java
@@ -16,6 +16,7 @@
*/
package org.apache.logging.log4j.core.async;
+import aQute.bnd.annotation.baseline.BaselineIgnore;
import com.lmax.disruptor.EventTranslatorVararg;
import com.lmax.disruptor.dsl.Disruptor;
import java.util.List;
@@ -62,6 +63,10 @@ import org.apache.logging.log4j.util.StringMap;
* and they will flush to disk at the end of each batch. This means that even with immediateFlush=false, there will
* never be any items left in the buffer; all log events will all be written to disk in a very efficient manner.
*/
+// We changed the constructor from `public` to package-private.
+// The constructor was effectively package-private, since the {@link AsyncLoggerDisruptor} class in its signature is
+// package-private, without any public implementation.
+@BaselineIgnore("2.24.3")
public class AsyncLogger extends Logger implements EventTranslatorVararg<RingBufferLogEvent> {
// Implementation note: many methods in this class are tuned for performance. MODIFY WITH CARE!
// Specifically, try to keep the hot methods to 35 bytecodes or less:
@@ -86,16 +91,14 @@ public class AsyncLogger extends Logger implements EventTranslatorVararg<RingBuf
/**
* Constructs an {@code AsyncLogger} with the specified context, name and message factory.
*
- * @param context context of this logger
- * @param name name of this logger
- * @param messageFactory message factory of this logger
- * @param loggerDisruptor helper class that logging can be delegated to. This object owns the Disruptor.
+ * @param context The {@link LoggerContext} this logger is associated with, never {@code null}.
+ * @param name The logger name, never {@code null}.
+ * @param messageFactory The message factory to be used.
+ * Passing a {@code null} value is deprecated, but supported for backward compatibility.
+ * @param loggerDisruptor Helper class that logging can be delegated to. This object owns the Disruptor.
*/
- public AsyncLogger(
- final LoggerContext context,
- final String name,
- final MessageFactory messageFactory,
- final AsyncLoggerDisruptor loggerDisruptor) {
+ AsyncLogger(
+ LoggerContext context, String name, MessageFactory messageFactory, AsyncLoggerDisruptor loggerDisruptor) {
super(context, name, messageFactory);
this.loggerDisruptor = loggerDisruptor;
includeLocation = privateConfig.loggerConfig.isIncludeLocation();
@@ -145,7 +148,7 @@ public class AsyncLogger extends Logger implements EventTranslatorVararg<RingBuf
getTranslatorType().log(fqcn, location, level, marker, message, throwable);
}
- abstract class TranslatorType {
+ abstract static class TranslatorType {
abstract void log(
final String fqcn,
final StackTraceElement location,
@@ -400,7 +403,7 @@ public class AsyncLogger extends Logger implements EventTranslatorVararg<RingBuf
if (!isReused(message)) {
InternalAsyncUtil.makeMessageImmutable(message);
}
- StackTraceElement location = null;
+ StackTraceElement location;
// calls the translateTo method on this AsyncLogger
if (!disruptor
.getRingBuffer()