diff --git a/src/java.management/share/classes/javax/management/AttributeList.java b/src/java.management/share/classes/javax/management/AttributeList.java index 932c4974244f3..c85b119f3a06c 100644 --- a/src/java.management/share/classes/javax/management/AttributeList.java +++ b/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}.

* - *

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 type-safe, 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.

+ *

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}.

* * @since 1.5 */ @@ -64,9 +60,6 @@ the asList() method so you can write */ public class AttributeList extends ArrayList { - private transient volatile boolean typeSafe; - private transient volatile boolean tainted; - /* Serial version */ private static final long serialVersionUID = -4077085769279709076L; @@ -145,14 +138,6 @@ public AttributeList(List list) { * @return a {@code List} whose contents * reflect the contents of this {@code AttributeList}. * - *

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.

- * * @throws IllegalArgumentException if this {@code AttributeList} contains * an element that is not an {@code Attribute}. * @@ -160,9 +145,7 @@ public AttributeList(List list) { */ @SuppressWarnings("unchecked") public List asList() { - typeSafe = true; - if (tainted) - adding((Collection) this); // will throw IllegalArgumentException + adding((Collection) this); return (List) (List) this; } @@ -257,15 +240,12 @@ public boolean addAll(int index, AttributeList list) { /* * Override all of the methods from ArrayList 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 - * type-safe 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 - * type-safe 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 - * type-safe 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 - * type-safe 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 - * type-safe 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) { diff --git a/src/java.management/share/classes/javax/management/relation/RoleList.java b/src/java.management/share/classes/javax/management/relation/RoleList.java index fbbc3df64364b..f16a441ad462b 100644 --- a/src/java.management/share/classes/javax/management/relation/RoleList.java +++ b/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. + + *

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}.

* * @since 1.5 */ @@ -55,9 +59,6 @@ the asList() method so you can write */ public class RoleList extends ArrayList { - private transient boolean typeSafe; - private transient boolean tainted; - /* Serial version */ private static final long serialVersionUID = 5568344346499649313L; @@ -121,25 +122,13 @@ public RoleList(List list) throws IllegalArgumentException { * @return a {@code List} whose contents * reflect the contents of this {@code RoleList}. * - *

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.

- * * @throws IllegalArgumentException if this {@code RoleList} contains * an element that is not a {@code Role}. * * @since 1.6 */ public List 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; - } } diff --git a/src/java.management/share/classes/javax/management/relation/RoleUnresolvedList.java b/src/java.management/share/classes/javax/management/relation/RoleUnresolvedList.java index dae506e5e5f2b..3d1e4223cbae8 100644 --- a/src/java.management/share/classes/javax/management/relation/RoleUnresolvedList.java +++ b/src/java.management/share/classes/javax/management/relation/RoleUnresolvedList.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 @@ * representing roles not retrieved from a relation due to a problem * encountered when trying to access (read or write) the roles. * + *

It is not permitted to add objects to a {@code RoleUnresolvedList} that are + * not instances of {@code RoleUnresolved}. This will produce an {@code IllegalArgumentException} + * when calling methods in this class, or when using {@code listIterator} and {@code add} or {@code set}.

+ * * @since 1.5 */ /* We cannot extend ArrayList because our legacy @@ -54,9 +58,6 @@ the asList() method so you can write */ public class RoleUnresolvedList extends ArrayList { - private transient boolean typeSafe; - private transient boolean tainted; - /* Serial version */ private static final long serialVersionUID = 4054902803091433324L; @@ -121,25 +122,13 @@ public RoleUnresolvedList(List list) * @return a {@code List} whose contents * reflect the contents of this {@code RoleUnresolvedList}. * - *

If this method has ever been called on a given - * {@code RoleUnresolvedList} instance, a subsequent attempt to add - * an object to that instance which is not a {@code RoleUnresolved} - * will fail with an {@code IllegalArgumentException}. For compatibility - * reasons, a {@code RoleUnresolvedList} on which this method has never - * been called does allow objects other than {@code RoleUnresolved}s to - * be added.

- * * @throws IllegalArgumentException if this {@code RoleUnresolvedList} * contains an element that is not a {@code RoleUnresolved}. * * @since 1.6 */ public List asList() { - if (!typeSafe) { - if (tainted) - checkTypeSafe(this); - typeSafe = true; - } + checkTypeSafe(this); return Util.cast(this); } @@ -158,8 +147,7 @@ public void add(RoleUnresolved role) throws IllegalArgumentException { if (role == null) { - String excMsg = "Invalid parameter"; - throw new IllegalArgumentException(excMsg); + throw new IllegalArgumentException("Invalid parameter"); } super.add(role); } @@ -184,10 +172,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); } @@ -210,10 +196,8 @@ public void set(int index, IndexOutOfBoundsException { if (role == null) { - String excMsg = "Invalid parameter"; - throw new IllegalArgumentException(excMsg); + throw new IllegalArgumentException("Invalid parameter"); } - super.set(index, role); } @@ -236,7 +220,6 @@ public boolean addAll(RoleUnresolvedList roleList) if (roleList == null) { return true; } - return (super.addAll(roleList)); } @@ -261,10 +244,8 @@ public boolean addAll(int index, IndexOutOfBoundsException { if (roleList == null) { - String excMsg = "Invalid parameter"; - throw new IllegalArgumentException(excMsg); + throw new IllegalArgumentException("Invalid parameter"); } - return (super.addAll(index, roleList)); } @@ -274,48 +255,53 @@ public boolean addAll(int index, * ever been called on this instance. */ + /** + * {@inheritDoc} + * @throws IllegalArgumentException if {@code o} is not a {@code RoleUnresolved}. + */ @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 RoleUnresolved}. + */ @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 RoleUnresolved}. + */ @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 RoleUnresolved}. + */ @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 RoleUnresolved}. + */ @Override public Object set(int index, Object element) { - if (!tainted) - tainted = isTainted(element); - if (typeSafe) - checkTypeSafe(element); + checkTypeSafe(element); return super.set(index, element); } @@ -342,28 +328,4 @@ private static void checkTypeSafe(Collection c) { throw new IllegalArgumentException(e); } } - - /** - * Returns true if o is a non-RoleUnresolved object. - */ - private static boolean isTainted(Object o) { - try { - checkTypeSafe(o); - } catch (IllegalArgumentException e) { - return true; - } - return false; - } - - /** - * Returns true if c contains any non-RoleUnresolved objects. - */ - private static boolean isTainted(Collection c) { - try { - checkTypeSafe(c); - } catch (IllegalArgumentException e) { - return true; - } - return false; - } } diff --git a/test/jdk/javax/management/MBeanServer/AttributeListTypeSafeTest.java b/test/jdk/javax/management/MBeanServer/AttributeListTypeSafeTest.java index 602e1f2ebd288..219632784f0a5 100644 --- a/test/jdk/javax/management/MBeanServer/AttributeListTypeSafeTest.java +++ b/test/jdk/javax/management/MBeanServer/AttributeListTypeSafeTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2008, 2015, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2008, 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 @@ -23,7 +23,7 @@ /* * @test - * @bug 6336968 + * @bug 6336968 8359809 * @summary Test adding non-Attribute values to an AttributeList. * @author Eamonn McManus */ @@ -38,28 +38,14 @@ public class AttributeListTypeSafeTest { private static String failure; public static void main(String[] args) throws Exception { - // Test calling asList after adding non-Attribute by various means + // Test adding non-Attribute by various means for (Op op : Op.values()) { AttributeList alist = new AttributeList(); - alist.add(new Attribute("foo", "bar")); - doOp(alist, op); - String what = "asList() after calling " + op + " with non-Attribute"; + alist.add(new Attribute("foo", "bar")); // Add actual Attribute + alist.add(null); + String what = "Using " + op + " with non-Attribute"; try { - List lista = alist.asList(); - fail(what + ": succeeded but should not have"); - } catch (IllegalArgumentException e) { - System.out.println("OK: " + what + ": got IllegalArgumentException"); - } - } - - // Test adding non-Attribute by various means after calling asList - for (Op op : Op.values()) { - AttributeList alist = new AttributeList(); - List lista = alist.asList(); - lista.add(new Attribute("foo", "bar")); - String what = op + " with non-Attribute after calling asList()"; - try { - doOp(alist, op); + doOp(alist, op); // Add some other non-Attribute, should fail fail(what + ": succeeded but should not have"); } catch (IllegalArgumentException e) { System.out.println("OK: " + what + ": got IllegalArgumentException"); diff --git a/test/jdk/javax/management/generified/ListTypeCheckTest.java b/test/jdk/javax/management/generified/ListTypeCheckTest.java index a28ffba92d407..c1e312d22a0ac 100644 --- a/test/jdk/javax/management/generified/ListTypeCheckTest.java +++ b/test/jdk/javax/management/generified/ListTypeCheckTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2005, 2015, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2005, 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 @@ -23,8 +23,8 @@ /* * @test - * @bug 6250772 - * @summary Test that *List objects are checked after asList is called. + * @bug 6250772 8359809 + * @summary Test that *List objects are checked * @author Eamonn McManus * * @run clean ListTypeCheckTest @@ -39,39 +39,45 @@ /* For compatibility reasons, the classes AttributeList, RoleList, * and RoleUnresolvedList all extend ArrayList even though - * logically they should extend ArrayList etc. They are - * all specified to have a method asList() with return type - * List etc, and to refuse to add any object other than - * an Attribute etc once this method has been called, but not before. + * logically they should extend ArrayList etc. + * + * Before JDK-8359809, their method asList() had to be called, to make + * the class refuse to add any object other than the intended type. */ public class ListTypeCheckTest { public static void main(String[] args) throws Exception { Class[] classes = { AttributeList.class, RoleList.class, RoleUnresolvedList.class, }; - for (Class c : classes) - test((Class) c); + Object[] objects = { + new Attribute("myAttr", "myVal"), new Role("myRole", new ArrayList()), + new RoleUnresolved("myRoleUnresolved", new ArrayList(), RoleStatus.NO_ROLE_WITH_NAME) + }; + for (int i = 0; i < classes.length; i++) { + test((Class) classes[i], objects[i]); + } } - private static void test(Class c) throws Exception { + private static void test(Class c, Object o) throws Exception { System.out.println("Testing " + c.getName()); ArrayList al = c.newInstance(); - test(al); + test(al, o); } - private static void test(ArrayList al) throws Exception { - test(al, true); + private static void test(ArrayList al, Object o) throws Exception { + test0(al, o); al.clear(); Method m = al.getClass().getMethod("asList"); - m.invoke(al); - test(al, false); + m.invoke(al); // Calling asList() does not change behaviour + test0(al, o); } - private static void test(ArrayList al, boolean allowsBogus) throws Exception { - for (int i = 0; i < 5; i++) { + private static void test0(ArrayList al, Object o) throws Exception { + for (int i = 0; i < 7; i++) { try { switch (i) { case 0: + // Add the wrong kind of element, will fail: al.add("yo"); break; case 1: @@ -86,14 +92,27 @@ private static void test(ArrayList al, boolean allowsBogus) throws Exception { case 4: al.set(0, "foo"); break; + case 5: + // Add the correct kind of element, so we can test ListIterator. + al.add(o); + ListIterator iter = al.listIterator(); + Object x = iter.next(); + iter.set("blah"); // Test "set", should fail like the others. + break; + case 6: + // Add the correct kind of element, so we can test ListIterator. + al.add(o); + ListIterator iter2 = al.listIterator(); + Object x2 = iter2.next(); + iter2.add("blah"); // Test "add", should fail like the others. + break; default: throw new Exception("test wrong"); } - if (!allowsBogus) - throw new Exception("op allowed but should fail"); + // All cases above should have caused an Exception: + throw new Exception("op " + i + " allowed but should fail on " + al.getClass()); } catch (IllegalArgumentException e) { - if (allowsBogus) - throw new Exception("got exception but should not", e); + System.out.println("op " + i + " got expected " + e + " on " + al.getClass()); } } }