From 82d861a93a0587533b062a1dc755843c1ea8c9ad Mon Sep 17 00:00:00 2001 From: JWT Date: Wed, 12 Feb 2025 12:12:47 +0100 Subject: [PATCH 1/3] Improve validation for AsyncWaitStrategyFactoryConfig for null/empty 'factoryClassName' (#3159) --- .../AsyncWaitStrategyFactoryConfigTest.java | 33 +++++++++++++++++++ ...log4j2-asyncwaitfactoryconfig-3159-nok.xml | 20 +++++++++++ .../async/AsyncWaitStrategyFactoryConfig.java | 10 ++++-- ...syncWaitStrategyFactoryConfig_guardNPE.xml | 8 +++++ 4 files changed, 68 insertions(+), 3 deletions(-) create mode 100644 log4j-core-test/src/test/resources/log4j2-asyncwaitfactoryconfig-3159-nok.xml create mode 100644 src/changelog/.2.x.x/3159_fix_AsyncWaitStrategyFactoryConfig_guardNPE.xml 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 42cdcb3071c..31272c2c041 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 @@ -20,10 +20,12 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; 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.junit.LoggerContextSource; import org.apache.logging.log4j.core.test.junit.Named; @@ -69,6 +71,37 @@ 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} a {@code null} + * factory class-name throws an exception. + */ + @Test + void testInvalidProgrammaticConfiguration3159WithNullFactoryClassName() { + 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 testInvalidProgrammaticConfiguration3159WithEmptyFactoryClassName() { + assertThrows(IllegalArgumentException.class, () -> AsyncWaitStrategyFactoryConfig.newBuilder() + .withFactoryClassName("")); + } + @Test @LoggerContextSource("AsyncWaitStrategyIncorrectFactoryConfigTest.xml") 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. + From bcfb19314ea8c41d641e8ca0c50a67c1f98b052b Mon Sep 17 00:00:00 2001 From: JWT Date: Wed, 12 Feb 2025 12:18:23 +0100 Subject: [PATCH 2/3] Moved and updated 3159 changelog entry. (#3159) --- .../3159_fix_AsyncWaitStrategyFactoryConfig_guardNPE.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) rename src/changelog/{.2.x.x => 2.25.0}/3159_fix_AsyncWaitStrategyFactoryConfig_guardNPE.xml (78%) diff --git a/src/changelog/.2.x.x/3159_fix_AsyncWaitStrategyFactoryConfig_guardNPE.xml b/src/changelog/2.25.0/3159_fix_AsyncWaitStrategyFactoryConfig_guardNPE.xml similarity index 78% rename from src/changelog/.2.x.x/3159_fix_AsyncWaitStrategyFactoryConfig_guardNPE.xml rename to src/changelog/2.25.0/3159_fix_AsyncWaitStrategyFactoryConfig_guardNPE.xml index 938f065991b..3b4dbb35ab5 100644 --- a/src/changelog/.2.x.x/3159_fix_AsyncWaitStrategyFactoryConfig_guardNPE.xml +++ b/src/changelog/2.25.0/3159_fix_AsyncWaitStrategyFactoryConfig_guardNPE.xml @@ -2,7 +2,7 @@ + type="fixed"> - Add improved validation to AsyncWaitStrategyFactoryConfig for null/empty factoryClassName. GitHub issue #3159. + Add improved validation to AsyncWaitStrategyFactoryConfig for null/empty factoryClassName. From 246f8480edc947c19bf60e2a64a32015a4caee89 Mon Sep 17 00:00:00 2001 From: JWT Date: Sun, 16 Feb 2025 20:48:11 +0100 Subject: [PATCH 3/3] Moved changelog to .2.x.x per other PR Code-Review (#3159) --- .../3159_fix_AsyncWaitStrategyFactoryConfig_guardNPE.xml | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename src/changelog/{2.25.0 => .2.x.x}/3159_fix_AsyncWaitStrategyFactoryConfig_guardNPE.xml (100%) diff --git a/src/changelog/2.25.0/3159_fix_AsyncWaitStrategyFactoryConfig_guardNPE.xml b/src/changelog/.2.x.x/3159_fix_AsyncWaitStrategyFactoryConfig_guardNPE.xml similarity index 100% rename from src/changelog/2.25.0/3159_fix_AsyncWaitStrategyFactoryConfig_guardNPE.xml rename to src/changelog/.2.x.x/3159_fix_AsyncWaitStrategyFactoryConfig_guardNPE.xml