changeset 58367:52a9226db842 records

records: review comments - SUID is allowable but ignored during deserialization
author chegar
date Tue, 22 Oct 2019 11:26:02 +0100
parents b3e8bdfcb968
children 31b6ebe43579
files src/java.base/share/classes/java/io/ObjectStreamClass.java test/jdk/java/io/Serializable/records/RecordClassTest.java test/jdk/java/io/Serializable/records/SerialVersionUIDTest.java test/jdk/java/io/Serializable/records/migration/plain/AssignableFromImpl.java test/jdk/java/io/Serializable/records/migration/record/AssignableFromImpl.java
diffstat 5 files changed, 127 insertions(+), 17 deletions(-) [+]
line wrap: on
line diff
--- a/src/java.base/share/classes/java/io/ObjectStreamClass.java	Mon Oct 21 14:41:49 2019 +0100
+++ b/src/java.base/share/classes/java/io/ObjectStreamClass.java	Tue Oct 22 11:26:02 2019 +0100
@@ -270,6 +270,9 @@
     public long getSerialVersionUID() {
         // REMIND: synchronize instead of relying on volatile?
         if (suid == null) {
+            if (isRecord)
+                return 0L;
+
             suid = AccessController.doPrivileged(
                 new PrivilegedAction<Long>() {
                     public Long run() {
@@ -506,11 +509,7 @@
                         return null;
                     }
 
-                    if (isRecord) {
-                        suid = 0L;
-                    } else {
                     suid = getDeclaredSUID(cl);
-                    }
                     try {
                         fields = getSerialFields(cl);
                         computeFieldOffsets();
--- a/test/jdk/java/io/Serializable/records/RecordClassTest.java	Mon Oct 21 14:41:49 2019 +0100
+++ b/test/jdk/java/io/Serializable/records/RecordClassTest.java	Tue Oct 22 11:26:02 2019 +0100
@@ -78,18 +78,20 @@
     @DataProvider(name = "recordClasses")
     public Object[][] recordClasses() {
         return new Object[][] {
-            new Object[] { Foo.class    },
-            new Object[] { Bar.class    },
-            new Object[] { Baz.class    },
-            new Object[] { Wibble.class },
-            new Object[] { Wobble.class },
-            new Object[] { Wubble.class },
+            new Object[] { Foo.class    , 0L         },
+            new Object[] { Bar.class    , 987654321L },
+            new Object[] { Baz.class    , 0L         },
+            new Object[] { Wibble.class , 12345678L  },
+            new Object[] { Wobble.class , 0L         },
+            new Object[] { Wubble.class , 0L         },
         };
     }
 
     /** Tests that the serialized and deserialized instances are equal. */
     @Test(dataProvider = "recordClasses")
-    public void testClassSerialization(Class<?> recordClass) throws Exception {
+    public void testClassSerialization(Class<?> recordClass, long unused)
+        throws Exception
+    {
         out.println("\n---");
         out.println("serializing : " + recordClass);
         var deserializedClass = serializeDeserialize(recordClass);
@@ -98,17 +100,17 @@
         assertEquals(deserializedClass, recordClass);
     }
 
-    /** Tests that the generated SUID is always 0 for all Serializable record classes. */
+    /** Tests that the SUID is always 0 unless explicitly declared. */
     @Test(dataProvider = "recordClasses")
-    public void testSerialVersionUID(Class<?> recordClass) {
+    public void testSerialVersionUID(Class<?> recordClass, long expectedUID) {
         out.println("\n---");
         ObjectStreamClass osc = ObjectStreamClass.lookup(recordClass);
         out.println("ObjectStreamClass::lookup  : " + osc);
-        assertEquals(osc.getSerialVersionUID(), 0L);
+        assertEquals(osc.getSerialVersionUID(), expectedUID);
 
         osc = ObjectStreamClass.lookupAny(recordClass);
         out.println("ObjectStreamClass::lookupAny: " + osc);
-        assertEquals(osc.getSerialVersionUID(), 0L);
+        assertEquals(osc.getSerialVersionUID(), expectedUID);
     }
 
     // --- not Serializable
--- /dev/null	Thu Jan 01 00:00:00 1970 +0000
+++ b/test/jdk/java/io/Serializable/records/SerialVersionUIDTest.java	Tue Oct 22 11:26:02 2019 +0100
@@ -0,0 +1,107 @@
+/*
+ * Copyright (c) 2019, 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @summary Basic tests for SUID in the serial stream
+ * @compile --enable-preview -source 14 SerialVersionUIDTest.java
+ * @run testng/othervm --enable-preview SerialVersionUIDTest
+ * @run testng/othervm/java.security.policy=empty_security.policy --enable-preview SerialVersionUIDTest
+ */
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.DataInputStream;
+import java.io.IOException;
+import java.io.ObjectOutputStream;
+import java.io.Serializable;
+import org.testng.annotations.DataProvider;
+import org.testng.annotations.Test;
+import static java.io.ObjectStreamConstants.STREAM_MAGIC;
+import static java.io.ObjectStreamConstants.STREAM_VERSION;
+import static java.io.ObjectStreamConstants.TC_CLASSDESC;
+import static java.io.ObjectStreamConstants.TC_OBJECT;
+import static java.lang.System.out;
+import static org.testng.Assert.assertEquals;
+
+public class SerialVersionUIDTest {
+
+    record R1 () implements Serializable {
+        private static final long serialVersionUID = 1L;
+    }
+
+    record R2 (int x, int y) implements Serializable {
+        private static final long serialVersionUID = 0L;
+    }
+
+    record R3 () implements Serializable { }
+
+    record R4 (String s) implements Serializable { }
+
+    record R5 (long l) implements Serializable {
+        private static final long serialVersionUID = 5678L;
+    }
+
+    @DataProvider(name = "recordObjects")
+    public Object[][] recordObjects() {
+        return new Object[][] {
+            new Object[] { new R1(),        1L    },
+            new Object[] { new R2(1, 2),    0L    },
+            new Object[] { new R3(),        0L    },
+            new Object[] { new R4("s"),     0L    },
+            new Object[] { new R5(7L),      5678L },
+        };
+    }
+
+    @Test(dataProvider = "recordObjects")
+    public void testSerialize(Object objectToSerialize, long expectedUID)
+        throws Exception
+    {
+        out.println("\n---");
+        out.println("serializing : " + objectToSerialize);
+        byte[] bytes = serialize(objectToSerialize);
+
+        ByteArrayInputStream bais = new ByteArrayInputStream(bytes);
+        DataInputStream dis = new DataInputStream(bais);
+
+        // sanity
+        assertEquals(dis.readShort(), STREAM_MAGIC);
+        assertEquals(dis.readShort(), STREAM_VERSION);
+        assertEquals(dis.readByte(), TC_OBJECT);
+        assertEquals(dis.readByte(), TC_CLASSDESC);
+        assertEquals(dis.readUTF(), objectToSerialize.getClass().getName());
+
+        // verify that the UID is as expected
+        assertEquals(dis.readLong(), expectedUID);
+    }
+
+    // --- infra
+
+    static <T> byte[] serialize(T obj) throws IOException {
+        ByteArrayOutputStream baos = new ByteArrayOutputStream();
+        ObjectOutputStream oos = new ObjectOutputStream(baos);
+        oos.writeObject(obj);
+        oos.close();
+        return baos.toByteArray();
+    }
+}
--- a/test/jdk/java/io/Serializable/records/migration/plain/AssignableFromImpl.java	Mon Oct 21 14:41:49 2019 +0100
+++ b/test/jdk/java/io/Serializable/records/migration/plain/AssignableFromImpl.java	Tue Oct 22 11:26:02 2019 +0100
@@ -24,7 +24,8 @@
 public class AssignableFromImpl
     implements AssignableFrom, java.io.Serializable
 {
-    private static final long serialVersionUID = 0L;
+    // ignored during record deserialization, but enforced for non-record deserialization
+    private static final long serialVersionUID = 1L;
 
     private final Number number;
 
--- a/test/jdk/java/io/Serializable/records/migration/record/AssignableFromImpl.java	Mon Oct 21 14:41:49 2019 +0100
+++ b/test/jdk/java/io/Serializable/records/migration/record/AssignableFromImpl.java	Tue Oct 22 11:26:02 2019 +0100
@@ -24,5 +24,6 @@
 public record AssignableFromImpl (Number number)
     implements AssignableFrom, java.io.Serializable
 {
-    private static final long serialVersionUID = 1L;  // should be ignored
+    // ignored during record deserialization, but enforced for non-record deserialization
+    private static final long serialVersionUID = 1L;
 }