On Tue, 2 Sep 2025 14:27:20 GMT, Sean Coffey <[email protected]> wrote:

>> The `javax.net.debug` TLS debug option is buggy since TLSv1.3 implementation 
>> was introduced many years ago.
>> 
>> Where "ssl" was previously a value to obtain all TLS debug traces (except 
>> network type dumps, verbose data), it now prints only a few lines for a 
>> standard client TLS connection. 
>> 
>> The property parsing was also lax and allowed users to declare verbose 
>> logging options by themselves where the documentation stated that such 
>> verbose options were only meant to be used in conjunction with other TLS 
>> options :
>> 
>> 
>>         System.err.println("help           print the help messages");
>>         System.err.println("expand         expand debugging information");
>>         System.err.println();
>>         System.err.println("all            turn on all debugging");
>>         System.err.println("ssl            turn on ssl debugging");
>>         System.err.println();
>>         System.err.println("The following can be used with ssl:");
>>         System.err.println("\trecord       enable per-record tracing");
>>         System.err.println("\thandshake    print each handshake message");
>>         System.err.println("\tkeygen       print key generation data");
>>         System.err.println("\tsession      print session activity");
>>         System.err.println("\tdefaultctx   print default SSL 
>> initialization");
>>         System.err.println("\tsslctx       print SSLContext tracing");
>>         System.err.println("\tsessioncache print session cache tracing");
>>         System.err.println("\tkeymanager   print key manager tracing");
>>         System.err.println("\ttrustmanager print trust manager tracing");
>>         System.err.println("\tpluggability print pluggability tracing");
>>         System.err.println();
>>         System.err.println("\thandshake debugging can be widened with:");
>>         System.err.println("\tdata         hex dump of each handshake 
>> message");
>>         System.err.println("\tverbose      verbose handshake message 
>> printing");
>>         System.err.println();
>>         System.err.println("\trecord debugging can be widened with:");
>>         System.err.println("\tplaintext    hex dump of record plaintext");
>>         System.err.println("\tpacket       print raw SSL/TLS packets");
>> 
>> 
>> as part of this patch, I've also moved the log call to the more performant 
>> friendly 
>> `System.Logger#log(java.lang.System.Logger.Level,java.util.function.Supplier)`
>>  method. 
>> 
>> the output has changed slightly with respect to that  - less verbose
>> 
>> e.g. old...
>
> Sean Coffey has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Further review comments, copyright years also

A couple more minor comments. 

All copyrights look good.  (Thanks.)

src/java.base/share/classes/sun/security/ssl/SSLLogger.java line 217:

> 215:                 "print each handshake message");
> 216:         System.err.printf("      %-12s   %s%n", "verbose",
> 217:                 "-verbose handshake message printing (widens 
> handshake)");

Minor nit.  Having "-" at the beginning of the textual description on these 
"wideners" looks odd to my eye.  "-" are usually for in front of the command 
args.  e.g.  

    The following filters can be used with ssl:
        ....
        handshake      print each handshake message
          verbose        -verbose handshake message printing (widens handshake)
        record         enable per-record tracing
          packet         -print raw SSL/TLS packets (widens record)
          plaintext      -hex dump of record plaintext (widens record)

src/java.base/share/classes/sun/security/ssl/SSLLogger.java line 236:

> 234:         System.err.printf("    %-14s %s%n", "trustmanager",
> 235:                 "print trust manager tracing");
> 236:         System.err.println();

Do we want to include something like this at the bottom?:

> Adding filters to "ssl" will filter log messages to include just those 
> categories.  If "ssl" is specified by itself, all non-widening filters are 
> enabled.

-------------

PR Review: https://git.openjdk.org/jdk/pull/18764#pullrequestreview-3214120399
PR Review Comment: https://git.openjdk.org/jdk/pull/18764#discussion_r2342697466
PR Review Comment: https://git.openjdk.org/jdk/pull/18764#discussion_r2342710103

Reply via email to