1. Why make the following update? removeAll() is a method of AbstractSet.
private static class SecureSet<E>
- extends AbstractSet<E>
- implements java.io.Serializable {
+ implements Set<E>, java.io.Serializable {
2. Per Java coding conversions, please always use braces even for single
line conditional statement. For example:
1365 if (o == this)
1366 return true;
->
if (o == this) {
return true;
}
3. The implementation of SecureSet.equals() may not follow the spec of
equals() method. See item 8 of "Effective Java".
1368 if (!(o instanceof Set))
1369 return false;
->
1368 if (!(o instanceof SecureSet))
1369 return false;
Or I think you can re-use the spec of AbstractSet.equals() if SecureSet
extends AbstractSet.
Otherwise, looks fine to me.
Xuelei
On 6/20/2014 9:11 AM, Jamil Nimeh wrote:
> Hello all,
>
> This revision for the fix for 8015081 has the following test changes:
>
> Removal of the static byte buffers for serialized data in lieu of a more
> dynamic approach. A stripped down version of Subject in its own package
> is now being compiled alongside SubjectNullTests.java. This version of
> Subject allows the creation and serialization of Subjects with null
> elements in the principals SecureSet. Immediately following
> serialization, this special Subject's class designation is overwritten
> to be javax.security.auth.Subject. When deserialization occurs, the
> correct (JDK 9) version of Subject will be used and null element checks
> will take place.
>
> Thanks go to Weijun Wang for this testing approach.
>
> http://cr.openjdk.java.net/~weijun/8015081/webrev.07
>
> --Jamil
>
