6616825: JMX query returns no value in 1.0 compatibility mode - deserialization bug in readObject()

Reviewed-by: emcmanus

jdk/src/share/classes/javax/management/ObjectName.java

@@ -38,9 +38,6 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.Hashtable;
 import java.util.Map;
-import javax.management.MBeanServer;
-import javax.management.MalformedObjectNameException;
-import javax.management.QueryExp;
 
 /**
  * <p>Represents the object name of an MBean, or a pattern that can
@@ -1160,9 +1157,19 @@ public class ObjectName implements Comparable<ObjectName>, QueryExp {
             //
             //in.defaultReadObject();
             final ObjectInputStream.GetField fields = in.readFields();
+            String propListString =
+                    (String)fields.get("propertyListString", "");
+
+            // 6616825: take care of property patterns
+            final boolean propPattern =
+                    fields.get("propertyPattern" , false);
+            if (propPattern) {
+                propListString =
+                        (propListString.length()==0?"*":(propListString+",*"));
+            }
+
             cn = (String)fields.get("domain", "default")+
-                ":"+
-                (String)fields.get("propertyListString", "");
+                ":"+ propListString;
         } else {
             // Read an object serialized in the new serial form
             //
@@ -1796,6 +1803,7 @@ public class ObjectName implements Comparable<ObjectName>, QueryExp {
      * @return True if <code>object</code> is an ObjectName whose
      * canonical form is equal to that of this ObjectName.
      */
