-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Improve Robustness for TLS 1.3 #46734
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
/azp run java - cosmos - tests |
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
/azp run java - cosmos - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - Thanks
/azp run java - cosmos - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this 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 |
.../main/java/com/azure/cosmos/implementation/directconnectivity/rntbd/RntbdRequestManager.java
Show resolved
Hide resolved
.../java/com/azure/cosmos/implementation/directconnectivity/rntbd/RntbdRequestManagerTests.java
Show resolved
Hide resolved
.../java/com/azure/cosmos/implementation/directconnectivity/rntbd/RntbdRequestManagerTests.java
Show resolved
Hide resolved
/azp run java - cosmos - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run java - cosmos - tests |
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks
…to tvaron3/tls1.3fix # Conflicts: # sdk/cosmos/azure-cosmos/CHANGELOG.md
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.