RFR: 8044609: javax.net.debug options not working and documented as expected [v26]
Bradford Wetmore
wetmore at openjdk.org
Wed Jan 28 06:34:27 UTC 2026
On Tue, 27 Jan 2026 11:44:06 GMT, Sean Coffey <coffeys at openjdk.org> 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 with a new target base due to a merge or a rebase. The pull request now contains 48 commits:
>
> - correct NamedGroup.java merge
> - Merge branch 'master' into 8044609-ssl
> - fix up test bug ID
> - fix up files post merge
> - Merge branch 'master' into 8044609-ssl
> - prep for isOn() merge
> - Merge branch 'master' into 8044609-ssl
> - Merge branch 'master' into 8044609-ssl
> - Merge branch 'master' into 8044609-ssl
> - Incorporate review comments from Brad
> - ... and 38 more: https://git.openjdk.org/jdk/compare/c69275dd...6b5c692c
Nothing major. We did most of the heavy lifting last year. Without looking at the calls sites yet, I like how the enum turned out. I am looking forward to seeing how the enums actually fit.
Just a few nits+requested comments.
I've hit the `SSLLogger.java` code pretty hard, now off to the other 500+ call sites. ;)
I'm going to check the Levels and the categories and see if we can make them more consistent, with ears towards:
[JDK-8338702](https://bugs.openjdk.org/browse/JDK-8338702)
[JDK-8344158](https://bugs.openjdk.org/browse/JDK-8344158)
src/java.base/share/classes/sun/security/ssl/SSLLogger.java line 52:
> 50: /**
> 51: * Implementation of SSL logger.
> 52: *
If you feel up to it, you could add `<p>` where needed.
Also, lines 298, 302, and 308.
src/java.base/share/classes/sun/security/ssl/SSLLogger.java line 58:
> 56: * and applications can customize and configure the logger or use external
> 57: * logging mechanisms. If the system property "javax.net.debug" is defined
> 58: * and non-empty, a private debug logger implemented in this class is used.
add something like...
the private debug logger in this class simply outputs to `System.err`.
src/java.base/share/classes/sun/security/ssl/SSLLogger.java line 62:
> 60: public final class SSLLogger implements System.Logger {
> 61: private static final System.Logger logger;
> 62: // high level boolean to track whether "all" or "ssl" option
Minor nit:
This makes it sounds like the boolean state `true` might be `all` and `false` is `ssl`.
Suggest maybe "to track whether logging is active (i.e. all/ssl)."
src/java.base/share/classes/sun/security/ssl/SSLLogger.java line 105:
> 103: if (Opt.HANDSHAKE.on && p.contains("verbose")) {
> 104: Opt.HANDSHAKE_VERBOSE.on = true;
> 105: }
Not absolutely needed given the options currently defined, but you could `replaceFirst()` on these 3 as well.
Your call.
src/java.base/share/classes/sun/security/ssl/SSLLogger.java line 130:
> 128: }
> 129: }
> 130: // javax.net.debug would be misconfigured property with respect
Nit: add extra line.
src/java.base/share/classes/sun/security/ssl/SSLLogger.java line 149:
> 147: return logging;
> 148: }
> 149: /**
Nit: add a line.
src/java.base/share/classes/sun/security/ssl/SSLLogger.java line 157:
> 155: }
> 156:
> 157: public static void severe(String msg, Object... params) {
Since we're down here, these don't really need to be `public`. Package-private is fine.
src/java.base/share/classes/sun/security/ssl/SSLLogger.java line 237:
> 235: System.err.printf("%nAdding valid filter options to \"ssl\" will log" +
> 236: " messages to include%njust those filtered categories.%n");
> 237: System.err.printf("%nIf \"ssl\" is specified by itself," +
Maybe reverse 235 and 237. ssl by itself, then ssl+filter? I've gone back/forth on this quite a bit. Do you have a preference?
src/java.base/share/classes/sun/security/ssl/SSLLogger.java line 296:
> 294: return level != Level.OFF;
> 295: }
> 296: /**
Nit: add a line.
src/java.base/share/classes/sun/security/ssl/SSLLogger.java line 311:
> 309: * Enabling subcomponents fine-tunes (filters) debug output.
> 310: */
> 311: public enum Opt {
Does this need to be public?
src/java.base/share/classes/sun/security/ssl/SSLLogger.java line 465:
> 463: formatParameters(parameters) :
> 464: Utilities.indent(formatParameters(parameters)))
> 465: };
Why did the indent get changed?
src/java.base/share/classes/sun/security/ssl/SSLLogger.java line 490:
> 488: isFirst = false;
> 489: } else {
> 490: builder.append(",\n");
Why did this get changed from `LINE_SEP` to `\n`. All of the other line separators in this file are still `LINE_SEP`.
src/java.base/share/classes/sun/security/ssl/SSLLogger.java line 494:
> 492:
> 493: if (parameter instanceof Throwable) {
> 494: builder.append(formatThrowable((Throwable)parameter));
Nit: from the Java Coding conventions, Section 8.2:
Casts should be followed by a blank. Examples:
myMethod((byte) aNum, (Object) x);
myFunc((int) (cp + 5), ((int) (i + 3))
I don't have a strong preference here.
If you do revert back, there are several places that got changed here.
-------------
PR Review: https://git.openjdk.org/jdk/pull/18764#pullrequestreview-3713815602
PR Review Comment: https://git.openjdk.org/jdk/pull/18764#discussion_r2734229865
PR Review Comment: https://git.openjdk.org/jdk/pull/18764#discussion_r2734137947
PR Review Comment: https://git.openjdk.org/jdk/pull/18764#discussion_r2734242861
PR Review Comment: https://git.openjdk.org/jdk/pull/18764#discussion_r2734786277
PR Review Comment: https://git.openjdk.org/jdk/pull/18764#discussion_r2734786894
PR Review Comment: https://git.openjdk.org/jdk/pull/18764#discussion_r2734896804
PR Review Comment: https://git.openjdk.org/jdk/pull/18764#discussion_r2734793448
PR Review Comment: https://git.openjdk.org/jdk/pull/18764#discussion_r2734912268
PR Review Comment: https://git.openjdk.org/jdk/pull/18764#discussion_r2734915243
PR Review Comment: https://git.openjdk.org/jdk/pull/18764#discussion_r2734920494
PR Review Comment: https://git.openjdk.org/jdk/pull/18764#discussion_r2734926518
PR Review Comment: https://git.openjdk.org/jdk/pull/18764#discussion_r2734982921
PR Review Comment: https://git.openjdk.org/jdk/pull/18764#discussion_r2734985908
More information about the security-dev
mailing list