3.1: Reset KeepAliveTimeout on HTTP/2 ping (#24858)

* Reset KeepAliveTimeout on HTTP/2 ping (#24644)

* Fix flaky keepalive ping test (#24804)

* Fix flaky keepalive ping test

* Clean up stream

* Remove quarantine attribute

* Fix test

Author: JamesNK

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

@@ -747,6 +747,12 @@ private Task ProcessPingFrameAsync(in ReadOnlySequence<byte> payload)
                 throw new Http2ConnectionErrorException(CoreStrings.FormatHttp2ErrorUnexpectedFrameLength(_incomingFrame.Type, 8), Http2ErrorCode.FRAME_SIZE_ERROR);
             }
 
+            // Incoming ping resets connection keep alive timeout
+            if (TimeoutControl.TimerReason == TimeoutReason.KeepAlive)
+            {
+                TimeoutControl.ResetTimeout(Limits.KeepAliveTimeout.Ticks, TimeoutReason.KeepAlive);
+            }
+
             if (_incomingFrame.PingAck)
             {
                 // TODO: verify that payload is equal to the outgoing PING frame

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

@@ -120,6 +120,76 @@ await ExpectAsync(Http2FrameType.HEADERS,
             _mockTimeoutHandler.VerifyNoOtherCalls();
         }
 
+        [Fact]
+        public async Task PING_WithinKeepAliveTimeout_ResetKeepAliveTimeout()
+        {
+            var mockSystemClock = _serviceContext.MockSystemClock;
+            var limits = _serviceContext.ServerOptions.Limits;
+
+            _timeoutControl.Initialize(mockSystemClock.UtcNow.Ticks);
+
+            CreateConnection();
+
+            await InitializeConnectionAsync(_noopApplication);
+
+            // Connection starts and sets keep alive timeout
+            _mockTimeoutControl.Verify(c => c.SetTimeout(It.IsAny<long>(), TimeoutReason.KeepAlive), Times.Once);
+            _mockTimeoutControl.Verify(c => c.ResetTimeout(It.IsAny<long>(), TimeoutReason.KeepAlive), Times.Never);
+
+            await SendPingAsync(Http2PingFrameFlags.NONE);
+            await ExpectAsync(Http2FrameType.PING,
+                withLength: 8,
+                withFlags: (byte)Http2PingFrameFlags.ACK,
+                withStreamId: 0);
+
+            // Server resets keep alive timeout
+            _mockTimeoutControl.Verify(c => c.ResetTimeout(It.IsAny<long>(), TimeoutReason.KeepAlive), Times.Once);
+        }
+
+        [Fact]
+        public async Task PING_NoKeepAliveTimeout_DoesNotResetKeepAliveTimeout()
+        {
+            var mockSystemClock = _serviceContext.MockSystemClock;
+            var limits = _serviceContext.ServerOptions.Limits;
+
+            _timeoutControl.Initialize(mockSystemClock.UtcNow.Ticks);
+
+            CreateConnection();
+
+            await InitializeConnectionAsync(_echoApplication);
+
+            // Connection starts and sets keep alive timeout
+            _mockTimeoutControl.Verify(c => c.SetTimeout(It.IsAny<long>(), TimeoutReason.KeepAlive), Times.Once);
+            _mockTimeoutControl.Verify(c => c.ResetTimeout(It.IsAny<long>(), TimeoutReason.KeepAlive), Times.Never);
+            _mockTimeoutControl.Verify(c => c.CancelTimeout(), Times.Never);
+
+            // Stream will stay open because it is waiting for request body to end
+            await StartStreamAsync(1, _browserRequestHeaders, endStream: false);
+
+            // Starting a stream cancels the keep alive timeout
+            _mockTimeoutControl.Verify(c => c.CancelTimeout(), Times.Once);
+
+            await SendPingAsync(Http2PingFrameFlags.NONE);
+            await ExpectAsync(Http2FrameType.PING,
+                withLength: 8,
+                withFlags: (byte)Http2PingFrameFlags.ACK,
+                withStreamId: 0);
+
+            // Server doesn't reset keep alive timeout because it isn't running
+            _mockTimeoutControl.Verify(c => c.ResetTimeout(It.IsAny<long>(), TimeoutReason.KeepAlive), Times.Never);
+
+            // End stream
+            await SendDataAsync(1, _helloWorldBytes, endStream: true);
+            await ExpectAsync(Http2FrameType.HEADERS,
+                withLength: 37,
+                withFlags: (byte)Http2HeadersFrameFlags.END_HEADERS,
+                withStreamId: 1);
+            await ExpectAsync(Http2FrameType.DATA,
+                withLength: _helloWorldBytes.Length,
+                withFlags: (byte)Http2DataFrameFlags.NONE,
+                withStreamId: 1);
+        }
+
         [Fact]
         public async Task HEADERS_ReceivedWithoutAllCONTINUATIONs_WithinRequestHeadersTimeout_AbortsConnection()
         {