diff --git a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/async/AsyncWaitStrategyFactoryConfigTest.java b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/async/AsyncWaitStrategyFactoryConfigTest.java index bdc2bad1b14..0de62c02ec2 100644 --- a/log4j-core-test/src/test/java/org/apache/logging/log4j/core/async/AsyncWaitStrategyFactoryConfigTest.java +++ b/log4j-core-test/src/test/java/org/apache/logging/log4j/core/async/AsyncWaitStrategyFactoryConfigTest.java @@ -17,17 +17,19 @@ package org.apache.logging.log4j.core.async; import static org.hamcrest.MatcherAssert.assertThat; -import static org.junit.Assert.assertNull; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; import com.lmax.disruptor.WaitStrategy; import com.lmax.disruptor.YieldingWaitStrategy; import org.apache.logging.log4j.core.LoggerContext; +import org.apache.logging.log4j.core.config.Configuration; import org.apache.logging.log4j.core.test.appender.ListAppender; import org.apache.logging.log4j.core.test.categories.AsyncLoggers; import org.apache.logging.log4j.core.test.junit.LoggerContextSource; import org.apache.logging.log4j.core.test.junit.Named; import org.junit.experimental.categories.Category; +import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; @Category(AsyncLoggers.class) @@ -68,6 +70,46 @@ public void testIncorrectConfigWaitStrategyFactory(final LoggerContext context) assertNull(asyncWaitStrategyFactory); } + /** + * Test that when XML element {@code AsyncWaitFactory} has no 'class' attribute. + * + * @param configuration the configuration + */ + @Test + @LoggerContextSource("log4j2-asyncwaitfactoryconfig-3159-nok.xml") + void testInvalidBuilderConfiguration3159(final Configuration configuration) { + assertNull(configuration.getAsyncWaitStrategyFactory(), "The AsyncWaitStrategyFactory should be null."); + } + + /** + * Test that when programmatically building a {@link AsyncWaitStrategyFactoryConfig} not setting a valid + * factory class-name throws an exception. + */ + @Test + void testInvalidProgrammaticConfiguration3159WithFactoryClassNameNotSet() { + Assertions.assertNull(AsyncWaitStrategyFactoryConfig.newBuilder().build()); + } + + /** + * Test that when programmatically building a {@link AsyncWaitStrategyFactoryConfig} a {@code null} + * factory class-name throws an exception. + */ + @Test + void testInvalidProgrammaticConfiguration3159WithFactoryClassNameNull() { + Assertions.assertThrows(IllegalArgumentException.class, () -> AsyncWaitStrategyFactoryConfig.newBuilder() + .withFactoryClassName(null)); + } + + /** + * Test that when programmatically building a {@link AsyncWaitStrategyFactoryConfig} a blank ({@code ""}) + * factory class-name throws an exception. + */ + @Test + void testInvalidProgrammaticConfiguration3159WithFactoryClassNameEmpty() { + Assertions.assertThrows(IllegalArgumentException.class, () -> AsyncWaitStrategyFactoryConfig.newBuilder() + .withFactoryClassName("")); + } + @Test @LoggerContextSource("AsyncWaitStrategyIncorrectFactoryConfigTest.xml") public void testIncorrectWaitStrategyFallsBackToDefault( diff --git a/log4j-core-test/src/test/resources/log4j2-asyncwaitfactoryconfig-3159-nok.xml b/log4j-core-test/src/test/resources/log4j2-asyncwaitfactoryconfig-3159-nok.xml new file mode 100644 index 00000000000..21512a602bc --- /dev/null +++ b/log4j-core-test/src/test/resources/log4j2-asyncwaitfactoryconfig-3159-nok.xml @@ -0,0 +1,20 @@ + + + + + diff --git a/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncWaitStrategyFactoryConfig.java b/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncWaitStrategyFactoryConfig.java index e57abcfff10..890d377cd26 100644 --- a/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncWaitStrategyFactoryConfig.java +++ b/log4j-core/src/main/java/org/apache/logging/log4j/core/async/AsyncWaitStrategyFactoryConfig.java @@ -16,12 +16,12 @@ */ package org.apache.logging.log4j.core.async; -import java.util.Objects; import org.apache.logging.log4j.core.Core; 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.config.plugins.validation.constraints.Required; +import org.apache.logging.log4j.core.util.Assert; import org.apache.logging.log4j.status.StatusLogger; import org.apache.logging.log4j.util.LoaderUtil; @@ -41,7 +41,7 @@ public class AsyncWaitStrategyFactoryConfig { private final String factoryClassName; public AsyncWaitStrategyFactoryConfig(final String factoryClassName) { - this.factoryClassName = Objects.requireNonNull(factoryClassName, "factoryClassName"); + this.factoryClassName = Assert.requireNonEmpty(factoryClassName, "factoryClassName"); } @PluginBuilderFactory @@ -67,12 +67,16 @@ public String getFactoryClassName() { } public B withFactoryClassName(final String className) { - this.factoryClassName = className; + this.factoryClassName = + Assert.requireNonEmpty(className, "The 'className' argument must not be null or empty."); return asBuilder(); } @Override public AsyncWaitStrategyFactoryConfig build() { + if (!isValid()) { + return null; + } return new AsyncWaitStrategyFactoryConfig(factoryClassName); } diff --git a/src/changelog/.2.x.x/3159_fix_AsyncWaitStrategyFactoryConfig_guardNPE.xml b/src/changelog/.2.x.x/3159_fix_AsyncWaitStrategyFactoryConfig_guardNPE.xml new file mode 100644 index 00000000000..938f065991b --- /dev/null +++ b/src/changelog/.2.x.x/3159_fix_AsyncWaitStrategyFactoryConfig_guardNPE.xml @@ -0,0 +1,8 @@ + + + + Add improved validation to AsyncWaitStrategyFactoryConfig for null/empty factoryClassName. GitHub issue #3159. +