8275137: jdk.unsupported/sun.reflect.ReflectionFactory.readObjectNoDataForSerialization uses wrong signature

Reviewed-by: dfuchs

src/java.base/share/classes/jdk/internal/reflect/ReflectionFactory.java

@@ -478,7 +478,7 @@ public class ReflectionFactory {
     }
 
     public final MethodHandle readObjectNoDataForSerialization(Class<?> cl) {
-        return findReadWriteObjectForSerialization(cl, "readObjectNoData", ObjectInputStream.class);
+        return findReadWriteObjectForSerialization(cl, "readObjectNoData", null);
     }
 
     public final MethodHandle writeObjectForSerialization(Class<?> cl) {
@@ -493,7 +493,8 @@ public class ReflectionFactory {
         }
 
         try {
-            Method meth = cl.getDeclaredMethod(methodName, streamClass);
+            Method meth = streamClass == null ? cl.getDeclaredMethod(methodName)
+                    : cl.getDeclaredMethod(methodName, streamClass);
             int mods = meth.getModifiers();
             if (meth.getReturnType() != Void.TYPE ||
                     Modifier.isStatic(mods) ||

src/jdk.unsupported/share/classes/sun/reflect/ReflectionFactory.java

@@ -144,9 +144,9 @@ public class ReflectionFactory {
     /**
      * Returns a direct MethodHandle for the {@code readObjectNoData} method on
      * a Serializable class.
-     * The first argument of {@link MethodHandle#invoke} is the serializable
-     * object and the second argument is the {@code ObjectInputStream} passed to
-     * {@code readObjectNoData}.
+     * The only argument of {@link MethodHandle#invoke} is the serializable
+     * object, which {@code readObjectNoData} is called on. No arguments are
+     * passed to the {@code readObjectNoData} method.
      *
      * @param cl a Serializable class
      * @return  a direct MethodHandle for the {@code readObjectNoData} method

test/jdk/sun/reflect/ReflectionFactory/ReflectionFactoryTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2021, 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
@@ -29,6 +29,7 @@ import java.io.ObjectInput;
 import java.io.ObjectInputStream;
 import java.io.ObjectOutput;
 import java.io.ObjectOutputStream;
+import java.io.ObjectStreamException;
 import java.io.OptionalDataException;
 import java.io.Serializable;
 import java.lang.invoke.MethodHandle;
@@ -45,11 +46,11 @@ import org.testng.TestNG;
 
 /*
  * @test
- * @bug 8137058 8164908 8168980
- * @run testng ReflectionFactoryTest
- * @run testng/othervm/policy=security.policy ReflectionFactoryTest
+ * @bug 8137058 8164908 8168980 8275137
  * @summary Basic test for the unsupported ReflectionFactory
  * @modules jdk.unsupported
+ * @run testng ReflectionFactoryTest
+ * @run testng/othervm/policy=security.policy ReflectionFactoryTest
  */
 
 public class ReflectionFactoryTest {
@@ -81,8 +82,7 @@ public class ReflectionFactoryTest {
      */
     @Test(dataProvider="ClassConstructors")
     static void testConstructor(Class<?> type)
-        throws NoSuchMethodException, InstantiationException,
-            IllegalAccessException, InvocationTargetException
+            throws InstantiationException, IllegalAccessException, InvocationTargetException
     {
         @SuppressWarnings("unchecked")
         Constructor<?> c = factory.newConstructorForSerialization(type);
@@ -131,8 +131,8 @@ public class ReflectionFactoryTest {
         }
 
         Assert.assertEquals(((Foo)o).foo(), expectedFoo);
-        if (o instanceof Baz) {
-            Assert.assertEquals(((Baz)o).baz(), expectedBaz);
+        if (o instanceof Baz b) {
+            Assert.assertEquals(b.baz(), expectedBaz);
         }
     }
 
@@ -178,7 +178,7 @@ public class ReflectionFactoryTest {
     }
 
     /**
-     * Test newConstructorForExternalization returns the constructor and it can be called.
+     * Tests that newConstructorForExternalization returns the constructor and it can be called.
      * @throws NoSuchMethodException - error
      * @throws InstantiationException - error
      * @throws IllegalAccessException - error
@@ -186,8 +186,7 @@ public class ReflectionFactoryTest {
      */
     @Test
     static void newConstructorForExternalization()
-            throws NoSuchMethodException, InstantiationException,
-            IllegalAccessException, InvocationTargetException {
+            throws InstantiationException, IllegalAccessException, InvocationTargetException {
         Constructor<?> cons = factory.newConstructorForExternalization(Ext.class);
         Ext ext = (Ext)cons.newInstance();
         Assert.assertEquals(ext.ext, 1, "Constructor not run");
@@ -251,7 +250,7 @@ public class ReflectionFactoryTest {
             Assert.assertFalse(ser2.readObjectNoDataCalled, "readObjectNoData should not be called");
             Assert.assertFalse(ser2.readResolveCalled, "readResolve should not be called");
 
-            readObjectNoDataMethod.invoke(ser2, ois);
+            readObjectNoDataMethod.invoke(ser2);
             Assert.assertTrue(ser2.readObjectCalled, "readObject should have been called");
             Assert.assertTrue(ser2.readObjectNoDataCalled, "readObjectNoData not called");
             Assert.assertFalse(ser2.readResolveCalled, "readResolve should not be called");
@@ -283,12 +282,12 @@ public class ReflectionFactoryTest {
 
         public Ser() {}
 
-        private void readObject(ObjectInputStream ois) throws IOException {
+        private void readObject(ObjectInputStream ois) throws IOException, ClassNotFoundException {
             Assert.assertFalse(writeObjectCalled, "readObject called too many times");
             readObjectCalled = ois.readBoolean();
         }
 
-        private void readObjectNoData(ObjectInputStream ois) throws IOException {
+        private void readObjectNoData() throws ObjectStreamException {
             Assert.assertFalse(readObjectNoDataCalled, "readObjectNoData called too many times");
             readObjectNoDataCalled = true;
         }
@@ -299,13 +298,13 @@ public class ReflectionFactoryTest {
             oos.writeBoolean(writeObjectCalled);
         }
 
-        private Object writeReplace() {
+        private Object writeReplace() throws ObjectStreamException {
             Assert.assertFalse(writeReplaceCalled, "writeReplace called too many times");
             writeReplaceCalled = true;
             return this;
         }
 
-        private Object readResolve() {
+        private Object readResolve() throws ObjectStreamException {
             Assert.assertFalse(readResolveCalled, "readResolve called too many times");
             readResolveCalled = true;
             return this;
@@ -313,7 +312,7 @@ public class ReflectionFactoryTest {
     }
 
     /**
-     * Test the constructor of OptionalDataExceptions.
+     * Tests the constructor of OptionalDataExceptions.
      */
     @Test
     static void newOptionalDataException() {
@@ -324,8 +323,6 @@ public class ReflectionFactoryTest {
 
     }
 
-
-
     // Main can be used to run the tests from the command line with only testng.jar.
     @SuppressWarnings("raw_types")
     @Test(enabled = false)