diff --git a/src/libraries/System.Net.Http/src/Resources/Strings.resx b/src/libraries/System.Net.Http/src/Resources/Strings.resx
index 8af485a17fee14..41314d822c3792 100644
--- a/src/libraries/System.Net.Http/src/Resources/Strings.resx
+++ b/src/libraries/System.Net.Http/src/Resources/Strings.resx
@@ -423,6 +423,9 @@
The initial HTTP/2 stream window size must be between {0} and {1}.
+
+ The HTTP/2 connection was aborted because the size of the frame queue exceeded an internal limit.
+
This method is not implemented by this class.
diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs
index cc53c81c94e7d6..e1adf9e7e58cf5 100644
--- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs
+++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs
@@ -75,6 +75,10 @@ internal sealed partial class Http2Connection : HttpConnectionBase
private Http2ProtocolErrorCode? _goAwayErrorCode;
+ // Cap number of untransmitted PING and SETTING ACKs and PING requests
+ private const int MaxQueuedFireAndForgetFrames = 1000;
+ private int _queuedFireAndForgetFrames;
+
private const int MaxStreamId = int.MaxValue;
// Temporary workaround for request burst handling on connection start.
@@ -926,7 +930,7 @@ private void ProcessSettingsFrame(FrameHeader frameHeader, bool initialFrame = f
// Send acknowledgement
// Don't wait for completion, which could happen asynchronously.
- LogExceptions(SendSettingsAckAsync());
+ QueueSettingsAck();
}
}
@@ -1007,7 +1011,7 @@ private void ProcessPingFrame(FrameHeader frameHeader)
}
else
{
- LogExceptions(SendPingAsync(pingContentLong, isAck: true));
+ QueuePing(pingContentLong, isAck: true);
}
_incomingBuffer.Discard(frameHeader.PayloadLength);
}
@@ -1288,20 +1292,35 @@ private async Task ProcessOutgoingFramesAsync()
}
}
- private Task SendSettingsAckAsync() =>
- PerformWriteAsync(FrameHeader.Size, this, static (thisRef, writeBuffer) =>
+ private void QueueSettingsAck()
+ {
+ if (!TryIncrementQueuedFireAndForgetFrames())
+ {
+ return;
+ }
+
+ LogExceptions(PerformWriteAsync(FrameHeader.Size, this, static (thisRef, writeBuffer) =>
{
if (NetEventSource.Log.IsEnabled()) thisRef.Trace("Started writing.");
FrameHeader.WriteTo(writeBuffer.Span, 0, FrameType.Settings, FrameFlags.Ack, streamId: 0);
+ thisRef.DecrementQueuedFireAndForgetFrames();
+
return true;
- });
+ }));
+ }
/// The 8-byte ping content to send, read as a big-endian integer.
/// Determine whether the frame is ping or ping ack.
- private Task SendPingAsync(long pingContent, bool isAck = false) =>
- PerformWriteAsync(FrameHeader.Size + FrameHeader.PingLength, (thisRef: this, pingContent, isAck), static (state, writeBuffer) =>
+ private void QueuePing(long pingContent, bool isAck = false)
+ {
+ if (!TryIncrementQueuedFireAndForgetFrames())
+ {
+ return;
+ }
+
+ LogExceptions(PerformWriteAsync(FrameHeader.Size + FrameHeader.PingLength, (thisRef: this, pingContent, isAck), static (state, writeBuffer) =>
{
if (NetEventSource.Log.IsEnabled()) state.thisRef.Trace($"Started writing. {nameof(pingContent)}={state.pingContent}");
@@ -1311,8 +1330,11 @@ private Task SendPingAsync(long pingContent, bool isAck = false) =>
FrameHeader.WriteTo(span, FrameHeader.PingLength, FrameType.Ping, state.isAck ? FrameFlags.Ack : FrameFlags.None, streamId: 0);
BinaryPrimitives.WriteInt64BigEndian(span.Slice(FrameHeader.Size), state.pingContent);
+ state.thisRef.DecrementQueuedFireAndForgetFrames();
+
return true;
- });
+ }));
+ }
private Task SendRstStreamAsync(int streamId, Http2ProtocolErrorCode errorCode) =>
PerformWriteAsync(FrameHeader.Size + FrameHeader.RstStreamLength, (thisRef: this, streamId, errorCode), static (s, writeBuffer) =>
@@ -1815,6 +1837,28 @@ private bool ForceSendConnectionWindowUpdate()
return true;
}
+ private bool TryIncrementQueuedFireAndForgetFrames()
+ {
+ if (Interlocked.Increment(ref _queuedFireAndForgetFrames) > MaxQueuedFireAndForgetFrames)
+ {
+ if (NetEventSource.Log.IsEnabled()) this.Trace("Number of untransmitted PING and SETTING frames exceeded limit.");
+
+ // Close connection when there is too much outstanding frames
+ var ex = new HttpIOException(HttpRequestError.Unknown, SR.net_http_http2_frame_limit_exceeded);
+ Abort(ex);
+
+ return false;
+ }
+
+ return true;
+ }
+
+ private void DecrementQueuedFireAndForgetFrames()
+ {
+ int pending = Interlocked.Decrement(ref _queuedFireAndForgetFrames);
+ Debug.Assert(pending >= 0);
+ }
+
/// Abort all streams and cause further processing to fail.
/// Exception causing Abort to be called.
private void Abort(Exception abortException)
@@ -2173,7 +2217,7 @@ private void VerifyKeepAlive()
_keepAlivePingTimeoutTimestamp = now + _keepAlivePingTimeout;
long pingPayload = Interlocked.Increment(ref _keepAlivePingPayload);
- LogExceptions(SendPingAsync(pingPayload));
+ QueuePing(pingPayload);
return;
}
break;
diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2StreamWindowManager.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2StreamWindowManager.cs
index 470cacbe2de34c..359e78b1995e64 100644
--- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2StreamWindowManager.cs
+++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2StreamWindowManager.cs
@@ -220,7 +220,7 @@ internal void OnDataOrHeadersReceived(Http2Connection connection, bool sendWindo
// Send a PING
_pingCounter--;
if (NetEventSource.Log.IsEnabled()) connection.Trace($"[FlowControl] Sending RTT PING with payload {_pingCounter}");
- connection.LogExceptions(connection.SendPingAsync(_pingCounter, isAck: false));
+ connection.QueuePing(_pingCounter, isAck: false);
_pingSentTimestamp = now;
_state = State.PingSent;
}
diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs
index 16463469d10986..bf62e5d0e070c3 100644
--- a/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs
+++ b/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs
@@ -3206,6 +3206,176 @@ await conn.WriteFrameAsync(
await Task.WhenAll(connectionTasks).ConfigureAwait(false);
}
+ [ConditionalFact(typeof(SocketsHttpHandlerTest_Http2), nameof(SupportsAlpn))]
+ public async Task Http2_SettingInFlightLimitExceeded()
+ {
+ // test that client abort connection when server send too much PING/SETTINGs while not processing ACK replies
+
+ await Http2LoopbackServer.CreateClientAndServerAsync(
+ async uri =>
+ {
+ using HttpClient client = CreateHttpClient();
+
+ Exception e = await Assert.ThrowsAsync(() => client.GetAsync(uri));
+ Assert.True(e.Message.StartsWith(SR.net_http_http2_frame_limit_exceeded), "Bad Exception Message");
+
+ // test that we can open connection after failure
+ await client.GetAsync(uri);
+ },
+ async server =>
+ {
+ server.AllowMultipleConnections = true;
+
+ await using Http2LoopbackConnection con = await server.AcceptConnectionAsync();
+ while (true)
+ {
+ try
+ {
+ await con.WriteFrameAsync(new Frame(0, FrameType.Settings, FrameFlags.None, 0));
+ }
+ catch (IOException)
+ {
+ break;
+ }
+ }
+
+ // second connection will be more lucky
+ await server.AcceptConnectionSendResponseAndCloseAsync(HttpStatusCode.OK, "ok");
+ });
+ }
+
+ [ConditionalFact(typeof(SocketsHttpHandlerTest_Http2), nameof(SupportsAlpn))]
+ public async Task Http2_PingInFlightLimitExceeded()
+ {
+ // almost exhaust client queue and test that following asynchronous PING issued by client trips
+
+ BlockedWriteNetworkStream? clientNetStream = null;
+
+ await Http2LoopbackServer.CreateClientAndServerAsync(
+ async uri =>
+ {
+ using HttpClientHandler handler = CreateHttpClientHandler(allowAllCertificates: true);
+ SocketsHttpHandler socketsHandler = GetUnderlyingSocketsHttpHandler(handler);
+
+ socketsHandler.ConnectCallback += async (ctx, ct) =>
+ {
+ DnsEndPoint dnsEndPoint = ctx.DnsEndPoint;
+ IPAddress[] addresses = await Dns.GetHostAddressesAsync(dnsEndPoint.Host, dnsEndPoint.AddressFamily, ct);
+
+ var s = new Socket(SocketType.Stream, ProtocolType.Tcp);
+ await s.ConnectAsync(addresses, dnsEndPoint.Port, ct);
+
+ clientNetStream = new BlockedWriteNetworkStream(s, true);
+ return clientNetStream;
+ };
+ socketsHandler.KeepAlivePingDelay = TimeSpan.FromSeconds(1);
+ socketsHandler.KeepAlivePingPolicy = HttpKeepAlivePingPolicy.Always;
+ socketsHandler.KeepAlivePingTimeout = TimeSpan.MaxValue;
+
+ using HttpClient client = CreateHttpClient(handler);
+ client.DefaultRequestVersion = HttpVersion.Version20;
+ client.DefaultVersionPolicy = HttpVersionPolicy.RequestVersionExact;
+
+ Exception e = await Assert.ThrowsAsync(() => client.GetAsync(uri));
+ Assert.True(e.Message.StartsWith(SR.net_http_http2_frame_limit_exceeded), "Bad Exception Message");
+
+ // test that we can connect new connection after fail
+ await client.GetAsync(uri);
+ },
+ async server =>
+ {
+ server.AllowMultipleConnections = true;
+
+ await using Http2LoopbackConnection con = await server.AcceptConnectionAsync();
+
+ // wait for initial frames from client
+ await con.ReadSettingsAsync();
+ await con.ReadRequestHeaderAsync();
+ PingFrame firstPing = await con.ReadPingAsync();
+
+ long writeCntBeforeSettingsAck = clientNetStream.StartBlocking();
+
+ // ACK for following SETTINGS will attemp flushing queue on client side and block its processing
+ await con.WriteFrameAsync(new SettingsFrame());
+
+ // wait before client attemp to send ACK and channel get blocked.
+ // Without this wait it may combine SETTINGs ACK and following PING ACKs into
+ // single buffer which will cause dequeueing them from queue and we fail to
+ // preload queue to expect 1000 entries as required by test later.
+ while (clientNetStream.WriteCallCount == writeCntBeforeSettingsAck)
+ {
+ await Task.Delay(50);
+ }
+
+ // fill the client queue with replies to PING, 1000 exactly fit the queue
+ for (long i = 0; i < 1000; i++)
+ {
+ await con.WriteFrameAsync(new PingFrame(i, FrameFlags.None, 0));
+ }
+
+ // now confirm ping we received soon after initiating connection to eneble new heart beat ping
+ // this should trip and close connection
+ await con.WriteFrameAsync(new PingFrame(firstPing.Data, FrameFlags.Ack, 0));
+
+ // test that server/client are operational after failure
+ await server.AcceptConnectionSendResponseAndCloseAsync();
+ }, new Http2Options()
+ {
+ UseSsl = false, // SSL introduce buffering which supress effect of blocking
+ EnableTransparentPingResponse = false // need to observe initial ping
+ });
+ }
+
+ private class BlockedWriteNetworkStream : NetworkStream
+ {
+ private object _lock = new object();
+ private bool _block;
+ private long _writes;
+
+ public long WriteCallCount
+ {
+ get
+ {
+ lock (_lock)
+ {
+ return _writes;
+ }
+ }
+ }
+
+ public BlockedWriteNetworkStream(Socket socket, bool ownsSocket) : base(socket, ownsSocket) { }
+
+ public long StartBlocking()
+ {
+ lock (_lock)
+ {
+ _block = true;
+ return _writes;
+ }
+ }
+
+ public override Task WriteAsync(byte[] buffer, int offset, int count, CancellationToken cancellationToken) =>
+ WriteAsync(buffer.AsMemory(offset, count), cancellationToken).AsTask();
+
+ public override async ValueTask WriteAsync(ReadOnlyMemory buffer, CancellationToken cancellationToken = default)
+ {
+ bool block;
+ lock (_lock)
+ {
+ _writes++;
+ block = _block;
+ }
+
+ if (block)
+ {
+ await Task.Delay(TimeSpan.FromDays(1), cancellationToken);
+ throw new Exception("Long delay was canceled early");
+ }
+
+ await base.WriteAsync(buffer, cancellationToken);
+ }
+ }
+
private async Task VerifySendTasks(IReadOnlyList> sendTasks)
{
await TestHelper.WhenAllCompletedOrAnyFailed(sendTasks.ToArray()).ConfigureAwait(false);