Runtime: [API] Add per request headers override to HttpClient convenience methods

[benaadams]

Motivation

Custom per request http headers importance are on the rise due to their role in authentication and authentication in newer api security models.

The way to send custom per request headers with HttpClient is via SendAsync and adding them to the HttpRequestMessage.

However that isn't very discoverable and is a big jump from using the convenience methods GetAsync, PostAsync, etc

This can lead people to changing the DefaultRequestHeaders on the HttpClient instead: SO, SO, SO, etc

Which is very dangerous if the HttpClient is used as a static; which is also an official suggested practice

HttpClient is intended to be instantiated once and re-used throughout the life of an application. Especially in server applications, creating a new HttpClient instance for every request will exhaust the number of sockets available under heavy loads. This will result in SocketException errors.

And in popular blogs "You're using HttpClient wrong and it is destabilizing your software"

This is dramatic. If you have any kind of load at all you need to remember these two things:

1. Make your HttpClient static.
2. Do not dispose of or wrap your HttpClient in a using unless you explicitly are looking for a particular behaviour (such as causing your services to fail).

Both changing DefaultRequestHeaders per request and concurrently making requests from a shared HttpClient will end very badly.

Proposal

Add overloads to the convenience methods GetAsync, PostAsync, etc to allow for customer headers on a per request basis from a shared HttpClient

public partial class HttpClient
{
    Task<HttpResponseMessage> DeleteAsync(string requestUri, HttpRequestHeaders requestHeaders) { throw null; }
    Task<HttpResponseMessage> DeleteAsync(string requestUri, HttpRequestHeaders requestHeaders, CancellationToken cancellationToken) { throw null; }
    Task<HttpResponseMessage> DeleteAsync(Uri requestUri, HttpRequestHeaders requestHeaders) { throw null; }
    Task<HttpResponseMessage> DeleteAsync(Uri requestUri, HttpRequestHeaders requestHeaders, CancellationToken cancellationToken) { throw null; }
    Task<HttpResponseMessage> GetAsync(string requestUri, HttpRequestHeaders requestHeaders) { throw null; }
    Task<HttpResponseMessage> GetAsync(string requestUri, HttpRequestHeaders requestHeaders, HttpCompletionOption completionOption) { throw null; }
    Task<HttpResponseMessage> GetAsync(string requestUri, HttpRequestHeaders requestHeaders, HttpCompletionOption completionOption, CancellationToken cancellationToken) { throw null; }
    Task<HttpResponseMessage> GetAsync(string requestUri, HttpRequestHeaders requestHeaders, CancellationToken cancellationToken) { throw null; }
    Task<HttpResponseMessage> GetAsync(Uri requestUri, HttpRequestHeaders requestHeaders) { throw null; }
    Task<HttpResponseMessage> GetAsync(Uri requestUri, HttpRequestHeaders requestHeaders, HttpCompletionOption completionOption) { throw null; }
    Task<HttpResponseMessage> GetAsync(Uri requestUri, HttpRequestHeaders requestHeaders, HttpCompletionOption completionOption, CancellationToken cancellationToken) { throw null; }
    Task<HttpResponseMessage> GetAsync(Uri requestUri, HttpRequestHeaders requestHeaders, CancellationToken cancellationToken) { throw null; }
    Task<byte[]> GetByteArrayAsync(string requestUri, HttpRequestHeaders requestHeaders) { throw null; }
    Task<byte[]> GetByteArrayAsync(Uri requestUri, HttpRequestHeaders requestHeaders) { throw null; }
    Task<Stream> GetStreamAsync(string requestUri, HttpRequestHeaders requestHeaders) { throw null; }
    Task<Stream> GetStreamAsync(Uri requestUri, HttpRequestHeaders requestHeaders) { throw null; }
    Task<string> GetStringAsync(string requestUri, HttpRequestHeaders requestHeaders) { throw null; }
    Task<string> GetStringAsync(Uri requestUri, HttpRequestHeaders requestHeaders) { throw null; }
    Task<HttpResponseMessage> PostAsync(string requestUri, HttpContent content, HttpRequestHeaders requestHeaders) { throw null; }
    Task<HttpResponseMessage> PostAsync(string requestUri, HttpContent content, HttpRequestHeaders requestHeaders, CancellationToken cancellationToken) { throw null; }
    Task<HttpResponseMessage> PostAsync(Uri requestUri, HttpContent content, HttpRequestHeaders requestHeaders) { throw null; }
    Task<HttpResponseMessage> PostAsync(Uri requestUri, HttpContent content, HttpRequestHeaders requestHeaders, CancellationToken cancellationToken) { throw null; }
    Task<HttpResponseMessage> PutAsync(string requestUri, HttpContent content, HttpRequestHeaders requestHeaders) { throw null; }
    Task<HttpResponseMessage> PutAsync(string requestUri, HttpContent content, HttpRequestHeaders requestHeaders, CancellationToken cancellationToken) { throw null; }
    Task<HttpResponseMessage> PutAsync(Uri requestUri, HttpContent content, HttpRequestHeaders requestHeaders) { throw null; }
    Task<HttpResponseMessage> PutAsync(Uri requestUri, HttpContent content, HttpRequestHeaders requestHeaders, CancellationToken cancellationToken) { throw null; }
}

