RFR: 8015081

[Xuelei Fan]

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
>