Re: SSLLogger.java (was Re: Code Review Request: TLS 1.3 Implementation)

2018-06-11 Thread Xuelei Fan
Hi Daniel, It's a good idea. I added your comment to the TLS 1.3 implementation issues tracking enhancement: https://bugs.openjdk.java.net/browse/JDK-8204636 Thanks, Xuelei On 6/11/2018 2:28 AM, Daniel Fuchs wrote: Hi Xuelei, Just a note that it might be a better idea to rework the imp

Re: SSLLogger.java (was Re: Code Review Request: TLS 1.3 Implementation)

2018-06-11 Thread Daniel Fuchs
Hi Xuelei, Just a note that it might be a better idea to rework the implementation of SSLLogger/SSLConsoleLogger a bit in order to have SSLLogger implement System.Logger. This would ensure that the SSLLogger class is skipped when looking for the caller, when the underlying logger is a logger ret

Re: SSLLogger.java (was Re: Code Review Request: TLS 1.3 Implementation)

2018-06-10 Thread Xuelei Fan
Update: http://hg.openjdk.java.net/jdk/sandbox/rev/e4fe7c97b1de On 6/9/2018 2:42 AM, Seán Coffey wrote: Some comments on SSLLogger also : formatCaller() uses getStackTrace() to walk the stack. It's probably more expensive than using the newer Stackwalker class. Could it be replaced with somet

Re: SSLLogger.java (was Re: Code Review Request: TLS 1.3 Implementation)

2018-06-09 Thread Seán Coffey
Some comments on SSLLogger also : formatCaller() uses getStackTrace() to walk the stack. It's probably more expensive than using the newer Stackwalker class. Could it be replaced with something like : return StackWalker.getInstance().walk(s -> s.dropWhile((f -> f.g

Re: SSLLogger.java (was Re: Code Review Request: TLS 1.3 Implementation)

2018-06-07 Thread Xuelei Fan
I got your ideas. Let's see if we can make a change, a little bit later. Thanks, Xuelei On 6/7/2018 6:23 PM, Weijun Wang wrote: My final comment on this class: Since SSLConsoleLogger is a Logger child, its log(..., params) method should follow the Logger::log spec, which has /**

Re: SSLLogger.java (was Re: Code Review Request: TLS 1.3 Implementation)