Notes

There are also suggestions for going further for increased safety

I would opt for taking headers at construction time and freeze them, or allow to pass frozen headers through a param or TLS...etc

Perhaps freezing the DefaultRequestHeaders on first request; and throwing an InvalidOperationException if they are modified after the first request has been initiated? As modifying the default headers while requests are being initiated in parallel will lead to nonsensical outcome; and modifying them and submitting a request can't be an atomic operation.

/cc @davidsh, @CIPop, @Priya91 @xoofx @khellang

[darrelmiller]

I can't say I'm enthusiastic about adding even more overloads to HttpClient. I would be happy with being able to do this:

            httpClient.SendAsync(new HttpRequestMessage()
            {
                RequestUri = new Uri("http://api.example.org"),
                Method = HttpMethod.Post,
                Content = new StringContent("Hello World"),
                Headers = new HttpRequestHeaders() { new AuthenticationHeaderValue("bearer","token")}
            });

Unfortunately you can't because Headers is a read-only property.

[davidfowl]

Why not a Func instead?

[benaadams]

Alternative, could be extension methods that call through to SendAsync; would be able to ship OOB then also and work at .NET Standard 1.1?

public static class HttpClientExtensions
{
    Task<HttpResponseMessage> GetAsync(
        this HttpClient client, 
        string requestUri, 
        HttpRequestHeaders requestHeaders);
}

[benaadams]

Why not a Func instead?

Something like?

Task<HttpResponseMessage> GetAsync(
    string requestUri, 
    Action<HttpRequestMessage, object> modifyRequest, 
    object state);

Might not help with discoverablity for new users though?

[davidfowl]

@benaadams yea or maybe:

C# Task<HttpResponseMessage> GetAsync( string requestUri, Func<HttpRequestMessage, object, Task> modifyRequest, object state);

Might not help with discoverablity for new users though?

True, but less allocatey.

[tmds]

Custom per request http headers importance are on the rise due to their role in authentication and authentication in newer api security models.

I guess this involves a TokenManager of some sorts. Would be nice to see how that fits into the proposed APIs.

[stevedesmond-ca]

I like the idea of passing in a headers object rather than a modifier func, though that could lead to devs doing the same thing and just modifying the same shared header collection.

Can this be solved with documentation? I feel like there's a lot of "you're doing it wrong" out there without a great discussion of trade-offs, e.g.

• when to use a singleton HttpClient vs not
• what happens when you do vs don't (good and bad for both)
• when to use the happy path convenience methods vs creating a message and passing it into the lower-level Send()
• default headers means default (stop changing them)

OK, now I kind of want to do this...

[darrelmiller]

I agree with @stevedesmond-ca that documentation would be a really good solution to this problem. There really isn't much out there on HTTP and there is plenty to cover. I know because I have 6 hours worth of unfinished Pluralsight course material just on HttpClient.

If you take @davidfowl 's Func suggestion to the extreme you end up close to what I have been working with for a long time now which is the notion of a Link object which implements IRequestFactory. It's like the client version of an ActionResult and encapsulates everything about creating a HttpRequestMessage.

So you end up doing this:

var httpClient = new HttpClient();
var link = new Link() { Target= new Uri("http://example.org/foo" };
var response = await httpClient.SendAsync(link.CreateRequest());

and you have the option of going all the way to stuff like this,

var httpClient = new HttpClient();
var link = new UPSTrackingLink() { 
                     TrackingNumber = "Z12323D2323S"
                };
var response = await httpClient.SendAsync(link.CreateRequest());

https://github.com/tavis-software/tavis.link#examples

There are a ton of other benefits to this approach, but I will not attempt to derail further :-)

[darrelmiller]

@tmds I would use a HTTP Message handler just like on the server. Like this https://github.com/tavis-software/Tavis.Auth/blob/master/src/Auth/AuthMessageHandler.cs

[benaadams]

HttpRequestHeaders doesn't have a public constructor so am trying a different api

[darrelmiller]

@benaadams It used to, 8 years ago, before it got re-written :-) http://www.bizcoder.com/httpclient-the-basics

[tmds]

@tmds I would use a HTTP Message handler just like on the server.

@darrelmiller 👍 👍

[adrianm64]

Sorry to jump in late here and I have probably misunderstood something.

