RFR 8072394: java.security.cert.PolicyQualifierInfo needs value-based equality

[Florian Weimer]

On 02/20/2015 11:23 PM, Sean Mullan wrote:
> On 02/17/2015 09:30 AM, Florian Weimer wrote:
>> On 02/16/2015 11:13 PM, Sean Mullan wrote:
>>
>>>> Based on that, PolicyQualifierInfo should have implemented value-based
>>>> equals() and hashCode(), and the identity-based set is just a bug. 
>>>> (But
>>>> the requirement I cited is a stronger requirement the Set would not
>>>> enforce.)
>>>>
>>>> However, I think it's too late to fix this bug now.  That's why I just
>>>> added the identity counter.  If you want the behavioral change instead,
>>>> I can implement that as well.
>>>
>>> Maybe it's not too late. This is not a commonly used class, and the
>>> compatibility risk is probably fairly low. If you code up the changes, I
>>> can file a CCC on your behalf.
>>
>> Updated webrev: <http://cr.openjdk.java.net/~fweimer/8072394/webrev.01/>
> 
> You need to add a description for the overridden equals/hashCode
> methods, ex:

Okay, I didn't know that.

I don't like specifying the hash code so strongly.  But if we want to
fix it at Arrays.hashCode(byte[]), it's probably better to implement
Comparable as well, to aid the collision resolution in HashMap.  The new
webrev does that:

  <http://cr.openjdk.java.net/~fweimer/8072394/webrev.02/>

I also compute the hashcode unconditionally in the constructor because
PolicyQualifierInfo objects are used with sets, so their hashcode is so
often needed.

Funny how I wanted to make things go faster, and ended up introducing a
slowdown.  But correctness over performance.

Most policy qualifier sets in X.509 certificates seem to have just one
element, so I looked at optimizing that, but there's a lot of copying
going on inside the implementation, and I'd have to special-case the one
element case in multiple locations.  Switching from a HashSet to a
TreeSet would be externally visible, so that doesn't seem to be a good
idea, either.

We need an optimized immutable set class in the JDK. :-)

-- 
Florian Weimer / Red Hat Product Security