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 99f10cfe29e597eba69ba8280cd184796a920acf
Author: opencode <[email protected]>
AuthorDate: Thu Oct 15 10:18:58 2026 +0200

    Report the actual old value in attribute change notifications
    
    BaseModelMBean.setAttribute() sent every AttributeChangeNotification
    with a null old value since the code fetching the previous value had
    been commented out (a leftover from the removed attribute cache),
    which breaks the AttributeChangeNotification contract for listeners
    that rely on the old value.
    
    Retrieve the previous value via getAttribute() before invoking the
    setter. Attributes without a getter, or whose getter fails, keep the
    unknown old value null so write-only or failing-getter sets behave
    exactly as before. As a consequence, a set to null is now reported
    (old and new values both null is still treated as no change).
    
    Add a unit test covering the old and new values of the notification
    emitted by setAttribute().
---
 .../apache/tomcat/util/modeler/BaseModelMBean.java | 10 ++++-
 .../modeler/TestBaseModelMBeanNotifications.java   | 48 +++++++++++++++++++++-
 2 files changed, 54 insertions(+), 4 deletions(-)

diff --git a/java/org/apache/tomcat/util/modeler/BaseModelMBean.java 
b/java/org/apache/tomcat/util/modeler/BaseModelMBean.java
index d1f09b57ae..a3fa451082 100644
--- a/java/org/apache/tomcat/util/modeler/BaseModelMBean.java
+++ b/java/org/apache/tomcat/util/modeler/BaseModelMBean.java
@@ -354,9 +354,15 @@ public class BaseModelMBean implements DynamicMBean, 
MBeanRegistration, ModelMBe
                     sm.getString("baseModelMBean.nullAttributeName"));
         }
 
+        // Get the old value for the change notification below. The attribute
+        // may be write-only or the getter may fail, in which case the old
+        // value remains unknown (null).
         Object oldValue = null;
-        // if( getAttMap.get(name) != null )
-        // oldValue=getAttribute( name );
+        try {
+            oldValue = getAttribute(name);
+        } catch (Exception ignore) {
+            // Old value unknown - ignore
+        }
 
         Method m = managedBean.getSetter(name, this, resource);
 
diff --git 
a/test/org/apache/tomcat/util/modeler/TestBaseModelMBeanNotifications.java 
b/test/org/apache/tomcat/util/modeler/TestBaseModelMBeanNotifications.java
index 4d92b2c934..0d378aa7ab 100644
--- a/test/org/apache/tomcat/util/modeler/TestBaseModelMBeanNotifications.java
+++ b/test/org/apache/tomcat/util/modeler/TestBaseModelMBeanNotifications.java
@@ -33,15 +33,34 @@ public class TestBaseModelMBeanNotifications {
     private static final class TesterNotificationListener implements 
NotificationListener {
 
         private final List<String> received = new ArrayList<>();
+        private final List<AttributeChangeNotification> changes = new 
ArrayList<>();
 
         @Override
         public void handleNotification(Notification notification, Object 
handback) {
-            String name = notification instanceof AttributeChangeNotification ?
-                    ((AttributeChangeNotification) 
notification).getAttributeName() : notification.getType();
+            String name;
+            if (notification instanceof AttributeChangeNotification acn) {
+                changes.add(acn);
+                name = acn.getAttributeName();
+            } else {
+                name = notification.getType();
+            }
             received.add(name + ":" + handback);
         }
     }
 
+    public static class TesterResource {
+
+        private String value;
+
+        public String getValue() {
+            return value;
+        }
+
+        public void setValue(String value) {
+            this.value = value;
+        }
+    }
+
     private static void send(BaseModelMBean mbean, String attributeName) 
throws Exception {
         mbean.sendAttributeChangeNotification(new Attribute(attributeName, 
"old"),
                 new Attribute(attributeName, "new"));
@@ -126,6 +145,31 @@ public class TestBaseModelMBeanNotifications {
         Assert.assertEquals(List.of("A:hb"), listener2.received);
     }
 
+    @Test
+    public void testSetAttributeNotificationContainsOldValue() throws 
Exception {
+        ManagedBean managedBean = new ManagedBean();
+        managedBean.setName("Value");
+        managedBean.setType("Value");
+        AttributeInfo attributeInfo = new AttributeInfo();
+        attributeInfo.setName("value");
+        attributeInfo.setDescription("The value");
+        attributeInfo.setType("java.lang.String");
+        managedBean.addAttribute(attributeInfo);
+
+        TesterResource resource = new TesterResource();
+        resource.setValue("initial");
+        BaseModelMBean mbean = (BaseModelMBean) 
managedBean.createMBean(resource);
+
+        TesterNotificationListener listener = new TesterNotificationListener();
+        mbean.addAttributeChangeNotificationListener(listener, "value", "hb");
+        mbean.setAttribute(new Attribute("value", "updated"));
+
+        Assert.assertEquals(1, listener.changes.size());
+        AttributeChangeNotification change = listener.changes.get(0);
+        Assert.assertEquals("initial", change.getOldValue());
+        Assert.assertEquals("updated", change.getNewValue());
+    }
+
     @Test
     public void testNonAttributeFilterIsNotRemovedByName() throws Exception {
         BaseModelMBean mbean = new BaseModelMBean();


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to