Why is everyone so eager to reuse the HttpClient? I have read the linked articles but they all seem to miss the fact that HttpClient class is more or less just a holder class for headers, timeout and some other things. All methods hitting the network are delegated into the pipeline via HttpMessageHandler.SendAsync.
There is no network state in HttpClient, Dispose does nothing if you pass in your own HttpMessageHandler.

As long as you use the same HttpMessageHandler you can create and dispose HttpClients as much as you want without any penalty (except some memory and a cancellation token). I use code like this:

private static Lazy<HttpClientHandler> _defaultHandler = new Lazy<HttpClientHandler>()

// Allow users to specify their own handler e.g. for logging
public HttpMessageHandler MessageHandler { get; set; }

private HttpClient CreateClient() {
    var client = new HttpClient(MessageHandler ?? _defaultHandler.Value, disposeHandler: false);
    // set base url, headers etc
    return client;
}

public async Task<string> GetStringAsync(Uri uri) {
    using (var client = CreateClient()) {
        return await client.GetStringAsync(uri);
    }
}

Makes it a lot easier to deal with changing headers, timeout etc.

In hindsight it would probably been better to not have the factory-like default constructor and named the class HttpMessageFacade. It would have been slightly more difficult to use but less confusing about what it is.

Edit: forgot an await
Edit2: adaptor -> facade (I should really blow the dust of my GoF book :-))

[robertmiles3]

FWIW, the extension methods of ServiceStack’s HTTP Utils are a treat. Adding headers is super easy, and something along the lines of @davidfowl suggestion.

http://docs.servicestack.net/http-utils

[yahorsi]

Could we consider adding overload that just takes Bearer token and so adds corresponding header? That would make many today's call's very simple. Or that is too specific?
'

[yahorsi]

@adrianm64 According to sources there is no network sattce in the HttpClientHandler as well. The only thing it does in the Dispose is a call to the ServicePointManager.CloseConnectionGroups(this.connectionGroupName);

So, theoretically, it should be just enough to set disposeHandler: false

PS: Strange, made the test, if HttpClientHandler is newed every time and disposeHandler: false then we see lot's of ports in the CLOSE_WAIT stait. If HttpClientHandler is reused then everything is fine. I twice looked at the HttpClientHandler code and do not understand that. Might be related to the connection group name

[adrianm64]

@tom3m it is the ServicePointManager.CloseConnectionGroups you want to avoid. You can either do that by never disposing the HttpClient or by using the disposeHandler: false.

[geoffkizer]

You can use collection initializer syntax to do this:

            var t = httpClient.SendAsync(new HttpRequestMessage()
            {
                RequestUri = new Uri("http://microsoft.com"),
                Method = HttpMethod.Get,
                Headers = { { "My-Header", "HeaderValue" } }
            });

It won't work with typed header values since there's no Add method for that, but we could look at adding something like that.

[ericsampson]

@karelz we ran into this too (per-message custom headers for getasync) because like Ben said originally this is getting to be a more common pattern, is there any interest in reviving this request in some form? Thanks!

[karelz]

Eventually, we will be interested. Most likely post-3.0.
It has 10 heart-votes, which is on higher end (although I sort only by thumbs up votes - now I have to remember to look at heart-sort as well)

[ericsampson]

@karelz would this be a good candidate for a community member to submit a PR as a starting point?

[karelz]

We need API discussion first. It adds lots of methods - is it worth it? That's the important question IMO.

[vladiliescu]

The current API makes it _really really_ hard to fall into the pit of success. As Ben mentioned, the default instinct is to use the convenience methods (GetAsync, PostAsync, DeleteAsync etc), but then how do you set the headers in Get&Delete? You go and modify the DefaultHeaders of course. But the HttpClient is static because that's what all the guides say. And then all hell ensues, but only sometimes. This is such an easy trap to fall into.

[yahorsi]

IMHO it should have been designed as stateless API, where all parameters needed are passed via method parameters, and of course it should not be IDisposable. And then you could have statefull API on the top of the statefull, when someone would prefer to save some e.g. headers to be used in all requests and so use concrete instance as that instance holds the state

[adrianm64]

@yahorsi, you already have that.
HttpMessageHandler is what you call stateless (everything passed via parameters) and HttpClient is stateful (holds default headers etc).

Just use a static HttpMessageHandler and then create a new HttpClient whenever you need to alter default headers/timeout etc.

[yahorsi]

HttpMessageHandler

@adrianm64 Not sure what you mean. SendAsync is protected abstract in the HttpMessageHandler and receives HttpRequestMessage.

[darrelmiller]

@yahorsi @adrianm64 You need to use HttpMessageInvoker if you want to call a HttpMessageHandler directly.

@vladiliescu The convenience methods assume that you are not going to want to set per-request headers. This is not a terrible assumption for a reasonable number of scenarios. Having to fall back on creating a HttpRequestMessage and then use SendAsync if you want more control is, in my opinion, a decent way of peeling back the layer's of the onion.

