Expose SNI hostname in ITlsHandshakeFeature for Kestrel/HttpSys

[karimsalem1]

Expose SNI hostname in ITlsHandshakeFeature for Kestrel/HttpSys

• You've read the Contributor Guide and Code of Conduct.
• You've included unit or integration tests for your change, where applicable.
• You've included inline docs for your change, where applicable.
• There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.

Summary of the changes (Less than 80 chars)

Exposing SNI HostName in the ITlsHandshakeFeature interface.

Description

Exposing SNI HostName in the ITlsHandshakeFeature interface to allow users to retrieve the server_name used in the ClientHello of the TLS handshake. This PR applies to both Kestrel and HttpSys servers.

Issue opened and API approved here: #34525

[ghost]

Thanks for your PR, @karimsalem1. Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

[Tratcher]

Are you planning to do IIS as well?

[karimsalem1]

Are you planning to do IIS as well?

Will do in another PR.

[Tratcher]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 3 pipeline(s).

[halter73]

Trying to use null vs String.Empty to distinguish between different conditions is discouraged, it's not self-descriptive. Is there a requirement to know if the client didn't support SNI as compared to sending an empty value? I don't think there's a practical difference from the server side, it requires an SNI value or it doesn't.

Given this, should we update the api to be string HostName => "";? Is there any value to knowing whether it's been explicitly implemented or not? I'm okay with it as-is, but making it non-nullable could reduce unnecessary null checks.

[Tratcher]

Good call, return string.Empty from the default implementation.

[halter73]

I've updated the API review issue. Hopefully we can quickly approve it again Thursday.

[ghost]

@karimsalem1, this change will be considered for inclusion in the blog post for the release it'll ship in. Nice work!

Please ensure that the original comment in this thread contains a clear explanation of what the change does, why it's important (what problem does it solve?), and, if relevant, include things like code samples and/or performance numbers.

This content may not be exactly what goes into the blog post, but it will help the team putting together the announcement.

Thanks!