8343395: SSLLogger doesn't work for formatted messages

[coffeys]

trafficstars

SLSLogger is broken when used with with System.Logger (-Djavax.net.debug mode)

SSL Debug messages don't use format specifiers. As a result, any custom format data isn't printed. Proposed solution is to append the SSL custom format output to the original debug message. Similar approach used for SSLConsoleLogger mode.

Used this opportunity to delete some old commented code and to replace use of "\n" with System.lineSeparator()

DebugPropertyValuesTest also updated to test new logic.

---

Progress

• [ ] Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• [x] Change must not contain extraneous whitespace
• [x] Commit message must refer to an issue

Issue

• JDK-8343395: SSLLogger doesn't work for formatted messages (Bug - P4)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/25934/head:pull/25934
$ git checkout pull/25934

Update a local copy of the PR:
$ git checkout pull/25934
$ git pull https://git.openjdk.org/jdk.git pull/25934/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 25934

View PR using the GUI difftool:
$ git pr show -t 25934

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/25934.diff

Using Webrev

Link to Webrev Comment

[bridgekeeper[bot]]

:wave: Welcome back coffeys! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk[bot]]

@coffeys This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8343395: SSLLogger doesn't work for formatted messages

Reviewed-by: weijun

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 371 new commits pushed to the master branch:

• 695e36b0031be4d013ad149a0f23c36c0669c422: 8367927: Remove 8043571-related tests from problemlists
• 16458b60c9ccdfac60140c8186f31d5d8a57f2f9: 8367725: Incorrect reading of oop in SuspendResumeManager::suspend while thread is blocked
• 1512d889dee2adb6d4536202dc7f205e5daf6fe7: 8348278: Trim InitialRAMPercentage to improve startup in default modes
• ... and 368 more: https://git.openjdk.org/jdk/compare/2b44ed70707175f87ba962d8a6ce6bbc2c8737bf...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[openjdk[bot]]

@coffeys The following label will be automatically applied to this pull request:

• security

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[mlbridge[bot]]

Webrevs

• 06: Full - Incremental (beba07b2)
• 05: Full - Incremental (bb0df658)
• 04: Full - Incremental (82c5427b)
• 03: Full (4b740d23)
• 02: Full - Incremental (e42c2001)
• 01: Full - Incremental (51eb0f1b)
• 00: Full (9e589a63)

[bridgekeeper[bot]]

@coffeys This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply issue a /touch or /keepalive command to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

[openjdk[bot]]

@coffeys this pull request can not be integrated into master due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:

git checkout 8343395-formatter
git fetch https://git.openjdk.org/jdk.git master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push

[bridgekeeper[bot]]

@coffeys This pull request has been inactive for more than 8 weeks and will now be automatically closed. If you would like to continue working on this pull request in the future, feel free to reopen it! This can be done using the /open pull request command.

[coffeys]

/open

[openjdk[bot]]

@coffeys This pull request is now open

[wangweij]

Does this mean the parameters argument in the SSLSimpleFormatter::format method is always empty? And then all those messageCompactFormatWithParas and messageFormatWithParas are useless?

[coffeys]

Does this mean the parameters argument in the SSLSimpleFormatter::format method is always empty? And then all those messageCompactFormatWithParas and messageFormatWithParas are useless?

SSLSimpleFormatter.formatParameters(params) is still used and contains logic to decode JSSE Objects for logging purposes.

On your specific question regarding messageCompactFormatWithParas / messageFormatWithParas

correct - they appear to have been designed for SSLConsoleLogger use. The System.Logger option delegates back to the logging framework regarding output format.

The default System Logger format already includes some fields. e.g. :

Sep 18, 2025 9:10:00 AM sun.security.ssl.SSLLogger log
FINE: Ignore disabled cipher suite: TLS_ECDH_RSA_WITH_AES_256_GCM_SHA384

[wangweij]

Since you effectively combine the message and the extra objects, the "expand" style of log is modified, from

  "message"     : "Produced ClientHello handshake message",
  "specifics"   : [
     ...
   ]

to

  "message"     : "Produced ClientHello handshake message:...

Although the current output is no pure legal JSON, the combination makes it worse if there are quotation marks inside the text for extra objects.

Update: When SSLConsoleLogger implements the log method with a params argument, it breaks the contract to modify the format argument to a bare message, which does not include any format information. One workaround I can think of now is to check instanceof before your current change and only combine them when it's not an SSLConsoleLogger.

BTW, I don't think it's worth a lambda there. isLoggable is already true.

[coffeys]

thanks Weijun. I'd missed that point. Yes- I've already tested the solution where we test for "instanceof SSLConsoleLogger" - seems to work fine, I've updated the test case also.

wondering if the System Logger should attempt to print to a similar format. Will get back to you on this.

[coffeys]

Thanks for the review Weijun.

/integrate

[openjdk[bot]]

Going to push as commit 47efe3c794c241b7534eac597b3dd03d571677f1. Since your change was applied there have been 403 commits pushed to the master branch:

• bdfe05b595d86c62f7dad78549023a3426423679: 8368071: Compilation throughput regressed 2X-8X after JDK-8355003
• c3aaa8751acfd795207f1a509b6e170e6a753c69: 8361955: [GCC static analyzer] libjdwp/threadControl.c threadControl_setPendingInterrupt error: dereference of NULL 'node'
• 58270b757c0bdf82bf753fa304b829e3b64196e4: 8346839: [TESTBUG] "java/awt/textfield/setechochartest4/setechochartest4.java" failed because the test frame disappears on clicking "Click Several Times" button
• ... and 400 more: https://git.openjdk.org/jdk/compare/2b44ed70707175f87ba962d8a6ce6bbc2c8737bf...master

Your commit was automatically rebased without conflicts.

[openjdk[bot]]

@coffeys Pushed as commit 47efe3c794c241b7534eac597b3dd03d571677f1.

:bulb: You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.