This is an automated email from the ASF dual-hosted git repository.
vy pushed a commit to branch 2.x
in repository https://gitbox.apache.org/repos/asf/logging-log4j2.git
The following commit(s) were added to refs/heads/2.x by this push:
new 3c60ae0c04 Fix exceptions in the JMX integration (#4185)
3c60ae0c04 is described below
commit 3c60ae0c04d2f310e33c77d6e22a50023d80193a
Author: Sebastien Tardif <[email protected]>
AuthorDate: Fri Aug 14 01:18:49 2026 -0700
Fix exceptions in the JMX integration (#4185)
Signed-off-by: Sebastien Tardif <[email protected]>
Co-authored-by: Ramanathan <[email protected]>
Co-authored-by: Volkan Yazıcı <[email protected]>
---
.../org/apache/log4j/jmx/AppenderDynamicMBean.java | 8 ++
.../org/apache/log4j/jmx/LoggerDynamicMBean.java | 8 +-
.../apache/log4j/jmx/AppenderDynamicMBeanTest.java | 83 +++++++++++++++++
.../apache/log4j/jmx/LoggerDynamicMBeanTest.java | 102 +++++++++++++++++++++
src/changelog/.2.x.x/4185_fix_jmx_mbean_npe.xml | 12 +++
5 files changed, 212 insertions(+), 1 deletion(-)
diff --git
a/log4j-1.2-api/src/main/java/org/apache/log4j/jmx/AppenderDynamicMBean.java
b/log4j-1.2-api/src/main/java/org/apache/log4j/jmx/AppenderDynamicMBean.java
index 2ee683240a..053388fb18 100644
--- a/log4j-1.2-api/src/main/java/org/apache/log4j/jmx/AppenderDynamicMBean.java
+++ b/log4j-1.2-api/src/main/java/org/apache/log4j/jmx/AppenderDynamicMBean.java
@@ -187,6 +187,14 @@ public class AppenderDynamicMBean extends
AbstractDynamicMBean {
} 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);
}
diff --git
a/log4j-1.2-api/src/main/java/org/apache/log4j/jmx/LoggerDynamicMBean.java
b/log4j-1.2-api/src/main/java/org/apache/log4j/jmx/LoggerDynamicMBean.java
index 7be0f070f5..5d6825dd2d 100644
--- a/log4j-1.2-api/src/main/java/org/apache/log4j/jmx/LoggerDynamicMBean.java
+++ b/log4j-1.2-api/src/main/java/org/apache/log4j/jmx/LoggerDynamicMBean.java
@@ -63,10 +63,16 @@ public class LoggerDynamicMBean extends
AbstractDynamicMBean implements Notifica
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);
diff --git
a/log4j-1.2-api/src/test/java/org/apache/log4j/jmx/AppenderDynamicMBeanTest.java
b/log4j-1.2-api/src/test/java/org/apache/log4j/jmx/AppenderDynamicMBeanTest.java
new file mode 100644
index 0000000000..597bd3822a
--- /dev/null
+++
b/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())));
+ }
+}
diff --git
a/log4j-1.2-api/src/test/java/org/apache/log4j/jmx/LoggerDynamicMBeanTest.java
b/log4j-1.2-api/src/test/java/org/apache/log4j/jmx/LoggerDynamicMBeanTest.java
new file mode 100644
index 0000000000..8bd45969d5
--- /dev/null
+++
b/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;
+ }
+}
diff --git a/src/changelog/.2.x.x/4185_fix_jmx_mbean_npe.xml
b/src/changelog/.2.x.x/4185_fix_jmx_mbean_npe.xml
new file mode 100644
index 0000000000..65e2259a3f
--- /dev/null
+++ b/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>
