fix: null-check LoggerDynamicMBean addAppender and AppenderDynamicMBean setLayout

log4j-1.2-api/src/main/java/org/apache/log4j/jmx/AppenderDynamicMBean.java

@@ -187,6 +187,14 @@ public Object invoke(final String operationName, final Object params[], final St
         } else if (operationName.equals("setLayout")) {
             final Layout layout =
                     (Layout) OptionConverter.instantiateByClassName((String) params[0], Layout.class, null);
+            if (layout == null) {
+                final String message = "Could not instantiate layout class [" + params[0] + "] for appender ["
+                        + getAppenderName(appender) + "].";
+                cat.error(message);
+                // Fail via MBeanException so JMX clients can distinguish failure from
+                // success (setLayout is declared void; a return string is not reliable).
+                throw new MBeanException(new IllegalArgumentException(message), message);
+            }
             appender.setLayout(layout);
             registerLayoutMBean(layout);
         }

log4j-1.2-api/src/main/java/org/apache/log4j/jmx/LoggerDynamicMBean.java

@@ -63,10 +63,16 @@ public LoggerDynamicMBean(final Logger logger) {
         buildDynamicMBeanInfo();
     }
 
-    void addAppender(final String appenderClass, final String appenderName) {
+    void addAppender(final String appenderClass, final String appenderName) throws MBeanException {
         cat.debug("addAppender called with " + appenderClass + ", " + appenderName);
         final Appender appender =
                 (Appender) OptionConverter.instantiateByClassName(appenderClass, org.apache.log4j.Appender.class, null);
+        if (appender == null) {
+            final String message =
+                    "Could not instantiate appender class [" + appenderClass + "] for name [" + appenderName + "].";
+            cat.error(message);
+            throw new MBeanException(new IllegalArgumentException(message), message);
+        }
         appender.setName(appenderName);
         logger.addAppender(appender);
 

log4j-1.2-api/src/test/java/org/apache/log4j/jmx/AppenderDynamicMBeanTest.java

@@ -0,0 +1,83 @@
+/*
+ * 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.log4j.jmx;
+
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import javax.management.MBeanException;
+import javax.management.MBeanServer;
+import javax.management.MBeanServerFactory;
+import javax.management.ObjectName;
+import org.apache.log4j.ConsoleAppender;
+import org.apache.log4j.PatternLayout;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Regression for JMX {@code setLayout}: instantiateByClassName may return null.
+ */
+class AppenderDynamicMBeanTest {
+
+    private static final String[] SET_LAYOUT_SIGNATURE = {String.class.getName()};
+
+    private MBeanServer server;
+
+    @BeforeEach
+    void createMBeanServer() {
+        server = MBeanServerFactory.newMBeanServer();
+    }
+
+    /**
+     * Registration is required: {@code preRegister} injects the server that
+     * {@code registerLayoutMBean} dereferences on the success path.
+     */
+    private AppenderDynamicMBean registerAppenderMBean(final ConsoleAppender appender) throws Exception {
+        final AppenderDynamicMBean mbean = new AppenderDynamicMBean(appender);
+        server.registerMBean(mbean, new ObjectName("log4j:appender=" + appender.getName()));
+        return mbean;
+    }
+
+    @Test
+    void setLayoutFailsWhenClassCannotBeInstantiated() throws Exception {
+        final ConsoleAppender appender = new ConsoleAppender();
+        appender.setName("jmx-layout-test");
+        final AppenderDynamicMBean mbean = registerAppenderMBean(appender);
+
+        final MBeanException thrown = assertThrows(
+                MBeanException.class,
+                () -> mbean.invoke(
+                        "setLayout", new Object[] {"this.class.does.not.exist.MissingLayout"}, SET_LAYOUT_SIGNATURE));
+        assertTrue(thrown.getMessage().contains("Could not instantiate layout class"));
+        assertInstanceOf(IllegalArgumentException.class, thrown.getTargetException());
+        assertNull(appender.getLayout());
+    }
+
+    @Test
+    void setLayoutStillAttachesValidLayout() throws Exception {
+        final ConsoleAppender appender = new ConsoleAppender();
+        appender.setName("jmx-layout-valid");
+        final AppenderDynamicMBean mbean = registerAppenderMBean(appender);
+
+        mbean.invoke("setLayout", new Object[] {PatternLayout.class.getName()}, SET_LAYOUT_SIGNATURE);
+        assertInstanceOf(PatternLayout.class, appender.getLayout());
+        assertTrue(server.isRegistered(
+                new ObjectName("log4j:appender=" + appender.getName() + ",layout=" + PatternLayout.class.getName())));
+    }
+}

log4j-1.2-api/src/test/java/org/apache/log4j/jmx/LoggerDynamicMBeanTest.java

@@ -0,0 +1,102 @@
+/*
+ * 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.log4j.jmx;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertInstanceOf;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+import java.util.Enumeration;
+import javax.management.MBeanException;
+import javax.management.MBeanServer;
+import javax.management.MBeanServerFactory;
+import javax.management.ObjectName;
+import org.apache.log4j.Appender;
+import org.apache.log4j.ConsoleAppender;
+import org.apache.log4j.Logger;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+/**
+ * Regression for invalid JMX {@code addAppender} class names: instantiation can
+ * return null, which must not NPE when calling {@code setName} and must not be
+ * reported to the client as a successful invocation.
+ */
+class LoggerDynamicMBeanTest {
+
+    private static final String[] ADD_APPENDER_SIGNATURE = {String.class.getName(), String.class.getName()};
+
+    private MBeanServer server;
+
+    @BeforeEach
+    void createMBeanServer() {
+        server = MBeanServerFactory.newMBeanServer();
+    }
+
+    /**
+     * Register through an {@link MBeanServer} so the bean goes through
+     * {@code preRegister}, matching real JMX use (and {@code AppenderDynamicMBeanTest}).
+     */
+    private LoggerDynamicMBean registerLoggerMBean(final Logger logger) throws Exception {
+        final LoggerDynamicMBean mbean = new LoggerDynamicMBean(logger);
+        final String name = logger.getName().isEmpty() ? "root" : logger.getName();
+        // Same ObjectName shape as HierarchyDynamicMBean.addLoggerMBean.
+        server.registerMBean(mbean, new ObjectName("log4j", "logger", name));
+        return mbean;
+    }
+
+    @Test
+    void addAppenderFailsWhenClassCannotBeInstantiated() throws Exception {
+        final Logger logger = Logger.getLogger("jmx.LoggerDynamicMBeanTest.invalid");
+        final LoggerDynamicMBean mbean = registerLoggerMBean(logger);
+
+        final MBeanException thrown = assertThrows(
+                MBeanException.class,
+                () -> mbean.invoke(
+                        "addAppender",
+                        new Object[] {"this.class.does.not.exist.MissingAppender", "should-not-attach"},
+                        ADD_APPENDER_SIGNATURE));
+        assertTrue(thrown.getMessage().contains("Could not instantiate appender class"));
+        assertInstanceOf(IllegalArgumentException.class, thrown.getTargetException());
+        assertFalse(hasAppenderNamed(logger, "should-not-attach"));
+    }
+
+    @Test
+    void addAppenderStillAttachesValidAppender() throws Exception {
+        final Logger logger = Logger.getLogger("jmx.LoggerDynamicMBeanTest.valid");
+        final LoggerDynamicMBean mbean = registerLoggerMBean(logger);
+
+        final Object result = mbean.invoke(
+                "addAppender", new Object[] {ConsoleAppender.class.getName(), "console-jmx"}, ADD_APPENDER_SIGNATURE);
+        assertTrue(hasAppenderNamed(logger, "console-jmx"));
+        // Legacy success return value, pinned so the fix cannot silently change it.
+        assertEquals("Hello world.", result);
+    }
+
+    private static boolean hasAppenderNamed(final Logger logger, final String name) {
+        final Enumeration enumeration = logger.getAllAppenders();
+        while (enumeration.hasMoreElements()) {
+            final Appender appender = (Appender) enumeration.nextElement();
+            if (name.equals(appender.getName())) {
+                return true;
+            }
+        }
+        return false;
+    }
+}

src/changelog/.2.x.x/4185_fix_jmx_mbean_npe.xml

@@ -0,0 +1,12 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<entry xmlns="https://logging.apache.org/xml/ns"
+       xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+       xsi:schemaLocation="
+           https://logging.apache.org/xml/ns
+           https://logging.apache.org/xml/ns/log4j-changelog-0.xsd"
+       type="fixed">
+    <issue id="4185" link="https://github.com/apache/logging-log4j2/pull/4185"/>
+    <description format="asciidoc">
+        Fix exceptions in JMX integration
+    </description>
+</entry>