dotnet / dotnet/runtime

Broken error handling in WinHttpHandler

Open
#121,913 3 comments 0 reactions 0 assignees View on GitHub
area-System.Net.Http bug
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

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.