Allow instantiate HttpRequestHeaders and merge it into another instance

[YehudahA]

System.Net.Http.HttpRequestHeaders is a comfortable class, and it stores all general headers [UserAgent, Connection, Encoding etc.] and custom headers.

This class used by HttpClient as DefaultRequestHeaders property, for merge default headers into every HttpRequestMessage.

Example for internal using of this for set default headers: HttpClient.PrepareRequestMessage
The internal logic that merge headers: HttpRequestHeaders.AddHeaders

Unfortunately, This class have 2 limitations:

1. It doesn't have any public constructor.
2. The logic of merge HttpRequestHeaders into another instance of HttpRequestHeaders, is not public.

I want to extend HttpClient or HttpMessageHandler behavior, so I can create per-host DefaultRequestHeaders. Something like this:

HttpRequestHeaders googleDefaultHeaders = new HttpRequestHeaders
{
    { "X-MY-HEADER-1", "123" },
    { "X-MY-HEADER-2", "456" },
};
googleDefaultHeaders.UserAgent = "XYZ;
googleDefaultHeaders.Connection = false;
httpClient.PerHostDefaultHeaders.Add(new Uri("HTTP://www.google.com"), googleDefaultHeaders);

[Sorry for my bad English ):]

[davidsh]

Can you provide a link to the source code you are talking about?

[davidsh]

If you want to set additional headers per request and based on a custom set of rules, it might be better to use a customer HttpMessageHandler. You can create a new custom handler based on a DelegatingHandler class:
https://docs.microsoft.com/en-us/dotnet/api/System.Net.Http.DelegatingHandler?view=netframework-4.7&viewFallbackFrom=netcore-1.1.0

Then you can use this handler when you create an HttpClient(), i.e.

var handler = new CustomHandler();
var client = new HttpClient(handler);

Using a custom handler such as this is the recommended best-practice way of adding custom behavior like this (i.e. adding request headers based on custom rules).

[davidsh]

cc: @karelz @stephentoub

[YehudahA]

@davidsh, I want to write something like this:

class Program
{
    static void Main(string[] args)
    {
        var client = new MyHttpClient();
        client.DefaultRequestHeaders.Add("X-SHARED", "XYZ");

        HttpRequestHeaders googleDefaultHeaders = new HttpRequestHeaders
        {
            { "X-MY-HEADER-1", "123" },
            { "X-MY-HEADER-2", "456" },
        };
        client.PerHostDefaultHeaders.Add(new Uri("HTTP://www.google.com"), googleDefaultHeaders);

        HttpRequestHeaders twitterDefaultHeaders = new HttpRequestHeaders
        {
            { "X-MY-HEADER-A", "BBBB" },
            { "X-MY-HEADER-B", "BABABA" },
        };
        client.PerHostDefaultHeaders.Add(new Uri("HTTP://www.twitter.com"), twitterDefaultHeaders);

        // Do something
    }
}

public class MyHttpClient : HttpClient
{
    public Dictionary<Uri, HttpRequestHeaders> PerHostDefaultHeaders { get; } = new Dictionary<Uri, HttpRequestHeaders>();

    public override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
    {
        var key = PerHostDefaultHeaders.Keys.FirstOrDefault(x => x.IsBaseOf(request.RequestUri));
        if (key != null)
        {
            request.Headers.AddHeaders(PerHostDefaultHeaders[key]);
        }

        return base.SendAsync(request, cancellationToken);
    }
}

This code have 2 problems:

1. HttpRequestHeaders doesn't hava any public ctor.
2. HttpRequestHeaders has AddHeaders method, but it's internal. This method merge a source HttpRequestHeaders into the target.

[karelz]

@YehudahA did you consider using the suggestion from @davidsh? Would that be a viable alternative?

[YehudahA]

@karelz, I know DelegatingHandler and I have experience with it, but as I understood @davidsh suggestion, I did not understand how it solved the problem.

public class MyHandler : DelegatingHandler
{
    protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
    {
        switch(request.RequestUri.Host.ToLower())
        {
            case "www.google.com":
                if(!request.Headers.ConnectionClose.HasValue)
                {
                    request.Headers.ConnectionClose = true;
                }
                if(!request.Headers.Contains("X-My-Header"))
                {
                    request.Headers.Add("X-My-Header", "XXX");
                }
                break;
            case "xxx.xxx.com":
                // Set general headers
                // set custom headers
                break;
        }

        return base.SendAsync(request, cancellationToken);
    }
}

1. The code is ugly.
2. It's very hard to merge all general headers [userAgent, AcceptEncoding, AcceptEncoding, AcceptLanguage etc.]

System.Net.Http.HttpClient does it with a single line of code: AddHaders. Why I cannot do the same?
Let see here: https://github.com/dotnet/corefx/blob/5d20e4df940dc9991af5cbb3f5ecc824e3151741/src/System.Net.Http/src/System/Net/Http/HttpClient.cs#L639

[karelz]

Did you consider adding your PerHostDefaultHeaders API on the handler? Why is it uglier in that case?
Yes, it is a bit more work (~50 lines of code), but it achieves exactly what you asked for, right?

If this is common task in real-world apps (so far we got only 1 report), we could consider adding it into HttpClient. Right now the Handler extensibility seems to be much more flexible and general and achieves your goal.

In general we want to add APIs which are widely used, important. If we add API for every single thing anyone will ask, we will create monster classes which people will have trouble to use, because it will be too hard to figure out what it is they need to do / have to do, etc. Again, for common tasks it definitely makes sense to add APIs.

Makes sense?

[YehudahA]

@karelz, I have two problems:

1. HttpRequestHeaders doesn't have any public ctor, so I cannot use PerHostDefaultHeaders .
2. I want to merge HttpRequestHeaders into the request headers, without special handling for every header. I looked into the internal code behind ``AddHeaders``` and I see that it's very complex.

It is not a new feature. It exists already, and it is used internally. All I want is to make it public.

[karelz]

Sadly, there is no such thing as "just make the API public" :(. Public APIs need extra care - you have to think through all angles, all usage cases. You have to understand how it impacts your ability to evolve the API in future.
It is a lot of work.

Key questions: Who is going to benefit form the API? How common the API is? Are we going to regret the API in 3-5 years, because there will be something better/shinier?
What is the cost? (incl. long-term maintenance, adding tests, backporting it to other platforms, etc.)
Is adding this API more important than adding other APIs in the space? (not just for you, but for the platform and all its users)
Do other platforms have this API?
If the API is not mainstream, why cannot we rely on a workaround? (the one above seems plausible)

If you are still interested, please read our API process and feel free to submit the proposal with all/most questions above answered, demonstrating need for the API in the ecosystem/API surface and demonstrating understanding of all its impact in all dimensions.

If there is compelling case, we will definitely consider it.

[YehudahA]

@karelz, What more information do you expect from me?
[I rewrite the first message in this thread]

[karelz]

@YehudahA your description talks about implementation details. It doesn't talk about scenarios, usage patterns and other key questions I raised above.

Tip & tricks:

• Do not include any implementation details in API proposal, save it for after the API is approved.
• Follow the API process, incl. the format in the good example - it steers you to described the right things in your API proposal.

[karelz]

dotnet/corefx#23544 seems to be reasonable way how to solve the problem.