Runtime: [Preview2] Make SocketsHttpHandler default Handler

[karelz]

We want to switch default HttpClientHandler to SocketsHttpHandler for 2.1 (starting with Preview2).

We need to keep the escape route to fall back to WinHttp & LibCurl handlers (when explicitly set).
Most relevant code is here:
https://github.com/dotnet/corefx/blob/master/src/System.Net.Http/src/System/Net/Http/HttpClientHandler.cs#L17-L37

Part of the task is also to:

1. Verify all CoreFX tests pass (esp. Data which depends on Http stack) - incl. outer loop CI legs
2. Verify that WCF tests pass (please work with Paulo & WCF team to run all their test suites, ideally prior to checkin)
3. Keep testing WinHttp & LibCurl handlers (e.g. change ManagedHandler Http test suite to PlatformHandler Http test suite)

cc @geoffkizer

[geoffkizer]

cc @stephentoub @dotnet/ncl

Presumably, we will do this work in the Preview2 branch, right?

[geoffkizer]

@pjanotti How are the WCF stress tests looking?

I wonder if there are Data stress tests we could run as well...

[karelz]

@geoffkizer we will do it first in master. If Preview2 is branched out by the time we check in the change, we will port the change into Preview2 branch.

We also need to keep testing WinHttp & LibCurl handlers (swap ManagedHandler tests to PlatformHandler tests).

[geoffkizer]

Assuming we do this, will we expose CurlHandler publicly? Or only using the env var? WinHttpHandler is already public.

[geoffkizer]

We also need to keep testing WinHttp & LibCurl handlers (swap ManagedHandler tests to PlatformHandler tests).

Yeah. The tricky thing here will be that if we don't make CurlHandler public, there's not a direct way to construct it. This is solvable, just a little tricky.

[stephentoub]

I don't think it should be public. Ideally we would retire it as soon as possible :)

[stephentoub]

Presumably, we will do this work in the Preview2 branch, right?

I think we do it in master, and then if we'be already forked off for preview 2, we port the change to the release branch.

[pjanotti]

@geoffkizer regarding WCF Stress: I tried various variations regarding #threads types of streaming and I am starting to make some variations with very reduced timeouts, and so far I only got timeouts (which is good and expected, while I bit frustrating to me since I was hoping to find some bugs).

Do we have some fuzzing planned around it? This could be a good avenue to find any issues and get more confidence on the new handler.

[stephentoub]

which is good and expected, while a bit frustrating to me since I was hoping to find some bugs

Sorry but not sorry :smile:

[mconnew]

I have a few questions. Is the AppContext switch stable enough that WCF can rely on it to detect which is used? I'd like to add back in our usage of the Expect 100-Continue header and get rid of our priming HEAD request but only when using SocketsHttpHandler.
Is there a way to override this on a per usage basis? For example, there might be a scenario discovered that is broken for WCF when using SocketsHttpHandler but another scenario with an HTTP request not using WCF to a server which requires SocketsHttpHandler, both in the same process. This is especially a concern when this implementation is so new.

[davidsh]

Just to clarify, but this change will not affect UAP version of HttpClientHandler. That will continue to use WinRT underneath.

[mconnew]

@davidsh, that matters a LOT. WinRT HttpClient has some issues which we can't work around. Are there some dependencies missing on WinRT that would be needed to use SocketsHttpHandler?

[davidsh]

Are there some dependencies missing on WinRT that would be needed to use SocketsHttpHandler?

I think in theory SocketsHttpHandler can certainly be used for UAP. However, right now it doesn't have HTTP/2 support. So, that would be a regression in functionality. And regressing UAP functionality could affect Store apps etc.

Right now, this GitHub issue is about non-UAP and just replacing WinHttpHandler and CurlHandler with a single SocketsHttpHandler. There isn't any specific timeline being thought about for doing this also for UAP. Perhaps @karelz can comment further on the timeline and goals for that.

[mconnew]

