On 11/28/2011 12:34 PM, Weijun Wang wrote: > > On 11/28/2011 11:27 AM, Xuelei Fan wrote: >> webrev: http://cr.openjdk.java.net/~xuelei/7115524/webrev.02/ >> >> I think the class is special in that in the class the client is not >> really want to establish a HTTPS/TLS connection with the server. The >> purpose of the class is to get the server certificate. It does not >> matter whether the TLS connection is negotiated or not. So the class >> only need a parser that can read the server certificate during the TLS >> handshaking. The "security" functions (for example, encryption >> decryption digest, etc.) do not make sense. In such situation, we really >> don't mind what's the underlying providers. >> >> Does it make sense? > > Yes. > > But I really don't think you need to change this behavior. If this class > is not used by anyone else except KeyTool, there is no difference > between a static field and a local variable. I really don't like static > fields. > I think it is only used by KeyTool at present. But I really hate to see multiple SSLContext instances and TM/TM instances in an application, it is designed to be immutable.
I have no special preference about static fields and local variables. If the local variable is always the same across the class life cycle, I prefer static field; otherwise, I prefer local variable. >> >> I make a update, so that even the TLS handshaking failed or other IO >> exceptions, once we are able to get the server certificate, we will >> use it. > > This is good, and you can add a comment on this. > There is a two-line comment in the update. Thanks, Xuelei > Thanks > Max > >> >> Thanks, >> Xuelei >> >> >> On 11/27/2011 10:31 PM, Xuelei Fan wrote: >>> On 11/27/2011 7:07 PM, Weijun Wang wrote: >>>> >>>> >>>> On 11/27/2011 05:48 PM, Xuelei Fan wrote: >>>>> >>>>> >>>>> Sent from my iPad >>>>> >>>>> On Nov 27, 2011, at 5:01 PM, Weijun Wang<weijun.w...@oracle.com> >>>>> wrote: >>>>> >>>>>> Anyway, I find this a little unsafe. One can add a new SSL security >>>>>> provider at run time, and then run this method, and the new provider >>>>>> can be loaded. >>>>>> >>>>> I did not follow your ideas. The new provider is loaded, and what's >>>>> the worries then? >>>> >>>> Typo, I meant "cannot". >>>> >>>> 1. Run this method, the default SSL provider loaded >>>> 2. Add a new SSL security provider >>>> 3. Run this method again, still the old provider. >>>> >>> Got it. But this class is very special in that the security provider may >>> be useless. >>> >>> It is a little complicated. I will call you tomorrow to explain more. I >>> think we still have space to improve the stability and performance based >>> on the latest update. >>> >>> Thanks, >>> Xuelei