Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Fix HttpClientHandler ServerCertificateCustomValidationCallback for UWP #21905

Merged
davidsh merged 2 commits into from
Jul 7, 2017
Merged

Fix HttpClientHandler ServerCertificateCustomValidationCallback for UWP #21905

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1dd0427
Fix HttpClientHandler ServerCertificateCustomValidationCallback for UWP
davidsh Jul 5, 2017
e3657ee
Address more PR feedback
davidsh Jul 6, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpRequestCallback.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ private static void OnRequestError(WinHttpRequestState state, Interop.WinHttp.WI
{
// TODO: Issue #2165. We need to pass in the cancellation token from the
// user's ReadAsync() call into the TrySetCanceled().
Debug.WriteLine("RequestCallback: QUERY_DATA_AVAILABLE - ERROR_WINHTTP_OPERATION_CANCELLED");
WinHttpTraceHelper.Trace("RequestCallback: QUERY_DATA_AVAILABLE - ERROR_WINHTTP_OPERATION_CANCELLED");
state.LifecycleAwaitable.SetCanceled();
}
else
Expand All @@ -365,7 +365,7 @@ private static void OnRequestError(WinHttpRequestState state, Interop.WinHttp.WI
{
// TODO: Issue #2165. We need to pass in the cancellation token from the
// user's ReadAsync() call into the TrySetCanceled().
Debug.WriteLine("RequestCallback: API_READ_DATA - ERROR_WINHTTP_OPERATION_CANCELLED");
WinHttpTraceHelper.Trace("RequestCallback: API_READ_DATA - ERROR_WINHTTP_OPERATION_CANCELLED");
state.LifecycleAwaitable.SetCanceled();
}
else
Expand All @@ -379,7 +379,7 @@ private static void OnRequestError(WinHttpRequestState state, Interop.WinHttp.WI
{
// TODO: Issue #2165. We need to pass in the cancellation token from the
// user's WriteAsync() call into the TrySetCanceled().
Debug.WriteLine("RequestCallback: API_WRITE_DATA - ERROR_WINHTTP_OPERATION_CANCELLED");
WinHttpTraceHelper.Trace("RequestCallback: API_WRITE_DATA - ERROR_WINHTTP_OPERATION_CANCELLED");
state.TcsInternalWriteDataToRequestStream.TrySetCanceled();
}
else
Expand Down
3 changes: 3 additions & 0 deletions src/System.Net.Http/src/Resources/Strings.resx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -298,4 +298,7 @@
<data name="net_http_libcurl_revocation_notsupported" xml:space="preserve">
<value>The handler does not support changing revocation settings with this combination of libcurl ({0}) and its SSL backend ("{1}").</value>
</data>
<data name="net_http_feature_requires_Windows10Version1607" xml:space="preserve">
<value>Using this feature requires Windows 10 Version 1607.</value>
</data>
</root>
12 changes: 7 additions & 5 deletions src/System.Net.Http/src/System.Net.Http.csproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,9 @@
<ItemGroup Condition="'$(TargetGroup)' == 'uap'">
<Reference Include="Windows" />
<Reference Include="mscorlib" />
<Reference Include="System.Runtime.InteropServices.WindowsRuntime" />
<Reference Include="System.Runtime.WindowsRuntime" />
<Reference Include="System.Security.Cryptography.X509Certificates" />
<Reference Include="System.Text.Encoding.Extensions" />
<Compile Include="$(CommonPath)\System\NotImplemented.cs">
<Link>Common\System\NotImplemented.cs</Link>
Expand All @@ -389,11 +391,11 @@
<Compile Include="$(CommonPath)\System\Threading\Tasks\TaskToApm.cs">
<Link>Common\System\Threading\Tasks\TaskToApm.cs</Link>
</Compile>
<Compile Include="netcore50\System\Net\cookie.cs" />
<Compile Include="netcore50\System\Net\cookieexception.cs" />
<Compile Include="netcore50\System\Net\CookieHelper.cs" />
<Compile Include="netcore50\System\Net\HttpHandlerToFilter.cs" />
<Compile Include="netcore50\System\Net\HttpClientHandler.cs" />
<Compile Include="uap\System\Net\cookie.cs" />
<Compile Include="uap\System\Net\cookieexception.cs" />
<Compile Include="uap\System\Net\CookieHelper.cs" />
<Compile Include="uap\System\Net\HttpHandlerToFilter.cs" />
<Compile Include="uap\System\Net\HttpClientHandler.cs" />
</ItemGroup>
<ItemGroup Condition="'$(TargetGroup)' == 'netcoreapp' OR '$(TargetGroup)' == 'uap'">
<Reference Include="Microsoft.Win32.Primitives" />
Expand Down
0 .../src/netcore50/System/Net/CookieHelper.cs → ...t.Http/src/uap/System/Net/CookieHelper.cs
View file
Open in desktop
File renamed without changes.
156 changes: 147 additions & 9 deletions ...netcore50/System/Net/HttpClientHandler.cs → ...p/src/uap/System/Net/HttpClientHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,12 @@
using System.Net;
using System.Net.Http.Headers;
using System.Net.Security;
using System.Runtime.ExceptionServices;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using System.Runtime.InteropServices.WindowsRuntime;
using System.Security.Authentication;
using System.Security.Cryptography;
using System.Security.Cryptography.X509Certificates;
using System.Threading;
using System.Threading.Tasks;
Expand All @@ -20,22 +23,32 @@
using RTHttpCacheReadBehavior = Windows.Web.Http.Filters.HttpCacheReadBehavior;
using RTHttpCacheWriteBehavior = Windows.Web.Http.Filters.HttpCacheWriteBehavior;
using RTHttpCookieUsageBehavior = Windows.Web.Http.Filters.HttpCookieUsageBehavior;
using RTHttpRequestMessage = Windows.Web.Http.HttpRequestMessage;
using RTPasswordCredential = Windows.Security.Credentials.PasswordCredential;
using RTCertificate = Windows.Security.Cryptography.Certificates.Certificate;
using RTCertificateQuery = Windows.Security.Cryptography.Certificates.CertificateQuery;
using RTCertificateStores = Windows.Security.Cryptography.Certificates.CertificateStores;
using RTChainValidationResult = Windows.Security.Cryptography.Certificates.ChainValidationResult;
using RTHttpServerCustomValidationRequestedEventArgs = Windows.Web.Http.Filters.HttpServerCustomValidationRequestedEventArgs;
using RTIBuffer = Windows.Storage.Streams.IBuffer;

namespace System.Net.Http
{
public partial class HttpClientHandler : HttpMessageHandler
{
private const string RequestMessageLookupKey = "System.Net.Http.HttpRequestMessage";
private const string SavedExceptionDispatchInfoLookupKey = "System.Runtime.ExceptionServices.ExceptionDispatchInfo";
private const string ClientAuthenticationOID = "1.3.6.1.5.5.7.3.2";
private static Oid s_serverAuthOid = new Oid("1.3.6.1.5.5.7.3.1", "1.3.6.1.5.5.7.3.1");
private static readonly Lazy<bool> s_RTCookieUsageBehaviorSupported =
new Lazy<bool>(InitRTCookieUsageBehaviorSupported);
private static bool RTCookieUsageBehaviorSupported => s_RTCookieUsageBehaviorSupported.Value;
private static readonly Lazy<bool> s_RTNoCacheSupported =
new Lazy<bool>(InitRTNoCacheSupported);
private static bool RTNoCacheSupported => s_RTNoCacheSupported.Value;
private static readonly Lazy<bool> s_RTServerCustomValidationRequestedSupported =
new Lazy<bool>(InitRTServerCustomValidationRequestedSupported);
private static bool RTServerCustomValidationRequestedSupported => s_RTServerCustomValidationRequestedSupported.Value;

#region Fields

Expand All @@ -55,6 +68,7 @@ public partial class HttpClientHandler : HttpMessageHandler
private IWebProxy _proxy;
private X509Certificate2Collection _clientCertificates;
private IDictionary<String, Object> _properties; // Only create dictionary when required.
private Func<HttpRequestMessage, X509Certificate2, X509Chain, SslPolicyErrors, bool> _serverCertificateCustomValidationCallback;

#endregion Fields

Expand Down Expand Up @@ -291,18 +305,23 @@ public X509CertificateCollection ClientCertificates

public Func<HttpRequestMessage, X509Certificate2, X509Chain, SslPolicyErrors, bool> ServerCertificateCustomValidationCallback
{
// TODO: Not yet implemented. Issue #7623.
get{ return null; }
get
{
return _serverCertificateCustomValidationCallback;
}
set
{
CheckDisposedOrStarted();
if (value != null)
{
/*
throw new PlatformNotSupportedException(String.Format(CultureInfo.InvariantCulture,
SR.net_http_value_not_supported, value, nameof(ServerCertificateCustomValidationCallback)));
*/
if (!RTServerCustomValidationRequestedSupported)
{
throw new PlatformNotSupportedException(string.Format(CultureInfo.InvariantCulture,
SR.net_http_feature_requires_Windows10Version1607));
}
}

_serverCertificateCustomValidationCallback = value;
}
}

Expand Down Expand Up @@ -349,6 +368,8 @@ public HttpClientHandler()
{
_rtFilter = new RTHttpBaseProtocolFilter();
_handlerToFilter = new HttpHandlerToFilter(_rtFilter);
_handlerToFilter.RequestMessageLookupKey = RequestMessageLookupKey;
_handlerToFilter.SavedExceptionDispatchInfoLookupKey = SavedExceptionDispatchInfoLookupKey;
_diagnosticsPipeline = new DiagnosticsHandler(_handlerToFilter);

_clientCertificateOptions = ClientCertificateOption.Manual;
Expand Down Expand Up @@ -573,7 +594,7 @@ protected internal override async Task<HttpResponseMessage> SendAsync(HttpReques
}
catch (Exception ex)
{
// Convert back to the expected exception type
// Convert back to the expected exception type.
throw new HttpRequestException(SR.net_http_client_execution_error, ex);
}

Expand Down Expand Up @@ -645,10 +666,37 @@ private void SetOperationStarted()
{
if (!_operationStarted)
{
// Since this is the first operation, we set the proxy and server credentials on the
// WinRT filter based on the .NET handler's property settings.
// Since this is the first operation, we set all the necessary WinRT filter properties.
SetFilterProxyCredential();
SetFilterServerCredential();
if (_serverCertificateCustomValidationCallback != null)
{
Debug.Assert(RTServerCustomValidationRequestedSupported);

// The WinRT layer uses a different model for the certificate callback. The callback is
// considered "extra" validation. We need to explicitly ignore errors so that the callback
// will get called.
//
// In addition, the WinRT layer restricts some errors so that they cannot be ignored, such
Copy link
Copy Markdown

@CIPop CIPop Jul 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't find any documentation on this here or here.

Copy link
Copy Markdown
Member

@Diego-Perez-Botero Diego-Perez-Botero Jul 5, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From your first link:

Default OS validation of the server certificate is performed before raising this event. If the certificate fails this validation, the connection is terminated and your event handler is not called.

In order to skip parts of the OS validation (not recommended for production scenarios), use the IgnorableServerCertificateErrors property to specify the errors that you want to ignore. Then as long as the certificate does not have any other errors, the OS validation will be considered successful and your event handler will be called.

Copy link
Copy Markdown
Member

@Diego-Perez-Botero Diego-Perez-Botero Jul 5, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or are you asking specifically about the ChainValidationResult values that the app is allowed to ignore?

Copy link
Copy Markdown
Contributor Author

@davidsh davidsh Jul 5, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree the documentation is not complete in all WinRT areas.

For example, trying to do this:

var filter = new Windows.Web.Http.Filters.HttpBaseProtocolFilter();
filter.IgnorableServerCertificateErrors.Add(ChainValidationResult.Revoked);

results in this exception:

System.ArgumentException: The parameter is incorrect.
'item': Provided value is not an ignorable ChainValidationResult value.

// as "Revoked". This will result in behavior differences between UWP and other platforms.
// The following errors cannot be ignored right now in the WinRT layer:
//
// ChainValidationResult.BasicConstraintsError
// ChainValidationResult.InvalidCertificateAuthorityPolicy
// ChainValidationResult.InvalidSignature
// ChainValidationResult.OtherErrors
// ChainValidationResult.Revoked
Copy link
Copy Markdown
Member

@Diego-Perez-Botero Diego-Perez-Botero Jul 5, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ChainValidationResult.Revoked can be ignored, but not ChainValidationResult.IncompleteChain.

Update: This was based on the Wininet flags that are currently available in the OS (see WINHTTP_OPTION_SECURITY_FLAGS in MSDN), but it turns out that WinRT has its own list of ignorable conditions.

Copy link
Copy Markdown
Member

@Diego-Perez-Botero Diego-Perez-Botero Jul 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be more future-proof to just add ALL ChainValidationResult values to the IgnorableServerCertificateErrors, and just comment that some of them will not effectively be ignored?

Copy link
Copy Markdown
Contributor Author

@davidsh davidsh Jul 5, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Diego-Perez-Botero See my comment above. Passing in ChainValidation.Revoked to the filter.IgnorableServerCertificateErrors.Add returns an error.

Copy link
Copy Markdown
Contributor Author

@davidsh davidsh Jul 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be more future-proof to just add ALL ChainValidationResult values to the IgnorableServerCertificateErrors, and just comment that some of them will not effectively be ignored?

Yes, but passing in ones that can't be ignored throws exceptions from the Add() method. And I didn't want to add exception throwing noise all the time even if we catch it. It adds noise to debugging sessions.

Copy link
Copy Markdown
Member

@Diego-Perez-Botero Diego-Perez-Botero Jul 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. I didn't know that exceptions were thrown for "unignorable" values :)

// ChainValidationResult.UnknownCriticalExtension
_rtFilter.IgnorableServerCertificateErrors.Add(RTChainValidationResult.Expired);
_rtFilter.IgnorableServerCertificateErrors.Add(RTChainValidationResult.IncompleteChain);
_rtFilter.IgnorableServerCertificateErrors.Add(RTChainValidationResult.InvalidName);
_rtFilter.IgnorableServerCertificateErrors.Add(RTChainValidationResult.RevocationFailure);
_rtFilter.IgnorableServerCertificateErrors.Add(RTChainValidationResult.RevocationInformationMissing);
_rtFilter.IgnorableServerCertificateErrors.Add(RTChainValidationResult.Untrusted);
_rtFilter.IgnorableServerCertificateErrors.Add(RTChainValidationResult.WrongUsage);
_rtFilter.ServerCustomValidationRequested += RTServerCertificateCallback;
Copy link
Copy Markdown
Member

@Diego-Perez-Botero Diego-Perez-Botero Jul 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this need to be done in "light up" fashion (checking that the custom validation callback API surface is available)? This property was added in 14393, so I'm not sure if "light up" logic is in order.

Copy link
Copy Markdown
Contributor Author

@davidsh davidsh Jul 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I added light-up for this. The fallback is to ignore any callback.

Copy link
Copy Markdown
Member

@bartonjs bartonjs Jul 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fallback is to ignore any callback.

I think the fallback has to be to throw. Not all callbacks are ignoring errors, some of them are imposing restrictions. You can't let a connection transmit data if a custom callback didn't get to have its say.

Copy link
Copy Markdown
Contributor Author

@davidsh davidsh Jul 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can't let a connection transmit data if a custom callback didn't get to have its say.

This is a good point. I think this demonstrates one example where we have to throw an exception at runtime if the light-up detection fails and we are trying to use that feature.

We can either throw an exception during the property setter of the callback. That would be a PNSE (PlatformNotSupportedException). Otherwise, if we validate everything during .SendAsync, we'll need to throw an HttpRequestException with perhaps a PNSE inside that.

I'm leaning toward throwing at the property setter since it is can be calculated right away that the feature won't work since we're running on a Windows 10 OS version before that callback was implemented in WinRT.

cc: @Diego-Perez-Botero @DavidGoll @CIPop

Copy link
Copy Markdown
Member

@stephentoub stephentoub Jul 7, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can either throw an exception during the property setter or the callback

For reference, we have a similar issue with CurlHandler, and we throw the PNSE during the request/callback. While in some situations we could detect this based purely on inspecting the libcurl version/build being used, in general we don't know whether a callback is supported until we try to register it with libcurl and it gives us an answer about whether it could hook it up.

}

_operationStarted = true;
}
}
Expand Down Expand Up @@ -684,6 +732,96 @@ private static bool InitRTNoCacheSupported()
"NoCache");
}

private static bool InitRTServerCustomValidationRequestedSupported()
{
return RTApiInformation.IsEventPresent(
"Windows.Web.Http.Filters.HttpBaseProtocolFilter",
"ServerCustomValidationRequested");
}

private void RTServerCertificateCallback(RTHttpBaseProtocolFilter sender, RTHttpServerCustomValidationRequestedEventArgs args)
{
bool success = RTServerCertificateCallbackHelper(
args.RequestMessage,
args.ServerCertificate,
args.ServerIntermediateCertificates,
args.ServerCertificateErrors);

if (!success)
{
args.Reject();
}
}

private bool RTServerCertificateCallbackHelper(
RTHttpRequestMessage requestMessage,
RTCertificate cert,
IReadOnlyList<RTCertificate> intermediateCerts,
IReadOnlyList<RTChainValidationResult> certErrors)
{
// Convert WinRT certificate to .NET certificate.
X509Certificate2 serverCert = ConvertPublicKeyCertificate(cert);

// Create .NET X509Chain from the WinRT information. We need to rebuild the chain since WinRT only
// gives us an array of intermediate certificates and not a X509Chain object.
var serverChain = new X509Chain();
SslPolicyErrors sslPolicyErrors = SslPolicyErrors.None;
foreach (RTCertificate intermediateCert in intermediateCerts)
{
serverChain.ChainPolicy.ExtraStore.Add(ConvertPublicKeyCertificate(cert));
}
serverChain.ChainPolicy.RevocationMode = X509RevocationMode.Online; // WinRT always checks revocation.
serverChain.ChainPolicy.RevocationFlag = X509RevocationFlag.ExcludeRoot;
// Authenticate the remote party: (e.g. when operating in client mode, authenticate the server).
serverChain.ChainPolicy.ApplicationPolicy.Add(s_serverAuthOid);
if (!serverChain.Build(serverCert))
{
sslPolicyErrors |= SslPolicyErrors.RemoteCertificateChainErrors;
}

// Determine name-mismatch error from the existing WinRT information since .NET X509Chain.Build does not
// return that in the X509Chain.ChainStatus fields.
foreach (RTChainValidationResult result in certErrors)
{
if (result == RTChainValidationResult.InvalidName)
{
sslPolicyErrors |= SslPolicyErrors.RemoteCertificateNameMismatch;
break;
}
}

// Get the .NET HttpRequestMessage we saved in the property bag of the WinRT HttpRequestMessage.
HttpRequestMessage request = (HttpRequestMessage)requestMessage.Properties[RequestMessageLookupKey];

// Call the .NET callback.
bool success = false;
try
{
success = _serverCertificateCustomValidationCallback(request, serverCert, serverChain, sslPolicyErrors);
}
catch (Exception ex)
{
// Save the exception info. We will return it later via the SendAsync response processing.
requestMessage.Properties.Add(
SavedExceptionDispatchInfoLookupKey,
ExceptionDispatchInfo.Capture(ex));
}
finally
{
serverChain.Dispose();
serverCert.Dispose();
}

return success;
}

private X509Certificate2 ConvertPublicKeyCertificate(RTCertificate cert)
{
// Convert Windows X509v2 cert to .NET X509v2 cert.
RTIBuffer blob = cert.GetCertificateBlob();
return new X509Certificate2(blob.ToArray());
}

#endregion Helpers
}
}
29 changes: 27 additions & 2 deletions ...tcore50/System/Net/HttpHandlerToFilter.cs → ...src/uap/System/Net/HttpHandlerToFilter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using System.Collections.Generic;
using System.Diagnostics;
using System.IO;
using System.Runtime.ExceptionServices;
using System.Runtime.InteropServices.WindowsRuntime;
using System.Threading;
using System.Threading.Tasks;
Expand Down Expand Up @@ -39,6 +40,9 @@ internal HttpHandlerToFilter(RTHttpBaseProtocolFilter filter)
_filterMaxVersionSet = 0;
}

internal string RequestMessageLookupKey { get; set; }
internal string SavedExceptionDispatchInfoLookupKey { get; set; }

protected internal override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancel)
{
if (request == null)
Expand All @@ -47,9 +51,27 @@ protected internal override async Task<HttpResponseMessage> SendAsync(HttpReques
}
cancel.ThrowIfCancellationRequested();


RTHttpRequestMessage rtRequest = await ConvertRequestAsync(request).ConfigureAwait(false);
RTHttpResponseMessage rtResponse = await _next.SendRequestAsync(rtRequest).AsTask(cancel).ConfigureAwait(false);

RTHttpResponseMessage rtResponse;
try
{
rtResponse = await _next.SendRequestAsync(rtRequest).AsTask(cancel).ConfigureAwait(false);
}
catch (TaskCanceledException)
Copy link
Copy Markdown
Member

@stephentoub stephentoub Jul 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just TaskCanceledException, or any OperationCanceledException?

Copy link
Copy Markdown
Contributor Author

@davidsh davidsh Jul 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I copied this pattern from here:
https://github.com/dotnet/corefx/blob/master/src/System.Net.Http/src/System/Net/Http/DiagnosticsHandler.cs#L100

If it's wrong there, we can fix both in later PR.

{
throw;
}
catch (Exception)
Copy link
Copy Markdown

@CIPop CIPop Jul 5, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if there are two separate kinds of Exception types (one saved and a completely different one here)?

Copy link
Copy Markdown
Contributor Author

@davidsh davidsh Jul 6, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First, I changed the "savedException" of type Exception to be a "savedExceptionDispatchInfo" of type ExceptionDispatchInfo as recommended by @stephentoub.

If there is a saved ExceptionDispatchInfo, we will use it because at the point it happened, the user callback (.NET callback) threw an unhandled exception. We need to report that. It also stops all further processing of the HTTP request, so there are no other exceptions we need to worry about.

When such an unhandled exception is thrown by the user provided .NET callback, our WinRTCertificateCallback wrapper traps that then calls Reject() on the WinRT callback layer. That causes the WinRT layer to throw an exception. That is really the exception we are trapping here in that case. And so, we need to see if we saved the ExceptionDispatchInfo object because that is really the root cause of why the WinRT layer threw an exception. And so, we need to propagate that.

{
object info;
if (rtRequest.Properties.TryGetValue(SavedExceptionDispatchInfoLookupKey, out info))
{
((ExceptionDispatchInfo)info).Throw();
}

throw;
}

// Update in case of redirects
request.RequestUri = rtRequest.RequestUri;
Expand All @@ -64,6 +86,9 @@ private async Task<RTHttpRequestMessage> ConvertRequestAsync(HttpRequestMessage
{
RTHttpRequestMessage rtRequest = new RTHttpRequestMessage(new RTHttpMethod(request.Method.Method), request.RequestUri);

// Add a reference from the WinRT object back to the .NET object.
rtRequest.Properties.Add(RequestMessageLookupKey, request);

// We can only control the Version on the first request message since the WinRT API
// has this property designed as a filter/handler property. In addition the overall design
// of HTTP/2.0 is such that once the first request is using it, all the other requests
Expand Down
0 ...t.Http/src/netcore50/System/Net/cookie.cs → ...tem.Net.Http/src/uap/System/Net/cookie.cs
View file
Open in desktop
File renamed without changes.
0 ...c/netcore50/System/Net/cookieexception.cs → ...ttp/src/uap/System/Net/cookieexception.cs
View file
Open in desktop
File renamed without changes.
6 changes: 4 additions & 2 deletions src/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.AcceptAllCerts.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ namespace System.Net.Http.Functional.Tests

public class HttpClientHandler_DangerousAcceptAllCertificatesValidator_Test
{
// TODO: https://github.com/dotnet/corefx/issues/7812
private static bool ClientSupportsDHECipherSuites => (!PlatformDetection.IsWindows || PlatformDetection.IsWindows10Version1607OrGreater);

[Fact]
public void SingletonReturnsTrue()
{
Expand Down Expand Up @@ -57,9 +60,8 @@ await TestHelper.WhenAllCompletedOrAnyFailed(
new object[] { Configuration.Http.WrongHostNameCertRemoteServer },
};

[ActiveIssue(7812, TestPlatforms.Windows)]
[OuterLoop] // TODO: Issue #11345
[Theory]
[ConditionalTheory(nameof(ClientSupportsDHECipherSuites))]
Copy link
Copy Markdown
Member

@bartonjs bartonjs Jul 6, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a negative behavior that we should be validating? Like a kind of exception?

Copy link
Copy Markdown
Contributor Author

@davidsh davidsh Jul 6, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please refer to #7812. This is to prevent CI failures. We don't want these tests running on Windows OS versions that don't have the fixes to the DHE handshake bugs. It is not a feature we want to test, per se.

[MemberData(nameof(InvalidCertificateServers))]
public async Task InvalidCertificateServers_CertificateValidationDisabled_Succeeds(string url)
{
Expand Down
Loading