Broken error handling in WinHttpHandler
- Dominant language
- C#
- Stars
- 18.3k
- Forks
- 5.6k
- PR merge metrics
- PR metrics pending
Description
### Description
WinHttpHandler's StartRequestAsync() contains an outer try-catch, where a Task object (`sendRequestBodyTask`) with the result from sending a request body is awaited in the finally-block without any error-handling:
https://github.com/dotnet/runtime/blob/6a06e6629ab7e97216ebff3bba840e0d209d93f5/src/libraries/System.Net.Http.WinHttpHandler/src/System/Net/Http/WinHttpHandler.cs#L1018-L1038
The Task returned from StartRequestAsync() is never awaited or chained to a task continuation, so it is not expected to end up in a faulted state with an Exception (a TaskCompletionSource is used to chain a result or an exception back to the caller in other code paths). But the code path where `sendRequestBodyTask` is awaited makes it more than likely that the Task _will_ end up in a faulted state. This will lead to unobserved exceptions which could cause an application to crash (depending on how the event handler for unobserved exceptions is wired up). For example:
```
System.AggregateException: A Task's exception(s) were not observed either by Waiting on the Task or accessing its Exception property. As a result, the unobserved exception was rethrown by the finalizer thread. (Error while copying content to a stream.)
---> System.Net.Http.HttpRequestException: Error while copying content to a stream.
---> System.IO.IOException: The write operation failed, see inner exception.
---> System.Net.Http.WinHttpException (80072F78, 12152): Error 12152 calling WinHttpWriteData, 'The server returned an invalid or unrecognized response'.
--- End of inner exception stack trace ---
at [... HttpContent-specific stack trace ...]
at System.Net.Http.HttpContent.g__WaitAsync|56_0(ValueTask copyTask)
--- End of inner exception stack trace ---
at System.Net.Http.HttpContent.g__WaitAsync|56_0(ValueTask copyTask)
at System.Net.Http.WinHttpHandler.InternalSendRequestBodyAsync(WinHttpRequestState state, WinHttpChunkMode chunkedModeForSend)
at System.Net.Http.WinHttpHandler.StartRequestAsync(WinHttpRequestState state)
--- End of inner exception stack trace ---
```
### Reproduction Steps
The issue can be reproduced consistently by using a fake HttpContent object that emits a synthetic exception, as shown below (reproducing the issue consistently in a test using real code and real exceptions is a lot harder, due to timing difficulties). The complexity of the code used to reproduce the problem is merely from being able to force it to happen deterministically -- in practice, however, this issue may occur for any simple HTTP request that sends a request body (i.e. POST/PUT/etc. request).
```csharp
private sealed class CustomHttpContent : HttpContent
{
private readonly TaskCompletionSource calledTcs;
private readonly TaskCompletionSource blockTcs;
public CustomHttpContent(TaskCompletionSource calledTcs, TaskCompletionSource blockTcs)
{
this.calledTcs = calledTcs;
this.blockTcs = blockTcs;
}
///
protected override async Task SerializeToStreamAsync(Stream stream, TransportContext context)
{
calledTcs.TrySetResult(null);
await blockTcs.Task.ConfigureAwait(false);
}
///
protected override bool TryComputeLength(out long length)
{
length = 10000;
return true;
}
}
[Test]
public async Task WinHttp_Error_Handling()
{
var uhTcs = new TaskCompletionSource();
var uoTcs = new TaskCompletionSource();
AppDomain.CurrentDomain.UnhandledException += (sender, args) =>
{
uhTcs.TrySetResult(args.ExceptionObject);
};
TaskScheduler.UnobservedTaskException += (sender, args) =>
{
uoTcs.TrySetResult(args.Exception);
};
var tcpListener = new TcpListener(IPAddress.Loopback, 0);
tcpListener.Start();
int serverPort = ((IPEndPoint)tcpListener.LocalEndpoint).Port;
WeakReference weakReference;
using (var handler = new WinHttpHandler())
using (var httpClient = new HttpClient(handler))
{
weakReference = new WeakReference(handler);
httpClient.BaseAddress = new Uri($"http://localhost:{serverPort}/path");
var request = new HttpRequestMessage(HttpMethod.Post, "/subpath");
var sendContentCalledTcs = new TaskCompletionSource();
var blockSendContentTcs = new TaskCompletionSource();
request.Content = new CustomHttpContent(sendContentCalledTcs, blockSendContentTcs);
Task acceptSocketTask = tcpListener.AcceptSocketAsync();
Task sendTask = httpClient.SendAsync(request);
Socket socket = await acceptSocketTask.ConfigureAwait(false);
byte[] buffer = new byte[1024];
socket.Receive(buffer, 0, 100, SocketFlags.None);
await sendContentCalledTcs.Task.ConfigureAwait(false);
blockSendContentTcs.TrySetException(new IOException("Synthetic error"));
try
{
await sendTask.ConfigureAwait(false);
}
catch (Exception e)
{
Console.WriteLine(e);
}
}
int millisecondsDelay = 2;
while (weakReference.TryGetTarget(out _))
{
await Task.Delay(millisecondsDelay, CancellationToken.None).ConfigureAwait(false);
millisecondsDelay *= 2;
#pragma warning disable S1215 // "GC.Collect" should not be called
GC.Collect(GC.MaxGeneration, GCCollectionMode.Forced, true);
GC.WaitForPendingFinalizers();
GC.Collect(GC.MaxGeneration, GCCollectionMode.Forced, true);
#pragma warning restore S1215 // "GC.Collect" should not be called
}
Console.WriteLine(uhTcs.Task.Status);
Console.WriteLine(uoTcs.Task.Status);
if (uhTcs.Task.Status == TaskStatus.RanToCompletion)
{
object result = await uhTcs.Task.ConfigureAwait(false);
if (result is Exception e)
{
Console.WriteLine("UnhandledException: " + e);
ExceptionDispatchInfo.Capture(e).Throw();
}
}
if (uoTcs.Task.Status == TaskStatus.RanToCompletion)
{
object result = await uoTcs.Task.ConfigureAwait(false);
if (result is Exception e)
{
Console.WriteLine("UnobservedTaskException: " + e);
ExceptionDispatchInfo.Capture(e).Throw();
}
}
}
```
### Expected behavior
Use of WinHttpHandler should never result in unobserved exceptions.
### Actual behavior
Use of WinHttpHandler does result in unobserved exceptions.
### Regression?
Not known to be a regression.
### Known Workarounds
No known workarounds.
### Configuration
- Seen on both .NET Framework and .NET "core" (currently using .NET 8). Not dependent on a specific versions of .NET.
- Seen on all versions of System.Net.Http.WinHttpHandler since 8.0.2 (have not tried earlier versions).
- Applies to Windows only, since that is the only OS where WinHttpHandler is supported
- Architecture: x64
### Other information
_No response_
Contributor guide
Assessment
This issue has not been assessed yet.