This is an automated email from the ASF dual-hosted git repository.
markt-asf pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push:
new 92cb4c500a Follow up to "Keep naming resource entries consistent when
updated..."
92cb4c500a is described below
commit 92cb4c500abd975a587cc18c0696b20f1a275b9a
Author: Mark Thomas <[email protected]>
AuthorDate: Thu Oct 8 15:08:47 2026 +0100
Follow up to "Keep naming resource entries consistent when updated..."
- consistent validation of new attribute
- align MBean with ContextEnvironment
- improve roll-back
- format code
---
.../apache/catalina/mbeans/BaseCatalinaMBean.java | 3 +
.../catalina/mbeans/BaseNamingResourceMBean.java | 78 ++++++++++++++++++++++
.../org/apache/catalina/mbeans/ConnectorMBean.java | 3 +-
.../org/apache/catalina/mbeans/ContainerMBean.java | 7 +-
.../catalina/mbeans/ContextEnvironmentMBean.java | 54 ++++++++++-----
java/org/apache/catalina/mbeans/ContextMBean.java | 3 +-
.../catalina/mbeans/ContextResourceLinkMBean.java | 26 ++++----
.../catalina/mbeans/ContextResourceMBean.java | 41 +++++++-----
.../mbeans/DataSourceUserDatabaseMBean.java | 2 +-
.../apache/catalina/mbeans/LocalStrings.properties | 2 +
java/org/apache/catalina/mbeans/MBeanDumper.java | 1 +
java/org/apache/catalina/mbeans/MBeanUtils.java | 4 +-
java/org/apache/catalina/mbeans/ServiceMBean.java | 4 +-
13 files changed, 167 insertions(+), 61 deletions(-)
diff --git a/java/org/apache/catalina/mbeans/BaseCatalinaMBean.java
b/java/org/apache/catalina/mbeans/BaseCatalinaMBean.java
index f5ac3f28fa..f91652b592 100644
--- a/java/org/apache/catalina/mbeans/BaseCatalinaMBean.java
+++ b/java/org/apache/catalina/mbeans/BaseCatalinaMBean.java
@@ -40,6 +40,7 @@ public abstract class BaseCatalinaMBean<T> extends
BaseModelMBean {
* Returns the managed resource associated with this MBean.
*
* @return the managed resource
+ *
* @throws MBeanException if the resource cannot be retrieved
*/
protected T doGetManagedResource() throws MBeanException {
@@ -57,7 +58,9 @@ public abstract class BaseCatalinaMBean<T> extends
BaseModelMBean {
* Creates a new instance of the specified class.
*
* @param type the fully qualified class name
+ *
* @return the new instance
+ *
* @throws MBeanException if the instance cannot be created
*/
protected static Object newInstance(String type) throws MBeanException {
diff --git a/java/org/apache/catalina/mbeans/BaseNamingResourceMBean.java
b/java/org/apache/catalina/mbeans/BaseNamingResourceMBean.java
new file mode 100644
index 0000000000..16320e743a
--- /dev/null
+++ b/java/org/apache/catalina/mbeans/BaseNamingResourceMBean.java
@@ -0,0 +1,78 @@
+/*
+ * 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 javax.management.Attribute;
+import javax.management.RuntimeOperationsException;
+
+import org.apache.juli.logging.Log;
+import org.apache.tomcat.util.res.StringManager;
+
+/**
+ * Abstract base class for Naming Resource MBeans.
+ *
+ * @param <T> the type of the managed resource
+ */
+public abstract class BaseNamingResourceMBean<T> extends BaseCatalinaMBean<T> {
+
+ private static final StringManager sm =
StringManager.getManager(BaseNamingResourceMBean.class);
+
+
+ /**
+ * Validate the provided attribute. Checks include:
+ * <ul>
+ * <li>attribute is not null</li>
+ * <li>attribute name is not null</li>
+ * <li>attribute name is not "name"</li>
+ * </ul>
+ *
+ * @param attribute The attribute to validate
+ *
+ * @return {@code true} if the attribute is valid, {@code false} if the
caller should stop further processing.
+ */
+ protected boolean validateAttribute(Attribute attribute) {
+ // Validate the input parameters
+ if (attribute == null) {
+ throw new RuntimeOperationsException(new
IllegalArgumentException(sm.getString("mBean.nullAttribute")),
+ sm.getString("mBean.nullAttribute"));
+ }
+
+ String name = attribute.getName();
+ if (name == null) {
+ throw new RuntimeOperationsException(new
IllegalArgumentException(sm.getString("mBean.nullName")),
+ sm.getString("mBean.nullName"));
+ }
+
+ if ("name".equals(name)) {
+ // Updating the name actually needs removing and adding back the
+ // component under the new name. Ignore the change, as the other
+ // naming resource MBeans do.
+ getLog().info(sm.getString("mBean.nameChange"));
+ return false;
+ }
+
+ return true;
+ }
+
+
+ /**
+ * Enables sub-classes to provide the correct logger for any log messages.
+ *
+ * @return The logger to use
+ */
+ protected abstract Log getLog();
+}
diff --git a/java/org/apache/catalina/mbeans/ConnectorMBean.java
b/java/org/apache/catalina/mbeans/ConnectorMBean.java
index 6205bd8148..8145a0f28b 100644
--- a/java/org/apache/catalina/mbeans/ConnectorMBean.java
+++ b/java/org/apache/catalina/mbeans/ConnectorMBean.java
@@ -27,8 +27,7 @@ import org.apache.tomcat.util.IntrospectionUtils;
import org.apache.tomcat.util.res.StringManager;
/**
- * A <strong>ModelMBean</strong> implementation for the
<code>org.apache.catalina.connector.Connector</code>
- * component.
+ * A <strong>ModelMBean</strong> implementation for the
<code>org.apache.catalina.connector.Connector</code> component.
*/
public class ConnectorMBean extends ClassNameMBean<Connector> {
diff --git a/java/org/apache/catalina/mbeans/ContainerMBean.java
b/java/org/apache/catalina/mbeans/ContainerMBean.java
index 3466bb0a7e..aaf4c46b51 100644
--- a/java/org/apache/catalina/mbeans/ContainerMBean.java
+++ b/java/org/apache/catalina/mbeans/ContainerMBean.java
@@ -37,8 +37,8 @@ import org.apache.catalina.startup.ContextConfig;
import org.apache.catalina.startup.HostConfig;
/**
- * MBean wrapper for ContainerBase instances, providing JMX management
operations for child containers,
- * valves, and lifecycle listeners.
+ * MBean wrapper for ContainerBase instances, providing JMX management
operations for child containers, valves, and
+ * lifecycle listeners.
*/
public class ContainerMBean extends BaseCatalinaMBean<ContainerBase> {
/**
@@ -115,8 +115,7 @@ public class ContainerMBean extends
BaseCatalinaMBean<ContainerBase> {
*
* @param valveType ClassName of the valve to be added
*
- * @return the MBean name of the new valve, or null if the valve was not
- * added or has no registered MBean
+ * @return the MBean name of the new valve, or null if the valve was not
added or has no registered MBean
*
* @throws MBeanException if adding the valve failed
*/
diff --git a/java/org/apache/catalina/mbeans/ContextEnvironmentMBean.java
b/java/org/apache/catalina/mbeans/ContextEnvironmentMBean.java
index 02387a43dc..09c2372549 100644
--- a/java/org/apache/catalina/mbeans/ContextEnvironmentMBean.java
+++ b/java/org/apache/catalina/mbeans/ContextEnvironmentMBean.java
@@ -20,10 +20,10 @@ import javax.management.Attribute;
import javax.management.AttributeNotFoundException;
import javax.management.MBeanException;
import javax.management.ReflectionException;
-import javax.management.RuntimeOperationsException;
import org.apache.juli.logging.Log;
import org.apache.juli.logging.LogFactory;
+import org.apache.tomcat.util.ExceptionUtils;
import org.apache.tomcat.util.descriptor.web.ContextEnvironment;
import org.apache.tomcat.util.descriptor.web.NamingResources;
import org.apache.tomcat.util.res.StringManager;
@@ -32,7 +32,11 @@ import org.apache.tomcat.util.res.StringManager;
* A <strong>ModelMBean</strong> implementation for the
* <code>org.apache.tomcat.util.descriptor.web.ContextEnvironment</code>
component.
*/
-public class ContextEnvironmentMBean extends
BaseCatalinaMBean<ContextEnvironment> {
+public class ContextEnvironmentMBean extends
BaseNamingResourceMBean<ContextEnvironment> {
+
+ private static final StringManager sm =
StringManager.getManager(ContextEnvironmentMBean.class);
+ private static final Log log =
LogFactory.getLog(ContextEnvironmentMBean.class);
+
/**
* Default constructor for ContextEnvironmentMBean.
@@ -40,31 +44,31 @@ public class ContextEnvironmentMBean extends
BaseCatalinaMBean<ContextEnvironmen
public ContextEnvironmentMBean() {
}
- private static final Log log =
LogFactory.getLog(ContextEnvironmentMBean.class);
- private static final StringManager sm =
StringManager.getManager(ContextEnvironmentMBean.class);
@Override
public void setAttribute(Attribute attribute)
throws AttributeNotFoundException, MBeanException,
ReflectionException {
- if (attribute == null) {
- throw new RuntimeOperationsException(new
IllegalArgumentException(sm.getString("mBean.nullAttribute")),
- sm.getString("mBean.nullAttribute"));
+ if (!validateAttribute(attribute)) {
+ return;
}
- ContextEnvironment ce = doGetManagedResource();
+ String name = attribute.getName();
+ Object value = attribute.getValue();
- if ("name".equals(attribute.getName())) {
- // Updating the name actually needs removing and adding back the
- // component under the new name. Ignore the change, as the other
- // naming resource MBeans do.
- log.info(sm.getString("mBean.nameChange"));
- return;
- }
+ ContextEnvironment ce = doGetManagedResource();
String oldType = ce.getType();
String oldValue = ce.getValue();
String oldLookup = ce.getLookupName();
+ String oldDescription = ce.getDescription();
+ boolean oldOverride = ce.getOverride();
+
+ // Entries with injection targets but no value are effectively ignored
+ if (ce.getInjectionTargets() != null &&
!ce.getInjectionTargets().isEmpty() && "value".equals(name) &&
+ (value == null || (value instanceof String s && s.isEmpty())))
{
+ throw new
IllegalArgumentException(sm.getString("contextEnvironment.ignore.injectionNoValue"));
+ }
super.setAttribute(attribute);
@@ -75,16 +79,30 @@ public class ContextEnvironmentMBean extends
BaseCatalinaMBean<ContextEnvironmen
try {
nr.removeEnvironment(ce.getName());
nr.addEnvironment(ce);
- } catch (IllegalArgumentException iae) {
+ } catch (Throwable t) {
+ ExceptionUtils.handleThrowable(t);
// The change is not acceptable. Restore the previous state
// before passing the failure to the caller, so the entry is
// not lost.
ce.setType(oldType);
ce.setValue(oldValue);
ce.setLookupName(oldLookup);
- nr.addEnvironment(ce);
- throw iae;
+ ce.setDescription(oldDescription);
+ ce.setOverride(oldOverride);
+ try {
+ nr.addEnvironment(ce);
+ } catch (Throwable t1) {
+ ExceptionUtils.handleThrowable(t1);
+ t.addSuppressed(t1);
+ }
+ throw t;
}
}
}
+
+
+ @Override
+ protected Log getLog() {
+ return log;
+ }
}
diff --git a/java/org/apache/catalina/mbeans/ContextMBean.java
b/java/org/apache/catalina/mbeans/ContextMBean.java
index 23c27e1437..825a7ea552 100644
--- a/java/org/apache/catalina/mbeans/ContextMBean.java
+++ b/java/org/apache/catalina/mbeans/ContextMBean.java
@@ -26,8 +26,7 @@ import org.apache.tomcat.util.descriptor.web.FilterMap;
import org.apache.tomcat.util.descriptor.web.SecurityConstraint;
/**
- * A <strong>ModelMBean</strong> implementation for the
- * <code>org.apache.catalina.Context</code> component.
+ * A <strong>ModelMBean</strong> implementation for the
<code>org.apache.catalina.Context</code> component.
*/
public class ContextMBean extends BaseCatalinaMBean<Context> {
diff --git a/java/org/apache/catalina/mbeans/ContextResourceLinkMBean.java
b/java/org/apache/catalina/mbeans/ContextResourceLinkMBean.java
index ee438d127e..743d58e8df 100644
--- a/java/org/apache/catalina/mbeans/ContextResourceLinkMBean.java
+++ b/java/org/apache/catalina/mbeans/ContextResourceLinkMBean.java
@@ -32,7 +32,11 @@ import org.apache.tomcat.util.res.StringManager;
* A <strong>ModelMBean</strong> implementation for the
* <code>org.apache.tomcat.util.descriptor.web.ContextResourceLink</code>
component.
*/
-public class ContextResourceLinkMBean extends
BaseCatalinaMBean<ContextResourceLink> {
+public class ContextResourceLinkMBean extends
BaseNamingResourceMBean<ContextResourceLink> {
+
+ private static final Log log =
LogFactory.getLog(ContextResourceLinkMBean.class);
+ private static final StringManager sm =
StringManager.getManager(ContextResourceLinkMBean.class);
+
/**
* Default constructor for ContextResourceLinkMBean.
@@ -40,8 +44,6 @@ public class ContextResourceLinkMBean extends
BaseCatalinaMBean<ContextResourceL
public ContextResourceLinkMBean() {
}
- private static final Log log =
LogFactory.getLog(ContextResourceLinkMBean.class);
- private static final StringManager sm =
StringManager.getManager(ContextResourceLinkMBean.class);
@Override
public Object getAttribute(String name) throws AttributeNotFoundException,
MBeanException, ReflectionException {
@@ -80,26 +82,18 @@ public class ContextResourceLinkMBean extends
BaseCatalinaMBean<ContextResourceL
public void setAttribute(Attribute attribute)
throws AttributeNotFoundException, MBeanException,
ReflectionException {
- // Validate the input parameters
- if (attribute == null) {
- throw new RuntimeOperationsException(new
IllegalArgumentException(sm.getString("mBean.nullAttribute")),
- sm.getString("mBean.nullAttribute"));
+ if (!validateAttribute(attribute)) {
+ return;
}
String name = attribute.getName();
Object value = attribute.getValue();
- if (name == null) {
- throw new RuntimeOperationsException(new
IllegalArgumentException(sm.getString("mBean.nullName")),
- sm.getString("mBean.nullName"));
- }
ContextResourceLink crl = doGetManagedResource();
switch (name) {
case "global" -> crl.setGlobal((String) value);
case "description" -> crl.setDescription((String) value);
- // Updating the name actually needs removing and adding back the
component under the new name
- case "name" -> log.info(sm.getString("mBean.nameChange"));
case "type" -> crl.setType((String) value);
default -> crl.setProperty(name, value == null ? null :
value.toString());
}
@@ -112,4 +106,10 @@ public class ContextResourceLinkMBean extends
BaseCatalinaMBean<ContextResourceL
nr.addResourceLink(crl);
}
}
+
+
+ @Override
+ protected Log getLog() {
+ return log;
+ }
}
diff --git a/java/org/apache/catalina/mbeans/ContextResourceMBean.java
b/java/org/apache/catalina/mbeans/ContextResourceMBean.java
index 669bc7726e..9919793e2c 100644
--- a/java/org/apache/catalina/mbeans/ContextResourceMBean.java
+++ b/java/org/apache/catalina/mbeans/ContextResourceMBean.java
@@ -24,6 +24,7 @@ import javax.management.RuntimeOperationsException;
import org.apache.juli.logging.Log;
import org.apache.juli.logging.LogFactory;
+import org.apache.tomcat.util.ExceptionUtils;
import org.apache.tomcat.util.descriptor.web.ContextResource;
import org.apache.tomcat.util.descriptor.web.NamingResources;
import org.apache.tomcat.util.res.StringManager;
@@ -32,7 +33,11 @@ import org.apache.tomcat.util.res.StringManager;
* A <strong>ModelMBean</strong> implementation for the
* <code>org.apache.tomcat.util.descriptor.web.ContextResource</code>
component.
*/
-public class ContextResourceMBean extends BaseCatalinaMBean<ContextResource> {
+public class ContextResourceMBean extends
BaseNamingResourceMBean<ContextResource> {
+
+ private static final Log log =
LogFactory.getLog(ContextResourceMBean.class);
+ private static final StringManager sm =
StringManager.getManager(ContextResourceMBean.class);
+
/**
* Default constructor for ContextResourceMBean.
@@ -40,8 +45,6 @@ public class ContextResourceMBean extends
BaseCatalinaMBean<ContextResource> {
public ContextResourceMBean() {
}
- private static final Log log =
LogFactory.getLog(ContextResourceMBean.class);
- private static final StringManager sm =
StringManager.getManager(ContextResourceMBean.class);
@Override
public Object getAttribute(String name) throws AttributeNotFoundException,
MBeanException, ReflectionException {
@@ -82,27 +85,19 @@ public class ContextResourceMBean extends
BaseCatalinaMBean<ContextResource> {
public void setAttribute(Attribute attribute)
throws AttributeNotFoundException, MBeanException,
ReflectionException {
- // Validate the input parameters
- if (attribute == null) {
- throw new RuntimeOperationsException(new
IllegalArgumentException(sm.getString("mBean.nullAttribute")),
- sm.getString("mBean.nullAttribute"));
+ if (!validateAttribute(attribute)) {
+ return;
}
+
String name = attribute.getName();
Object value = attribute.getValue();
- if (name == null) {
- throw new RuntimeOperationsException(new
IllegalArgumentException(sm.getString("mBean.nullName")),
- sm.getString("mBean.nullName"));
- }
ContextResource cr = doGetManagedResource();
-
String oldType = cr.getType();
switch (name) {
case "auth" -> cr.setAuth((String) value);
case "description" -> cr.setDescription((String) value);
- // Updating the name actually needs removing and adding back the
component under the new name
- case "name" -> log.info(sm.getString("mBean.nameChange"));
case "scope" -> cr.setScope((String) value);
case "type" -> cr.setType((String) value);
default -> cr.setProperty(name, value == null ? null :
value.toString());
@@ -115,14 +110,26 @@ public class ContextResourceMBean extends
BaseCatalinaMBean<ContextResource> {
try {
nr.removeResource(cr.getName());
nr.addResource(cr);
- } catch (IllegalArgumentException iae) {
+ } catch (Throwable t) {
+ ExceptionUtils.handleThrowable(t);
// The change is not acceptable. Restore the previous type
// before passing the failure to the caller, so the entry is
// not lost.
cr.setType(oldType);
- nr.addResource(cr);
- throw iae;
+ try {
+ nr.addResource(cr);
+ } catch (Throwable t1) {
+ ExceptionUtils.handleThrowable(t1);
+ t.addSuppressed(t1);
+ }
+ throw t;
}
}
}
+
+
+ @Override
+ protected Log getLog() {
+ return log;
+ }
}
diff --git a/java/org/apache/catalina/mbeans/DataSourceUserDatabaseMBean.java
b/java/org/apache/catalina/mbeans/DataSourceUserDatabaseMBean.java
index 71e2d582da..e5ed2f16d4 100644
--- a/java/org/apache/catalina/mbeans/DataSourceUserDatabaseMBean.java
+++ b/java/org/apache/catalina/mbeans/DataSourceUserDatabaseMBean.java
@@ -115,7 +115,7 @@ public class DataSourceUserDatabaseMBean extends
BaseModelMBean {
/**
* Create a new Group and return the corresponding name.
*
- * @param groupname Group name of the new group
+ * @param groupname Group name of the new group
* @param description Description of the new group
*
* @return the new group name
diff --git a/java/org/apache/catalina/mbeans/LocalStrings.properties
b/java/org/apache/catalina/mbeans/LocalStrings.properties
index 9de716f0da..99697e075b 100644
--- a/java/org/apache/catalina/mbeans/LocalStrings.properties
+++ b/java/org/apache/catalina/mbeans/LocalStrings.properties
@@ -13,6 +13,8 @@
# See the License for the specific language governing permissions and
# limitations under the License.
+contextEnvironment.ignore.injectionNoValue=Updates that set the value to null
or empty are ignored if there are injection targets defined
+
globalResources.create=Creating MBeans for Global JNDI Resources in Context
[{0}]
globalResources.createError=Exception processing global JNDI Resources
globalResources.createError.operation=Operation not supported error creating
MBeans
diff --git a/java/org/apache/catalina/mbeans/MBeanDumper.java
b/java/org/apache/catalina/mbeans/MBeanDumper.java
index 3230afffd9..bb213677c3 100644
--- a/java/org/apache/catalina/mbeans/MBeanDumper.java
+++ b/java/org/apache/catalina/mbeans/MBeanDumper.java
@@ -180,6 +180,7 @@ public class MBeanDumper {
* Escape a string value for display.
*
* @param value the value to escape
+ *
* @return the escaped value
*/
public static String escape(String value) {
diff --git a/java/org/apache/catalina/mbeans/MBeanUtils.java
b/java/org/apache/catalina/mbeans/MBeanUtils.java
index 9ef621e758..4f6a9f563c 100644
--- a/java/org/apache/catalina/mbeans/MBeanUtils.java
+++ b/java/org/apache/catalina/mbeans/MBeanUtils.java
@@ -489,8 +489,8 @@ public class MBeanUtils {
*/
static ObjectName createObjectName(String domain, User user) throws
MalformedObjectNameException {
- return new ObjectName(domain + ":type=User,username=" +
ObjectName.quote(user.getUsername()) +
- ",database=" +
ObjectName.quote(user.getUserDatabase().getId()));
+ return new ObjectName(domain + ":type=User,username=" +
ObjectName.quote(user.getUsername()) + ",database=" +
+ ObjectName.quote(user.getUserDatabase().getId()));
}
diff --git a/java/org/apache/catalina/mbeans/ServiceMBean.java
b/java/org/apache/catalina/mbeans/ServiceMBean.java
index 4fdff6a942..10172b9bba 100644
--- a/java/org/apache/catalina/mbeans/ServiceMBean.java
+++ b/java/org/apache/catalina/mbeans/ServiceMBean.java
@@ -23,8 +23,8 @@ import org.apache.catalina.Service;
import org.apache.catalina.connector.Connector;
/**
- * JMX MBean wrapper for a {@link Service} instance. Provides operations to
- * add connectors and executors, and to query connectors and executors.
+ * JMX MBean wrapper for a {@link Service} instance. Provides operations to
add connectors and executors, and to query
+ * connectors and executors.
*/
public class ServiceMBean extends BaseCatalinaMBean<Service> {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
