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