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 something like :
return StackWalker.getInstance().walk(s ->
s.dropWhile((f ->
f.getClassName().startsWith("sun.security.ssl.SSLLogger")))
.map(f -> f.getClassName() + ":" +
f.getLineNumber())
.findFirst().orElse("unknown caller"));
Good point!
Looks like this method is not used :
406 private static boolean isLoggerImpl(StackTraceElement ste) {
407 String cn = ste.getClassName();
408 System.out.println("class name: " + cn);
409 return cn.equals("sun.security.ssl.SSLLogger");
410 }
Removed.
Also the SSLSimpleFormatter class has dateFormat variable stored in a
ThreadLocal. Is there a reason for that ?
SimpleDateFormat is not multiple thread-safe. There were some expected
issues if using SimpleDateFormat instance directly in multiple threads
environment.
Thanks,
Xuelei
regards,
Sean.
On 08/06/2018 02:40, Xuelei Fan wrote:
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
/**
* Logs a message with resource bundle and an optional list of
* parameters.
* ...
* @param format the string message format in {@link
* java.text.MessageFormat} format, (or a key in the message
* catalog if {@code bundle} is not {@code null}); can be
{@code null}.
* @param params an optional list of parameters to the
message (may be
* none).
* ...
*/
public void log(Level level, ResourceBundle bundle, String
format,
Object... params);
Especially, the String parameter is in MessageFormat format (that's
why it's named "format" but not "message"). However,
SSLConsoleLogger::log is always called with format being a plain
string and params are stacked in the output separated by command and
newline.
My opinion is that this is not the normal style of using a Logger and
we should not allow system logger (through -Djavax.net.debug=). If
you still see benefit in that, can we make this change at the moment?
- logger.log(level, msg, formatted);
+ if (logger instanceof SSLConsoleLogger) {
+ logger.log(level, msg, formatted);
+ } else {
+ logger.log(level, msg + ": " + formatted);
+ }
Thanks
Max
On Jun 7, 2018, at 11:24 AM, Weijun Wang <weijun.w...@oracle.com>
wrote:
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 <xuelei....@oracle.com> wrote:
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 debug logger is specified by
System.getLogger("javax.net.ssl"),
* and applications can customize and configure the logger or use
external
* logging mechanisms. If the system property "javax.net.debug" is
defined
* and non-empty, a private debug logger implemented in this class
is used.
*/
On 6/6/2018 6:37 PM, Weijun Wang wrote:
But will any application use the logger named "javax.net.debug"?
I think that's only for JSSE.
If using System Logger, the "javax.net.debug" system property will
be used to turn on/off the logger (define with empty or not defined).
Xuelei
On Jun 7, 2018, at 9:35 AM, Xuelei Fan<xuelei....@oracle.com>
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 themselves if needed. That's also the purpose
why we introduce the System Logger in the provider.
