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
