Merge changes from tls-channel to prevent accidentally calling SSLEng…

[vbabanin]

See related bug report: marianobarrios/tls-channel#197
The PR from upstream has been manually merged: marianobarrios/tls-channel#201

JAVA-5797

[Copilot]

Pull Request Overview

This PR merges upstream changes to prevent unintended duplicate calls to SSLEngine.beginHandshake by replacing the negotiated flag with separate handshakeStarted and handshakeCompleted flags.

• Replaces the "negotiated" flag with "handshakeStarted" and "handshakeCompleted".
• Adds a guard to prevent multiple invocations of SSLEngine.beginHandshake while retaining backward compatibility.

[Copilot]

Setting handshakeCompleted to true immediately may lead to an inconsistent handshake state if the subsequent session initialization callback fails. Consider moving this assignment to after a successful execution of the callback or resetting it on failure.

Suggested change

	handshakeCompleted = true;
	// call client code
	try {
	initSessionCallback.accept(engine.getSession());
	// call client code
	try {
	initSessionCallback.accept(engine.getSession());
	handshakeCompleted = true; // set only after successful callback execution

[rozza]

Perhaps a bad recommendation? However, best to confirm if initSessionCallback could be called twice and if that has any ramifications.

[vbabanin]

Yeah, I think in this case the Copilot suggestion does not apply.
Based on the TLSChannelImpl javadoc, the callback is only invoked when the TLS session is established or re-established:

"Register a callback function to be executed when the TLS session is established (or re-established)..."

mongo-java-driver/driver-core/src/main/com/mongodb/internal/connection/tlschannel/TlsChannelBuilder.java

Line 93 in 7cc4be2

public T withSessionInitCallback(Consumer<SSLSession> sessionInitCallback) {

If the callback fails, it doesn't indicate that the session was not established or in partial state. Internally, we still mark the handshake as completed to prevent re-establishing the session again on the next read or write. So the callback doesn’t control session validity - it's more of a post-handshake hook. Also, we rely on the default session callback, which is a no-op. Reference:

mongo-java-driver/driver-core/src/main/com/mongodb/internal/connection/tlschannel/TlsChannelBuilder.java

Line 34 in 7cc4be2

Consumer<SSLSession> sessionInitCallback = session -> {};

Given this, I believe we are okay and we can copy the logic from the upstream.

[Copilot]

Pull Request Overview

This PR merges upstream changes from the tls-channel project to prevent unintended SSLEngine renegotiation by introducing explicit handshake state tracking.

• Replace the single negotiated flag with handshakeStarted and handshakeCompleted for clearer handshake lifecycle management.
• Add a guard in doHandshake() to skip engine.beginHandshake() on subsequent calls when not forcing a renegotiation.
• Ensure initSessionCallback is invoked once per completed handshake.

Comments suppressed due to low confidence (4)

driver-core/src/main/com/mongodb/internal/connection/tlschannel/impl/TlsChannelImpl.java:162

• Consider adding a Javadoc comment explaining the role of the handshakeStarted flag to clarify its lifecycle and prevent future confusion.

private boolean handshakeStarted = false;

driver-core/src/main/com/mongodb/internal/connection/tlschannel/impl/TlsChannelImpl.java:164

• Consider documenting the handshakeCompleted flag, indicating that it tracks whether the TLS handshake has finished to improve code readability.

private volatile boolean handshakeCompleted = false;

driver-core/src/main/com/mongodb/internal/connection/tlschannel/impl/TlsChannelImpl.java:531

• Consider adding a unit test for the force=true handshake path after handshakeCompleted to verify that the handshake restarts correctly and doesn’t skip necessary steps.

if (!force && handshakeCompleted) {

driver-core/src/main/com/mongodb/internal/connection/tlschannel/impl/TlsChannelImpl.java:162

• [nitpick] You might consolidate handshakeStarted and handshakeCompleted into a single enum state (e.g., NOT_STARTED, IN_PROGRESS, COMPLETED) to reduce complexity and prevent inconsistent flag usage.

private boolean handshakeStarted = false;

[vbabanin]

Added a test case, as upstream didn't include one to cover this change.