Skip to content

Commit 5fbd1eb

Browse files
authored
Reset KeepAliveTimeout on HTTP/2 ping (#24644)
1 parent 942b6de commit 5fbd1eb

File tree

2 files changed

+64
-0
lines changed

2 files changed

+64
-0
lines changed

src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -808,6 +808,12 @@ private Task ProcessPingFrameAsync(in ReadOnlySequence<byte> payload)
808808
throw new Http2ConnectionErrorException(CoreStrings.FormatHttp2ErrorUnexpectedFrameLength(_incomingFrame.Type, 8), Http2ErrorCode.FRAME_SIZE_ERROR);
809809
}
810810

811+
// Incoming ping resets connection keep alive timeout
812+
if (TimeoutControl.TimerReason == TimeoutReason.KeepAlive)
813+
{
814+
TimeoutControl.ResetTimeout(Limits.KeepAliveTimeout.Ticks, TimeoutReason.KeepAlive);
815+
}
816+
811817
if (_incomingFrame.PingAck)
812818
{
813819
// TODO: verify that payload is equal to the outgoing PING frame

src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TimeoutTests.cs

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,64 @@ await ExpectAsync(Http2FrameType.HEADERS,
120120
_mockTimeoutHandler.VerifyNoOtherCalls();
121121
}
122122

123+
[Fact]
124+
public async Task PING_WithinKeepAliveTimeout_ResetKeepAliveTimeout()
125+
{
126+
var mockSystemClock = _serviceContext.MockSystemClock;
127+
var limits = _serviceContext.ServerOptions.Limits;
128+
129+
_timeoutControl.Initialize(mockSystemClock.UtcNow.Ticks);
130+
131+
CreateConnection();
132+
133+
await InitializeConnectionAsync(_noopApplication);
134+
135+
// Connection starts and sets keep alive timeout
136+
_mockTimeoutControl.Verify(c => c.SetTimeout(It.IsAny<long>(), TimeoutReason.KeepAlive), Times.Once);
137+
_mockTimeoutControl.Verify(c => c.ResetTimeout(It.IsAny<long>(), TimeoutReason.KeepAlive), Times.Never);
138+
139+
await SendPingAsync(Http2PingFrameFlags.NONE);
140+
await ExpectAsync(Http2FrameType.PING,
141+
withLength: 8,
142+
withFlags: (byte)Http2PingFrameFlags.ACK,
143+
withStreamId: 0);
144+
145+
// Server resets keep alive timeout
146+
_mockTimeoutControl.Verify(c => c.ResetTimeout(It.IsAny<long>(), TimeoutReason.KeepAlive), Times.Once);
147+
}
148+
149+
[Fact]
150+
public async Task PING_NoKeepAliveTimeout_DoesNotResetKeepAliveTimeout()
151+
{
152+
var mockSystemClock = _serviceContext.MockSystemClock;
153+
var limits = _serviceContext.ServerOptions.Limits;
154+
155+
_timeoutControl.Initialize(mockSystemClock.UtcNow.Ticks);
156+
157+
CreateConnection();
158+
159+
await InitializeConnectionAsync(_noopApplication);
160+
161+
// Connection starts and sets keep alive timeout
162+
_mockTimeoutControl.Verify(c => c.SetTimeout(It.IsAny<long>(), TimeoutReason.KeepAlive), Times.Once);
163+
_mockTimeoutControl.Verify(c => c.ResetTimeout(It.IsAny<long>(), TimeoutReason.KeepAlive), Times.Never);
164+
_mockTimeoutControl.Verify(c => c.CancelTimeout(), Times.Never);
165+
166+
await StartStreamAsync(1, _browserRequestHeaders, endStream: false);
167+
168+
// Starting a stream cancels the keep alive timeout
169+
_mockTimeoutControl.Verify(c => c.CancelTimeout(), Times.Once);
170+
171+
await SendPingAsync(Http2PingFrameFlags.NONE);
172+
await ExpectAsync(Http2FrameType.PING,
173+
withLength: 8,
174+
withFlags: (byte)Http2PingFrameFlags.ACK,
175+
withStreamId: 0);
176+
177+
// Server doesn't reset keep alive timeout because it isn't running
178+
_mockTimeoutControl.Verify(c => c.ResetTimeout(It.IsAny<long>(), TimeoutReason.KeepAlive), Times.Never);
179+
}
180+
123181
[Fact]
124182
public async Task HEADERS_ReceivedWithoutAllCONTINUATIONs_WithinRequestHeadersTimeout_AbortsConnection()
125183
{

0 commit comments

Comments
 (0)