From 5fbd1eb00725a600a6885e877e9d822d64153db1 Mon Sep 17 00:00:00 2001
From: James Newton-King <james@newtonking.com>
Date: Tue, 11 Aug 2020 14:59:24 +1200
Subject: [PATCH] Reset KeepAliveTimeout on HTTP/2 ping (#24644)

---
 .../src/Internal/Http2/Http2Connection.cs     |  6 ++
 .../Http2/Http2TimeoutTests.cs                | 58 +++++++++++++++++++
 2 files changed, 64 insertions(+)

diff --git a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs
index eb43929378..698b392b3e 100644
--- a/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs
+++ b/src/Servers/Kestrel/Core/src/Internal/Http2/Http2Connection.cs
@@ -808,6 +808,12 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Internal.Http2
                 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
diff --git a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TimeoutTests.cs b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TimeoutTests.cs
index c433bb90be..05ad66065b 100644
--- a/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TimeoutTests.cs
+++ b/src/Servers/Kestrel/test/InMemory.FunctionalTests/Http2/Http2TimeoutTests.cs
@@ -120,6 +120,64 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core.Tests
             _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(_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);
+            _mockTimeoutControl.Verify(c => c.CancelTimeout(), Times.Never);
+
+            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);
+        }
+
         [Fact]
         public async Task HEADERS_ReceivedWithoutAllCONTINUATIONs_WithinRequestHeadersTimeout_AbortsConnection()
         {