Only select TLS1.2 when non-default SecurityProtocol selected. #5
Conversation
With changes from editor-tools/octokit.net#5.
| @@ -28,7 +28,10 @@ public HttpClientAdapter(Func<HttpMessageHandler> getHandler) | |||
| Ensure.ArgumentNotNull(getHandler, nameof(getHandler)); | |||
|
|
|||
| #if HAS_SERVICEPOINTMANAGER | |||
| ServicePointManager.SecurityProtocol |= SecurityProtocolType.Tls12; | |||
| if (ServicePointManager.SecurityProtocol != 0) | |||
There was a problem hiding this comment.
Took me a little while to understand what's going on here (thanks for the explanation).
Maybe we should add a comment that 0 corresponds to SecurityProtocolType.SystemDefault, which is new in .NET 4.7 (with a link to https://docs.microsoft.com/en-us/dotnet/api/system.net.securityprotocoltype?view=netframework-4.7). If I've understood correctly, the SystemDefault will default to using TLS 1.2.
There was a problem hiding this comment.
Because this is a fork, I expect that this change will be overwritten by the upstream change when that is made, so I expect this change to be shortlived. The issue opened on octokit.net by MS suggests using 0 so I expected that they will make a similar change; therefore I'm not sure it's worth calling it out here.
There was a problem hiding this comment.
I thought this was going to end up a PR to upstream as well.
I'll ✔️ as is. 👍
See: octokit#1914