Thread safety issues with `HttpClient` and `HttpClient.DefaultRequestHeaders.Authorization`

[zzullick]

Hello! Thanks again for this library. I found a potentially serious issue I was hoping we could discuss. Internally to SlackWebApiClient it uses an instance of HttpClient which can be passed in via optional constructors or if omitted constructed itself. The mechanism for passing the access token to Slack uses the DefaultRequestHeaders.Authorization to set the token. My problem is that I desired to use the same instance of HttpClient for the running lifetime of the application to ensure we don't have resource exhaustion as recommended at https://docs.microsoft.com/en-us/dotnet/api/system.net.http.httpclient?view=net-6.0. Consider the case where we use the same HttpClient but it is used for many different use cases, or even for multiple Slack workspaces (like a SaaS app serving many workspaces). In this case, the incorrect token may be used even if I change the DefaultRequestHeaders myself prior to passing an existing HttpClient to SlackWebApiClient because of race conditions related to the asynchronous threaded nature in which we're serving many disparate requests from Slack.

I believe to fix this issue, the following would be appropriate:

• Discontinue use of DefaultRequestHeaders globally to the HttpClient and instead use HttpRequestMessage and HttpRequestMessage.Headers.Authorization in places that utilize the client, like MakeJsonCall to guarantee we're always using the access token we expect for a particular operation
• Modify SlackWebApiClient to internally use a static instance of HttpClient if none are passed in to prevent resource exhaustion from many HttpClient being constructed for different use cases across an application

[stoiveyp]

Hi @zzullick - just wanted to respond to let you know I've seen the issues you've raised and thank you for doing so. Today is going to be a bit of a long day for me but I certainly hope to have time to address them in the next day or so 👍

[zzullick]

Hi @zzullick - just wanted to respond to let you know I've seen the issues you've raised and thank you for doing so. Today is going to be a bit of a long day for me but I certainly hope to have time to address them in the next day or so 👍

Hey no problem! Sorry for the triple hit, I was working a bit over the weekend and had accumulated them. I'm happy to help as well. Please let me know if you have any contributing guidelines. I think we're both on the Slack Community workspace as well so happy to chat anytime there in #lang-dotnet.

[stoiveyp]

This was a really good shout - I have hazy recollections of knowing I needed to look at this and then clearly forgot all about it once I got into building up the actual API support

Thankfully moving to HttpRequestMessage was a much quicker change thanks to the client interface. This is now in 3.13.0 and 3.14.0-beta1 👍