Skip to content

Commit 80640d4

Browse files
rzikmManickaP
rzikm
and
ManickaP
authored
[HTTP3] Correctly handle server closing a control stream (#73680)
* Handle HTTP3 server closing its controls stream * React to server closing inbound control stream * Update src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs Co-authored-by: Marie Píchová <[email protected]> Co-authored-by: Marie Píchová <[email protected]>
1 parent bb97114 commit 80640d4

File tree

5 files changed

+171
-53
lines changed

5 files changed

+171
-53
lines changed

‎src/libraries/Common/tests/System/Net/Http/Http3LoopbackConnection.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,9 @@ internal sealed class Http3LoopbackConnection : GenericLoopbackConnection
4949
private Http3LoopbackStream _inboundControlStream; // Inbound control stream from client
5050
private Http3LoopbackStream _outboundControlStream; // Our outbound control stream
5151

52+
public Http3LoopbackStream OutboundControlStream => _outboundControlStream ?? throw new Exception("Control stream has not been opened yet");
53+
public Http3LoopbackStream InboundControlStream => _inboundControlStream ?? throw new Exception("Inbound control stream has not been accepted yet");
54+
5255
public Http3LoopbackConnection(QuicConnection connection)
5356
{
5457
_connection = connection;

‎src/libraries/System.Net.Http/src/System.Net.Http.csproj

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -446,6 +446,7 @@
446446
<Reference Include="System.Diagnostics.DiagnosticSource" />
447447
<Reference Include="System.Diagnostics.Tracing" />
448448
<Reference Include="System.IO.Compression" />
449+
<Reference Include="System.Linq" />
449450
<Reference Include="System.Memory" />
450451
<Reference Include="System.Net.NameResolution" />
451452
<Reference Include="System.Net.NetworkInformation" />

‎src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3Connection.cs

Lines changed: 72 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
using System.Runtime.Versioning;
88
using System.Net.Quic;
99
using System.IO;
10+
using System.Linq;
1011
using System.Collections.Generic;
1112
using System.Diagnostics;
1213
using System.Globalization;
@@ -368,6 +369,16 @@ private async Task SendSettingsAsync()
368369
try
369370
{
370371
_clientControl = await _connection!.OpenOutboundStreamAsync(QuicStreamType.Unidirectional).ConfigureAwait(false);
372+
373+
// Server MUST NOT abort our control stream, setup a continuation which will react accordingly
374+
_ = _clientControl.WritesClosed.ContinueWith(t =>
375+
{
376+
if (t.Exception?.InnerException is QuicException ex && ex.QuicError == QuicError.StreamAborted)
377+
{
378+
Abort(HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.ClosedCriticalStream));
379+
}
380+
}, CancellationToken.None, TaskContinuationOptions.ExecuteSynchronously, TaskScheduler.Current);
381+
371382
await _clientControl.WriteAsync(_pool.Settings.Http3SettingsFrame, CancellationToken.None).ConfigureAwait(false);
372383
}
373384
catch (Exception ex)
@@ -571,70 +582,78 @@ private async Task ProcessServerStreamAsync(QuicStream stream)
571582
/// </summary>
572583
private async Task ProcessServerControlStreamAsync(QuicStream stream, ArrayBuffer buffer)
573584
{
574-
using (buffer)
585+
try
575586
{
576-
// Read the first frame of the control stream. Per spec:
577-
// A SETTINGS frame MUST be sent as the first frame of each control stream.
578-
579-
(Http3FrameType? frameType, long payloadLength) = await ReadFrameEnvelopeAsync().ConfigureAwait(false);
580-
581-
if (frameType == null)
587+
using (buffer)
582588
{
583-
// Connection closed prematurely, expected SETTINGS frame.
584-
throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.ClosedCriticalStream);
585-
}
589+
// Read the first frame of the control stream. Per spec:
590+
// A SETTINGS frame MUST be sent as the first frame of each control stream.
586591

587-
if (frameType != Http3FrameType.Settings)
588-
{
589-
throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.MissingSettings);
590-
}
592+
(Http3FrameType? frameType, long payloadLength) = await ReadFrameEnvelopeAsync().ConfigureAwait(false);
591593

592-
await ProcessSettingsFrameAsync(payloadLength).ConfigureAwait(false);
594+
if (frameType == null)
595+
{
596+
// Connection closed prematurely, expected SETTINGS frame.
597+
throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.ClosedCriticalStream);
598+
}
593599

594-
// Read subsequent frames.
600+
if (frameType != Http3FrameType.Settings)
601+
{
602+
throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.MissingSettings);
603+
}
595604