The original proposal by @benaadams talks about per-request headers for auth, but those should be done via a piece of middleware, not by inserting ad-hoc code before every call to the API.

There are very few headers, in my experience, that are per-request that would not be done more effectively as a a piece of middleware.

[vladiliescu]

@darrelmiller Agreed, manually creating an HttpRequestMessage and using it for SendAsync is by no means terrible. I do not think that this is what beginners (i.e. people not familiar with HttpClient) will use though. Beginners will use GetAsync and PostAsync and PutAsync because they're just more convenient. And when they'll need to set per-request headers they'll look at the overloads for Get/Put/Post and find nothing there. Some may look at other methods such as SendAsync (I know I wouldn't, why would I look at other methods when Get/Put/Post do almost exactly what I want :) ) and others may conclude that they need to change the DefaultHeaders.

The latter will encounter dragons down the road, and I think this should be mitigated.

[ericsampson]

and others may conclude that they need to change the DefaultHeaders.

The latter will encounter dragons down the road, and I think this should be mitigated.

aka 'Pit of Success'. This isn't an imaginary scenario, I've seen this issue happen (and fixed it) at a couple companies. Now maybe it can be argued that the original developers were naive or should have known better (used middleware etc), but the story that @vladiliescu laid out does happen FWIW.

[balivo]

Any news?

I have a scenario where my users add multiple vendors that will have their APIs consumed by my platform, directly "on behalf" of my user...

The per request header is really useful...

[scalablecory]

@balivo per-request headers are already supported; this issue is discussing possible convenience methods for anyone not wanting to manually instantiate a HttpRequestMessage like in https://github.com/dotnet/corefx/issues/23544#issuecomment-369419082.

[balivo]

Oh sorry @scalablecory I don't see that comment... SendAsync is available in Extensions NuGet?

[scalablecory]

@balivo SendAsync is an API on HttpClient, no extension needed.

[balivo]

Tks...

[scalablecory]

So the general recommendation is to have only a single HttpClient instance.

However, the real reason for this is that you only want a single HttpMessageHandler, because this is where all the connection pooling goodness comes from.

In this case, you can have your cake and eat it too, with a single shared instance and being able to continue using DefaultRequestHeaders with something like this:

```c#
using HttpClientHandler handler = new HttpClientHandler();

async Task WorkWithApiA()
{
using var client = new HttpClient(handler, false);
client.DefaultRequestHeaders.Add("X-Secret-Auth-Token", "foo");

await client.GetAsync("/");
// ...

}

async Task WorkWithApiB()
{
using var client = new HttpClient(handler, false);
client.DefaultRequestHeaders.Authorization = new System.Net.Http.Headers.AuthenticationHeaderValue("Bearer", "foo");

await client.GetAsync("/");
// ...

}
```

This is our recommended solution if you want to use default request headers against multiple APIs, while keeping to a single instance. We believe this will be cleaner than asking users to pass a collection to every convenience API call.

[scalablecory]

Perhaps freezing the DefaultRequestHeaders on first request; and throwing an InvalidOperationException if they are modified after the first request has been initiated?

This is going to be a tough sell because some usages of HttpClient can rely on this today. For instance, I've used an API where I did something like this:

```c#
var response = await client.PostAsync("/login", ...);
AuthResponse auth = await response.Content.ReadAsAsync()
client.DefaultRequestHeaders.Add("auth", auth.Token);

// continue using client for further requests.
```

I do agree that it is easy to misuse and needs some documentation.

[geoffkizer]

Modifying DefaultRequestHeaders after first use is a dangerous thing to do. If you're using the HttpClient from other threads (as recommended) you will get undefined behavior here.

[mr5z]

Just so I had a weird issue after setting up the authorization of the DefaultRequestHeaders.
I tried to modify the per-request headers, changing the Authorization value to null while DefaultRequestHeaders.Authorization is non-null but somehow the final request ended up using that from the default headers. I stopped using the DefaultRequestHeaders.

[balivo]

I agree with @geoffkizer... Modify DefaultRequestHeaders after first use is REALY dangerous... Use SendAsync is the best practice...

[jemiller0]

When is Microsoft going to release an HttpClient replacement that isn't a complete train wreck. You can't even update a default request header? This class seems like a land mine. Even Dispose() doesn't work properly. Honestly, I don't know what is wrong with the ASP.NET team that they make everything more complicated than it needs to be and programmer hostile. It is like someone thinks that every class needs to implement everyone of the 21 patterns from the Design Patterns book in every class. It's ridiculous. You would think HttpClient would be better than WebClient. It looks to me that you are doing a poor job of designing these classes and making them way more difficult to work with than they should be. That describes ASP.NET in general these days. Over engineered and difficult to work with.