Fix potential NPEs in DefaultComponentBuilder in addAttribute methods (apache#2791)

Author: JWT007

log4j-core/src/main/java/org/apache/logging/log4j/core/config/builder/api/ComponentBuilder.java

@@ -29,6 +29,10 @@ public interface ComponentBuilder<T extends ComponentBuilder<T>> extends Builder
 
     /**
      * Adds a String attribute.
+     * <p>
+     *   If the given {@code level} value is {@code null}, the component attribute with the given key
+     *   will be removed (if present).
+     * </p>
      * @param key The attribute key.
      * @param value The value of the attribute.
      * @return This ComponentBuilder.
@@ -37,6 +41,10 @@ public interface ComponentBuilder<T extends ComponentBuilder<T>> extends Builder
 
     /**
      * Adds a logging Level attribute.
+     * <p>
+     *   If the given {@code level} value is {@code null}, the component attribute with the given key
+     *   will be removed (if present).
+     * </p>
      * @param key The attribute key.
      * @param level The logging Level.
      * @return This ComponentBuilder.
@@ -45,6 +53,10 @@ public interface ComponentBuilder<T extends ComponentBuilder<T>> extends Builder
 
     /**
      * Adds an enumeration attribute.
+     * <p>
+     *   If the given {@code level} value is {@code null}, the component attribute with the given key
+     *   will be removed (if present).
+     * </p>
      * @param key The attribute key.
      * @param value The enumeration.
      * @return This ComponentBuilder.
@@ -69,6 +81,10 @@ public interface ComponentBuilder<T extends ComponentBuilder<T>> extends Builder
 
     /**
      * Adds an Object attribute.
+     * <p>
+     *   If the given {@code value} is {@code null}, the component attribute with the given key
+     *   will be removed (if present).
+     * </p>
      * @param key The attribute key.
      * @param value The object value.
      * @return This ComponentBuilder.

log4j-core/src/main/java/org/apache/logging/log4j/core/config/builder/impl/DefaultComponentBuilder.java

@@ -20,6 +20,8 @@
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
+
 import org.apache.logging.log4j.Level;
 import org.apache.logging.log4j.core.config.Configuration;
 import org.apache.logging.log4j.core.config.builder.api.Component;
@@ -57,43 +59,51 @@ public DefaultComponentBuilder(final CB builder, final String name, final String
         this.value = value;
     }
 
+    /** {@inheritDoc} */
     @Override
     public T addAttribute(final String key, final boolean value) {
         return put(key, Boolean.toString(value));
     }
 
+    /** {@inheritDoc} */
     @Override
     public T addAttribute(final String key, final Enum<?> value) {
-        return put(key, value.name());
+        return put(key, Optional.ofNullable(value).map(Enum::name).orElse(null));
     }
 
+    /** {@inheritDoc} */
     @Override
     public T addAttribute(final String key, final int value) {
         return put(key, Integer.toString(value));
     }
 
+    /** {@inheritDoc} */
     @Override
     public T addAttribute(final String key, final Level level) {
-        return put(key, level.toString());
+        return put(key, Optional.ofNullable(level).map(Level::toString).orElse(null));
     }
 
+    /** {@inheritDoc} */
     @Override
     public T addAttribute(final String key, final Object value) {
-        return put(key, value.toString());
+        return put(key, Optional.ofNullable(value).map(Object::toString).orElse(null));
     }
 
+    /** {@inheritDoc} */
     @Override
     public T addAttribute(final String key, final String value) {
         return put(key, value);
     }
 
+    /** {@inheritDoc} */
     @Override
     @SuppressWarnings("unchecked")
     public T addComponent(final ComponentBuilder<?> builder) {
         components.add(builder.build());
         return (T) this;
     }
 
+    /** {@inheritDoc} */
     @Override
     public Component build() {
         final Component component = new Component(type, name, value);
@@ -102,19 +112,37 @@ public Component build() {
         return component;
     }
 
+    /** {@inheritDoc} */
     @Override
     public CB getBuilder() {
         return builder;
     }
 
+    /** {@inheritDoc} */
     @Override
     public String getName() {
         return name;
     }
 
+    /**
+     * Puts the given key/value pair to the attribute map.
+     * <p>
+     *   If the value is {@code null} the component attribute with the given {@code key} will
+     *   instead be removed from the map.
+     * </p>
+     * @param key the key
+     * @param value the value
+     * @return this builder (for chaining)
+     */
     @SuppressWarnings("unchecked")
     protected T put(final String key, final String value) {
-        attributes.put(key, value);
+
+        if (value != null) {
+            attributes.put(key, value);
+        } else {
+            attributes.remove(key);
+        }
+
         return (T) this;
     }
 }

src/changelog/.2.x.x/2791_fix_DefaultComponenBuilder_null_value_attributes.xml

@@ -0,0 +1,11 @@
+<?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="2791" link="https://github.com/apache/logging-log4j2/issues/2791"/>
+  <description format="asciidoc">
+    Fix problem when null attribute values are set on DefaultComponentBuilder.
+    GitHub issue #2791.
+  </description>
+</entry>