596-
while (true)
597-
{
598-
(frameType, payloadLength) = await ReadFrameEnvelopeAsync().ConfigureAwait(false);
605+
await ProcessSettingsFrameAsync(payloadLength).ConfigureAwait(false);
606+
607+
// Read subsequent frames.
599608

600-
switch (frameType)
609+
while (true)
601610
{
602-
case Http3FrameType.GoAway:
603-
await ProcessGoAwayFrameAsync(payloadLength).ConfigureAwait(false);
604-
break;
605-
case Http3FrameType.Settings:
606-
// 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.
607-
throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.UnexpectedFrame);
608-
case Http3FrameType.Headers: // Servers should not send these frames to a control stream.
609-
case Http3FrameType.Data:
610-
case Http3FrameType.MaxPushId:
611-
case Http3FrameType.ReservedHttp2Priority: // These frames are explicitly reserved and must never be sent.
612-
case Http3FrameType.ReservedHttp2Ping:
613-
case Http3FrameType.ReservedHttp2WindowUpdate:
614-
case Http3FrameType.ReservedHttp2Continuation:
615-
throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.UnexpectedFrame);
616-
case Http3FrameType.PushPromise:
617-
case Http3FrameType.CancelPush:
618-
// 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.
619-
throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.IdError);
620-
case null:
621-
// 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).
622-
bool shuttingDown;
623-
lock (SyncObj)
624-
{
625-
shuttingDown = ShuttingDown;
626-
}
627-
if (!shuttingDown)
628-
{
629-
throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.ClosedCriticalStream);
630-
}
631-
return;
632-
default:
633-
await SkipUnknownPayloadAsync(frameType.GetValueOrDefault(), payloadLength).ConfigureAwait(false);
634-
break;
611+
(frameType, payloadLength) = await ReadFrameEnvelopeAsync().ConfigureAwait(false);
612+
613+
switch (frameType)
614+
{
615+
case Http3FrameType.GoAway:
616+
await ProcessGoAwayFrameAsync(payloadLength).ConfigureAwait(false);
617+
break;
618+
case Http3FrameType.Settings:
619+
// 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.
620+
throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.UnexpectedFrame);
621+
case Http3FrameType.Headers: // Servers should not send these frames to a control stream.
622+
case Http3FrameType.Data:
623+
case Http3FrameType.MaxPushId:
624+
case Http3FrameType.ReservedHttp2Priority: // These frames are explicitly reserved and must never be sent.
625+
case Http3FrameType.ReservedHttp2Ping:
626+
case Http3FrameType.ReservedHttp2WindowUpdate:
627+
case Http3FrameType.ReservedHttp2Continuation:
628+
throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.UnexpectedFrame);
629+
case Http3FrameType.PushPromise:
630+
case Http3FrameType.CancelPush:
631+
// 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.
632+
throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.IdError);
633+
case null:
634+
// 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).
635+
bool shuttingDown;
636+
lock (SyncObj)
637+
{
638+
shuttingDown = ShuttingDown;
639+
}
640+
if (!shuttingDown)
641+
{
642+
throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.ClosedCriticalStream);
643+
}
644+
return;
645+
default:
646+
await SkipUnknownPayloadAsync(frameType.GetValueOrDefault(), payloadLength).ConfigureAwait(false);
647+
break;
648+
}
635649
}
636650
}
637651
}
652+
catch (QuicException ex) when (ex.QuicError == QuicError.StreamAborted)
653+
{
654+
// Peers MUST NOT close the control stream
655+
throw HttpProtocolException.CreateHttp3ConnectionException(Http3ErrorCode.ClosedCriticalStream);
656+
}
638657

639658
async ValueTask<(Http3FrameType? frameType, long payloadLength)> ReadFrameEnvelopeAsync()
640659
{

‎src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -272,6 +272,11 @@ await Task.WhenAny(sendContentTask, readResponseTask).ConfigureAwait(false) == s
272272
Exception abortException = _connection.Abort(HttpProtocolException.CreateHttp3ConnectionException(code, SR.net_http_http3_connection_close));
273273
throw new HttpRequestException(SR.net_http_client_execution_error, abortException);
274274
}
275+
catch (QuicException ex) when (ex.QuicError == QuicError.OperationAborted && _connection.AbortException != null)
276+
{
277+
// we close the connection, propagate the AbortException
278+
throw new HttpRequestException(SR.net_http_client_execution_error, _connection.AbortException);
279+
}
275280
// It is possible for user's Content code to throw an unexpected OperationCanceledException.
276281
catch (OperationCanceledException ex) when (ex.CancellationToken == _requestBodyCancellationSource.Token || ex.CancellationToken == cancellationToken)
277282
{

‎src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http3.cs

Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1596,6 +1596,96 @@ public async Task ServerSendsTrailingHeaders_Success()
15961596

15971597
}
15981598

