From 1ff27f16dded59cfb64291445d8a8e1edd2a4d02 Mon Sep 17 00:00:00 2001 From: Geoff Kizer Date: Tue, 20 Feb 2018 05:20:20 -0800 Subject: [PATCH] SocketsHttpHandler: Don't reuse connection with extraneous received data (dotnet/corefx#27265) * don't reuse connection with extraneous received data * add tracking issue Commit migrated from https://github.com/dotnet/corefx/commit/5a57de7524fa3c3a2ba2b4f643f30464f917e14f --- .../Net/Http/SocketsHttpHandler/HttpConnection.cs | 8 +++++++- .../FunctionalTests/SocketsHttpHandlerTest.cs | 22 +++++++++++++++++++++- 2 files changed, 28 insertions(+), 2 deletions(-) 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 --- 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 --- 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 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()) -- 2.7.4