@davidsh, so this is a great example for my earlier question about changing it per usage. WCF doesn't use HTTP/2 at all but it does really need Expect 100 Continue support as well as mutual Kerberos authentication (I've opened an issue and this looks relatively simple to do in SocketsHttpHandler) and access to the SSL context binding info, none of which are on any roadmap for support in the WinRT HttpClient implementation that I'm aware of. Plus the WinRT HttpClient has more restrictions on where a client certificate comes from and doesn't allow you to ignore all certificate validation errors. So at least for WCF there's a LOT missing from the WinRT HttpClient that WCF needs that the SocketsHttpHandler would provide us. If there was a way to optionally turn on SocketsHttpHandler for WCF but leave other usages using the WinRT HttpClient unless explicitly switched, that would be really useful.

[davidsh]

There are few other interesting things that the WinRT implementation does (via the Windows.Web.Http implementation). For example, it is aware of being a UAP app and it knows how to properly set the parent window handle in order for the not-widely-known dialog to come up for permissions use of a private key for a client cert (if the client cert is coming from the per-user certificate store and not the per-app certificate store).

There are probably a bunch of other interesting things that would have to be tested in order to switch the default of HttpClientHandler on UAP to use SocketsHttpHandler. So, I think that would need to be scheduled before this could be changed on UAP platform.

[geoffkizer]

@mconnew Both SocketsHttpHandler and WinHttpHandler are public, so you can explicitly use these instead of HttpClientHandler if you want control over which is used.

[mconnew]

@geoffkizer, does this mean SocketsHttpHandler and WinHttpHandler are/will be supported on UWP, just not the default? The general idea is I want end developers to be able to choose. Otherwise we need to provide a mechanism in WCF to allow them to specify.

[geoffkizer]

does this mean SocketsHttpHandler and WinHttpHandler are/will be supported on UWP, just not the default

Not for 2.1. As @davidsh said above, this could be considered in the future.

[davidsh]

And WinHttpHandler will never be supported for UWP. The WinHTTP native APIs are not even in the Modern SDK so they can't be used in a Store app.

[stephentoub]

This is solvable, just a little tricky.

Regarding testing, here's what I suggest we do...

Today, we've got a bunch of base classes that contain tests. These tests use virtual CreateHttpClient{Handler}() to create the HttpClient or HttpClientHandler to use and run tests with that. We then have some derived types for SocketsHttpHandler that overrides CreateHttpClient{Handler}() to construct an HttpClient or HttpClientHandler with SocketsHttpHandler.

I suggest we keep the base classes, but make them abstract and make CreateHttpClientHandler abstract. We keep the SocketsHttpHandler testing derived types, but we change its CreateHttpClientHandler() to just new HttpClientHandler(), since SocketsHttpHandler will now be the default. We then add another set of derived types to test the native implementations. And as we're currently doing in SocketsHttpHandler's derived testing types:
https://github.com/dotnet/corefx/blob/master/src/System.Net.Http/tests/FunctionalTests/HttpClientTestBase.cs#L29
we use reflection to access this ctor:
https://github.com/dotnet/corefx/blob/f7bec37564c1b61450090336b9cc76b64480d915/src/System.Net.Http/src/System/Net/Http/HttpClientHandler.Unix.cs#L24
https://github.com/dotnet/corefx/blob/f7bec37564c1b61450090336b9cc76b64480d915/src/System.Net.Http/src/System/Net/Http/HttpClientHandler.Windows.cs#L26
I generally don't like using reflection for testing, but in this case I think it's minimal enough that the benefits outweigh the costs.

We'll also probably need/want to change the various IsXx methods, like IsWinHttpHandler, but it should be fairly straightforward.

[stephentoub]

Made default in https://github.com/dotnet/corefx/pull/27821

[ghost]

@stephentoub, for the build-from-source scenario, is it ok to relax this check: https://github.com/dotnet/corefx/blob/9a2e5d8f0e7f816f983bce75f5bba22811df4994/src/Native/Unix/System.Net.Http.Native/CMakeLists.txt#L4-L6 and make corefx native build without libcurl?

[karelz]

We still need to build CurlHandler as part of CoreFX (you can disable SocketsHttpHandler), so we need to keep it.

[stephentoub]

It's possible it could be made optional, i.e. System.Net.Http.Native.so builds if you have libcurl and is skipped otherwise.