SocketsHttpHandler: Don't reuse connection with extraneous received data (dotnet...

* don't reuse connection with extraneous received data

* add tracking issue

Commit migrated from https://github.com/dotnet/corefx/commit/5a57de7524fa3c3a2ba2b4f643f30464f917e14f

diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs
index 6f83457..8c28bdf 100644 (file)
--- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs
+++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs
@@ -1251,6 +1251,13 @@ namespace System.Net.Http
                 }
             }
 
+            // If we have extraneous data in the read buffer, don't reuse the connection;
+            // otherwise we'd interpret this as part of the next response.
+            if (_readOffset != _readLength)
+            {
+                _connectionClose = true;
+            }
+
             // If server told us it's closing the connection, don't put this back in the pool.
             // And if we incurred an error while transferring request content, also skip the pool.
             if (!_connectionClose &&
@@ -1268,7 +1275,6 @@ namespace System.Net.Http
                     // at any point to understand if the connection has been closed or if errant data
                     // has been sent on the connection by the server, either of which would mean we
                     // should close the connection and not use it for subsequent requests.
-                    Debug.Assert(_readLength == _readOffset, $"{_readLength} != {_readOffset}");
                     _readAheadTask = _stream.ReadAsync(_readBuffer, 0, _readBuffer.Length);
 
                     // Put connection back in the pool.

diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs
index 162dc19..147bc84 100644 (file)
--- a/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs
+++ b/src/libraries/System.Net.Http/tests/FunctionalTests/SocketsHttpHandlerTest.cs
@@ -622,7 +622,8 @@ namespace System.Net.Http.Functional.Tests
     {
         protected override bool UseSocketsHttpHandler => true;
 
-        // TODO: Currently the subsequent tests sometimes fail/hang with WinHttpHandler / CurlHandler.
+        // TODO: ISSUE #27272
+        // Currently the subsequent tests sometimes fail/hang with WinHttpHandler / CurlHandler.
         // In theory they should pass with any handler that does appropriate connection pooling.
         // We should understand why they sometimes fail there and ideally move them to be
         // used by all handlers this test project tests.
@@ -690,6 +691,25 @@ namespace System.Net.Http.Functional.Tests
         }
 
         [Fact]
+        public async Task ServerSendsGarbageAfterInitialRequest_SubsequentRequestUsesDifferentConnection()
+        {
+            using (HttpClient client = CreateHttpClient())
+            {
+                await LoopbackServer.CreateServerAsync(async (server, uri) =>
+                {
+                    // Make multiple requests iteratively.
+                    for (int i = 0; i < 2; i++)
+                    {
+                        Task<string> request = client.GetStringAsync(uri);
+                        string response = LoopbackServer.GetHttpResponse() + "here is a bunch of garbage";
+                        await server.AcceptConnectionSendCustomResponseAndCloseAsync(response);
+                        await request;
+                    }
+                });
+            }
+        }
+
+        [Fact]
         public async Task ServerSendsConnectionClose_SubsequentRequestUsesDifferentConnection()
         {
             using (HttpClient client = CreateHttpClient())