On Wed, 1 May 2024 18:43:21 GMT, Roger Riggs <rri...@openjdk.org> wrote:
> The issue reported a ClassCastException "cannot assign instance of
> java.util.CollSer to field of type java.util.Map"
> while deserializing an object referring to an immutable Map that contained a
> reference to a class that was not available.
> Immutable Collections such as Map utilize a serialization proxy in their
> serialized form.
> During deserialization the serialization proxy (a private implementation
> class) was attempted to be set in a field resulting in the
> ClassCastException. The ClassCastException and bug hid the ClassCastException
> that should have been thrown.
>
> When reading record fields or fields of a class, the results of
> deserialization of individual fields are recorded as dependencies of the
> object being constructed.
> The apparent bug is that the summary of those dependencies is not checked
> between reading the fields and invoking the constructor to create the record
> or assigning the fields to an object being constructed.
src/java.base/share/classes/java/io/ObjectInputStream.java line 2376:
> 2374: if (handles.lookupException(passHandle) != null) {
> 2375: return null; // slot marked with exception, don't
> create record
> 2376: }
It's proper to avoid creating a record instance in this case. However,
returning null is new behavior; this initially seemed a bit odd. The calling
method `readOrdinaryObject()` does allow a null return if the class couldn't be
resolved, and the body of `readOrdinaryObject()` does allow `obj` to be null
throughout. So, returning `null` from here is correct, though it took me a
while to puzzle out. :-)
I'd suggest adding some docs to `readRecord()` to indicate that it returns the
record instance if it could be created successfully, null if the record class
couldn't be resolved, or throws an error or other exception if one occurred
when instantiation was attempted.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/19043#discussion_r1604052829