2018-06-07 Thread Weijun Wang
My final comment on this class: Since SSLConsoleLogger is a Logger child, its log(..., params) method should follow the Logger::log spec, which has /** * Logs a message with resource bundle and an optional list of * parameters. * ... * @param format th

Re: SSLLogger.java (was Re: Code Review Request: TLS 1.3 Implementation)

2018-06-06 Thread Weijun Wang
I think I understand. So if a user sets it to empty, they will also need to create a customized logger/formatter like SSLLogger that is able to output a parameter without a placeholder in msg. --Max > On Jun 7, 2018, at 9:51 AM, Xuelei Fan wrote: > > In this implementation, the SunJSSE loggin

Re: SSLLogger.java (was Re: Code Review Request: TLS 1.3 Implementation)

2018-06-06 Thread Xuelei Fan
In this implementation, the SunJSSE logging is updated to (the class comment lines): /** * Implementation of SSL logger. * * If the system property "javax.net.debug" is not defined, the debug logging * is turned off. If the system property "javax.net.debug" is defined as * empty, the deb

Re: SSLLogger.java (was Re: Code Review Request: TLS 1.3 Implementation)

2018-06-06 Thread Weijun Wang
But will any application use the logger named "javax.net.debug"? I think that's only for JSSE. > On Jun 7, 2018, at 9:35 AM, Xuelei Fan wrote: > > I see your concerns now. Currently, we don't fine the format if using System > logger. Applications can define the format and output style for th

Re: SSLLogger.java (was Re: Code Review Request: TLS 1.3 Implementation)

2018-06-06 Thread Xuelei Fan
I see your concerns now. Currently, we don't fine the format if using System logger. Applications can define the format and output style for themselves if needed. That's also the purpose why we introduce the System Logger in the provider. Xuelei On 6/6/2018 6:10 PM, Weijun Wang wrote: I a

Re: SSLLogger.java (was Re: Code Review Request: TLS 1.3 Implementation)

2018-06-06 Thread Weijun Wang
I assume this output is for the internal SSLLogger. I was asking what would be printed if someone only set "-Djavax.net.debug" and a System logger is used. --Max > On Jun 7, 2018, at 8:54 AM, Xuelei Fan wrote: > > > > On 6/6/2018 5:46 PM, Weijun Wang wrote: >>> On Jun 7, 2018, at 8:41 AM, Xu

Re: SSLLogger.java (was Re: Code Review Request: TLS 1.3 Implementation)

2018-06-06 Thread Xuelei Fan
On 6/6/2018 5:46 PM, Weijun Wang wrote: On Jun 7, 2018, at 8:41 AM, Xuelei Fan wrote: On 6/6/2018 4:21 PM, Weijun Wang wrote: On Jun 7, 2018, at 12:27 AM, Xuelei Fan wrote: On 6/6/2018 5:41 AM, Weijun Wang wrote: There are lots of calls like RSAClientKeyExchangeMessage ckem =

Re: SSLLogger.java (was Re: Code Review Request: TLS 1.3 Implementation)

2018-06-06 Thread Weijun Wang
> On Jun 7, 2018, at 8:41 AM, Xuelei Fan wrote: > > > > On 6/6/2018 4:21 PM, Weijun Wang wrote: >>> On Jun 7, 2018, at 12:27 AM, Xuelei Fan wrote: >>> >>> On 6/6/2018 5:41 AM, Weijun Wang wrote: There are lots of calls like RSAClientKeyExchangeMessage ckem = ne

Re: SSLLogger.java (was Re: Code Review Request: TLS 1.3 Implementation)

2018-06-06 Thread Xuelei Fan
On 6/6/2018 4:21 PM, Weijun Wang wrote: On Jun 7, 2018, at 12:27 AM, Xuelei Fan wrote: On 6/6/2018 5:41 AM, Weijun Wang wrote: There are lots of calls like RSAClientKeyExchangeMessage ckem = new RSAClientKeyExchangeMessage(shc, message); if (SSLLogger.isOn && SSLLogge

Re: SSLLogger.java (was Re: Code Review Request: TLS 1.3 Implementation)

2018-06-06 Thread Weijun Wang
> On Jun 7, 2018, at 12:27 AM, Xuelei Fan wrote: > > On 6/6/2018 5:41 AM, Weijun Wang wrote: >> There are lots of calls like >>RSAClientKeyExchangeMessage ckem = >>new RSAClientKeyExchangeMessage(shc, message); >>if (SSLLogger.isOn && SSLLogger.isOn("ssl,handshake")) { >>

Re: SSLLogger.java (was Re: Code Review Request: TLS 1.3 Implementation)

2018-06-06 Thread Xuelei Fan
On 6/6/2018 5:41 AM, Weijun Wang wrote: There are lots of calls like RSAClientKeyExchangeMessage ckem = new RSAClientKeyExchangeMessage(shc, message); if (SSLLogger.isOn && SSLLogger.isOn("ssl,handshake")) { SSLLogger.fine( "Consuming RSA ClientKeyExchange

SSLLogger.java (was Re: Code Review Request: TLS 1.3 Implementation)

2018-06-06 Thread Weijun Wang
There are lots of calls like RSAClientKeyExchangeMessage ckem = new RSAClientKeyExchangeMessage(shc, message); if (SSLLogger.isOn && SSLLogger.isOn("ssl,handshake")) { SSLLogger.fine( "Consuming RSA ClientKeyExchange handshake message", ckem); } which finally