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

Joe Darcy joe.darcy at oracle.com
Wed Oct 9 01:56:46 UTC 2019


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 where we are clearly using some form of 
>> alternate serialization like ASN.1 encoding. I need to double-check 
>> the review again (it's a bit more time consuming because I have to 
>> look at the code in more detail), but the two that I spotted so far are:
>>
>> src/java.base/share/classes/sun/security/x509/X509CertImpl.java
>> src/java.security.jgss/share/classes/sun/security/krb5/internal/KRBError.java 
>> (from the JDK-8231368 review)
>
> Ok, I double-checked everything. The only other class in the webrev 
> that uses an alternate serial form is:
>
> sun/security/x509/X509Key.java
>
> but since that only has one field that is not Serializable, it 
> probably is ok to leave as-is.
>
>
I've put the updated webrev at:

     http://cr.openjdk.java.net/~darcy/8231262.1/

The difference from the first webrev is I marked a field in X509Key.java 
as transient; diff of webrev patches:

 > --- old/src/java.base/share/classes/sun/security/x509/X509Key.java 
2019-10-08 18:37:02.143999531 -0700
 > +++ new/src/java.base/share/classes/sun/security/x509/X509Key.java 
2019-10-08 18:37:01.815999531 -0700
 > @@ -84,7 +84,7 @@
214,215c214,215
< +    @SuppressWarnings("serial") // Not statically typed as Serializable
<      private BitArray bitStringKey = null;
---
 > -    private BitArray bitStringKey = null;
 > +    private transient BitArray bitStringKey = null;

For src/java.base/share/classes/sun/security/x509/X509CertImpl.java, I 
don't see any of the read* or write* methods associated with 
serialization defined in the class. I would seem clearer to me to having 
the @SuppressWarning("serial") at each field, but if you'd prefer to 
have it at the class level, I'll defer to your judgement on the matter.

FYI, of the files being modified only 
src/java.base/share/classes/javax/security/auth/Subject.java uses 
readFields or putFields.

Thanks,

-Joe




More information about the security-dev mailing list