Code review request, CR 7174244, NPE in Krb5ProxyImpl.getServerKeys()

[Xuelei Fan]

On 6/6/2012 9:11 PM, Weijun Wang wrote:
> Hi Xuelei
> 
> I don't quite understand the bug report. Certainly it's good to prevent
> that NPE, but why must the cipher suites be ordered? It seems it's not
> ordered before the new HashMap.
No, it was not ordered because of a regression.

> Is this a regression or an RFE?
It's a regression. We used to use TreeSet to sort the cipher suites. But
later it is miss-updated to use ArrayList rather then TreeSet in a bug
fix. Then the order was lost.

We need to sort the cipher suites because the order is the preference
when TLS handshaking selects a proper cipher suite. Please refer to TLS
specification about the order, page 41, RFC5246:

   cipher_suites
      This is a list of the cryptographic options supported by the
      client, with the client's first preference first.

> You'd
> better add some explanation in the bug report.
> 
I will.

Thanks,
Xuelei

> Thanks
> Max
> 
> On 06/06/2012 08:27 PM, Xuelei Fan wrote:
>> Hi Weijun,
>>
>> Would you please review the fix for CR 7174244.
>>      http://cr.openjdk.java.net./~xuelei/7174244/webrev.00/
>>
>> There are mainly two updates in the fix. One is to have
>> Krb5ProxyImpl.getServerKeys()  to check null return value of
>> Krb5Util.getServiceCreds(). The other one is to have cipher suite order
>> to be independent from HashMap implementation.  TreeSet is used to
>> replace ArrayList so that the cipher suites order are sorted
>> automatically.
>>
>> Thanks,
>> Xuelei