Restore calls to CreateFatalHandshakeAlertToken on Android by rzikm · Pull Request #118765 · dotnet/runtime

rzikm · 2025-08-15T08:42:41Z

#117428 Introduced some PAL constants to adjust SslStream behavior with regards to sending custom alerts. The PR assumed that since SslStreamPal.ApplyAlertToken is a no-op (due to no API for creating custom alert being available), the entire SslStream.CreateFatalHandshakeAlertToken would be no-op as well. This was the case on Linux and OSX, but not so on Android.

To avoid a regression in .NET 10, this PR restores the call to CreateFatalHandshakeAlertToken, but ideally, the code should be examined as to why the last call to SslStream.HandshakeInternal is needed (as any internally generated alerts should've been returned as part of the previous call).

rzikm · 2025-08-15T08:43:00Z

/azp run runtime-extra-platforms

azure-pipelines · 2025-08-15T08:43:15Z

Azure Pipelines successfully started running 1 pipeline(s).

dotnet-policy-service · 2025-08-15T08:43:31Z

Tagging subscribers to this area: @dotnet/ncl, @bartonjs, @vcsjones
See info in area-owners.md if you want to be subscribed.

Copilot

Pull Request Overview

This PR fixes a regression introduced in PR #117428 that incorrectly assumed Android's SslStream behavior was identical to Linux/OSX regarding custom alert generation. The fix restores proper handshake teardown behavior on Android by setting CanGenerateCustomAlerts = true and removing an incorrect assertion.

Key changes:

Restores CanGenerateCustomAlerts = true for Android to enable proper handshake teardown
Removes incorrect debug assertion that was causing issues
Adds explanatory comment about Android's unique handshake requirements

_{You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.}

rzikm · 2025-08-15T11:06:44Z

Affected tests pass now, other CI failures should be unreltaed.

rzikm · 2025-08-15T11:12:08Z

/ba-g failing test is #118770 and is unrelated

* Enable smoke tests fixed in #118765 * Fix duplicate include

rzikm · 2025-08-15T08:42:41Z

Fixes#118118
Fixes#117963
Fixes#118350

#117428 Introduced some PAL constants to adjust SslStream behavior with regards to sending custom alerts. The PR assumed that since SslStreamPal.ApplyAlertToken is a no-op (due to no API for creating custom alert being available), the entire SslStream.CreateFatalHandshakeAlertToken would be no-op as well. This was the case on Linux and OSX, but not so on Android.

To avoid a regression in .NET 10, this PR restores the call to CreateFatalHandshakeAlertToken, but ideally, the code should be examined as to why the last call to SslStream.HandshakeInternal is needed (as any internally generated alerts should've been returned as part of the previous call).

rzikm · 2025-08-15T08:43:00Z

/azp run runtime-extra-platforms

azure-pipelines · 2025-08-15T08:43:15Z

Azure Pipelines successfully started running 1 pipeline(s).

dotnet-policy-service · 2025-08-15T08:43:31Z

Tagging subscribers to this area: @dotnet/ncl, @bartonjs, @vcsjones
See info in area-owners.md if you want to be subscribed.

Copilot

Pull Request Overview

This PR fixes a regression introduced in PR #117428 that incorrectly assumed Android's SslStream behavior was identical to Linux/OSX regarding custom alert generation. The fix restores proper handshake teardown behavior on Android by setting CanGenerateCustomAlerts = true and removing an incorrect assertion.

Key changes:

Restores CanGenerateCustomAlerts = true for Android to enable proper handshake teardown
Removes incorrect debug assertion that was causing issues
Adds explanatory comment about Android's unique handshake requirements

_{You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.}

rzikm · 2025-08-15T11:06:44Z

Affected tests pass now, other CI failures should be unreltaed.

rzikm · 2025-08-15T11:12:08Z

/ba-g failing test is #118770 and is unrelated

* Enable smoke tests fixed in #118765 * Fix duplicate include

rzikm · 2025-08-15T08:42:41Z

Fixes#118118
Fixes#117963
Fixes#118350

#117428 Introduced some PAL constants to adjust SslStream behavior with regards to sending custom alerts. The PR assumed that since SslStreamPal.ApplyAlertToken is a no-op (due to no API for creating custom alert being available), the entire SslStream.CreateFatalHandshakeAlertToken would be no-op as well. This was the case on Linux and OSX, but not so on Android.

To avoid a regression in .NET 10, this PR restores the call to CreateFatalHandshakeAlertToken, but ideally, the code should be examined as to why the last call to SslStream.HandshakeInternal is needed (as any internally generated alerts should've been returned as part of the previous call).

rzikm · 2025-08-15T08:43:00Z

/azp run runtime-extra-platforms

azure-pipelines · 2025-08-15T08:43:15Z

Azure Pipelines successfully started running 1 pipeline(s).

dotnet-policy-service · 2025-08-15T08:43:31Z

Tagging subscribers to this area: @dotnet/ncl, @bartonjs, @vcsjones
See info in area-owners.md if you want to be subscribed.

Copilot

Pull Request Overview

This PR fixes a regression introduced in PR #117428 that incorrectly assumed Android's SslStream behavior was identical to Linux/OSX regarding custom alert generation. The fix restores proper handshake teardown behavior on Android by setting CanGenerateCustomAlerts = true and removing an incorrect assertion.

Key changes:

Restores CanGenerateCustomAlerts = true for Android to enable proper handshake teardown
Removes incorrect debug assertion that was causing issues
Adds explanatory comment about Android's unique handshake requirements

_{You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.}

rzikm · 2025-08-15T11:06:44Z

Affected tests pass now, other CI failures should be unreltaed.

rzikm · 2025-08-15T11:12:08Z

/ba-g failing test is #118770 and is unrelated

* Enable smoke tests fixed in #118765 * Fix duplicate include

rzikm · 2025-08-15T08:42:41Z

Fixes#118118
Fixes#117963
Fixes#118350

#117428 Introduced some PAL constants to adjust SslStream behavior with regards to sending custom alerts. The PR assumed that since SslStreamPal.ApplyAlertToken is a no-op (due to no API for creating custom alert being available), the entire SslStream.CreateFatalHandshakeAlertToken would be no-op as well. This was the case on Linux and OSX, but not so on Android.

To avoid a regression in .NET 10, this PR restores the call to CreateFatalHandshakeAlertToken, but ideally, the code should be examined as to why the last call to SslStream.HandshakeInternal is needed (as any internally generated alerts should've been returned as part of the previous call).

rzikm · 2025-08-15T08:43:00Z

/azp run runtime-extra-platforms

azure-pipelines · 2025-08-15T08:43:15Z

Azure Pipelines successfully started running 1 pipeline(s).

dotnet-policy-service · 2025-08-15T08:43:31Z

Tagging subscribers to this area: @dotnet/ncl, @bartonjs, @vcsjones
See info in area-owners.md if you want to be subscribed.

Copilot

Pull Request Overview

This PR fixes a regression introduced in PR #117428 that incorrectly assumed Android's SslStream behavior was identical to Linux/OSX regarding custom alert generation. The fix restores proper handshake teardown behavior on Android by setting CanGenerateCustomAlerts = true and removing an incorrect assertion.

Key changes:

Restores CanGenerateCustomAlerts = true for Android to enable proper handshake teardown
Removes incorrect debug assertion that was causing issues
Adds explanatory comment about Android's unique handshake requirements

_{You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.}

rzikm · 2025-08-15T11:06:44Z

Affected tests pass now, other CI failures should be unreltaed.

rzikm · 2025-08-15T11:12:08Z

/ba-g failing test is #118770 and is unrelated

* Enable smoke tests fixed in #118765 * Fix duplicate include

rzikm · 2025-08-15T08:42:41Z

Fixes#118118
Fixes#117963
Fixes#118350

#117428 Introduced some PAL constants to adjust SslStream behavior with regards to sending custom alerts. The PR assumed that since SslStreamPal.ApplyAlertToken is a no-op (due to no API for creating custom alert being available), the entire SslStream.CreateFatalHandshakeAlertToken would be no-op as well. This was the case on Linux and OSX, but not so on Android.

To avoid a regression in .NET 10, this PR restores the call to CreateFatalHandshakeAlertToken, but ideally, the code should be examined as to why the last call to SslStream.HandshakeInternal is needed (as any internally generated alerts should've been returned as part of the previous call).

rzikm · 2025-08-15T08:43:00Z

/azp run runtime-extra-platforms

azure-pipelines · 2025-08-15T08:43:15Z

Azure Pipelines successfully started running 1 pipeline(s).

dotnet-policy-service · 2025-08-15T08:43:31Z

Tagging subscribers to this area: @dotnet/ncl, @bartonjs, @vcsjones
See info in area-owners.md if you want to be subscribed.

Copilot

Pull Request Overview

This PR fixes a regression introduced in PR #117428 that incorrectly assumed Android's SslStream behavior was identical to Linux/OSX regarding custom alert generation. The fix restores proper handshake teardown behavior on Android by setting CanGenerateCustomAlerts = true and removing an incorrect assertion.

Key changes:

Restores CanGenerateCustomAlerts = true for Android to enable proper handshake teardown
Removes incorrect debug assertion that was causing issues
Adds explanatory comment about Android's unique handshake requirements

_{You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.}

rzikm · 2025-08-15T11:06:44Z

Affected tests pass now, other CI failures should be unreltaed.

rzikm · 2025-08-15T11:12:08Z

/ba-g failing test is #118770 and is unrelated

* Enable smoke tests fixed in #118765 * Fix duplicate include

rzikm · 2025-08-15T08:42:41Z

Fixes#118118
Fixes#117963
Fixes#118350

#117428 Introduced some PAL constants to adjust SslStream behavior with regards to sending custom alerts. The PR assumed that since SslStreamPal.ApplyAlertToken is a no-op (due to no API for creating custom alert being available), the entire SslStream.CreateFatalHandshakeAlertToken would be no-op as well. This was the case on Linux and OSX, but not so on Android.

To avoid a regression in .NET 10, this PR restores the call to CreateFatalHandshakeAlertToken, but ideally, the code should be examined as to why the last call to SslStream.HandshakeInternal is needed (as any internally generated alerts should've been returned as part of the previous call).

rzikm · 2025-08-15T08:43:00Z

/azp run runtime-extra-platforms

azure-pipelines · 2025-08-15T08:43:15Z

Azure Pipelines successfully started running 1 pipeline(s).

dotnet-policy-service · 2025-08-15T08:43:31Z

Tagging subscribers to this area: @dotnet/ncl, @bartonjs, @vcsjones
See info in area-owners.md if you want to be subscribed.

Copilot

Pull Request Overview

This PR fixes a regression introduced in PR #117428 that incorrectly assumed Android's SslStream behavior was identical to Linux/OSX regarding custom alert generation. The fix restores proper handshake teardown behavior on Android by setting CanGenerateCustomAlerts = true and removing an incorrect assertion.

Key changes:

Restores CanGenerateCustomAlerts = true for Android to enable proper handshake teardown
Removes incorrect debug assertion that was causing issues
Adds explanatory comment about Android's unique handshake requirements

_{You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.}

rzikm · 2025-08-15T11:06:44Z

Affected tests pass now, other CI failures should be unreltaed.

rzikm · 2025-08-15T11:12:08Z

/ba-g failing test is #118770 and is unrelated

* Enable smoke tests fixed in #118765 * Fix duplicate include

rzikm · 2025-08-15T08:42:41Z

Fixes#118118
Fixes#117963
Fixes#118350

#117428 Introduced some PAL constants to adjust SslStream behavior with regards to sending custom alerts. The PR assumed that since SslStreamPal.ApplyAlertToken is a no-op (due to no API for creating custom alert being available), the entire SslStream.CreateFatalHandshakeAlertToken would be no-op as well. This was the case on Linux and OSX, but not so on Android.

To avoid a regression in .NET 10, this PR restores the call to CreateFatalHandshakeAlertToken, but ideally, the code should be examined as to why the last call to SslStream.HandshakeInternal is needed (as any internally generated alerts should've been returned as part of the previous call).

rzikm · 2025-08-15T08:43:00Z

/azp run runtime-extra-platforms

azure-pipelines · 2025-08-15T08:43:15Z

Azure Pipelines successfully started running 1 pipeline(s).

dotnet-policy-service · 2025-08-15T08:43:31Z

Tagging subscribers to this area: @dotnet/ncl, @bartonjs, @vcsjones
See info in area-owners.md if you want to be subscribed.

Copilot

Pull Request Overview

This PR fixes a regression introduced in PR #117428 that incorrectly assumed Android's SslStream behavior was identical to Linux/OSX regarding custom alert generation. The fix restores proper handshake teardown behavior on Android by setting CanGenerateCustomAlerts = true and removing an incorrect assertion.

Key changes:

Restores CanGenerateCustomAlerts = true for Android to enable proper handshake teardown
Removes incorrect debug assertion that was causing issues
Adds explanatory comment about Android's unique handshake requirements

_{You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.}

rzikm · 2025-08-15T11:06:44Z

Affected tests pass now, other CI failures should be unreltaed.

rzikm · 2025-08-15T11:12:08Z

/ba-g failing test is #118770 and is unrelated

* Enable smoke tests fixed in #118765 * Fix duplicate include

rzikm · 2025-08-15T08:42:41Z

Fixes#118118
Fixes#117963
Fixes#118350

#117428 Introduced some PAL constants to adjust SslStream behavior with regards to sending custom alerts. The PR assumed that since SslStreamPal.ApplyAlertToken is a no-op (due to no API for creating custom alert being available), the entire SslStream.CreateFatalHandshakeAlertToken would be no-op as well. This was the case on Linux and OSX, but not so on Android.

To avoid a regression in .NET 10, this PR restores the call to CreateFatalHandshakeAlertToken, but ideally, the code should be examined as to why the last call to SslStream.HandshakeInternal is needed (as any internally generated alerts should've been returned as part of the previous call).

rzikm · 2025-08-15T08:43:00Z

/azp run runtime-extra-platforms

azure-pipelines · 2025-08-15T08:43:15Z

Azure Pipelines successfully started running 1 pipeline(s).

dotnet-policy-service · 2025-08-15T08:43:31Z

Tagging subscribers to this area: @dotnet/ncl, @bartonjs, @vcsjones
See info in area-owners.md if you want to be subscribed.

Copilot

Pull Request Overview

This PR fixes a regression introduced in PR #117428 that incorrectly assumed Android's SslStream behavior was identical to Linux/OSX regarding custom alert generation. The fix restores proper handshake teardown behavior on Android by setting CanGenerateCustomAlerts = true and removing an incorrect assertion.

Key changes:

Restores CanGenerateCustomAlerts = true for Android to enable proper handshake teardown
Removes incorrect debug assertion that was causing issues
Adds explanatory comment about Android's unique handshake requirements

_{You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.}

rzikm · 2025-08-15T11:06:44Z

Affected tests pass now, other CI failures should be unreltaed.

rzikm · 2025-08-15T11:12:08Z

/ba-g failing test is #118770 and is unrelated

* Enable smoke tests fixed in #118765 * Fix duplicate include

Restore calls to CreateFatalHandshakeAlertToken on Android
49daf2f

github-actionsBot added the area-System.Net.Security label Aug 15, 2025

dotnet-policy-serviceBot assigned rzikm Aug 15, 2025

rzikm requested a review from liveans August 15, 2025 08:48

rzikm marked this pull request as ready for review August 15, 2025 10:35

CopilotAI review requested due to automatic review settings August 15, 2025 10:35

CopilotAI reviewed Aug 15, 2025
View reviewed changes

Comment threadsrc/libraries/System.Net.Security/src/System/Net/Security/SslStreamPal.Android.cs

liveans approved these changes Aug 15, 2025
View reviewed changes

rzikm enabled auto-merge (squash) August 15, 2025 11:06

rzikm mentioned this pull request Aug 15, 2025
[Test Failure] System.Security.Cryptography.X509Certificates.Tests.ChainTests.BuildChainForCertificateSignedWithDisallowedKey #118770
Closed

rzikm merged commit 2b21c73 into dotnet:mainAug 15, 2025
86 of 92 checks passed

davidnguyen-tech added a commit to davidnguyen-tech/runtime that referenced this pull request Aug 15, 2025
Enable smoke tests fixed indotnet#118765
7816d19

davidnguyen-tech mentioned this pull request Aug 15, 2025
[Android][CoreCLR] Re-Enable fixed networking tests #118772
Merged

build-analysisBot mentioned this pull request Aug 15, 2025
[Android][CoreCLR] System.Security.Cryptography.Tests killed by lowmemorykiller #118603
Open

davidnguyen-tech added a commit that referenced this pull request Aug 18, 2025
[Android][CoreCLR] Re-Enable fixed networking tests (#118772)
12f87d7
* Enable smoke tests fixed in #118765 * Fix duplicate include

github-actionsBot locked and limited conversation to collaborators Sep 15, 2025