8209992: Align SSLSocket and SSLEngine Javadocs

[artur-oracle]

Add a javadoc paragraph to SSLSocket.startHandshake() and SSLEngine.beginHandshake() methods explaining behavioral differences between TLSv1.3+ and TLSv1.2- versions.

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change requires CSR request JDK-8364223 to be approved

Issues

• JDK-8209992: Align SSLSocket and SSLEngine Javadocs (Bug - P4)
• JDK-8364223: Align SSLSocket and SSLEngine Javadocs (CSR)

Reviewers

• Bradford Wetmore (@bradfordwetmore - Reviewer)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 26484

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

Using diff file

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

Using Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back abarashev! 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]

@artur-oracle 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:

8209992: Align SSLSocket and SSLEngine Javadocs

Reviewed-by: wetmore

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 145 new commits pushed to the master branch:

• f40381e: 8356760: VectorAPI: Optimize VectorMask.fromLong for all-true/all-false cases
• e801e51: 8306324: StopThread results in thread being marked as interrupted, leading to unexpected InterruptedException
• 7ea08d3: 8362244: Devkit's Oracle Linux base OS keyword is incorrectly documented
• ... and 142 more: https://git.openjdk.org/jdk/compare/f8c8bcf4fd31509fdb40d32e8e16ba4fba1f987d...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]

@artur-oracle The following labels will be automatically applied to this pull request:

• net
• security

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

[mlbridge]

Webrevs

• 03: Full - Incremental (5779ff34)
• 02: Full - Incremental (8cf4e12f)
• 01: Full - Incremental (d5723cb8)
• 00: Full (0a243a22)

[bradfordwetmore]

The renegotiation wording should probably updated in both the SSLSocket/SSLEngine section. As was pointed out, TLSv1.3 is not a full renegotiation.

You could soften some of the wording with something like (needs wordsmithing):

The behavior of this method is protocol- and possibly implementation- dependent:  For 
example, in TLSv1.2 and earlier could do X, and in TLSv1.3 calling this method would do
Y.

I think the submitter was also asking for a similar layout/wording between the two, while highlighting the differences where needed. This would make the learning curve a bit smaller.

This probably will need a CSR.

[artur-oracle]

The renegotiation wording should probably updated in both the SSLSocket/SSLEngine section. As was pointed out, TLSv1.3 is not a full renegotiation.

You could soften some of the wording with something like (needs wordsmithing):

The behavior of this method is protocol- and possibly implementation- dependent:  For 
example, in TLSv1.2 and earlier could do X, and in TLSv1.3 calling this method would do
Y.

I think the submitter was also asking for a similar layout/wording between the two, while highlighting the differences where needed. This would make the learning curve a bit smaller.

This probably will need a CSR.

Good point, I pushed an update. While both methods are similar, they are not the same due to the very nature of SSLEngine. So I made the first 2 paragraphs of the javadoc the same for both methods, the highlighting the differences follows in next paragraphs.

[bradfordwetmore]

A few thoughts.

• We usually set the first sentence in a paragraph by itself in javadoc.
• Believe it or not, we had to artificially create a different method name when creating SSLEngine between startHandshake() and beginHandshake(): (start implies it completes synchronously, whereas begin only begins it. Yeah, that's a pretty vague meaning, but it was forced on us in JDK 1.5 when SSLEngine was created.) So should keep the wording similar in the javadoc.
• In TLSv1.3 and later versions: we don't know what the handshaking will be like in the next version of TLS, so I'd like to just keep as In TLSv1.3.

These comments apply to both SSLEngine/SSLSocket. Just the intro sentence needs to change for SSLEngine.
I'll review the changes in the CSR after you've looked over this.

Some minor nits/formats change suggestions:

diff --git a/src/java.base/share/classes/javax/net/ssl/SSLSocket.java b/src/java.base/share/classes/javax/net/ssl/SSLSocket.java
index ca930c7fb83..bfced714825 100644
--- a/src/java.base/share/classes/javax/net/ssl/SSLSocket.java
+++ b/src/java.base/share/classes/javax/net/ssl/SSLSocket.java
@@ -502,16 +502,18 @@ public abstract void removeHandshakeCompletedListener(
     /**
-     * Initiates handshaking on this connection. Common reasons include a need
-     * to initiate a new session, to use new encryption keys or to change
-     * cipher suites. To force complete reauthentication, the current session
-     * could be invalidated before starting this handshake.
+     * Starts handshaking on this {@code SSLSocket}.  (or for)
+     * Begins handshaking on this {@code SSLEngine}.
+     * <P>
+     * Common reasons include a need to initiate a new protected session,
+     * create new encryption keys, or to change cipher suites. To force
+     * complete reauthentication, the current session should be invalidated
+     * before starting this handshake.
      * <P>
      * The behavior of this method is protocol (and possibly implementation)
-     * dependent. For example, in TLSv1.3 and later versions calling this
-     * method after the connection has been established will force producing
-     * a KeyUpdate message. For prior TLS versions it will force a
-     * renegotiation (re-handshake).
+     * dependent. For example, in TLSv1.3 calling this method after the
+     * connection has been established will force a key update.  For prior TLS
+     * versions it will force a renegotiation (re-handshake).
      * <P>
      * If data has already been sent on the connection, it continues
      * to flow during this handshake.  When the handshake completes, this

[bradfordwetmore]

A couple minor fixes.

I'll approve next round.

[bradfordwetmore]

LGTM. Thanks for the updates.

[artur-oracle]

/integrate

[openjdk]

Going to push as commit 6c52b73.
Since your change was applied there have been 162 commits pushed to the master branch:

• 567c0c9: 8354244: Use random data in MinMaxRed_Long data arrays
• fc47555: 8364516: Serial: Move class unloading logic inside SerialFullGC::invoke_at_safepoint
• a9f3d3a: 8364177: JDK fails to build due to undefined symbol in libpng on LoongArch64
• ... and 159 more: https://git.openjdk.org/jdk/compare/f8c8bcf4fd31509fdb40d32e8e16ba4fba1f987d...master

Your commit was automatically rebased without conflicts.

[openjdk]

@artur-oracle Pushed as commit 6c52b73.

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