Skip to content
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

Fix SslServerAuthenticationOptions.ApplicationProtocols empty list error #55772

Merged
merged 4 commits into from
Jul 16, 2021

Conversation

Tratcher
Copy link
Member

@Tratcher Tratcher commented Jul 15, 2021

Fixes #55447 SslStream is inconsistent across platforms about checking if the ApplicationProtocol list is null vs empty. On Windows it checks both, but on other platforms it only checks for null so an empty list will cause unexpected failures on other platforms.

@ghost
Copy link

ghost commented Jul 15, 2021

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

Issue Details

#55447 (Draft while tests confirm original behavior)

Author: Tratcher
Assignees: -
Labels:

area-System.Net.Security

Milestone: -

@wfurt
Copy link
Member

wfurt commented Jul 16, 2021

Should we make same check for Windows & macOS so the ALPN handling is consistent? (even if it does not break on Windows right now?)

Copy link
Member

@wfurt wfurt left a comment

Choose a reason for hiding this comment

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

LGTM

@Tratcher
Copy link
Member Author

Should we make same check for Windows & macOS so the ALPN handling is consistent? (even if it does not break on Windows right now?)

Windows already has this check:

if (sslAuthenticationOptions.ApplicationProtocols != null && sslAuthenticationOptions.ApplicationProtocols.Count != 0)

I couldn't find the Mac code path at first, but I've fixed it now, as well as android.

@Tratcher Tratcher marked this pull request as ready for review July 16, 2021 20:52
@Tratcher Tratcher added this to the 6.0.0 milestone Jul 16, 2021
@Tratcher Tratcher merged commit 0851fee into dotnet:main Jul 16, 2021
@Tratcher Tratcher deleted the tratcher/alpn branch July 16, 2021 21:36
@ghost ghost locked as resolved and limited conversation to collaborators Aug 15, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Setting SslServerAuthenticationOptions.ApplicationProtocols to an empty list fails on Ubuntu
3 participants