Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

Commit

Permalink
Wrap cert callback leaked exceptions in WinHttpHandler (#24107)
Browse files Browse the repository at this point in the history
Similar to the fix done for CurlHandler (#21938), fix WinHttpHandler so
that leaked exceptions from the user-provided certificate callback are
properly wrapped.

The ManagedHandler still needs to be fixed since it is not wrapping
properly.

Contributes to #21904
  • Loading branch information
davidsh authored Sep 18, 2017
1 parent f472d66 commit ca5d83e
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 12 deletions.
12 changes: 12 additions & 0 deletions src/Common/src/System/Net/Http/WinHttpException.cs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@ public WinHttpException(int error, string message) : base(error, message)
this.HResult = ConvertErrorCodeToHR(error);
}

public WinHttpException(int error, string message, Exception innerException) : base(message, innerException)
{
this.HResult = ConvertErrorCodeToHR(error);
}

public static int ConvertErrorCodeToHR(int error)
{
// This method allows common error detection code to be used by consumers
Expand Down Expand Up @@ -52,6 +57,13 @@ public static WinHttpException CreateExceptionUsingError(int error)
return e;
}

public static WinHttpException CreateExceptionUsingError(int error, Exception innerException)
{
var e = new WinHttpException(error, GetErrorMessage(error), innerException);
ExceptionStackTrace.AddCurrentStack(e);
return e;
}

public static string GetErrorMessage(int error)
{
// Look up specific error message in WINHTTP.DLL since it is not listed in default system resources
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,7 @@ private static void OnRequestSendingRequest(WinHttpRequestState state)

X509Chain chain = null;
SslPolicyErrors sslPolicyErrors;
bool result = false;

try
{
Expand All @@ -281,16 +282,16 @@ private static void OnRequestSendingRequest(WinHttpRequestState state)
out chain,
out sslPolicyErrors);

bool result = state.ServerCertificateValidationCallback(
result = state.ServerCertificateValidationCallback(
state.RequestMessage,
serverCertificate,
chain,
sslPolicyErrors);
if (!result)
{
throw WinHttpException.CreateExceptionUsingError(
(int)Interop.WinHttp.ERROR_WINHTTP_SECURE_FAILURE);
}
}
catch (Exception ex)
{
throw WinHttpException.CreateExceptionUsingError(
(int)Interop.WinHttp.ERROR_WINHTTP_SECURE_FAILURE, ex);
}
finally
{
Expand All @@ -301,6 +302,12 @@ private static void OnRequestSendingRequest(WinHttpRequestState state)

serverCertificate.Dispose();
}

if (!result)
{
throw WinHttpException.CreateExceptionUsingError(
(int)Interop.WinHttp.ERROR_WINHTTP_SECURE_FAILURE);
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,14 +111,16 @@ public async Task UseCallback_CallbackReturnsFailure_ThrowsInnerSecurityFailureE

[OuterLoop] // TODO: Issue #11345
[Fact]
public async Task UseCallback_CallbackThrowsSpecificException_ThrowsInnerSpecificException()
public async Task UseCallback_CallbackThrowsSpecificException_SpecificExceptionPropagatesAsBaseException()
{
var handler = new WinHttpHandler();
handler.ServerCertificateValidationCallback = CustomServerCertificateValidationCallback;
using (var client = new HttpClient(handler))
{
_validationCallbackHistory.ThrowException = true;
await Assert.ThrowsAsync<CustomException>(() => client.GetAsync(System.Net.Test.Common.Configuration.Http.SecureRemoteEchoServer));
HttpRequestException ex = await Assert.ThrowsAsync<HttpRequestException>(() =>
client.GetAsync(System.Net.Test.Common.Configuration.Http.SecureRemoteEchoServer));
Assert.True(ex.GetBaseException() is CustomException);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,14 +204,18 @@ public async Task UseCallback_CallbackReturnsFailure_ThrowsException()
}
}

[ActiveIssue(21904, ~TargetFrameworkMonikers.Uap)]
[OuterLoop] // TODO: Issue #11345
[ConditionalFact(nameof(BackendSupportsCustomCertificateHandling))]
public async Task UseCallback_CallbackThrowsException_ExceptionPropagatesAsInnerException()
public async Task UseCallback_CallbackThrowsException_ExceptionPropagatesAsBaseException()
{
if (ManagedHandlerTestHelpers.IsEnabled)
{
return; // TODO #21904: ManagedHandler is not properly wrapping exception.
}

if (BackendDoesNotSupportCustomCertificateHandling) // can't use [Conditional*] right now as it's evaluated at the wrong time for the managed handler
{
Console.WriteLine($"Skipping {nameof(UseCallback_CallbackThrowsException_ExceptionPropagatesAsInnerException)}()");
Console.WriteLine($"Skipping {nameof(UseCallback_CallbackThrowsException_ExceptionPropagatesAsBaseException)}()");
return;
}

Expand All @@ -222,7 +226,7 @@ public async Task UseCallback_CallbackThrowsException_ExceptionPropagatesAsInner
handler.ServerCertificateCustomValidationCallback = delegate { throw e; };

HttpRequestException ex = await Assert.ThrowsAsync<HttpRequestException>(() => client.GetAsync(Configuration.Http.SecureRemoteEchoServer));
Assert.Same(e, ex.InnerException);
Assert.Same(e, ex.GetBaseException());
}
}

Expand Down

0 comments on commit ca5d83e

Please sign in to comment.