Improve validation for AsyncWaitStrategyFactoryConfig for null/empty 'factoryClassName' (apache#3159)

Author: jethomas-tsi

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(

log4j-core-test/src/test/resources/log4j2-asyncwaitfactoryconfig-3159-nok.xml

@@ -0,0 +1,20 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  ~ 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.
+  -->
+<Configuration status="warn">
+   <AsyncWaitStrategyFactory/> <!-- no 'class' attribute -->
+</Configuration>

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);
         }
 

src/changelog/.2.x.x/3159_fix_AsyncWaitStrategyFactoryConfig_guardNPE.xml

@@ -0,0 +1,8 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<entry xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+       xmlns="https://logging.apache.org/xml/ns"
+       xsi:schemaLocation="https://logging.apache.org/xml/ns https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"
+       type="changed">
+  <issue id="3159" link="https://github.com/apache/logging-log4j2/issues/3159"/>
+  <description format="asciidoc">Add improved validation to AsyncWaitStrategyFactoryConfig for null/empty factoryClassName. GitHub issue #3159.</description>
+</entry>