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

[Xuelei Fan]

On 8/20/2013 9:37 AM, Weijun Wang wrote:
> 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?
> 
It's a public internal class.  It's a little better to restrict the
usage of this constructor.

> 2. Is the code change sufficient? i.e. is there other places we need to
> take care of?
> 
No similar issue found on other krb5/ssl impl.

Thanks,
Xuelei

> --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
>>>>
>>