This is an automated email from the ASF dual-hosted git repository. rmaucher pushed a commit to branch 11.0.x in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 5e5e940ac923e9ce322dd3673e3d5436be725bfb Author: opencode <[email protected]> AuthorDate: Wed Oct 7 12:50:11 2026 +0200 Report failed property updates of the connector MBean The connector MBean read and wrote its properties through IntrospectionUtils but ignored the outcome. An update of an unknown property, or of a known property with a value that could not be converted, for example a port that is not a number, was reported to the JMX client as a successful change that did nothing, and reading an unknown attribute returned null instead of the required attribute not found error. Fail the update when the value could not be applied, and report an unknown attribute on read. An unset property keeps returning null, so the property is only rejected when no getter exists on the connector or on its protocol handler. Attribute dumps log the error for attributes that only exist for another protocol at debug level. --- .../org/apache/catalina/mbeans/ConnectorMBean.java | 29 +++++++- .../apache/catalina/mbeans/LocalStrings.properties | 1 + java/org/apache/catalina/mbeans/MBeanDumper.java | 9 +++ .../apache/catalina/mbeans/TestConnectorMBean.java | 85 ++++++++++++++++++++++ 4 files changed, 121 insertions(+), 3 deletions(-) diff --git a/java/org/apache/catalina/mbeans/ConnectorMBean.java b/java/org/apache/catalina/mbeans/ConnectorMBean.java index 1bdd2a8473..6205bd8148 100644 --- a/java/org/apache/catalina/mbeans/ConnectorMBean.java +++ b/java/org/apache/catalina/mbeans/ConnectorMBean.java @@ -51,7 +51,26 @@ public class ConnectorMBean extends ClassNameMBean<Connector> { } Connector connector = doGetManagedResource(); - return IntrospectionUtils.getProperty(connector, name); + Object value = IntrospectionUtils.getProperty(connector, name); + if (value == null) { + // An unset property has a null value, an unknown attribute has no + // getter. Only report an error for the latter. Protocol handler + // properties are read through the connector's getProperty. + String capitalized = IntrospectionUtils.capitalize(name); + if (!hasGetter(connector.getClass(), capitalized)) { + Object handler = connector.getProtocolHandler(); + if (handler == null || !hasGetter(handler.getClass(), capitalized)) { + throw new AttributeNotFoundException(sm.getString("mBean.attributeNotFound", name)); + } + } + } + return value; + } + + + private static boolean hasGetter(Class<?> clazz, String capitalizedProperty) { + return IntrospectionUtils.findMethod(clazz, "get" + capitalizedProperty, null) != null || + IntrospectionUtils.findMethod(clazz, "is" + capitalizedProperty, null) != null; } @@ -72,10 +91,14 @@ public class ConnectorMBean extends ClassNameMBean<Connector> { } Connector connector = doGetManagedResource(); + boolean set; if (value == null) { - IntrospectionUtils.setProperty(connector, name, null); + set = IntrospectionUtils.setProperty(connector, name, null); } else { - IntrospectionUtils.setProperty(connector, name, String.valueOf(value)); + set = IntrospectionUtils.setProperty(connector, name, String.valueOf(value)); + } + if (!set) { + throw new MBeanException(new IllegalArgumentException(sm.getString("mBean.setAttributeFail", name))); } } } diff --git a/java/org/apache/catalina/mbeans/LocalStrings.properties b/java/org/apache/catalina/mbeans/LocalStrings.properties index 5dc6a1311f..73107d3412 100644 --- a/java/org/apache/catalina/mbeans/LocalStrings.properties +++ b/java/org/apache/catalina/mbeans/LocalStrings.properties @@ -38,6 +38,7 @@ mBean.attributeNotFound=Cannot find attribute [{0}] mBean.nameChange=Cannot update name property mBean.nullAttribute=Attribute is null mBean.nullName=Attribute name is null +mBean.setAttributeFail=Cannot set attribute [{0}] mBeanDumper.getAttributeError=Error getting attribute [{0}] for object name [{1}] diff --git a/java/org/apache/catalina/mbeans/MBeanDumper.java b/java/org/apache/catalina/mbeans/MBeanDumper.java index 83d53e8b10..5812f9f16c 100644 --- a/java/org/apache/catalina/mbeans/MBeanDumper.java +++ b/java/org/apache/catalina/mbeans/MBeanDumper.java @@ -20,6 +20,7 @@ import java.lang.reflect.Array; import java.util.Set; import java.util.StringJoiner; +import javax.management.AttributeNotFoundException; import javax.management.JMRuntimeException; import javax.management.MBeanAttributeInfo; import javax.management.MBeanInfo; @@ -113,6 +114,14 @@ public class MBeanDumper { log.error(sm.getString("mBeanDumper.getAttributeError", attName, oname), rme); } continue; + } catch (AttributeNotFoundException e) { + // Attributes that are advertised in the descriptor for + // every protocol but handled by a single protocol do + // not exist on the other protocols + if (log.isDebugEnabled()) { + log.debug(sm.getString("mBeanDumper.getAttributeError", attName, oname), e); + } + continue; } catch (Throwable t) { ExceptionUtils.handleThrowable(t); log.error(sm.getString("mBeanDumper.getAttributeError", attName, oname), t); diff --git a/test/org/apache/catalina/mbeans/TestConnectorMBean.java b/test/org/apache/catalina/mbeans/TestConnectorMBean.java new file mode 100644 index 0000000000..d846466970 --- /dev/null +++ b/test/org/apache/catalina/mbeans/TestConnectorMBean.java @@ -0,0 +1,85 @@ +/* + * 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.catalina.mbeans; + +import java.util.Set; + +import javax.management.Attribute; +import javax.management.AttributeNotFoundException; +import javax.management.MBeanException; +import javax.management.MBeanServer; +import javax.management.ObjectInstance; +import javax.management.ObjectName; + +import org.junit.Assert; +import org.junit.Test; + +import org.apache.catalina.startup.Tomcat; +import org.apache.catalina.startup.TomcatBaseTest; +import org.apache.tomcat.util.modeler.Registry; + +public class TestConnectorMBean extends TomcatBaseTest { + + @Test + public void testAttributeFailuresAreReported() throws Exception { + Tomcat tomcat = getTomcatInstance(); + tomcat.start(); + + MBeanServer server = Registry.getRegistry(null).getMBeanServer(); + + Set<ObjectInstance> connectors = server.queryMBeans(new ObjectName("*:type=Connector,*"), null); + Assert.assertEquals(1, connectors.size()); + ObjectName oname = connectors.iterator().next().getObjectName(); + + // A value that cannot be converted. This used to be reported as a + // successful update that did nothing. + Object port = server.getAttribute(oname, "port"); + + try { + server.setAttribute(oname, new Attribute("port", "not-a-number")); + Assert.fail("Setting an invalid port value must fail"); + } catch (MBeanException e) { + Assert.assertTrue(String.valueOf(e.getCause()), e.getCause() instanceof IllegalArgumentException); + } + + // The failed update must not have changed the value + Assert.assertEquals(port, server.getAttribute(oname, "port")); + + // An unknown attribute. Reading used to return null and writing used + // to do nothing, both without an error. + try { + server.getAttribute(oname, "notAnAttribute"); + Assert.fail("Reading an unknown attribute must fail"); + } catch (AttributeNotFoundException e) { + // Expected + } + + try { + server.setAttribute(oname, new Attribute("notAnAttribute", "x")); + Assert.fail("Writing an unknown attribute must fail"); + } catch (MBeanException e) { + // Expected + } + + // A protocol handler property that is simply unset reads as null + Assert.assertNull(server.getAttribute(oname, "relaxedPathChars")); + + // A valid property can still be written and read + server.setAttribute(oname, new Attribute("useBodyEncodingForURI", "true")); + Assert.assertEquals(Boolean.TRUE, server.getAttribute(oname, "useBodyEncodingForURI")); + } +} --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
