Runtime: Setting Http version when initialize HttpClient

[Code-ScottLe]

Right now, HttpClient will default to 1.1 on all requests, unless specifies directly on each HttpRequestMessage with SendAsync() (per #4870).

It would be nice if we can initialize the client with the Http version of choice once, and use it for every API that HttpClient provides.

Rejected API Proposal (attempt)

Motivation: Change default Http protocol version to the latest supported version by default to get the best performance.

public static class HttpVersion
{
    public static readonly Version Unknown = new Version(0, 0);
    public static readonly Version Version10 = new Version(1, 0);
    public static readonly Version Version11 = new Version(1, 1);
    public static readonly Version Version20 = new Version(2, 0);
+   public static readonly Version Default = new Version(int.MaxValue, int.MaxValue); // Use the highest version that the platform supports
}

Note: Similar to SecurityProtocolType.SystemDefault

[Code-ScottLe]

cc @karelz

[karelz]

@davidsh @geoffkizer any thoughts / pros / cons?

cc @stephentoub

[davidsh]

Technically, it would be a breaking change to change the default of HttpRequestMessage.Version from 1.1 to 2.0. However, the default on UWP is 2.0. And now that all the platform stacks support 2.0, it is probably an ok thing to do this for 2.1 release.

Keep in mind that .NET Framework default is still 1.1 via the inbox System.Net.Http.dll. However, once someone pulls in the System.Net.Http.dll via NuGet, the default will change to 2.0. That will break .NET Framework as it will throw an exception when getting 2.0.

So, we will need to have an if-def NET46 around this change so that on NET46 builds, it will stay 1.1 and otherwise switch to 2.0.

[Code-ScottLe]

Can we do something like initializing the HttpClientHandler with a version so the HttpClient can use that to override the version on each HttpRequestMessage that it sends out?

So with that, we don't have to modify the default value of HttpRequestMessage and the client has to specify the version specifically to that Client alone.

[stephentoub]

Technically, it would be a breaking change to change the default of HttpRequestMessage.Version from 1.1 to 2.0.

Is the request here to change the default, or to add a property or something to HttpClientHandler, e.g. DefaultVersion, that would be used if a version wasn't set on the HttpRequestMessage?

[davidsh]

I don't think we should add a new property for this. We should just change the default of HttpRequestMessage.Version to (2,0) except for NET46 platform. For servers that don't support 2.0, it will fallback to 1.1 anyways.

[davidsh]

See:
https://github.com/dotnet/corefx/blob/master/src/System.Net.Http/src/System/Net/Http/HttpUtilities.cs#L18

We just need to change it from:

```c#
internal static Version DefaultRequestVersion =>

if uap

HttpVersionInternal.Version20;

else

HttpVersionInternal.Version11;

endif

 to:

```c#
internal static Version DefaultRequestVersion =>
#if !net46
    HttpVersionInternal.Version20;
#else
    HttpVersionInternal.Version11;
#endif

[Code-ScottLe]

The idea of defining build directive is faster, but that is still leaving this problem open, with the case of user wants to specify very specifically the earlier version of Http that they want to use (1.1 or 1.0) for various reason. Do you think introducing a new property gonna bring more trouble? I'm kinda torn in between.

[davidsh]

Devs that need more granular control over the HttpRequestMessage.Version already can simply use the HttpClient.SendAsync() method which takes an HttpRequestMessage. The other methods of HttpClient are convenience methods (GetAsync, PostAsync).

I would prefer not cluttering up HttpClient with properties that aren't strictly necessary.

I do think, though, that it is time to change the default of HttpRequestMessage.Version to be 2.0. That would provide immediate value to folks wanting to have most requests start using HTTP/2.0 protocol.

[stephentoub]

cc: @Tratcher

[karelz]

cc @davidfowl

[Code-ScottLe]

@davidsh Valid points. But then again, if you have to makes like 50+ calls and each time you have to make the HttpResponseMessage again just to be able to use the version of choice, it's very error prone and would be nice if we can just set it once and leverage all of the convenience methods like you mentioned above. I thought it is a good improvement from the user perspective, but i also see it from the API maintainers' as well, we need more info on this.

[davidsh]

We should probably split this discussion into 2 parts.

1. Change the default of HttpRequestMessage.Version to 2.0 (except for NET46)

2. Add a new property to HttpClient such as:

c# public Version DefaultRequestVersion { get; set; }

And the default value of this new property can be debated but should probably be 1.1 to avoid app-compat issues and NET46 issues.

[Tratcher]

Since HTTP/2 is more of a transport concern I see no reason to couple it to HttpReqeuestMessage.Version or HttpClient. I would sooner expect a setting on the HttpClientHandler to prefer HTTP/2 over HTTP/1.1, use ALPN to do the negotiation with the server, and I'd enable it by default as the ALPN should handle most of the back-compat issues.

Do you really need to manually switch back and for between HTTP/2 and HTTP/1.1?

[Code-ScottLe]

In my case, I need to switch to 2.0. If we moved the default to 2.0 like @davidsh 's proposal. It will solve the problem that I'm having, but not sure if that gonna break everybody else. I imagine that somebody would need such an option to go back and debating which would be the best option to handle such need.

As of it right now, the only way to modify the version is by setting the Version property of HttpRequestMessage, so I am not sure if I understand this part right.

Since HTTP/2 is more of a transport concern I see no reason to couple it to HttpReqeuestMessage.Version or HttpClient

[Tratcher]

@Code-ScottLe HTTP/2 primarily addresses the wire format and connection management, it has little impact on anything in HttpRequestMessage or HttpResponseMessage. HTTP/2 could be a HttpClientHandler opt-in/out setting rather than hard-coding individual requests to HTTP/2.

[davidsh]

On UWP platform, the underlying WinRT Windows.Web.Http.HttpClient has the HTTP/2 as a property on the HttpBaseProtocolFilter (similar to HttpClientHandler). And on WinRT, there is no Version field for Windows.Web.Http.HttpRequestMessage. This was done that way because HTTP/2 is a connection transport thing and it can't really be turned on/off per HTTP request. And WinRT decided that it wasn't useful to have a Version field on HttpRequestMessage because Wininet ignores that field and only uses HTTP/1.1 (and not HTTP/1.0).

When using WinRT underneath System.Net.Http (on UWP), we had to ignore the HttpRequestMessage.Version field for any request messages sent after the first one that used the HttpClientHandler.

The abstract object model for HttpClient and hence HttpRequestMessage doesn't care or know about the transport HttpClientHandler. So from that point of view, there is an expectation that the Version field on HttpRequestMessage would be used.

With all of that said, I still think for the current HttpClient API, the easiest path is what I suggested earlier:

Change the default of HttpRequestMessage.Version to 2.0 (except for NET46)

Doing this would enable the most scenarios quickly and allows devs to use HttpClient and the GetAsync, PostAsync, etc. APIs and have them default to using HTTP/2.0 (where the server supports it).

[Tratcher]

@davidsh If you set HttpRequestMessage.Version to 2.0 does it gracefully fall back to 1.1 if the server does not negotiate 2.0 via ALPN?

[davidsh]

In general, yes. The current UWP version of System.Net.Http is always setting HTTP/2 on the requests by default. We haven't had any customers report any problems.

We can continue to test more with current CurlHandler (HTTP stack used on .NET Core *nix) as well as WinHTTP (HTTP stack used on .NET Core Windows). But I think that in general, it works the way it is designed. I.e., offer HTTP/2 to the server and if the server doesn't support it, then downgrade to HTTP/1.

[karelz]

Triage: We should change the default of HttpRequestMessage.Version. We should verify it works ok with 1.1-only server.

[geoffkizer]

I think this is fine, but just to clarify:

I think we are saying that HttpRequestMessage.Version effectively means "don't try to send a higher version than this".

It seems kind of unfortunate that it's defined the way it is, since future HTTP versions will always require us to update the default here. I.e. when HTTP3 (or whatever) comes out, we'll want to change the default to 3.0.

It seems like it would be better to change the default to be something that means "the highest version supported by both client and server". Then this would just automatically work in the future without changes. Perhaps we could redefine (0, 0) to mean this?

Right now the only reason anyone should ever set this value is if they have a server that they know doesn't support 1.1 (only 1.0). Even then it's not clear that this is all that useful, since an HTTP/1.1 request should usually be able to be handled properly by a 1.0 server.

[davidsh]

It seems like it would be better to change the default to be something that means "the highest version supported by both client and server". Then this would just automatically work in the future without changes. Perhaps we could redefine (0, 0) to mean this?

We did something similar to this when we added the new SecurityProtocolType.SystemDefault enum value in .NET Framework:

https://msdn.microsoft.com/en-us/library/system.net.securityprotocoltype(v=vs.110).aspx

And for the corresponding SslProtocols enum type, we use the SslProtcols.None value as meaning the same thing:
https://msdn.microsoft.com/en-us/library/system.security.authentication.sslprotocols(v=vs.110).aspx

Yes, we could define a sentinel value to mean "system default" which could change as the platform changes. But we already used Version(0,0) to mean "Unknown"

So, we would probably need something like this:

https://github.com/dotnet/corefx/blob/master/src/System.Net.Primitives/ref/System.Net.Primitives.cs#L282

c# public static class HttpVersion { public static readonly Version Unknown = new Version(0, 0); public static readonly Version Version10 = new Version(1, 0); public static readonly Version Version11 = new Version(1, 1); public static readonly Version Version20 = new Version(2, 0); public static readonly Version SystemDefault = new Version(int.MaxValue, int.MaxValue); // Use the highest version that the platform supports }

[geoffkizer]

Yeah, using MaxValue is probably better.

I'd call this property "Default" as it's really up to the handler, not the system, to decide. In the case of WinHttp, the system default is the handler default. In the case of the managed handler, it's not based on system.

One other random thought: we could make the default null. Probably dangerous for compat, but semantically it makes a lot of sense.

[karelz]

I am fine with change to Default - it makes more sense as @geoffkizer points out.

null might have some compat problems, I suggest to avoid it.
For example: HttpRequestMessage.Version considers null as invalid argument today:
https://github.com/dotnet/corefx/blob/6bf1e7c3032d3c24d5a41bbd880549ef4a8de2da/src/System.Net.Http/src/System/Net/Http/HttpRequestMessage.cs#L30-L43

[terrajobst]

As proposed, the API has a couple of issues:

• The name Default implies policy. We may want to separate the sentinel from whether it's actually the default. For example UseHighestSupported.
• The still has problems as people might compare version numbers against the sentinel which is nonsensical. Instead of that, we should consider changing the APIs that take a version to indicate the way want the highest version by, for example, also accepting a bool.

it would help to see which APIs are expected to handle this value and see code samples for both consumer and producer.

[karelz]

Maybe we should refrain from adding Default and just simply upgrade .NET Core default to Version20 - @davidsh @geoffkizer any thoughts?

[davidsh]

Maybe we should refrain from adding Default and just simply upgrade .NET Core default to Version20 - @davidsh @geoffkizer any thoughts?

That was my initial proposal. I'm fine with that for now.

[geoffkizer]

I'm fine too.

[karelz]

OK, flipping back to enhancement - we will just change the default (on Core). It should be fairly straightforward to do. Anyone interested to do it?