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 <[email protected]> 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 <[email protected]> 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<[email protected]> 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.
>
