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

Reply via email to