-
Notifications
You must be signed in to change notification settings - Fork 1.3k
CSHARP-2843: Disable TLS renegotiation when possible #1520
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
Conversation
sslStream.AuthenticateAsClient(options); | ||
#elif (NETSTANDARD2_1_OR_GREATER) | ||
var options = GetAuthOptions(targetHost); | ||
sslStream.AuthenticateAsClientAsync(options, default).GetAwaiter().GetResult(); |
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.
There is no sync version in net2_1. Also the sslstream sync version fallbacks to async.
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.
The cancellationToken
parameter should be passed into AuthenticateAsClientAsync
, not a default
one.
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.
Thanks! The older API didn't accept cancellationToken
, so this was carried over.
sslStream.AuthenticateAsClient(options); | ||
#elif (NETSTANDARD2_1_OR_GREATER) | ||
var options = GetAuthOptions(targetHost); | ||
sslStream.AuthenticateAsClientAsync(options, default).GetAwaiter().GetResult(); |
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.
The cancellationToken
parameter should be passed into AuthenticateAsClientAsync
, not a default
one.
|
||
#if (NETSTANDARD2_1_OR_GREATER || NET6_0_OR_GREATER) | ||
var options = GetAuthOptions(targetHost); | ||
await sslStream.AuthenticateAsClientAsync(options, default).ConfigureAwait(false); |
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.
Same here. The cancellationToken
parameter should be passed into AuthenticateAsClientAsync
, not a default
one.
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.
Thanks done.
@@ -106,6 +122,17 @@ private void DisposeStreamIgnoringExceptions(Stream stream) | |||
} | |||
} | |||
|
|||
#if (NETSTANDARD2_1_OR_GREATER || NET6_0_OR_GREATER) | |||
private SslClientAuthenticationOptions GetAuthOptions(string targetHost) => new() |
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.
We typically avoid abbreviations. GetAuthenticationOptions
or GetSslClientAuthenticationOptions
.
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.
Done.
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
No description provided.