8359809: AttributeList, RoleList and UnresolvedRoleList should never accept other types of Object

Reviewed-by: sspitsyn

Author: kevinjwalls

src/java.management/share/classes/javax/management/AttributeList.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1999, 2024, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1999, 2025, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -37,13 +37,9 @@
  * {@link MBeanServerConnection#setAttributes setAttributes} methods of
  * {@link MBeanServer} and {@link MBeanServerConnection}.</p>
  *
- * <p id="type-safe">For compatibility reasons, it is possible, though
- * highly discouraged, to add objects to an {@code AttributeList} that are
- * not instances of {@code Attribute}.  However, an {@code AttributeList}
- * can be made <em>type-safe</em>, which means that an attempt to add
- * an object that is not an {@code Attribute} will produce an {@code
- * IllegalArgumentException}.  An {@code AttributeList} becomes type-safe
- * when the method {@link #asList()} is called on it.</p>
+ * <p>It is not permitted to add objects to an {@code AttributeList} that are
+ * not instances of {@code Attribute}.  This will produce an {@code IllegalArgumentException}
+ * when calling methods in this class, or when using {@code listIterator} and {@code add} or {@code set}.</p>
  *
  * @since 1.5
  */
@@ -64,9 +60,6 @@ the asList() method so you can write
 */
 public class AttributeList extends ArrayList<Object> {
 
-    private transient volatile boolean typeSafe;
-    private transient volatile boolean tainted;
-
     /* Serial version */
     private static final long serialVersionUID = -4077085769279709076L;
 
@@ -145,24 +138,14 @@ public AttributeList(List<Attribute> list) {
      * @return a {@code List<Attribute>} whose contents
      * reflect the contents of this {@code AttributeList}.
      *
-     * <p>If this method has ever been called on a given
-     * {@code AttributeList} instance, a subsequent attempt to add
-     * an object to that instance which is not an {@code Attribute}
-     * will fail with an {@code IllegalArgumentException}. For compatibility
-     * reasons, an {@code AttributeList} on which this method has never
-     * been called does allow objects other than {@code Attribute}s to
-     * be added.</p>
-     *
      * @throws IllegalArgumentException if this {@code AttributeList} contains
      * an element that is not an {@code Attribute}.
      *
      * @since 1.6
      */
     @SuppressWarnings("unchecked")
     public List<Attribute> asList() {
-        typeSafe = true;
-        if (tainted)
-            adding((Collection<?>) this);  // will throw IllegalArgumentException
+        adding((Collection<?>) this);
         return (List<Attribute>) (List<?>) this;
     }
 
@@ -257,15 +240,12 @@ public boolean addAll(int index, AttributeList list)  {
 
     /*
      * Override all of the methods from ArrayList<Object> that might add
-     * a non-Attribute to the List, and disallow that if asList has ever
-     * been called on this instance.
+     * a non-Attribute to the List, and disallow.
      */
 
     /**
      * {@inheritDoc}
-     * @throws IllegalArgumentException if this {@code AttributeList} is
-     * <a href="#type-safe">type-safe</a> and {@code element} is not an
-     * {@code Attribute}.
+     * @throws IllegalArgumentException if {@code element} is not an {@code Attribute}.
      */
     @Override
     public boolean add(Object element) {
@@ -275,9 +255,7 @@ public boolean add(Object element) {
 
     /**
      * {@inheritDoc}
-     * @throws IllegalArgumentException if this {@code AttributeList} is
-     * <a href="#type-safe">type-safe</a> and {@code element} is not an
-     * {@code Attribute}.
+     * @throws IllegalArgumentException if {@code element} is not an {@code Attribute}.
      */
     @Override
     public void add(int index, Object element) {
@@ -287,9 +265,7 @@ public void add(int index, Object element) {
 
     /**
      * {@inheritDoc}
-     * @throws IllegalArgumentException if this {@code AttributeList} is
-     * <a href="#type-safe">type-safe</a> and {@code c} contains an
-     * element that is not an {@code Attribute}.
+     * @throws IllegalArgumentException if {@code c} contains an element that is not an {@code Attribute}.
      */
     @Override
     public boolean addAll(Collection<?> c) {
@@ -299,9 +275,7 @@ public boolean addAll(Collection<?> c) {
 
     /**
      * {@inheritDoc}
-     * @throws IllegalArgumentException if this {@code AttributeList} is
-     * <a href="#type-safe">type-safe</a> and {@code c} contains an
-     * element that is not an {@code Attribute}.
+     * @throws IllegalArgumentException if {@code c} contains an element that is not an {@code Attribute}.
      */
     @Override
     public boolean addAll(int index, Collection<?> c) {
@@ -311,9 +285,7 @@ public boolean addAll(int index, Collection<?> c) {
 
     /**
      * {@inheritDoc}
-     * @throws IllegalArgumentException if this {@code AttributeList} is
-     * <a href="#type-safe">type-safe</a> and {@code element} is not an
-     * {@code Attribute}.
+     * @throws IllegalArgumentException if {@code element} is not an {@code Attribute}.
      */
     @Override
     public Object set(int index, Object element) {
@@ -324,10 +296,7 @@ public Object set(int index, Object element) {
     private void adding(Object x) {
         if (x == null || x instanceof Attribute)
             return;
-        if (typeSafe)
-            throw new IllegalArgumentException("Not an Attribute: " + x);
-        else
-            tainted = true;
+        throw new IllegalArgumentException("Not an Attribute: " + x);
     }
 
     private void adding(Collection<?> c) {

src/java.management/share/classes/javax/management/relation/RoleList.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2000, 2024, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2000, 2025, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -35,6 +35,10 @@
  * parameter when creating a relation, and when trying to set several roles in
  * a relation (via 'setRoles()' method). It is returned as part of a
  * RoleResult, to provide roles successfully retrieved.
+
+ * <p>It is not permitted to add objects to a {@code RoleList} that are
+ * not instances of {@code Role}.  This will produce an {@code IllegalArgumentException}
+ * when calling methods in this class, or when using {@code listIterator} and {@code add} or {@code set}.</p>
  *
  * @since 1.5
  */
@@ -55,9 +59,6 @@ the asList() method so you can write
 */
 public class RoleList extends ArrayList<Object> {
 
-    private transient boolean typeSafe;
-    private transient boolean tainted;
-
     /* Serial version */
     private static final long serialVersionUID = 5568344346499649313L;
 
@@ -121,25 +122,13 @@ public RoleList(List<Role> list) throws IllegalArgumentException {
      * @return a {@code List<Role>} whose contents
      * reflect the contents of this {@code RoleList}.
      *
-     * <p>If this method has ever been called on a given
-     * {@code RoleList} instance, a subsequent attempt to add
-     * an object to that instance which is not a {@code Role}
-     * will fail with an {@code IllegalArgumentException}. For compatibility
-     * reasons, a {@code RoleList} on which this method has never
-     * been called does allow objects other than {@code Role}s to
-     * be added.</p>
-     *
      * @throws IllegalArgumentException if this {@code RoleList} contains
      * an element that is not a {@code Role}.
      *
      * @since 1.6
      */
     public List<Role> asList() {
-        if (!typeSafe) {
-            if (tainted)
-                checkTypeSafe(this);
-            typeSafe = true;
-        }
+        checkTypeSafe(this);
         return Util.cast(this);
     }
 
@@ -158,8 +147,7 @@ public void add(Role role)
         throws IllegalArgumentException {
 
         if (role == null) {
-            String excMsg = "Invalid parameter";
-            throw new IllegalArgumentException(excMsg);
+            throw new IllegalArgumentException("Invalid parameter");
         }
         super.add(role);
     }
@@ -183,10 +171,8 @@ public void add(int index,
                IndexOutOfBoundsException {
 
         if (role == null) {
-            String excMsg = "Invalid parameter";
-            throw new IllegalArgumentException(excMsg);
+            throw new IllegalArgumentException("Invalid parameter");
         }
-
         super.add(index, role);
     }
 
@@ -208,11 +194,8 @@ public void set(int index,
                 IndexOutOfBoundsException {
 
         if (role == null) {
-            // Revisit [cebro] Localize message
-            String excMsg = "Invalid parameter.";
-            throw new IllegalArgumentException(excMsg);
+            throw new IllegalArgumentException("Invalid parameter");
         }
-
         super.set(index, role);
      }
 
@@ -236,7 +219,6 @@ public boolean addAll(RoleList roleList)
         if (roleList == null) {
             return true;
         }
-
         return (super.addAll(roleList));
     }
 
@@ -263,9 +245,7 @@ public boolean addAll(int index,
                IndexOutOfBoundsException {
 
         if (roleList == null) {
-            // Revisit [cebro] Localize message
-            String excMsg = "Invalid parameter.";
-            throw new IllegalArgumentException(excMsg);
+            throw new IllegalArgumentException("Invalid parameter");
         }
 
         return (super.addAll(index, roleList));
@@ -277,48 +257,53 @@ public boolean addAll(int index,
      * been called on this instance.
      */
 
+    /**
+     * {@inheritDoc}
+     * @throws IllegalArgumentException if {@code o} is not a {@code Role}.
+     */
     @Override
     public boolean add(Object o) {
-        if (!tainted)
-            tainted = isTainted(o);
-        if (typeSafe)
-            checkTypeSafe(o);
+        checkTypeSafe(o);
         return super.add(o);
     }
 
+    /**
+     * {@inheritDoc}
+     * @throws IllegalArgumentException if {@code element} is not a {@code Role}.
+     */
     @Override
     public void add(int index, Object element) {
-        if (!tainted)
-            tainted = isTainted(element);
-        if (typeSafe)
-            checkTypeSafe(element);
+        checkTypeSafe(element);
         super.add(index, element);
     }
 
+    /**
+     * {@inheritDoc}
+     * @throws IllegalArgumentException if {@code c} contains a member that is not a {@code Role}.
+     */
     @Override
     public boolean addAll(Collection<?> c) {
-        if (!tainted)
-            tainted = isTainted(c);
-        if (typeSafe)
-            checkTypeSafe(c);
+        checkTypeSafe(c);
         return super.addAll(c);
     }
 
+    /**
+     * {@inheritDoc}
+     * @throws IllegalArgumentException if {@code c} contains a member that is not a {@code Role}.
+     */
     @Override
     public boolean addAll(int index, Collection<?> c) {
-        if (!tainted)
-            tainted = isTainted(c);
-        if (typeSafe)
-            checkTypeSafe(c);
+        checkTypeSafe(c);
         return super.addAll(index, c);
     }
 
+    /**
+     * {@inheritDoc}
+     * @throws IllegalArgumentException if {@code element} is not a {@code Role}.
+     */
     @Override
     public Object set(int index, Object element) {
-        if (!tainted)
-            tainted = isTainted(element);
-        if (typeSafe)
-            checkTypeSafe(element);
+        checkTypeSafe(element);
         return super.set(index, element);
     }
 
@@ -345,28 +330,4 @@ private static void checkTypeSafe(Collection<?> c) {
             throw new IllegalArgumentException(e);
         }
     }
-
-    /**
-     * Returns true if o is a non-Role object.
-     */
-    private static boolean isTainted(Object o) {
-        try {
-            checkTypeSafe(o);
-        } catch (IllegalArgumentException e) {
-            return true;
-        }
-        return false;
-    }
-
-    /**
-     * Returns true if c contains any non-Role objects.
-     */
-    private static boolean isTainted(Collection<?> c) {
-        try {
-            checkTypeSafe(c);
-        } catch (IllegalArgumentException e) {
-            return true;
-        }
-        return false;
-    }
 }