+    @Override
     public boolean equals(Object object)  {
 
         // same object case
@@ -1819,6 +1827,7 @@ public class ObjectName implements Comparable<ObjectName>, QueryExp {
      * Returns a hash code for this object name.
      *
      */
+    @Override
     public int hashCode() {
         return _canonicalName.hashCode();
     }

jdk/test/javax/management/ObjectName/SerialCompatTest.java

@@ -23,9 +23,9 @@
 
 /*
  * @test
- * @bug 6211220
+ * @bug 6211220 6616825
  * @summary Test that jmx.serial.form=1.0 works for ObjectName
- * @author Eamonn McManus
+ * @author Eamonn McManus, Daniel Fuchs
  * @run clean SerialCompatTest
  * @run build SerialCompatTest
  * @run main/othervm SerialCompatTest
@@ -36,19 +36,8 @@ import java.util.*;
 import javax.management.ObjectName;
 
 public class SerialCompatTest {
-    public static void main(String[] args) throws Exception {
-        System.setProperty("jmx.serial.form", "1.0");
 
-        /* Check that we really are in jmx.serial.form=1.0 mode.
-           The property is frozen the first time the ObjectName class
-           is referenced so checking that it is set to the correct
-           value now is not enough.  */
-        ObjectStreamClass osc = ObjectStreamClass.lookup(ObjectName.class);
-        if (osc.getFields().length != 6) {
-            throw new Exception("Not using old serial form: fields: " +
-                                Arrays.asList(osc.getFields()));
-            // new serial form has no fields, uses writeObject
-        }
+    public static void check6211220() throws Exception {
 
         ObjectName on = new ObjectName("a:b=c");
         ByteArrayOutputStream bos = new ByteArrayOutputStream();
@@ -62,56 +51,214 @@ public class SerialCompatTest {
 
         // if the bug is present, these will get NullPointerException
         for (int i = 0; i <= 11; i++) {
+            String msg = "6211220 case(" + i + ")";
+            try {
+                switch (i) {
+                    case 0:
+                        check(msg, on1.getDomain().equals("a"));
+                        break;
+                    case 1:
+                        check(msg, on1.getCanonicalName().equals("a:b=c"));
+                        break;
+                    case 2:
+                        check(msg, on1.getKeyPropertyListString()
+                                .equals("b=c"));
+                        break;
+                    case 3:
+                        check(msg, on1.getCanonicalKeyPropertyListString()
+                                .equals("b=c"));
+                        break;
+                    case 4:
+                        check(msg, on1.getKeyProperty("b").equals("c"));
+                        break;
+                    case 5:
+                        check(msg, on1.getKeyPropertyList()
+                                .equals(Collections.singletonMap("b", "c")));
+                        break;
+                    case 6:
+                        check(msg, !on1.isDomainPattern());
+                        break;
+                    case 7:
+                        check(msg, !on1.isPattern());
+                        break;
+                    case 8:
+                        check(msg, !on1.isPropertyPattern());
+                        break;
+                    case 9:
+                        check(msg, on1.equals(on));
+                        break;
+                    case 10:
+                        check(msg, on.equals(on1));
+                        break;
+                    case 11:
+                        check(msg, on1.apply(on));
+                        break;
+                    default:
+                        throw new Exception(msg + ": Test incorrect");
+                }
+            } catch (Exception e) {
+                System.out.println(msg + ": Test failed with exception:");
+                e.printStackTrace(System.out);
+                failed = true;
+            }
+        }
+
+        if (failed) {
+            throw new Exception("Some tests for 6211220 failed");
+        } else {
+            System.out.println("All tests for 6211220 passed");
+        }
+    }
+
+    static void checkName(String testname, ObjectName on)
+            throws Exception {
+        ByteArrayOutputStream bos = new ByteArrayOutputStream();
+        ObjectOutputStream oos = new ObjectOutputStream(bos);
+        oos.writeObject(on);
+        oos.close();
+        byte[] bytes = bos.toByteArray();
+        ByteArrayInputStream bis = new ByteArrayInputStream(bytes);
+        ObjectInputStream ois = new ObjectInputStream(bis);
+        ObjectName on1 = (ObjectName) ois.readObject();
+        // if the bug is present, these will get NullPointerException
+        for (int i = 0; i <= 11; i++) {
+            String msg = testname + " case(" + i + ")";
             try {
                 switch (i) {
-                case 0:
-                    check(on1.getDomain().equals("a")); break;
-                case 1:
-                    check(on1.getCanonicalName().equals("a:b=c")); break;
-                case 2:
-                    check(on1.getKeyPropertyListString().equals("b=c")); break;
-                case 3:
-                    check(on1.getCanonicalKeyPropertyListString().equals("b=c"));
-                    break;
-                case 4:
-                    check(on1.getKeyProperty("b").equals("c")); break;
-                case 5:
-                    check(on1.getKeyPropertyList()
-                          .equals(Collections.singletonMap("b", "c"))); break;
-                case 6:
-                    check(!on1.isDomainPattern()); break;
-                case 7:
-                    check(!on1.isPattern()); break;
-                case 8:
-                    check(!on1.isPropertyPattern()); break;
-                case 9:
-                    check(on1.equals(on)); break;
-                case 10:
-                    check(on.equals(on1)); break;
-                case 11:
-                    check(on1.apply(on)); break;
-                default:
-                    throw new Exception("Test incorrect: case: " + i);
+                    case 0:
+                        check(msg, on1.getDomain().equals(on.getDomain()));
+                        break;
+                    case 1:
+                        check(msg, on1.getCanonicalName().
+                                equals(on.getCanonicalName()));
+                        break;
+                    case 2:
+                        check(msg, on1.getKeyPropertyListString().
+                                equals(on.getKeyPropertyListString()));
+                        break;
+                    case 3:
+                        check(msg, on1.getCanonicalKeyPropertyListString().
+                                equals(on.getCanonicalKeyPropertyListString()));
+                        break;
+                    case 4:
+                        for (Object ko : on1.getKeyPropertyList().keySet()) {
+                            final String key = (String) ko;
+                            check(msg, on1.getKeyProperty(key).
+                                    equals(on.getKeyProperty(key)));
+                        }
+                        for (Object ko : on.getKeyPropertyList().keySet()) {
+                            final String key = (String) ko;
+                            check(msg, on1.getKeyProperty(key).
+                                    equals(on.getKeyProperty(key)));
+                        }
+                    case 5:
+                        check(msg, on1.getKeyPropertyList()
+                                .equals(on.getKeyPropertyList()));
+                        break;
+                    case 6:
+                        check(msg, on1.isDomainPattern()==on.isDomainPattern());
+                        break;
+                    case 7:
+                        check(msg, on1.isPattern() == on.isPattern());
+                        break;
+                    case 8:
+                        check(msg,
+                              on1.isPropertyPattern()==on.isPropertyPattern());
+                        break;
+                    case 9:
+                        check(msg, on1.equals(on));
+                        break;
+                    case 10:
+                        check(msg, on.equals(on1));
+                        break;
+                    case 11:
+                        if (!on.isPattern()) {
+                            check(msg, on1.apply(on));
+                        }
+                        break;
+                    default:
+                        throw new Exception("Test incorrect: case: " + i);
                 }
             } catch (Exception e) {
-                System.out.println("Test failed with exception:");
+                System.out.println("Test (" + i + ") failed with exception:");
                 e.printStackTrace(System.out);
                 failed = true;
             }
         }
 
-        if (failed)
+    }
+    private static String[] names6616825 = {
+        "a:b=c", "a:b=c,*", "*:*", ":*", ":b=c", ":b=c,*",
+        "a:*,b=c", ":*", ":*,b=c", "*x?:k=\"x\\*z\"", "*x?:k=\"x\\*z\",*",
+        "*x?:*,k=\"x\\*z\"", "*x?:k=\"x\\*z\",*,b=c"
+    };
+
+    static void check6616825() throws Exception {
+        System.out.println("Testing 616825");
+        for (String n : names6616825) {
+            final ObjectName on;
+            try {
+                on = new ObjectName(n);
+            } catch (Exception x) {
+                failed = true;
+                System.out.println("Unexpected failure for 6616825 [" + n +
+                        "]: " + x);
+                x.printStackTrace(System.out);
+                continue;
+            }
+            try {
+                checkName("616825 " + n, on);
+            } catch (Exception x) {
+                failed = true;
+                System.out.println("6616825 failed for [" + n + "]: " + x);
+                x.printStackTrace(System.out);
+            }
+        }
+
+        if (failed) {
+            throw new Exception("Some tests for 6616825 failed");
+        } else {
+            System.out.println("All tests for 6616825 passed");
+        }
+    }
+
+    public static void main(String[] args) throws Exception {
+        System.setProperty("jmx.serial.form", "1.0");
+
+        /* Check that we really are in jmx.serial.form=1.0 mode.
+        The property is frozen the first time the ObjectName class
+        is referenced so checking that it is set to the correct
+        value now is not enough.  */
+        ObjectStreamClass osc = ObjectStreamClass.lookup(ObjectName.class);
+        if (osc.getFields().length != 6) {
+            throw new Exception("Not using old serial form: fields: " +
+                    Arrays.asList(osc.getFields()));
+        // new serial form has no fields, uses writeObject
+        }
+
+        try {
+            check6211220();
+        } catch (Exception x) {
+            System.err.println(x.getMessage());
+        }
+        try {
+            check6616825();
+        } catch (Exception x) {
+            System.err.println(x.getMessage());
+        }
+
+        if (failed) {
             throw new Exception("Some tests failed");
-        else
+        } else {
             System.out.println("All tests passed");
+        }
     }
 
-    private static void check(boolean condition) {
+    private static void check(String msg, boolean condition) {
         if (!condition) {
-            new Throwable("Test failed").printStackTrace(System.out);
+            new Throwable("Test failed " + msg).printStackTrace(System.out);
             failed = true;
         }
     }
-
     private static boolean failed;
 }