Hi Joe,
The addition of @SuppressWarnings(serial) hides a number of instances of
poor choices of types. Before they dissappear behind the suppress warnings,
the fix should be to correct the types used.
For example, in the serialization proxies for java.time, the Ser class
carelessly
has a field of type Object when it could/should be using Serializable.
The types being serialized and deserialized are known to be Serializable.
See the attach patches for details and a suggested fix.
Thanks, Roger
(p.s. the patch is attached twice, once as .patch and the other .txt.
I'd like to see if they both get through the mail).
On 9/18/19 5:38 PM, Joe Darcy wrote:
Hello,
As background, I'm working on a number of serialization-related
compile-time checks with the goal of enabling stricter javac lint
checking in the JDK build (and elsewhere).
One check is tracked by
JDK-8160675: Issue lint warning for non-serializable non-transient
instance fields in serializable type
As summarized in the bug description, it may be concerning if a
serializable class has non-transient instance fields whose types are
not Serializable. This can cause a serialization failure at runtime.
(Classes using the serialPersistentFields mechanism are excluded from
this check.)
A common example is an exception type -- all Throwable's are
Serializable -- which has a non-serializable field. If the fields
cannot be marked as transient, one approach to handle this robustly is
to have a writeObject method which null outs the field in question
when serializing and make the other methods in the exception
null-tolerant.
In other cases, the object pointed to by the non-serializable field
are conditionally serializable at runtime. This is the case for many
collection types. For example, a class may have a field of type
List<Foo> with the field set to an ArrayList<Foo> at runtime. While
the List interface does not extent Serializable, the ArrayList class
does implement Serializable and the class would serialize fine in
practice, assuming the Foo's were serialazable.
As a precursor to the adding a compile-time check to the build, please
review adding @SuppressWarnings("serial") to document the
non-serializable fields in the core libraries:
JDK-8231202: Suppress warnings on non-serializable non-transient
instance fields in serializable classes
http://cr.openjdk.java.net/~darcy/8231202.0/
Bugs for similar changes to client libs and security libs will be
filed and reviewed separately.
A more complete fix would add readObject/writeObject null handling to
AnnotationTypeMismatchExceptionProxy, but since this hasn't seemed to
be an issue since the type was introduced back in JDK 5.0, I just
added the annotation, as done elsewhere.
Thanks,
-Joe
# HG changeset patch
# Parent aad3466779bbcc2778d039ef7aee01ac75228b86
diff -r aad3466779bb -r f584c4228c70 src/java.base/share/classes/java/lang/invoke/SerializedLambda.java
--- a/src/java.base/share/classes/java/lang/invoke/SerializedLambda.java Thu Sep 19 14:40:54 2019 -0400
+++ b/src/java.base/share/classes/java/lang/invoke/SerializedLambda.java Thu Sep 19 15:04:48 2019 -0400
@@ -77,7 +77,7 @@ public final class SerializedLambda impl
private final int implMethodKind;
private final String instantiatedMethodType;
@SuppressWarnings("serial") // Not statically typed as Serializable
- private final Object[] capturedArgs;
+ private final Serializable[] capturedArgs;
/**
* Create a {@code SerializedLambda} from the low-level information present
@@ -114,7 +114,7 @@ public final class SerializedLambda impl
String implMethodName,
String implMethodSignature,
String instantiatedMethodType,
- Object[] capturedArgs) {
+ Serializable[] capturedArgs) {
this.capturingClass = capturingClass;
this.functionalInterfaceClass = functionalInterfaceClass;
this.functionalInterfaceMethodName = functionalInterfaceMethodName;
diff -r aad3466779bb -r f584c4228c70 src/java.base/share/classes/java/time/Ser.java
--- a/src/java.base/share/classes/java/time/Ser.java Thu Sep 19 14:40:54 2019 -0400
+++ b/src/java.base/share/classes/java/time/Ser.java Thu Sep 19 15:04:48 2019 -0400
@@ -61,6 +61,7 @@ import java.io.IOException;
import java.io.InvalidClassException;
import java.io.ObjectInput;
import java.io.ObjectOutput;
+import java.io.Serializable;
import java.io.StreamCorruptedException;
/**
@@ -112,8 +113,7 @@ final class Ser implements Externalizabl
/** The type being serialized. */
private byte type;
/** The object being serialized. */
- @SuppressWarnings("serial") // Not statically typed as Serializable
- private Object object;
+ private Serializable object;
/**
* Constructor for deserialization.
@@ -127,7 +127,7 @@ final class Ser implements Externalizabl
* @param type the type
* @param object the object
*/
- Ser(byte type, Object object) {
+ Ser(byte type, Serializable object) {
this.type = type;
this.object = object;
}
@@ -248,12 +248,12 @@ final class Ser implements Externalizabl
object = readInternal(type, in);
}
- static Object read(ObjectInput in) throws IOException, ClassNotFoundException {
+ static Serializable read(ObjectInput in) throws IOException, ClassNotFoundException {
byte type = in.readByte();
return readInternal(type, in);
}
- private static Object readInternal(byte type, ObjectInput in) throws IOException, ClassNotFoundException {
+ private static Serializable readInternal(byte type, ObjectInput in) throws IOException, ClassNotFoundException {
switch (type) {
case DURATION_TYPE: return Duration.readExternal(in);
case INSTANT_TYPE: return Instant.readExternal(in);
diff -r aad3466779bb -r f584c4228c70 src/java.base/share/classes/java/time/chrono/AbstractChronology.java
--- a/src/java.base/share/classes/java/time/chrono/AbstractChronology.java Thu Sep 19 14:40:54 2019 -0400
+++ b/src/java.base/share/classes/java/time/chrono/AbstractChronology.java Thu Sep 19 15:04:48 2019 -0400
@@ -731,7 +731,7 @@ public abstract class AbstractChronology
*/
@java.io.Serial
Object writeReplace() {
- return new Ser(Ser.CHRONO_TYPE, this);
+ return new Ser(Ser.CHRONO_TYPE, (Serializable)this);
}
/**
diff -r aad3466779bb -r f584c4228c70 src/java.base/share/classes/java/time/chrono/Ser.java
--- a/src/java.base/share/classes/java/time/chrono/Ser.java Thu Sep 19 14:40:54 2019 -0400
+++ b/src/java.base/share/classes/java/time/chrono/Ser.java Thu Sep 19 15:04:48 2019 -0400
@@ -61,6 +61,7 @@ import java.io.IOException;
import java.io.InvalidClassException;
import java.io.ObjectInput;
import java.io.ObjectOutput;
+import java.io.Serializable;
import java.io.StreamCorruptedException;
import java.time.LocalDate;
import java.time.LocalDateTime;
@@ -110,8 +111,7 @@ final class Ser implements Externalizabl
/** The type being serialized. */
private byte type;
/** The object being serialized. */
- @SuppressWarnings("serial") // Not statically typed as Serializable
- private Object object;
+ private Serializable object;
/**
* Constructor for deserialization.
@@ -125,7 +125,7 @@ final class Ser implements Externalizabl
* @param type the type
* @param object the object
*/
- Ser(byte type, Object object) {
+ Ser(byte type, Serializable object) {
this.type = type;
this.object = object;
}
@@ -231,11 +231,12 @@ final class Ser implements Externalizabl
return readInternal(type, in);
}
- private static Object readInternal(byte type, ObjectInput in) throws IOException, ClassNotFoundException {
+ private static Serializable readInternal(byte type, ObjectInput in) throws IOException,
+ ClassNotFoundException {
switch (type) {
- case CHRONO_TYPE: return AbstractChronology.readExternal(in);
- case CHRONO_LOCAL_DATE_TIME_TYPE: return ChronoLocalDateTimeImpl.readExternal(in);
- case CHRONO_ZONE_DATE_TIME_TYPE: return ChronoZonedDateTimeImpl.readExternal(in);
+ case CHRONO_TYPE: return (Serializable)AbstractChronology.readExternal(in);
+ case CHRONO_LOCAL_DATE_TIME_TYPE: return (Serializable)ChronoLocalDateTimeImpl.readExternal(in);
+ case CHRONO_ZONE_DATE_TIME_TYPE: return (Serializable)ChronoZonedDateTimeImpl.readExternal(in);
case JAPANESE_DATE_TYPE: return JapaneseDate.readExternal(in);
case JAPANESE_ERA_TYPE: return JapaneseEra.readExternal(in);
case HIJRAH_DATE_TYPE: return HijrahDate.readExternal(in);
diff -r aad3466779bb -r f584c4228c70 src/java.base/share/classes/java/time/zone/Ser.java
--- a/src/java.base/share/classes/java/time/zone/Ser.java Thu Sep 19 14:40:54 2019 -0400
+++ b/src/java.base/share/classes/java/time/zone/Ser.java Thu Sep 19 15:04:48 2019 -0400
@@ -68,6 +68,7 @@ import java.io.IOException;
import java.io.InvalidClassException;
import java.io.ObjectInput;
import java.io.ObjectOutput;
+import java.io.Serializable;
import java.io.StreamCorruptedException;
import java.time.ZoneOffset;
@@ -97,8 +98,7 @@ final class Ser implements Externalizabl
/** The type being serialized. */
private byte type;
/** The object being serialized. */
- @SuppressWarnings("serial") // Not statically typed as Serializable
- private Object object;
+ private Serializable object;
/**
* Constructor for deserialization.
@@ -112,7 +112,7 @@ final class Ser implements Externalizabl
* @param type the type
* @param object the object
*/
- Ser(byte type, Object object) {
+ Ser(byte type, Serializable object) {
this.type = type;
this.object = object;
}
@@ -184,12 +184,13 @@ final class Ser implements Externalizabl
object = readInternal(type, in);
}
- static Object read(DataInput in) throws IOException, ClassNotFoundException {
+ static Serializable read(DataInput in) throws IOException, ClassNotFoundException {
byte type = in.readByte();
return readInternal(type, in);
}
- private static Object readInternal(byte type, DataInput in) throws IOException, ClassNotFoundException {
+ private static Serializable readInternal(byte type, DataInput in) throws IOException,
+ ClassNotFoundException {
switch (type) {
case ZRULES:
return ZoneRules.readExternal(in);
# HG changeset patch
# Parent aad3466779bbcc2778d039ef7aee01ac75228b86
diff -r aad3466779bb -r f584c4228c70
src/java.base/share/classes/java/lang/invoke/SerializedLambda.java
--- a/src/java.base/share/classes/java/lang/invoke/SerializedLambda.java
Thu Sep 19 14:40:54 2019 -0400
+++ b/src/java.base/share/classes/java/lang/invoke/SerializedLambda.java
Thu Sep 19 15:04:48 2019 -0400
@@ -77,7 +77,7 @@ public final class SerializedLambda impl
private final int implMethodKind;
private final String instantiatedMethodType;
@SuppressWarnings("serial") // Not statically typed as Serializable
- private final Object[] capturedArgs;
+ private final Serializable[] capturedArgs;
/**
* Create a {@code SerializedLambda} from the low-level information present
@@ -114,7 +114,7 @@ public final class SerializedLambda impl
String implMethodName,
String implMethodSignature,
String instantiatedMethodType,
- Object[] capturedArgs) {
+ Serializable[] capturedArgs) {
this.capturingClass = capturingClass;
this.functionalInterfaceClass = functionalInterfaceClass;
this.functionalInterfaceMethodName = functionalInterfaceMethodName;
diff -r aad3466779bb -r f584c4228c70
src/java.base/share/classes/java/time/Ser.java
--- a/src/java.base/share/classes/java/time/Ser.java Thu Sep 19 14:40:54
2019 -0400
+++ b/src/java.base/share/classes/java/time/Ser.java Thu Sep 19 15:04:48
2019 -0400
@@ -61,6 +61,7 @@ import java.io.IOException;
import java.io.InvalidClassException;
import java.io.ObjectInput;
import java.io.ObjectOutput;
+import java.io.Serializable;
import java.io.StreamCorruptedException;
/**
@@ -112,8 +113,7 @@ final class Ser implements Externalizabl
/** The type being serialized. */
private byte type;
/** The object being serialized. */
- @SuppressWarnings("serial") // Not statically typed as Serializable
- private Object object;
+ private Serializable object;
/**
* Constructor for deserialization.
@@ -127,7 +127,7 @@ final class Ser implements Externalizabl
* @param type the type
* @param object the object
*/
- Ser(byte type, Object object) {
+ Ser(byte type, Serializable object) {
this.type = type;
this.object = object;
}
@@ -248,12 +248,12 @@ final class Ser implements Externalizabl
object = readInternal(type, in);
}
- static Object read(ObjectInput in) throws IOException,
ClassNotFoundException {
+ static Serializable read(ObjectInput in) throws IOException,
ClassNotFoundException {
byte type = in.readByte();
return readInternal(type, in);
}
- private static Object readInternal(byte type, ObjectInput in) throws
IOException, ClassNotFoundException {
+ private static Serializable readInternal(byte type, ObjectInput in) throws
IOException, ClassNotFoundException {
switch (type) {
case DURATION_TYPE: return Duration.readExternal(in);
case INSTANT_TYPE: return Instant.readExternal(in);
diff -r aad3466779bb -r f584c4228c70
src/java.base/share/classes/java/time/chrono/AbstractChronology.java
--- a/src/java.base/share/classes/java/time/chrono/AbstractChronology.java
Thu Sep 19 14:40:54 2019 -0400
+++ b/src/java.base/share/classes/java/time/chrono/AbstractChronology.java
Thu Sep 19 15:04:48 2019 -0400
@@ -731,7 +731,7 @@ public abstract class AbstractChronology
*/
@java.io.Serial
Object writeReplace() {
- return new Ser(Ser.CHRONO_TYPE, this);
+ return new Ser(Ser.CHRONO_TYPE, (Serializable)this);
}
/**
diff -r aad3466779bb -r f584c4228c70
src/java.base/share/classes/java/time/chrono/Ser.java
--- a/src/java.base/share/classes/java/time/chrono/Ser.java Thu Sep 19
14:40:54 2019 -0400
+++ b/src/java.base/share/classes/java/time/chrono/Ser.java Thu Sep 19
15:04:48 2019 -0400
@@ -61,6 +61,7 @@ import java.io.IOException;
import java.io.InvalidClassException;
import java.io.ObjectInput;
import java.io.ObjectOutput;
+import java.io.Serializable;
import java.io.StreamCorruptedException;
import java.time.LocalDate;
import java.time.LocalDateTime;
@@ -110,8 +111,7 @@ final class Ser implements Externalizabl
/** The type being serialized. */
private byte type;
/** The object being serialized. */
- @SuppressWarnings("serial") // Not statically typed as Serializable
- private Object object;
+ private Serializable object;
/**
* Constructor for deserialization.
@@ -125,7 +125,7 @@ final class Ser implements Externalizabl
* @param type the type
* @param object the object
*/
- Ser(byte type, Object object) {
+ Ser(byte type, Serializable object) {
this.type = type;
this.object = object;
}
@@ -231,11 +231,12 @@ final class Ser implements Externalizabl
return readInternal(type, in);
}
- private static Object readInternal(byte type, ObjectInput in) throws
IOException, ClassNotFoundException {
+ private static Serializable readInternal(byte type, ObjectInput in) throws
IOException,
+ ClassNotFoundException {
switch (type) {
- case CHRONO_TYPE: return AbstractChronology.readExternal(in);
- case CHRONO_LOCAL_DATE_TIME_TYPE: return
ChronoLocalDateTimeImpl.readExternal(in);
- case CHRONO_ZONE_DATE_TIME_TYPE: return
ChronoZonedDateTimeImpl.readExternal(in);
+ case CHRONO_TYPE: return
(Serializable)AbstractChronology.readExternal(in);
+ case CHRONO_LOCAL_DATE_TIME_TYPE: return
(Serializable)ChronoLocalDateTimeImpl.readExternal(in);
+ case CHRONO_ZONE_DATE_TIME_TYPE: return
(Serializable)ChronoZonedDateTimeImpl.readExternal(in);
case JAPANESE_DATE_TYPE: return JapaneseDate.readExternal(in);
case JAPANESE_ERA_TYPE: return JapaneseEra.readExternal(in);
case HIJRAH_DATE_TYPE: return HijrahDate.readExternal(in);
diff -r aad3466779bb -r f584c4228c70
src/java.base/share/classes/java/time/zone/Ser.java
--- a/src/java.base/share/classes/java/time/zone/Ser.java Thu Sep 19
14:40:54 2019 -0400
+++ b/src/java.base/share/classes/java/time/zone/Ser.java Thu Sep 19
15:04:48 2019 -0400
@@ -68,6 +68,7 @@ import java.io.IOException;
import java.io.InvalidClassException;
import java.io.ObjectInput;
import java.io.ObjectOutput;
+import java.io.Serializable;
import java.io.StreamCorruptedException;
import java.time.ZoneOffset;
@@ -97,8 +98,7 @@ final class Ser implements Externalizabl
/** The type being serialized. */
private byte type;
/** The object being serialized. */
- @SuppressWarnings("serial") // Not statically typed as Serializable
- private Object object;
+ private Serializable object;
/**
* Constructor for deserialization.
@@ -112,7 +112,7 @@ final class Ser implements Externalizabl
* @param type the type
* @param object the object
*/
- Ser(byte type, Object object) {
+ Ser(byte type, Serializable object) {
this.type = type;
this.object = object;
}
@@ -184,12 +184,13 @@ final class Ser implements Externalizabl
object = readInternal(type, in);
}
- static Object read(DataInput in) throws IOException,
ClassNotFoundException {
+ static Serializable read(DataInput in) throws IOException,
ClassNotFoundException {
byte type = in.readByte();
return readInternal(type, in);
}
- private static Object readInternal(byte type, DataInput in) throws
IOException, ClassNotFoundException {
+ private static Serializable readInternal(byte type, DataInput in) throws
IOException,
+ ClassNotFoundException {
switch (type) {
case ZRULES:
return ZoneRules.readExternal(in);
