search

EventDay / Infusionsoft.net

A C# Wrapper around the Infusionsoft.com API
15 stars 22 forks source link

Remove setting properties on ServicePointManager #23

Closed MisinformedDNA closed 9 years ago

MisinformedDNA commented 9 years ago

We shouldn't be setting anything on ServicePointManager. The first instance was fixed here. The second instance is still active here and, per my tests, does not appear to be needed. 

ServicePointManager.ServerCertificateValidationCallback += 
    (sender, certificate, chain, sslpolicyerrors) => true;

ServicePointManager affects the whole app-domain and thus all other (non-Infusionsoft) calls are affected. This means that developers, who are unaware of the internals of this library, will not know why their app is behaving in an unexpected way.

For instance, I was setting ServicePointManager.SecurityProtocol to be Tls to solve the POODLE issue for Facebook, but because this library also hard-coded it to Ssl3, it was overwriting my setting.

If we ever "need" to set something on ServicePointManager or another setting that affects the entire app-domain, we should instead give the user guidance as to proper settings or call it out in a more obvious way.

PR incoming.

trbngr commented 9 years ago

Absolutely correct.

This was a setting that solved a bug introduced by an Infusionsoft API change.

PR accepted. Thanks!