Re: RFR: 8313961: Enhance identification of special serialization methods [v2]
> This improves the identification of the serialization magic fields and > methods. Raffaello Giulietti has updated the pull request incrementally with one additional commit since the last revision: Removed a spurious, unintended file. - Changes: - all: https://git.openjdk.org/jdk/pull/15364/files - new: https://git.openjdk.org/jdk/pull/15364/files/bc93873f..b74bc7a3 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=15364&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=15364&range=00-01 Stats: 33 lines in 1 file changed: 0 ins; 33 del; 0 mod Patch: https://git.openjdk.org/jdk/pull/15364.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/15364/head:pull/15364 PR: https://git.openjdk.org/jdk/pull/15364
Re: RFR: 8313961: Enhance identification of special serialization methods [v2]
On Mon, 21 Aug 2023 16:04:49 GMT, Raffaello Giulietti
wrote:
>> This improves the identification of the serialization magic fields and
>> methods.
>
> Raffaello Giulietti has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Removed a spurious, unintended file.
src/java.base/share/classes/java/io/ObjectStreamClass.java line 1670:
> 1668: ObjectStreamField[] serialPersistentFields = null;
> 1669: try {
> 1670: Field f = getDeclaredField(cl, ObjectStreamField[].class,
> "serialPersistentFields");
This can technically be a breaking change, as it was supported for the field to
have a declared type that is assignable from `ObjectStreamField[]`, as long as
it held an `ObjectStreamField[]` instance at runtime, even if it wasn’t
officially supported.
class Example implements Serializable {
// This used to work before this patch in OpenJDK
private static final Object serialPersistentFields = new
ObjectStreamField[] {
// ...
};
// ...
}
-
PR Review Comment: https://git.openjdk.org/jdk/pull/15364#discussion_r1300341941
Re: RFR: 8313961: Enhance identification of special serialization methods [v2]
On Mon, 21 Aug 2023 16:13:37 GMT, ExE Boss wrote:
>> Raffaello Giulietti has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Removed a spurious, unintended file.
>
> src/java.base/share/classes/java/io/ObjectStreamClass.java line 1670:
>
>> 1668: ObjectStreamField[] serialPersistentFields = null;
>> 1669: try {
>> 1670: Field f = getDeclaredField(cl, ObjectStreamField[].class,
>> "serialPersistentFields");
>
> This can technically be a breaking change, as it was supported for the field
> to have a declared type that is assignable from `ObjectStreamField[]`, as
> long as it held an `ObjectStreamField[]` instance at runtime, even if it
> wasn’t officially supported.
>
>
> class Example implements Serializable {
> // This used to work before this patch in OpenJDK
> private static final Object serialPersistentFields = new
> ObjectStreamField[] {
> // ...
> };
>
> // ...
> }
True.
On the other hand, what about a .class file that includes all of (pseudo-Java)
private static final Object serialPersistentFields = new
ObjectStreamField[0];
private static final Cloneable serialPersistentFields = new
ObjectStreamField[0];
private static final SerializableserialPersistentFields = new
ObjectStreamField[0];
private static final ObjectStreamField[] serialPersistentFields = new
SubclassOfObjectStreamField[0];
Which one is the "preferred" field?
Perhaps the Java Object Serialization Specification should simply prohibit
multiple `serialPersistentFields`.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/15364#discussion_r1300400056
Re: RFR: 8313961: Enhance identification of special serialization methods [v2]
On Mon, 21 Aug 2023 16:57:57 GMT, Raffaello Giulietti
wrote:
>> src/java.base/share/classes/java/io/ObjectStreamClass.java line 1670:
>>
>>> 1668: ObjectStreamField[] serialPersistentFields = null;
>>> 1669: try {
>>> 1670: Field f = getDeclaredField(cl, ObjectStreamField[].class,
>>> "serialPersistentFields");
>>
>> This can technically be a breaking change, as it was supported for the field
>> to have a declared type that is assignable from `ObjectStreamField[]`, as
>> long as it held an `ObjectStreamField[]` instance at runtime, even if it
>> wasn’t officially supported.
>>
>>
>> class Example implements Serializable {
>> // This used to work before this patch in OpenJDK
>> private static final Object serialPersistentFields = new
>> ObjectStreamField[] {
>> // ...
>> };
>>
>> // ...
>> }
>
> True.
>
> On the other hand, what about a .class file that includes all of (pseudo-Java)
>
>
> private static final Object serialPersistentFields = new
> ObjectStreamField[0];
> private static final Cloneable serialPersistentFields = new
> ObjectStreamField[0];
> private static final SerializableserialPersistentFields = new
> ObjectStreamField[0];
> private static final ObjectStreamField[] serialPersistentFields = new
> SubclassOfObjectStreamField[0];
>
>
> Which one is the "preferred" field?
>
> Perhaps the Java Object Serialization Specification should simply prohibit
> multiple `serialPersistentFields`.
> This can technically be a breaking change, as it was supported for the field
> to have a declared type that is assignable from `ObjectStreamField[]`, as
> long as it held an `ObjectStreamField[]` instance at runtime, even if it
> wasn’t officially supported.
>
> ```java
> class Example implements Serializable {
> // This used to work before this patch in OpenJDK
> private static final Object serialPersistentFields = new
> ObjectStreamField[] {
> // ...
> };
>
> // ...
> }
> ```
That sort of behavior change would require a CSR; marking the PR accordingly.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/15364#discussion_r1302070033
