Skip to content

Conversation

tvaron3
Copy link
Member

@tvaron3 tvaron3 commented Sep 18, 2025

Description

It is possible that in tls 1.3 with certain netty dependencies that the SDK will get a duplicate handler exception because multiple handshake completion events are emitted by netty. This pr improves the resilience of the sdk in this scenario by only adding a handler once regardless of the amount of completion events emitted. It also adds some test coverage for the ssl implementation from the jdk.

@tvaron3
Copy link
Member Author

tvaron3 commented Sep 18, 2025

/azp run java - cosmos - tests

Copy link

Azure Pipelines will not run the associated pipelines, because the pull request was updated after the run command was issued. Review the pull request again and issue a new run command.

…to tvaron3/tls1.3fix

# Conflicts:
#	sdk/cosmos/azure-cosmos/CHANGELOG.md
@tvaron3
Copy link
Member Author

tvaron3 commented Sep 18, 2025

/azp run java - cosmos - tests

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

@FabianMeiswinkel FabianMeiswinkel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - Thanks

@tvaron3
Copy link
Member Author

tvaron3 commented Sep 18, 2025

/azp run java - cosmos - tests

@tvaron3 tvaron3 marked this pull request as ready for review September 18, 2025 18:13
@tvaron3 tvaron3 requested review from kirankumarkolli and a team as code owners September 18, 2025 18:13
@Copilot Copilot AI review requested due to automatic review settings September 18, 2025 18:13
@tvaron3 tvaron3 requested a review from a team as a code owner September 18, 2025 18:13
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR improves robustness for TLS 1.3 connections by preventing duplicate handler exceptions when multiple handshake completion events are emitted by Netty.

  • Added a defensive check to prevent adding IdleStateHandler multiple times when multiple SSL handshake completion events occur
  • Included test coverage for the SSL implementation behavior
  • Updated test configuration to force JDK SSL implementation instead of OpenSSL

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
sdk/cosmos/live-platform-matrix.json Added test configuration to disable OpenSSL and force JDK SSL implementation
sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/directconnectivity/rntbd/RntbdRequestManager.java Added null check to prevent duplicate IdleStateHandler additions during SSL handshake completion
sdk/cosmos/azure-cosmos/CHANGELOG.md Updated changelog to document the SSL handshake resilience improvement
sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/implementation/directconnectivity/rntbd/RntbdRequestManagerTests.java Added unit test to verify IdleStateHandler is only added once despite multiple SSL handshake completion events

@tvaron3
Copy link
Member Author

tvaron3 commented Sep 18, 2025

/azp run java - cosmos - tests

Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@FabianMeiswinkel
Copy link
Member

/azp run java - cosmos - tests

@FabianMeiswinkel FabianMeiswinkel enabled auto-merge (squash) September 19, 2025 14:58
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

Copy link
Member

@xinlian12 xinlian12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants