Re: JDK 14 RFR of JDK-8231262: Suppress warnings on non-serializable instance fields in security libs serializable classes

2019-10-09 Thread Joe Darcy
Hi Chris and Sean, I'll push a fix for JDK-8231262 with a single class-level suppression in X509CertImpl:     @SuppressWarnings("serial") // See writeReplace method in Certificate I've filed         JDK-8232062: Clarify serialization mechanisms of X509CertImpl for the follow-up work.

Re: JDK 14 RFR of JDK-8231262: Suppress warnings on non-serializable instance fields in security libs serializable classes

2019-10-09 Thread Chris Hegarty
On 09/10/2019 14:54, Sean Mullan wrote: ... X509CertImpl extends X509Certificate which extends Certificate. Certificate has a writeReplace method. Another possible follow-on is to add readObject methods, that unconditionally throw, to both X509Certificate and X509CertImpl, since

Re: JDK 14 RFR of JDK-8231262: Suppress warnings on non-serializable instance fields in security libs serializable classes

2019-10-09 Thread Sean Mullan
On 10/8/19 9:56 PM, Joe Darcy wrote: Hi Sean, Getting back to this review... On 9/26/2019 1:55 PM, Sean Mullan wrote: On 9/26/19 4:20 PM, Sean Mullan wrote: Would you prefer I revise the patch where there are multiple SuppressWarnings("serial") on fields to put a single one on the class

Re: JDK 14 RFR of JDK-8231262: Suppress warnings on non-serializable instance fields in security libs serializable classes

2019-10-08 Thread Joe Darcy
Hi Sean, Getting back to this review... On 9/26/2019 1:55 PM, Sean Mullan wrote: On 9/26/19 4:20 PM, Sean Mullan wrote: Would you prefer I revise the patch where there are multiple SuppressWarnings("serial") on fields to put a single one on the class instead? Yes, but only in the cases

Re: JDK 14 RFR of JDK-8231262: Suppress warnings on non-serializable instance fields in security libs serializable classes

2019-09-27 Thread Chris Hegarty
On 26/09/2019 21:10, Joe Darcy wrote: .. Yes; applying SuppressWarnings("serial") to the class will silence this new warning for all the fields. Will is also suppress other warnings, like on the serialization-related magic methods? Which would be unfortunate. -Chris.

Re: JDK 14 RFR of JDK-8231262: Suppress warnings on non-serializable instance fields in security libs serializable classes

2019-09-26 Thread Sean Mullan
On 9/26/19 4:20 PM, Sean Mullan wrote: Would you prefer I revise the patch where there are multiple SuppressWarnings("serial") on fields to put a single one on the class instead? Yes, but only in the cases where we are clearly using some form of alternate serialization like ASN.1 encoding. I

Re: JDK 14 RFR of JDK-8231262: Suppress warnings on non-serializable instance fields in security libs serializable classes

2019-09-26 Thread Sean Mullan
On 9/26/19 4:10 PM, Joe Darcy wrote: Hi Sean, On 9/26/2019 12:46 PM, Sean Mullan wrote: On 9/23/19 4:16 PM, Joe Darcy wrote: Hi Sean, On 9/23/2019 12:19 PM, Sean Mullan wrote: Hi Joe, It's a little odd to suppress the warnings in the X509CertImpl class since it is a subclass of

Re: JDK 14 RFR of JDK-8231262: Suppress warnings on non-serializable instance fields in security libs serializable classes

2019-09-26 Thread Joe Darcy
Hi Sean, On 9/26/2019 12:46 PM, Sean Mullan wrote: On 9/23/19 4:16 PM, Joe Darcy wrote: Hi Sean, On 9/23/2019 12:19 PM, Sean Mullan wrote: Hi Joe, It's a little odd to suppress the warnings in the X509CertImpl class since it is a subclass of java.security.cert.Certificate which implements

Re: JDK 14 RFR of JDK-8231262: Suppress warnings on non-serializable instance fields in security libs serializable classes

2019-09-26 Thread Sean Mullan
On 9/23/19 4:16 PM, Joe Darcy wrote: Hi Sean, On 9/23/2019 12:19 PM, Sean Mullan wrote: Hi Joe, It's a little odd to suppress the warnings in the X509CertImpl class since it is a subclass of java.security.cert.Certificate which implements the writeReplace method so these fields are not

Re: JDK 14 RFR of JDK-8231262: Suppress warnings on non-serializable instance fields in security libs serializable classes

2019-09-23 Thread Joe Darcy
Hi Sean, On 9/23/2019 12:19 PM, Sean Mullan wrote: Hi Joe, It's a little odd to suppress the warnings in the X509CertImpl class since it is a subclass of java.security.cert.Certificate which implements the writeReplace method so these fields are not serialized. Also for other classes like

Re: JDK 14 RFR of JDK-8231262: Suppress warnings on non-serializable instance fields in security libs serializable classes

2019-09-23 Thread Sean Mullan
Hi Joe, It's a little odd to suppress the warnings in the X509CertImpl class since it is a subclass of java.security.cert.Certificate which implements the writeReplace method so these fields are not serialized. Also for other classes like X509Key which are internal it is a little odd to

Re: JDK 14 RFR of JDK-8231262: Suppress warnings on non-serializable instance fields in security libs serializable classes

2019-09-21 Thread Joe Darcy
On 9/21/2019 4:15 AM, Chris Hegarty wrote: On 19 Sep 2019, at 18:32, Joe Darcy wrote: Hello, Ahead of augmenting javac's serial lint checks under JDK-8160675, it would be helpful to mark fields in security libs classes where the class is serializable, but a non-transient instance field

Re: JDK 14 RFR of JDK-8231262: Suppress warnings on non-serializable instance fields in security libs serializable classes

2019-09-21 Thread Chris Hegarty
> On 19 Sep 2019, at 18:32, Joe Darcy wrote: > > Hello, > > Ahead of augmenting javac's serial lint checks under JDK-8160675, it would be > helpful to mark fields in security libs classes where the class is > serializable, but a non-transient instance field does *not* have a > serialiable

Re: JDK 14 RFR of JDK-8231262: Suppress warnings on non-serializable instance fields in security libs serializable classes

2019-09-19 Thread Peter Firmstone
Hello, I'd make an exception for interfaces, often these are not serializable, but their implementations may be, in this case a warning would be spurious. Regards, Peter. On 20/09/2019 3:32 AM, Joe Darcy wrote: Hello, Ahead of augmenting javac's serial lint checks under JDK-8160675, it

JDK 14 RFR of JDK-8231262: Suppress warnings on non-serializable instance fields in security libs serializable classes

2019-09-19 Thread Joe Darcy
Hello, Ahead of augmenting javac's serial lint checks under JDK-8160675, it would be helpful to mark fields in security libs classes where the class is serializable, but a non-transient instance field does *not* have a serialiable type. Such classes may have difficulties being serialized at