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

Potentially breaking change in Octopus.Client dependency - ServerCertificateCustomValidationCallback & SslProtocols #121

Open
karelz opened this issue Jan 31, 2017 · 5 comments
Assignees

Comments

@karelz
Copy link

karelz commented Jan 31, 2017

We plan to release new System.Net.Http package which will break two APIs you use HttpClientHandler.ServerCertificateCustomValidationCallback and HttpClientHandler.SslProtocols - (usage in your code 1 and 2).
The reason is that the package causes a lot of grief to .NET developers (see https://github.com/dotnet/corefx/issues/11100), we have to revert the source code back to 4.6 code and there is no simple way how to implement this property correctly in that code base.

The breaking change (more details in https://github.com/dotnet/corefx/issues/11100 - see top-most post, section 'Impact of the change - Breaking changes'):

.2. HttpClientHandler.SslProtocols (introduced in System.Net.Http 4.1)

  • New behavior: Throws PlatformNotSupportedException
  • Workaround: Use ServicePointManager.SecurityProtocol instead (impacts the whole AppDomain, not just single HttpClientHandler as it did in System.Net.Http 4.1-4.3)

.3. HttpClientHandler.ServerCertificateCustomValidationCallback (introduced in System.Net.Http 4.1)

  • New behavior: Works fine, except that the first parameter of type HttpRequestMessage is always null
  • Workaround: Use ServicePointManager.ServerCertificateValidationCallback

Octopus.Client NuGet package is one of 4 NuGet packages directly affected by the breaking change in the APIs. Can you please assess if your library is affected and act accordingly?

Thank you and sorry for the inconvenience!
If you have any questions, please let us know.
cc @davidsh

@droyad droyad self-assigned this Jan 31, 2017
@droyad
Copy link
Contributor

droyad commented Jan 31, 2017

@karelz If I understand correctly, the functionality will be reverting back to what it was in the .NET Framework. That's actually good for us as then the net45 and netstandard builds have the same behaviour.

Thanks for the heads up.

@karelz
Copy link
Author

karelz commented Jan 31, 2017

@droyad yes, technically you are correct - these properties were NOT avaiable in net45/net46 at all. They were newly introduced in out-of-box package 4.1 of System.Net.Http. The new properties did NOT ship as part of the full = Desktop .NET Framework.

Let us know if you have further questions.

@PaulStovell
Copy link
Member

@karelz thanks for pro-actively reaching out! Much appreciated.

@karelz
Copy link
Author

karelz commented Feb 17, 2017

Sure thing. Note that the latest plan is that all "8 new APIs", which were added in 4.1 NuGet package of System.Net.Http, will be available in .NET Standard 2.0. However, they will throw MissingMethodException if you run library targeting .NET Standard 2.0 on Desktop. We will stop using the NuGet package System.Net.Http as part of .NET Standard from 2.0.
Running the same library (targeting .NET Standard 2.0 and using some of the "8 new APIs"), will work just fine on .NET Core.

@droyad
Copy link
Contributor

droyad commented Jun 8, 2017

When we do upgrade to netstandard2.0, we should keep a net451/net45 target as well and the IF DEFS that exclude the usage for those APIs. .NET Framework apps should then prefer the net45 targets over the netstandard ones.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants