Skip to content

[HTTP3] Correctly handle server closing a control stream #73680

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
rzikm merged 3 commits into from
Aug 11, 2022
+171 −53
Merged

[HTTP3] Correctly handle server closing a control stream #73680

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e0edb95
Handle HTTP3 server closing its controls stream
rzikm Aug 10, 2022
1581732
React to server closing inbound control stream
rzikm Aug 10, 2022
3a4bdff
Update src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpH…
rzikm Aug 10, 2022
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
3 changes: 3 additions & 0 deletions src/libraries/Common/tests/System/Net/Http/Http3LoopbackConnection.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,9 @@ internal sealed class Http3LoopbackConnection : GenericLoopbackConnection
private Http3LoopbackStream _inboundControlStream; // Inbound control stream from client
private Http3LoopbackStream _outboundControlStream; // Our outbound control stream

public Http3LoopbackStream OutboundControlStream => _outboundControlStream ?? throw new Exception("Control stream has not been opened yet");
public Http3LoopbackStream InboundControlStream => _inboundControlStream ?? throw new Exception("Inbound control stream has not been accepted yet");

public Http3LoopbackConnection(QuicConnection connection)
{
_connection = connection;
Expand Down
1 change: 1 addition & 0 deletions src/libraries/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 @@ -446,6 +446,7 @@
<Reference Include="System.Diagnostics.DiagnosticSource" />
<Reference Include="System.Diagnostics.Tracing" />
<Reference Include="System.IO.Compression" />
<Reference Include="System.Linq" />
<Reference Include="System.Memory" />
<Reference Include="System.Net.NameResolution" />
<Reference Include="System.Net.NetworkInformation" />
Expand Down
125 changes: 72 additions & 53 deletions src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using System.Runtime.Versioning;
using System.Net.Quic;
using System.IO;
using System.Linq;
Copy link
Member

@ManickaP ManickaP Aug 10, 2022

Choose a reason for hiding this comment

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

Suggested change
using System.Linq;

Copy link
Member Author

@rzikm rzikm Aug 11, 2022

Choose a reason for hiding this comment

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

oops, I missed this one and merged it, I will remove it when I next touch HTTP3

using System.Collections.Generic;
using System.Diagnostics;
using System.Globalization;
Expand Down Expand Up @@ -368,6 +369,16 @@ private async Task SendSettingsAsync()
try
{
_clientControl = await _connection!.OpenOutboundStreamAsync(QuicStreamType.Unidirectional).ConfigureAwait(false);

// Server MUST NOT abort our control stream, setup a continuation which will react accordingly
_ = _clientControl.WritesClosed.ContinueWith(t =>
{
if (t.Exception?.InnerException is QuicException ex && ex.QuicError == QuicError.StreamAborted)
{
Abort(HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.ClosedCriticalStream));
}
}, CancellationToken.None, TaskContinuationOptions.ExecuteSynchronously, TaskScheduler.Current);

await _clientControl.WriteAsync(_pool.Settings.Http3SettingsFrame, CancellationToken.None).ConfigureAwait(false);
}
catch (Exception ex)
Expand Down Expand Up @@ -571,70 +582,78 @@ private async Task ProcessServerStreamAsync(QuicStream stream)
/// </summary>
private async Task ProcessServerControlStreamAsync(QuicStream stream, ArrayBuffer buffer)
{
using (buffer)
try
{
// Read the first frame of the control stream. Per spec:
// A SETTINGS frame MUST be sent as the first frame of each control stream.

(Http3FrameType? frameType, long payloadLength) = await ReadFrameEnvelopeAsync().ConfigureAwait(false);

if (frameType == null)
using (buffer)
{
// Connection closed prematurely, expected SETTINGS frame.
throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.ClosedCriticalStream);
}
// Read the first frame of the control stream. Per spec:
// A SETTINGS frame MUST be sent as the first frame of each control stream.

if (frameType != Http3FrameType.Settings)
{
throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.MissingSettings);
}
(Http3FrameType? frameType, long payloadLength) = await ReadFrameEnvelopeAsync().ConfigureAwait(false);

await ProcessSettingsFrameAsync(payloadLength).ConfigureAwait(false);
if (frameType == null)
{
// Connection closed prematurely, expected SETTINGS frame.
throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.ClosedCriticalStream);
}

// Read subsequent frames.
if (frameType != Http3FrameType.Settings)
{
throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.MissingSettings);
}

while (true)
{
(frameType, payloadLength) = await ReadFrameEnvelopeAsync().ConfigureAwait(false);
await ProcessSettingsFrameAsync(payloadLength).ConfigureAwait(false);

// Read subsequent frames.

switch (frameType)
while (true)
{
case Http3FrameType.GoAway:
await ProcessGoAwayFrameAsync(payloadLength).ConfigureAwait(false);
break;
case Http3FrameType.Settings:
// If an endpoint receives a second SETTINGS frame on the control stream, the endpoint MUST respond with a connection error of type H3_FRAME_UNEXPECTED.
throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.UnexpectedFrame);
case Http3FrameType.Headers: // Servers should not send these frames to a control stream.
case Http3FrameType.Data:
case Http3FrameType.MaxPushId:
case Http3FrameType.ReservedHttp2Priority: // These frames are explicitly reserved and must never be sent.
case Http3FrameType.ReservedHttp2Ping:
case Http3FrameType.ReservedHttp2WindowUpdate:
case Http3FrameType.ReservedHttp2Continuation:
throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.UnexpectedFrame);
case Http3FrameType.PushPromise:
case Http3FrameType.CancelPush:
// Because we haven't sent any MAX_PUSH_ID frame, it is invalid to receive any push-related frames as they will all reference a too-large ID.
throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.IdError);
case null:
// End of stream reached. If we're shutting down, stop looping. Otherwise, this is an error (this stream should not be closed for life of connection).
bool shuttingDown;
lock (SyncObj)
{
shuttingDown = ShuttingDown;
}
if (!shuttingDown)
{
throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.ClosedCriticalStream);
}
return;
default:
await SkipUnknownPayloadAsync(frameType.GetValueOrDefault(), payloadLength).ConfigureAwait(false);
break;
(frameType, payloadLength) = await ReadFrameEnvelopeAsync().ConfigureAwait(false);

switch (frameType)
{
case Http3FrameType.GoAway:
await ProcessGoAwayFrameAsync(payloadLength).ConfigureAwait(false);
break;
case Http3FrameType.Settings:
// If an endpoint receives a second SETTINGS frame on the control stream, the endpoint MUST respond with a connection error of type H3_FRAME_UNEXPECTED.
throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.UnexpectedFrame);
case Http3FrameType.Headers: // Servers should not send these frames to a control stream.
case Http3FrameType.Data:
case Http3FrameType.MaxPushId:
case Http3FrameType.ReservedHttp2Priority: // These frames are explicitly reserved and must never be sent.
case Http3FrameType.ReservedHttp2Ping:
case Http3FrameType.ReservedHttp2WindowUpdate:
case Http3FrameType.ReservedHttp2Continuation:
throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.UnexpectedFrame);
case Http3FrameType.PushPromise:
case Http3FrameType.CancelPush:
// Because we haven't sent any MAX_PUSH_ID frame, it is invalid to receive any push-related frames as they will all reference a too-large ID.
throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.IdError);
case null:
// End of stream reached. If we're shutting down, stop looping. Otherwise, this is an error (this stream should not be closed for life of connection).
bool shuttingDown;
lock (SyncObj)
{
shuttingDown = ShuttingDown;
}
if (!shuttingDown)
{
throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.ClosedCriticalStream);
}
return;
default:
await SkipUnknownPayloadAsync(frameType.GetValueOrDefault(), payloadLength).ConfigureAwait(false);
break;
}
}
}
}
catch (QuicException ex) when (ex.QuicError == QuicError.StreamAborted)
{
// Peers MUST NOT close the control stream
throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.ClosedCriticalStream);
}

async ValueTask<(Http3FrameType? frameType, long payloadLength)> ReadFrameEnvelopeAsync()
{
Expand Down
5 changes: 5 additions & 0 deletions src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,11 @@ await Task.WhenAny(sendContentTask, readResponseTask).ConfigureAwait(false) == s
Exception abortException = _connection.Abort(HttpProtocolException.CreateHttp3ConnectionException(code, SR.net_http_http3_connection_close));
throw new HttpRequestException(SR.net_http_client_execution_error, abortException);
}
catch (QuicException ex) when (ex.QuicError == QuicError.OperationAborted && _connection.AbortException != null)
{
// we close the connection, propagate the AbortException
throw new HttpRequestException(SR.net_http_client_execution_error, _connection.AbortException);
}
// It is possible for user's Content code to throw an unexpected OperationCanceledException.
catch (OperationCanceledException ex) when (ex.CancellationToken == _requestBodyCancellationSource.Token || ex.CancellationToken == cancellationToken)
{
Expand Down
90 changes: 90 additions & 0 deletions src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http3.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1596,6 +1596,96 @@ public async Task ServerSendsTrailingHeaders_Success()

}

[Theory]
[InlineData(true)]
[InlineData(false)]
public async Task ServerClosesOutboundControlStream_ClientClosesConnection(bool graceful)
{
using Http3LoopbackServer server = CreateHttp3LoopbackServer();

SemaphoreSlim semaphore = new SemaphoreSlim(0);
Task serverTask = Task.Run(async () =>
{
await using Http3LoopbackConnection connection = (Http3LoopbackConnection)await server.EstablishGenericConnectionAsync();

// wait for incoming request
await using Http3LoopbackStream requestStream = await connection.AcceptRequestStreamAsync();

// abort the control stream
if (graceful)
{
await connection.OutboundControlStream.SendResponseBodyAsync(Array.Empty<byte>(), isFinal: true);
}
else
{
connection.OutboundControlStream.Abort(Http3LoopbackConnection.H3_INTERNAL_ERROR);
}

// wait for client task before tearing down the requestStream and connection
await semaphore.WaitAsync();
});

Task clientTask = Task.Run(async () =>
{
using HttpClient client = CreateHttpClient();

using HttpRequestMessage request = new()
{
Method = HttpMethod.Get,
RequestUri = server.Address,
Version = HttpVersion30,
VersionPolicy = HttpVersionPolicy.RequestVersionExact
};

await AssertProtocolErrorAsync(Http3LoopbackConnection.H3_CLOSED_CRITICAL_STREAM, () => client.SendAsync(request));
semaphore.Release();
});

await new[] { clientTask, serverTask }.WhenAllOrAnyFailed(200_000);
}

[Fact]
public async Task ServerClosesInboundControlStream_ClientClosesConnection()
{
using Http3LoopbackServer server = CreateHttp3LoopbackServer();

SemaphoreSlim semaphore = new SemaphoreSlim(0);
Task serverTask = Task.Run(async () =>
{
await using Http3LoopbackConnection connection = (Http3LoopbackConnection)await server.EstablishGenericConnectionAsync();

// wait for incoming request
(Http3LoopbackStream controlStream, Http3LoopbackStream requestStream) = await connection.AcceptControlAndRequestStreamAsync();

await using (controlStream)
await using (requestStream)
{
controlStream.Abort(Http3LoopbackConnection.H3_INTERNAL_ERROR);
// wait for client task before tearing down the requestStream and connection
await semaphore.WaitAsync();
}

});

Task clientTask = Task.Run(async () =>
{
using HttpClient client = CreateHttpClient();

using HttpRequestMessage request = new()
{
Method = HttpMethod.Get,
RequestUri = server.Address,
Version = HttpVersion30,
VersionPolicy = HttpVersionPolicy.RequestVersionExact
};

await AssertProtocolErrorAsync(Http3LoopbackConnection.H3_CLOSED_CRITICAL_STREAM, () => client.SendAsync(request));
semaphore.Release();
});

await new[] { clientTask, serverTask }.WhenAllOrAnyFailed(200_000);
}

private static async Task<QuicException> AssertThrowsQuicExceptionAsync(QuicError expectedError, Func<Task> testCode)
{
QuicException ex = await Assert.ThrowsAsync<QuicException>(testCode);
Expand Down