Include native underlying handler support in HttpClientHandler for iOS/tvOS/MacCatalyst and Android

[steveisok]

Completes the plan for net6 laid out in https://github.com/dotnet/designs/blob/main/accepted/2020/mono-convergence/platform-specific-httpclient.md#the-plan-for-net-5

This change supports using either the native HttpMessageHandler types that exist in the Xamarin repos (NSUrlSessionHandler for iOS/tvOS/Catalyst and AndroidMessageHandler for Android) or SocketsHttpHandler as the underlying handler in HttpClientHandler.

The behavior for new HttpClient() was established earlier in net6, but did not go all the way. For example, if the System.Net.Http.UseNativeHttpHandler feature switch was set to true, using HttpClient in different ways would lead to different underlying handlers.

Before this PR:

// System.Net.Http.UseNativeHttpHandler == true

new HttpClient();    // Chooses the native handler as the underlying handler

var handler = new HttpClientHandler();       // SocketsHttpHandler is the only choice 
new HttpClient(handler);

The change creates a handful of partial HttpClientHandler files in order to split out the platform specific parts. As you review the PR, you'll notice a bunch of if (IsSocketHandler) blocks. The intent of these are to make use of the linker and get linked out once the linker knows which handler is the active one.

get
{
    if (IsSocketHandler)
    {
        return _socketHandler!.UseCookies;
    }
    else
    {
        return GetUseCookies();
    }
}

Get and Set methods like GetUseCookies make it easier to tell the linker via DynamicDependency to preserve the reflection calls being made to the native underlying handler.

[DynamicDependency("get_UseCookies", "Xamarin.Android.Net.AndroidMessageHandler", "Mono.Android")]
private bool GetUseCookies() => (bool)InvokeNativeHandlerMethod("get_UseCookies");

It is important to point out that the underlying handler has to be derived from HttpMessageHandler. It cannot be HttpClientHandler or you'll end up with a circular dependency.

Note, there will need to be additional changes to Xamarin Android once this change goes in and before they take on the new version. We will coordinate appropriately.

/cc @dalexsoto @rolfbjarne @jonpryor @jonathanpeppers

[dotnet-issue-labeler]

Note regarding the new-api-needs-documentation label:

This serves as a reminder for when your PR is modifying a ref *.cs file and adding/modifying public APIs, to please make sure the API implementation in the src *.cs file is documented with triple slash comments, so the PR reviewers can sign off that change.

[ghost]

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Completes the plan for net6 laid out in https://github.com/dotnet/designs/blob/main/accepted/2020/mono-convergence/platform-specific-httpclient.md#the-plan-for-net-5

This change supports using either the native HttpMessageHandler types that exist in the Xamarin repos (NSUrlSessionHandler for iOS/tvOS/Catalyst and AndroidMessageHandler for Android) or SocketsHttpHandler as the underlying handler in HttpClientHandler.

The behavior for new HttpClient() was established earlier in net6, but did not go all the way. For example, if the System.Net.Http.UseNativeHttpHandler feature switch was set to true, using HttpClient in different ways would lead to different underlying handlers.

Before this PR:

// System.Net.Http.UseNativeHttpHandler == true

new HttpClient();    // Chooses the native handler as the underlying handler

var handler = new HttpClientHandler();       // SocketsHttpHandler is the only choice 
new HttpClient(handler);

The change creates a handful of partial HttpClientHandler files in order to split out the platform specific parts. As you review the PR, you'll notice a bunch of if (IsSocketHandler) blocks. The intent of these are to make use of the linker and get linked out once the linker knows which handler is the active one.

get
{
    if (IsSocketHandler)
    {
        return _socketHandler!.UseCookies;
    }
    else
    {
        return GetUseCookies();
    }
}

Get and Set methods like GetUseCookies make it easier to tell the linker via DynamicDependency to preserve the reflection calls being made to the native underlying handler.

[DynamicDependency("get_UseCookies", "Xamarin.Android.Net.AndroidMessageHandler", "Mono.Android")]
private bool GetUseCookies() => (bool)InvokeNativeHandlerMethod("get_UseCookies");

It is important to point out that the underlying handler has to be derived from HttpMessageHandler. It cannot be HttpClientHandler or you'll end up with a circular dependency.

Note, there will need to be additional changes to Xamarin Android once this change goes in and before they take on the new version. We will coordinate appropriately.

/cc @dalexsoto @rolfbjarne @jonpryor @jonathanpeppers

Author:	steveisok
Assignees:	-
Labels:	area-System.Net.Http, new-api-needs-documentation
Milestone:	-

[filipnavara]

This would be nice to have on Apple platforms. There's an underlying API with slightly different signature that is missing the X509Certificate2 wrapping and passes in the native Apple objects instead. In some earlier Xamarin issue/PR there was a code to do conversion between the two worlds. The reason it was not done directly was because of code size issues.

If this PR gets in then we should open a tracking issue to map this property (and possibly others too). Just putting it here as a reminder.

[filipnavara]

This should also be tracked in some issue. It's likely something that people will run into.

[filipnavara]

This seems to run counter to the documentation:

Particularly in test scenarios, a common pattern use HttpClient to connect to a server with a certificate that shouldn't be validated, such as a self-signed certificate. You commonly do this with HttpClientHandler by setting the ServerCertificateCustomValidationCallback property to a delegate that always returns True; this indicates that the certificate has passed validation. However, not all implementations support this callback, and some throw PlatformNotSupportedException.

The DangerousAcceptAnyServerCertificateValidator property addresses this limitation. The delegate returned by the DangerousAcceptAnyServerCertificateValidator property can be assigned to the ServerCertificateCustomValidationCallback property, as the following example does:

handler.ServerCertificateCustomValidationCallback = httpClientHandler.DangerousAcceptAnyServerCertificateValidator;

This gives HttpClientHandler implementations a known object reference identity that expresses the developer's intention. If the object stored in the DangerousAcceptAnyServerCertificateValidator property is reference equals to DangerousAcceptAnyServerCertificateValidator, the runtime is able to entirely disable validation on a platform that would otherwise throw a PlatformNotSupportedException.

[steveisok]

We may be able to pull something off w/ the native handlers. As for SocketsHttpHandler (on Android at least), I don't think we'll be able to be able to support it. Maybe we'll be able to apply an ugly tweak, but as of right now it's not likely.

[filipnavara]

The minimal implementation is to return static (...) => true delegate here in DangerousAcceptAnyServerCertificateValidator and then check for it by reference in ServerCertificateCustomValidationCallback setter and not throw PlatformNotSupportedException. At least that's how I read the documentation. It doesn't necessarily guarantee that the validation gets bypassed on platforms where that is not possible. I could be reading the documentation wrong though (and on second re-read I think I read it wrong 😩).

SockersHttpHandler on Android may be tricky and I am aware of the limitations in SslStream implementation. For something like NSUrlSessionHandler it would be possible to expose DangerousAcceptAnyTrust callback and assign it through reflection to TrustOverrideForUrl.

[marek-safar]

Would it make sense to substitute on both true and false values?

[steveisok]

From what I've tested, the current xml does substitute on true and false values.

[rolfbjarne]

CC @spouliot

[steveisok]

Failures are infra related.