Skip to content

Adding custom certificate validation #78 - #79

Closed
natanmejirovsky wants to merge 4 commits into
datalust:devfrom
natanmejirovsky:dev
Closed

Adding custom certificate validation #78#79
natanmejirovsky wants to merge 4 commits into
datalust:devfrom
natanmejirovsky:dev

Conversation

@natanmejirovsky

Copy link
Copy Markdown

No description provided.

Natan Mejirovsky and others added 2 commits December 26, 2019 14:02
Comment thread src/Seq.Api/Client/SeqApiClient.cs Outdated
/// <param name="apiKey">An API key to use when making requests to the server, if required.</param>
/// <param name="useDefaultCredentials">Whether default credentials will be sent with HTTP requests; the default is <c>true</c>.</param>
public SeqApiClient(string serverUrl, string apiKey = null, bool useDefaultCredentials = true)
/// <param name="serverCertificateCustomValidationCallback">Callback for custom certificate validation</param>

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that appending callback to existing HttpMessageHandler is a better solution than replacing the whole HttpMessageHandler. Because of that way no need for changing existing logic.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had the same thought at first, but, adding a parameter here will be a binary breaking change, which will make it harder to release, and, there are several more properties on HttpClientHandler that consumers may need to set in different situations in the future (Credentials/DefaultProxyCredentials/Proxy/PreAuthenticate, SslProtocols, MaxConnectionsPerServer) - which might cause further parameter list churn.

@nblumhardt

Copy link
Copy Markdown
Member

Thanks for the PR! 👍

I have a few minutes this morning, I might explore alternatives to the added parameter, I can see that it's a bit tricky to get everything lining up.

@nblumhardt

Copy link
Copy Markdown
Member

@nmkryon I've submitted #80 - would appreciate your feedback. Thanks!

@nblumhardt

Copy link
Copy Markdown
Member

Closed in favor of #80

@nblumhardt nblumhardt closed this Dec 30, 2019
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

Successfully merging this pull request may close these issues.

2 participants