RFR: 8015081

[Jamil Nimeh]

Hi Xuelei, thanks for the comments!

#1: Removal of SecureSet from AbstractSet's hierarchy was a request made 
by Sean earlier this month.  One of his concerns was to insulate 
SecureSet from changes/new methods added to AbstractSet, where the 
superclass method wouldn't work well with the implementation of SecureSet.

#2: No problem.  I'll look for any other instances of this and change them.

#3: I will change the run-time type check to look for SecureSet.

--Jamil

On 06/24/2014 08:51 AM, Xuelei Fan wrote:
> 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
>>