1599+
[Theory]
1600+
[InlineData(true)]
1601+
[InlineData(false)]
1602+
public async Task ServerClosesOutboundControlStream_ClientClosesConnection(bool graceful)
1603+
{
1604+
using Http3LoopbackServer server = CreateHttp3LoopbackServer();
1605+
1606+
SemaphoreSlim semaphore = new SemaphoreSlim(0);
1607+
Task serverTask = Task.Run(async () =>
1608+
{
1609+
await using Http3LoopbackConnection connection = (Http3LoopbackConnection)await server.EstablishGenericConnectionAsync();
1610+
1611+
// wait for incoming request
1612+
await using Http3LoopbackStream requestStream = await connection.AcceptRequestStreamAsync();
1613+
1614+
// abort the control stream
1615+
if (graceful)
1616+
{
1617+
await connection.OutboundControlStream.SendResponseBodyAsync(Array.Empty<byte>(), isFinal: true);
1618+
}
1619+
else
1620+
{
1621+
connection.OutboundControlStream.Abort(Http3LoopbackConnection.H3_INTERNAL_ERROR);
1622+
}
1623+
1624+
// wait for client task before tearing down the requestStream and connection
1625+
await semaphore.WaitAsync();
1626+
});
1627+
1628+
Task clientTask = Task.Run(async () =>
1629+
{
1630+
using HttpClient client = CreateHttpClient();
1631+
1632+
using HttpRequestMessage request = new()
1633+
{
1634+
Method = HttpMethod.Get,
1635+
RequestUri = server.Address,
1636+
Version = HttpVersion30,
1637+
VersionPolicy = HttpVersionPolicy.RequestVersionExact
1638+
};
1639+
1640+
await AssertProtocolErrorAsync(Http3LoopbackConnection.H3_CLOSED_CRITICAL_STREAM, () => client.SendAsync(request));
1641+
semaphore.Release();
1642+
});
1643+
1644+
await new[] { clientTask, serverTask }.WhenAllOrAnyFailed(200_000);
1645+
}
1646+
1647+
[Fact]
1648+
public async Task ServerClosesInboundControlStream_ClientClosesConnection()
1649+
{
1650+
using Http3LoopbackServer server = CreateHttp3LoopbackServer();
1651+
1652+
SemaphoreSlim semaphore = new SemaphoreSlim(0);
1653+
Task serverTask = Task.Run(async () =>
1654+
{
1655+
await using Http3LoopbackConnection connection = (Http3LoopbackConnection)await server.EstablishGenericConnectionAsync();
1656+
1657+
// wait for incoming request
1658+
(Http3LoopbackStream controlStream, Http3LoopbackStream requestStream) = await connection.AcceptControlAndRequestStreamAsync();
1659+
1660+
await using (controlStream)
1661+
await using (requestStream)
1662+
{
1663+
controlStream.Abort(Http3LoopbackConnection.H3_INTERNAL_ERROR);
1664+
// wait for client task before tearing down the requestStream and connection
1665+
await semaphore.WaitAsync();
1666+
}
1667+
1668+
});
1669+
1670+
Task clientTask = Task.Run(async () =>
1671+
{
1672+
using HttpClient client = CreateHttpClient();
1673+
1674+
using HttpRequestMessage request = new()
1675+
{
1676+
Method = HttpMethod.Get,
1677+
RequestUri = server.Address,
1678+
Version = HttpVersion30,
1679+
VersionPolicy = HttpVersionPolicy.RequestVersionExact
1680+
};
1681+
1682+
await AssertProtocolErrorAsync(Http3LoopbackConnection.H3_CLOSED_CRITICAL_STREAM, () => client.SendAsync(request));
1683+
semaphore.Release();
1684+
});
1685+
1686+
await new[] { clientTask, serverTask }.WhenAllOrAnyFailed(200_000);
1687+
}
1688+
15991689
private static async Task<QuicException> AssertThrowsQuicExceptionAsync(QuicError expectedError, Func<Task> testCode)
16001690
{
16011691
QuicException ex = await Assert.ThrowsAsync<QuicException>(testCode);

0 commit comments

Comments
 (0)