[release/6.0] [HTTP/3] Fix content stream read hang (#58460)

* Fix HTTP/3 content stream read hang

* Fix hang if full data frame is not available

Co-authored-by: Natalia Kondratyeva <knatalia@microsoft.com>

diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs
index 852507a..f2beed2 100644 (file)
--- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs
+++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http3RequestStream.cs
@@ -1051,6 +1051,14 @@ namespace System.Net.Http
                         _responseDataPayloadRemaining -= copyLen;
                         _recvBuffer.Discard(copyLen);
                         buffer = buffer.Slice(copyLen);
+
+                        // Stop, if we've reached the end of a data frame and start of the next data frame is not buffered yet
+                        // Waiting for the next data frame may cause a hang, e.g. in echo scenario
+                        // TODO: this is inefficient if data is already available in transport
+                        if (_responseDataPayloadRemaining == 0 && _recvBuffer.ActiveLength == 0)
+                        {
+                            break;
+                        }
                     }
                     else
                     {
@@ -1068,10 +1076,9 @@ namespace System.Net.Http
                         _responseDataPayloadRemaining -= bytesRead;
                         buffer = buffer.Slice(bytesRead);
 
-                        if (_responseDataPayloadRemaining == 0)
-                        {
-                            break;
-                        }
+                        // Stop, even if we are in the middle of a data frame. Waiting for the next data may cause a hang
+                        // TODO: this is inefficient if data is already available in transport
+                        break;
                     }
                 }
 
@@ -1113,6 +1120,14 @@ namespace System.Net.Http
                         _responseDataPayloadRemaining -= copyLen;
                         _recvBuffer.Discard(copyLen);
                         buffer = buffer.Slice(copyLen);
+
+                        // Stop, if we've reached the end of a data frame and start of the next data frame is not buffered yet
+                        // Waiting for the next data frame may cause a hang, e.g. in echo scenario
+                        // TODO: this is inefficient if data is already available in transport
+                        if (_responseDataPayloadRemaining == 0 && _recvBuffer.ActiveLength == 0)
+                        {
+                            break;
+                        }
                     }
                     else
                     {
@@ -1130,10 +1145,9 @@ namespace System.Net.Http
                         _responseDataPayloadRemaining -= bytesRead;
                         buffer = buffer.Slice(bytesRead);
 
-                        if (_responseDataPayloadRemaining == 0)
-                        {
-                            break;
-                        }
+                        // Stop, even if we are in the middle of a data frame. Waiting for the next data may cause a hang
+                        // TODO: this is inefficient if data is already available in transport
+                        break;
                     }
                 }
 

diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http3.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http3.cs
index febbf3c..2299a5a 100644 (file)
--- a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http3.cs
+++ b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http3.cs
@@ -1006,6 +1006,90 @@ namespace System.Net.Http.Functional.Tests
             connection.Dispose();
         }
 
+        [Theory]
+        [InlineData(1)] // frame fits into Http3RequestStream buffer
+        [InlineData(10)]
+        [InlineData(100)] // frame doesn't fit into Http3RequestStream buffer
+        [InlineData(1000)]
+        public async Task EchoServerStreaming_DifferentMessageSize_Success(int messageSize)
+        {
+            int iters = 5;
+            var message = new byte[messageSize];
+            var readBuffer = new byte[5 * messageSize]; // bigger than message
+            var random = new Random(0);
+
+            using Http3LoopbackServer server = CreateHttp3LoopbackServer();
+            Http3LoopbackConnection connection = null;
+            Http3LoopbackStream serverStream = null;
+
+            Task serverTask = Task.Run(async () =>
+            {
+                connection = (Http3LoopbackConnection)await server.EstablishGenericConnectionAsync();
+                serverStream = await connection.AcceptRequestStreamAsync();
+
+                HttpRequestData requestData = await serverStream.ReadRequestDataAsync(readBody: false).WaitAsync(TimeSpan.FromSeconds(30));
+
+                await serverStream.SendResponseHeadersAsync().ConfigureAwait(false);
+
+                while (true)
+                {
+                    (long? frameType, byte[] payload) = await serverStream.ReadFrameAsync();
+                    if (frameType == null)
+                    {
+                        // EOS
+                        break;
+                    }
+                    // echo back
+                    await serverStream.SendDataFrameAsync(payload).WaitAsync(TimeSpan.FromSeconds(30));
+                }
+                // send FIN
+                await serverStream.SendResponseBodyAsync(Array.Empty<byte>(), isFinal: true);
+            });
+
+            StreamingHttpContent requestContent = new StreamingHttpContent();
+
+            using HttpClient client = CreateHttpClient();
+            using HttpRequestMessage request = new()
+            {
+                Method = HttpMethod.Post,
+                RequestUri = server.Address,
+                Version = HttpVersion30,
+                VersionPolicy = HttpVersionPolicy.RequestVersionExact,
+                Content = requestContent
+            };
+
+            var responseTask = client.SendAsync(request, HttpCompletionOption.ResponseHeadersRead).WaitAsync(TimeSpan.FromSeconds(10));
+
+            Stream requestStream = await requestContent.GetStreamAsync();
+            // Send headers
+            await requestStream.FlushAsync();
+
+            using HttpResponseMessage response = await responseTask;
+
+            var responseStream = await response.Content.ReadAsStreamAsync();
+
+            for (int i = 0; i < iters; ++i)
+            {
+                random.NextBytes(message);
+                await requestStream.WriteAsync(message).AsTask().WaitAsync(TimeSpan.FromSeconds(10));
+                await requestStream.FlushAsync();
+
+                int bytesRead = await responseStream.ReadAsync(readBuffer).AsTask().WaitAsync(TimeSpan.FromSeconds(10));
+                Assert.Equal(bytesRead, messageSize);
+                Assert.Equal(message, readBuffer[..bytesRead]);
+            }
+            // Send FIN
+            requestContent.CompleteStream();
+            // Receive FIN
+            Assert.Equal(0, await responseStream.ReadAsync(readBuffer).AsTask().WaitAsync(TimeSpan.FromSeconds(10)));
+
+            await serverTask.WaitAsync(TimeSpan.FromSeconds(60));
+
+            serverStream.Dispose();
+            Assert.NotNull(connection);
+            connection.Dispose();
+        }
+
         public static TheoryData<HttpStatusCode, bool> StatusCodesTestData()
         {
             var statuses = Enum.GetValues(typeof(HttpStatusCode)).Cast<HttpStatusCode>().Where(s => s >= HttpStatusCode.OK); // exclude informational