RFR: 8333796: Add missing serialization functionality to sun.reflect.ReflectionFactory

[Alan Bateman]

On Thu, 13 Jun 2024 14:31:06 GMT, David M. Lloyd <duke at openjdk.org> wrote:

> Issue [JDK-8164908](https://bugs.openjdk.org/browse/JDK-8164908) added support for functionality required to continue to support IIOP and custom serializers in light of additional module-based restrictions on reflection. It was expected that these libraries would use `sun.misc.Unsafe` in order to access fields of serializable classes. However, with JEP 471, the methods necessary to do this are being removed.
> 
> To allow these libraries to continue to function, it is proposed to add two methods to `sun.reflect.ReflectionFactory` which will allow serialization libraries to acquire a method handle to generated `readObject`/`writeObject` methods which set or get the fields of the serializable class using the serialization `GetField`/`PutField` mechanism. These generated methods should be used by serialization libraries to serialize and deserialize classes which do not have a `readObject`/`writeObject` method or which use `ObjectInputStream.defaultReadObject`/`ObjectOutputStream.defaultWriteObject` to supplement default serialization.
> 
> It is also proposed to add methods which allow for the reading of serialization-specific private static final fields from classes which have them.
> 
> With the addition of these methods, serialization libraries no longer need to rely on `Unsafe` for serialization/deserialization activities.
> cc: @AlanBateman

I skimmed through the latest update (d137f43) and I think you've got this to a good place and a sensible/workable proposal. I've asked from help from others on the security review of this.

Right now, I'm still wonder if defaultReadObjectForSerialization/defaultWriteObjectForSerialization should return the same as readObjectForSerialization/writeObjectForSerialization when the readObject/writeObject methods are defined. This is more of a concern for a class with a readObject of course as that readObject will likely check invariants that would be bypassed if the serialization library always uses the defaultXXX methods.

I'd probably drop the "Of" suffix from serialPersistentFieldsOf and serialVersionUIDOf but naming isn't important right now.

src/java.base/share/classes/java/lang/invoke/ConstantBootstraps.java line 448:

> 446:      * @since 24
> 447:      */
> 448:     public static MethodHandle fieldSetterForSerialization(MethodHandles.Lookup lookup, String name, Class<MethodHandle> type, Class<?> cl) {

Is this left over from an earlier prototype? There shouldn't be any changes to the standard APIs in this proposal.

src/java.base/share/classes/java/lang/invoke/SerializationBytecodeGenerator.java line 43:

> 41:  * The private serialization bytecode generator used by {@code sun.misc.ReflectionFactory}.
> 42:  */
> 43: final class SerializationBytecodeGenerator {

I assume the starting point for the prototype should be a supporting class in jdk.internal.reflect rather than here.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/19702#issuecomment-2198027291
PR Review Comment: https://git.openjdk.org/jdk/pull/19702#discussion_r1656667254
PR Review Comment: https://git.openjdk.org/jdk/pull/19702#discussion_r1658215233