diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/AbstractAppender.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/AbstractAppender.java index 2ffbf15f55f..fd7bba386b1 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/AbstractAppender.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/AbstractAppender.java @@ -76,14 +76,14 @@ public String getName() { public Layout getOrCreateLayout() { if (layout == null) { - return PatternLayout.createDefaultLayout(); + return PatternLayout.createDefaultLayout(configuration); } return layout; } public Layout getOrCreateLayout(final Charset charset) { if (layout == null) { - return PatternLayout.newBuilder().withCharset(charset).build(); + return PatternLayout.newBuilder().withCharset(charset).withConfiguration(configuration).build(); } return layout; } diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/AbstractManager.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/AbstractManager.java index 72a8b4a14ed..86bd6803a92 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/AbstractManager.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/AbstractManager.java @@ -204,6 +204,13 @@ protected static StatusLogger logger() { return StatusLogger.getLogger(); } + /** + * For testing purposes. + */ + static int getManagerCount() { + return MAP.size(); + } + /** * May be overridden by managers to perform processing while the manager is being released and the * lock is held. A timeout is passed for implementors to use as they see fit. diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/OutputStreamAppender.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/OutputStreamAppender.java index dc6f35277e9..8b1fae69651 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/OutputStreamAppender.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/OutputStreamAppender.java @@ -57,10 +57,8 @@ public static class Builder> extends AbstractOutputStreamAp @Override public OutputStreamAppender build() { - final Layout layout = getLayout(); - final Layout actualLayout = layout == null ? PatternLayout.createDefaultLayout() - : layout; - return new OutputStreamAppender(getName(), actualLayout, getFilter(), getManager(target, follow, actualLayout), + final Layout layout = getOrCreateLayout(); + return new OutputStreamAppender(getName(), layout, getFilter(), getManager(target, follow, layout), ignoreExceptions, getPropertyArray()); } diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/WriterAppender.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/WriterAppender.java index 408fed1f8a2..6921d564fb3 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/WriterAppender.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/appender/WriterAppender.java @@ -16,11 +16,13 @@ */ package org.apache.logging.log4j.core.appender; +import java.io.Serializable; import java.io.Writer; import org.apache.logging.log4j.core.Appender; import org.apache.logging.log4j.core.Core; import org.apache.logging.log4j.core.Filter; +import org.apache.logging.log4j.core.Layout; import org.apache.logging.log4j.core.StringLayout; import org.apache.logging.log4j.core.config.Property; import org.apache.logging.log4j.core.config.plugins.Plugin; @@ -47,9 +49,13 @@ public static class Builder> extends AbstractAppender.Build @Override public WriterAppender build() { - final StringLayout layout = (StringLayout) getLayout(); - final StringLayout actualLayout = layout != null ? layout : PatternLayout.createDefaultLayout(); - return new WriterAppender(getName(), actualLayout, getFilter(), getManager(target, follow, actualLayout), + final Layout layout = getOrCreateLayout(); + if (!(layout instanceof StringLayout)) { + LOGGER.error("Layout must be a StringLayout to log to ServletContext"); + return null; + } + final StringLayout stringLayout = (StringLayout) layout; + return new WriterAppender(getName(), stringLayout, getFilter(), getManager(target, follow, stringLayout), isIgnoreExceptions(), getPropertyArray()); } diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/AbstractConfiguration.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/AbstractConfiguration.java index 8c2755b1ff5..d6da277da0f 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/config/AbstractConfiguration.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/config/AbstractConfiguration.java @@ -714,6 +714,12 @@ protected void doConfigure() { setParents(); } + public static Level getDefaultLevel() { + final String levelName = PropertiesUtil.getProperties().getStringProperty(DefaultConfiguration.DEFAULT_LEVEL, + Level.ERROR.name()); + return Level.valueOf(levelName); + } + protected void setToDefault() { // LOG4J2-1176 facilitate memory leak investigation setName(DefaultConfiguration.DEFAULT_NAME + "@" + Integer.toHexString(hashCode())); @@ -727,11 +733,7 @@ protected void setToDefault() { final LoggerConfig rootLoggerConfig = getRootLogger(); rootLoggerConfig.addAppender(appender, null, null); - final Level defaultLevel = Level.ERROR; - final String levelName = PropertiesUtil.getProperties().getStringProperty(DefaultConfiguration.DEFAULT_LEVEL, - defaultLevel.name()); - final Level level = Level.valueOf(levelName); - rootLoggerConfig.setLevel(level != null ? level : defaultLevel); + rootLoggerConfig.setLevel(getDefaultLevel()); } /** diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/AbstractLayout.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/AbstractLayout.java index 6a7f1753dd4..7496a44b4b8 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/AbstractLayout.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/AbstractLayout.java @@ -129,7 +129,7 @@ public AbstractLayout(final byte[] header, final byte[] footer) { * Constructs a layout with an optional header and footer. * * @param configuration - * The configuration + * The configuration. May be null. * @param header * The header to include when the stream is opened. May be null. * @param footer diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/AbstractStringLayout.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/AbstractStringLayout.java index 8997d41c305..3461507cfa2 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/AbstractStringLayout.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/AbstractStringLayout.java @@ -19,8 +19,11 @@ import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; +import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.core.LogEvent; import org.apache.logging.log4j.core.StringLayout; +import org.apache.logging.log4j.core.config.AbstractConfiguration; import org.apache.logging.log4j.core.config.Configuration; import org.apache.logging.log4j.core.config.LoggerConfig; import org.apache.logging.log4j.core.config.plugins.PluginBuilderAttribute; @@ -176,7 +179,7 @@ protected AbstractStringLayout(final Charset aCharset, final byte[] header, fina /** * Builds a new layout. - * @param config the configuration + * @param config the configuration. May be null. * @param aCharset the charset used to encode the header bytes, footer bytes and anything else that needs to be * converted from strings to bytes. * @param headerSerializer the header bytes serializer @@ -264,10 +267,19 @@ protected String serializeToString(final Serializer serializer) { if (serializer == null) { return null; } - final LoggerConfig rootLogger = getConfiguration().getRootLogger(); + final String loggerName; + final Level level; + if (configuration != null) { + final LoggerConfig rootLogger = configuration.getRootLogger(); + loggerName = rootLogger.getName(); + level = rootLogger.getLevel(); + } else { + loggerName = LogManager.ROOT_LOGGER_NAME; + level = AbstractConfiguration.getDefaultLevel(); + } // Using "" for the FQCN, does it matter? - final LogEvent logEvent = getLogEventFactory().createEvent(rootLogger.getName(), null, Strings.EMPTY, - rootLogger.getLevel(), null, null, null); + final LogEvent logEvent = getLogEventFactory().createEvent(loggerName, null, Strings.EMPTY, + level, null, null, null); return serializer.toSerializable(logEvent); } diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/PatternLayout.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/PatternLayout.java index 6ba5882b477..2f754c6c5dd 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/PatternLayout.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/layout/PatternLayout.java @@ -25,7 +25,6 @@ import org.apache.logging.log4j.core.Layout; import org.apache.logging.log4j.core.LogEvent; import org.apache.logging.log4j.core.config.Configuration; -import org.apache.logging.log4j.core.config.DefaultConfiguration; import org.apache.logging.log4j.core.config.Node; import org.apache.logging.log4j.core.config.plugins.Plugin; import org.apache.logging.log4j.core.config.plugins.PluginAttribute; @@ -243,7 +242,7 @@ private StringBuilder toText(final Serializer2 serializer, final LogEvent event, /** * Creates a PatternParser. - * @param config The Configuration. + * @param config The Configuration or {@code null}. * @return The PatternParser. */ public static PatternParser createPatternParser(final Configuration config) { @@ -763,10 +762,7 @@ public Builder withFooter(final String footer) { @Override public PatternLayout build() { - // fall back to DefaultConfiguration - if (configuration == null) { - configuration = new DefaultConfiguration(); - } + // should work with a null configuration return new PatternLayout(configuration, regexReplacement, pattern, patternSelector, charset, alwaysWriteExceptions, disableAnsi, noConsoleNoAnsi, header, footer); } diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/PatternParser.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/PatternParser.java index dc77f87c842..8ac65639a0c 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/PatternParser.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/pattern/PatternParser.java @@ -103,11 +103,11 @@ public PatternParser(final String converterKey) { * Constructor. * * @param config - * The current Configuration. + * The current Configuration or {@code null}. * @param converterKey * The key to lookup the converters. * @param expected - * The expected base Class of each Converter. + * The expected base Class of each Converter or {@code null}. */ public PatternParser(final Configuration config, final String converterKey, final Class expected) { this(config, converterKey, expected, null); @@ -117,13 +117,13 @@ public PatternParser(final Configuration config, final String converterKey, fina * Constructor. * * @param config - * The current Configuration. + * The current Configuration or {@code null}. * @param converterKey * The key to lookup the converters. * @param expectedClass - * The expected base Class of each Converter. + * The expected base Class of each Converter or {@code null}. * @param filterClass - * Filter the returned plugins after calling the plugin manager. + * Filter the returned plugins after calling the plugin manager, can be {@code null}. */ public PatternParser(final Configuration config, final String converterKey, final Class expectedClass, final Class filterClass) { diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/AbstractAppenderBuilderTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/AbstractAppenderBuilderTest.java new file mode 100644 index 00000000000..7481fb5a8ca --- /dev/null +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/appender/AbstractAppenderBuilderTest.java @@ -0,0 +1,37 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache license, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the license for the specific language governing permissions and + * limitations under the license. + */ +package org.apache.logging.log4j.core.appender; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import org.apache.logging.log4j.core.config.Configuration; +import org.apache.logging.log4j.core.config.DefaultConfiguration; +import org.junit.jupiter.api.Test; + +public class AbstractAppenderBuilderTest { + + @Test + public void testDefaultLayoutLeak() { + int expected = AbstractManager.getManagerCount(); + final Configuration configuration = new DefaultConfiguration(); + ConsoleAppender appender = ConsoleAppender.newBuilder().setConfiguration(configuration).setName("test").build(); + configuration.addAppender(appender); + configuration.start(); + configuration.stop(); + assertEquals(expected, AbstractManager.getManagerCount(), "No managers should be left after shutdown."); + } +} diff --git a/log4j-core/src/test/java/org/apache/logging/log4j/core/layout/AbstractStringLayoutTest.java b/log4j-core/src/test/java/org/apache/logging/log4j/core/layout/AbstractStringLayoutTest.java index b29b25dbd5d..0a7edf577dc 100644 --- a/log4j-core/src/test/java/org/apache/logging/log4j/core/layout/AbstractStringLayoutTest.java +++ b/log4j-core/src/test/java/org/apache/logging/log4j/core/layout/AbstractStringLayoutTest.java @@ -15,23 +15,33 @@ * limitations under the license. */ +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; + import java.nio.charset.Charset; import org.apache.logging.log4j.core.LogEvent; +import org.apache.logging.log4j.core.config.DefaultConfiguration; +import org.apache.logging.log4j.core.layout.AbstractStringLayout.Serializer; +import org.apache.logging.log4j.core.layout.PatternLayout.SerializerBuilder; import org.junit.jupiter.api.Test; -import static org.junit.jupiter.api.Assertions.*; - /** * Tests AbstractStringLayout. */ public class AbstractStringLayoutTest { + + // Configuration explicitly null + private static final Serializer serializer = new SerializerBuilder().setPattern(DefaultConfiguration.DEFAULT_PATTERN).build(); + static class ConcreteStringLayout extends AbstractStringLayout { public static int DEFAULT_STRING_BUILDER_SIZE = AbstractStringLayout.DEFAULT_STRING_BUILDER_SIZE; public static int MAX_STRING_BUILDER_SIZE = AbstractStringLayout.MAX_STRING_BUILDER_SIZE; public ConcreteStringLayout() { - super(Charset.defaultCharset()); + // Configuration explicitly null + super(null, Charset.defaultCharset(), serializer, serializer); } public static StringBuilder getStringBuilder() { @@ -76,4 +86,14 @@ public void testGetStringBuilderCapacityRestrictedToMax() throws Exception { "capacity, trimmed to MAX_STRING_BUILDER_SIZE"); assertEquals(0, sb3.length(), "empty, ready for use"); } + + @Test + public void testNullConfigurationIsAllowed() { + try { + final ConcreteStringLayout layout = new ConcreteStringLayout(); + layout.serializeToString(serializer); + } catch (NullPointerException e) { + fail(e); + } + } } \ No newline at end of file diff --git a/log4j-jakarta-web/src/main/java/org/apache/logging/log4j/web/appender/ServletAppender.java b/log4j-jakarta-web/src/main/java/org/apache/logging/log4j/web/appender/ServletAppender.java index ca0c141f547..4fc5bc9cfa8 100644 --- a/log4j-jakarta-web/src/main/java/org/apache/logging/log4j/web/appender/ServletAppender.java +++ b/log4j-jakarta-web/src/main/java/org/apache/logging/log4j/web/appender/ServletAppender.java @@ -23,13 +23,13 @@ import org.apache.logging.log4j.core.Filter; import org.apache.logging.log4j.core.Layout; import org.apache.logging.log4j.core.LogEvent; +import org.apache.logging.log4j.core.StringLayout; import org.apache.logging.log4j.core.appender.AbstractAppender; import org.apache.logging.log4j.core.config.Property; import org.apache.logging.log4j.core.config.plugins.Plugin; import org.apache.logging.log4j.core.config.plugins.PluginBuilderAttribute; import org.apache.logging.log4j.core.config.plugins.PluginBuilderFactory; import org.apache.logging.log4j.core.layout.AbstractStringLayout; -import org.apache.logging.log4j.core.layout.PatternLayout; import org.apache.logging.log4j.web.WebLoggerContextUtils; /** @@ -55,10 +55,8 @@ public ServletAppender build() { LOGGER.error("No servlet context is available"); return null; } - Layout layout = getLayout(); - if (layout == null) { - layout = PatternLayout.createDefaultLayout(); - } else if (!(layout instanceof AbstractStringLayout)) { + Layout layout = getOrCreateLayout(); + if (!(layout instanceof StringLayout)) { LOGGER.error("Layout must be a StringLayout to log to ServletContext"); return null; } diff --git a/log4j-web/src/main/java/org/apache/logging/log4j/web/appender/ServletAppender.java b/log4j-web/src/main/java/org/apache/logging/log4j/web/appender/ServletAppender.java index b3c47e28a1c..c9a70f6e6de 100644 --- a/log4j-web/src/main/java/org/apache/logging/log4j/web/appender/ServletAppender.java +++ b/log4j-web/src/main/java/org/apache/logging/log4j/web/appender/ServletAppender.java @@ -23,13 +23,13 @@ import org.apache.logging.log4j.core.Filter; import org.apache.logging.log4j.core.Layout; import org.apache.logging.log4j.core.LogEvent; +import org.apache.logging.log4j.core.StringLayout; import org.apache.logging.log4j.core.appender.AbstractAppender; import org.apache.logging.log4j.core.config.Property; import org.apache.logging.log4j.core.config.plugins.Plugin; import org.apache.logging.log4j.core.config.plugins.PluginBuilderAttribute; import org.apache.logging.log4j.core.config.plugins.PluginBuilderFactory; import org.apache.logging.log4j.core.layout.AbstractStringLayout; -import org.apache.logging.log4j.core.layout.PatternLayout; import org.apache.logging.log4j.web.WebLoggerContextUtils; /** @@ -55,10 +55,8 @@ public ServletAppender build() { LOGGER.error("No servlet context is available"); return null; } - Layout layout = getLayout(); - if (layout == null) { - layout = PatternLayout.createDefaultLayout(); - } else if (!(layout instanceof AbstractStringLayout)) { + final Layout layout = getOrCreateLayout(); + if (!(layout instanceof StringLayout)) { LOGGER.error("Layout must be a StringLayout to log to ServletContext"); return null; }