Code review request 8023230, The impl of KerberosClientKeyExchange maybe not exist

Weijun Wang weijun.wang at oracle.com
Tue Aug 20 01:37:31 UTC 2013


Looks fine now.

Or maybe you can just remove KerberosClientKeyExchange()? I remember an 
empty public constrictor is equivalent to no constructor. Of course you 
made it protected so it's a little different.

When I ask if you saw a failure, I had two different questions:

1. Is the code change necessary? i.e. if there is no impl at all, can a 
program go as far as this way to this class?

2. Is the code change sufficient? i.e. is there other places we need to 
take care of?

--Max

On 8/20/13 9:24 AM, Xuelei Fan wrote:
> new webrev: http://cr.openjdk.java.net/~xuelei/8023230/webrev.01/
>
> On 8/19/2013 9:53 PM, Weijun Wang wrote:
>> Only one change I don't understand:
>>
>>    73     public KerberosClientKeyExchange() {
>>    74         if (impl == null) {
>>    75             throw new IllegalStateException("Kerberos is
>> unavailable");
>>    76         }
>>    77     }
>>
>> It seems this constructor will be automatically called when constructing
>> an instance of its child class -- KerberosClientKeyExchangeImpl. Isn't
>> that impl itself? There seems to be a chicken-or-egg puzzle here.
>>
> Good catch.  It's a weird link between KerberosClientKeyExchangeImpl and
> KerberosClientKeyExchange.
>
> Then I would like to restrict the use of this constructor, and declare
> it as protected.   See above webrev.
>
>> Also, did you really spot a failure when KerberosClientKeyExchangeImpl
>> does not exist?
>>
> Not really.  But that's the purpose of existence of
> KerberosClientKeyExchangeImpl.  Krb5 implementation should be able to be
> removed from jsse.jar.
>
> Thanks,
> Xuelei
>
>> Thanks
>> Max
>>
>>
>> On 8/19/13 8:49 PM, Xuelei Fan wrote:
>>> Hi Weijun,
>>>
>>> Please review this update when you are available.
>>>
>>> webrev: http://cr.openjdk.java.net/~xuelei/8023230/webrev.00/
>>>
>>> If package sun.security.ssl.krb5 does not exist, the impl of
>>> KerberosClientKeyExchange (krb5.KerberosClientKeyExchangeImpl) will not
>>> present as well. Need to consider this case in the implementation of
>>> sun.security.ssl.KerberosClientKeyExchange.
>>>
>>> Thanks,
>>> Xuelei
>>>
>



More information about the security-dev mailing list