From bef51dfa379c2d662af4cd352cd39001a7ddea4a Mon Sep 17 00:00:00 2001 From: ManickaP Date: Thu, 2 Jul 2020 17:25:54 +0200 Subject: [PATCH 1/7] Recognizes 100-continue timeout and propagates errors --- .../Http/SocketsHttpHandler/HttpConnection.cs | 43 ++++++++++++++----- 1 file changed, 32 insertions(+), 11 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 a22e5d37f40996..d3308d38be39e1 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 @@ -15,6 +15,8 @@ using System.Text; using System.Threading; using System.Threading.Tasks; +using System; +using System.Runtime.ExceptionServices; namespace System.Net.Http { @@ -320,7 +322,7 @@ private Task WriteHexInt32Async(int value, bool async) public async Task SendAsyncCore(HttpRequestMessage request, bool async, CancellationToken cancellationToken) { - TaskCompletionSource? allowExpect100ToContinue = null; + TaskCompletionSource<(bool sendRequestContent, bool timerExpired)>? allowExpect100ToContinue = null; Task? sendRequestContentTask = null; Debug.Assert(_currentRequest == null, $"Expected null {nameof(_currentRequest)}."); Debug.Assert(RemainingBuffer.Length == 0, "Unexpected data in read buffer"); @@ -456,9 +458,9 @@ public async Task SendAsyncCore(HttpRequestMessage request, // Create a TCS we'll use to block the request content from being sent, and create a timer that's used // as a fail-safe to unblock the request content if we don't hear back from the server in a timely manner. // Then kick off the request. The TCS' result indicates whether content should be sent or not. - allowExpect100ToContinue = new TaskCompletionSource(); + allowExpect100ToContinue = new TaskCompletionSource<(bool sendRequestContent, bool timerExpired)>(); var expect100Timer = new Timer( - s => ((TaskCompletionSource)s!).TrySetResult(true), + tcs => ((TaskCompletionSource<(bool sendRequestContent, bool timerExpired)>)tcs!).TrySetResult((sendRequestContent: true, timerExpired: true)), allowExpect100ToContinue, _pool.Settings._expect100ContinueTimeout, Timeout.InfiniteTimeSpan); sendRequestContentTask = SendRequestContentWithExpect100ContinueAsync( request, allowExpect100ToContinue.Task, CreateRequestContentStream(request), expect100Timer, async, cancellationToken); @@ -518,7 +520,7 @@ public async Task SendAsyncCore(HttpRequestMessage request, // sending request body (if any). if (allowExpect100ToContinue != null && response.StatusCode == HttpStatusCode.Continue) { - allowExpect100ToContinue.TrySetResult(true); + allowExpect100ToContinue.TrySetResult((sendRequestContent: true, timerExpired: false)); allowExpect100ToContinue = null; } else if (response.StatusCode == HttpStatusCode.SwitchingProtocols) @@ -569,9 +571,9 @@ public async Task SendAsyncCore(HttpRequestMessage request, // to be closed. However, we may have also lost a race condition with the Expect: 100-continue timeout, so if it turns out // we've already started sending the payload (we weren't able to cancel it), then we don't need to force close the connection. // We also must not clone connection if we do NTLM or Negotiate authentication. - allowExpect100ToContinue.TrySetResult(false); + allowExpect100ToContinue.TrySetResult((sendRequestContent: false, timerExpired: false)); - if (!allowExpect100ToContinue.Task.Result) // if Result is true, the timeout already expired and we started sending content + if (!allowExpect100ToContinue.Task.Result.sendRequestContent) // if Result is true, the timeout already expired and we started sending content { _connectionClose = true; } @@ -581,7 +583,7 @@ public async Task SendAsyncCore(HttpRequestMessage request, // For any success status codes, for errors when the request content length is known to be small, // or for session-based authentication challenges, send the payload // (if there is one... if there isn't, Content is null and thus allowExpect100ToContinue is also null, we won't get here). - allowExpect100ToContinue.TrySetResult(true); + allowExpect100ToContinue.TrySetResult((sendRequestContent: true, timerExpired: false)); } } @@ -680,7 +682,7 @@ public async Task SendAsyncCore(HttpRequestMessage request, cancellationRegistration.Dispose(); // Make sure to complete the allowExpect100ToContinue task if it exists. - allowExpect100ToContinue?.TrySetResult(false); + allowExpect100ToContinue?.TrySetResult((sendRequestContent: false, timerExpired: false)); if (NetEventSource.IsEnabled) Trace($"Error sending request: {error}"); @@ -693,6 +695,14 @@ public async Task SendAsyncCore(HttpRequestMessage request, // hook up a continuation that will log it. if (sendRequestContentTask != null && !sendRequestContentTask.IsCompletedSuccessfully) { + if (sendRequestContentTask.IsFaulted && Volatile.Read(ref _disposed) == 1) + { + Exception? sendRequestContentException = sendRequestContentTask.Exception?.InnerException; + if (sendRequestContentException != null) + { + ExceptionDispatchInfo.Throw(sendRequestContentException); + } + } LogExceptions(sendRequestContentTask); } @@ -794,11 +804,12 @@ private async ValueTask SendRequestContentAsync(HttpRequestMessage request, Http } private async Task SendRequestContentWithExpect100ContinueAsync( - HttpRequestMessage request, Task allowExpect100ToContinueTask, HttpContentWriteStream stream, Timer expect100Timer, bool async, CancellationToken cancellationToken) + HttpRequestMessage request, Task<(bool sendRequestContent, bool timerExpired)> allowExpect100ToContinueTask, + HttpContentWriteStream stream, Timer expect100Timer, bool async, CancellationToken cancellationToken) { // Wait until we receive a trigger notification that it's ok to continue sending content. // This will come either when the timer fires or when we receive a response status line from the server. - bool sendRequestContent = await allowExpect100ToContinueTask.ConfigureAwait(false); + (bool sendRequestContent, bool timerExpired) = await allowExpect100ToContinueTask.ConfigureAwait(false); // Clean up the timer; it's no longer needed. expect100Timer.Dispose(); @@ -807,7 +818,17 @@ private async Task SendRequestContentWithExpect100ContinueAsync( if (sendRequestContent) { if (NetEventSource.IsEnabled) Trace($"Sending request content for Expect: 100-continue."); - await SendRequestContentAsync(request, stream, async, cancellationToken).ConfigureAwait(false); + try + { + await SendRequestContentAsync(request, stream, async, cancellationToken).ConfigureAwait(false); + } + catch (Exception) when (timerExpired) + { + // Tear down the connection if called from the timer thread because caller's thread will wait for server status line indefinitely + // or till HttpClient.Timeout tear the connection itself. + Dispose(); + throw; + } } else { From 4c212a79f3382d1ce774fc9974f9e871bfd394f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marie=20P=C3=ADchov=C3=A1?= Date: Fri, 3 Jul 2020 23:18:20 +0200 Subject: [PATCH 2/7] Added test. --- .../System/Net/Http/HttpClientHandlerTest.cs | 41 +++++++++++++++++++ .../tests/System/Net/Http}/ThrowingContent.cs | 2 +- ...ttp.WinHttpHandler.Functional.Tests.csproj | 2 + .../System.Net.Http.Functional.Tests.csproj | 5 ++- .../ThrowingContent.netcore.cs | 18 ++++++++ 5 files changed, 65 insertions(+), 3 deletions(-) rename src/libraries/{System.Net.Http/tests/FunctionalTests => Common/tests/System/Net/Http}/ThrowingContent.cs (93%) create mode 100644 src/libraries/System.Net.Http/tests/FunctionalTests/ThrowingContent.netcore.cs diff --git a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs index 3d9355bac4d7c8..56e2351401a43a 100644 --- a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs +++ b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs @@ -2116,6 +2116,47 @@ await server.AcceptConnectionAsync(async connection => }); } + [Fact] + public async Task SendAsync_Expect100Continue_RequestBodyFails_ThrowsContentException() + { + if (IsWinHttpHandler && UseVersion >= HttpVersion20.Value) + { + return; + } + if (!TestAsync && UseVersion >= HttpVersion20.Value) + { + return; + } + + var clientFinished = new TaskCompletionSource(); + const string ExpectedMessage = "ThrowingContentException"; + + await LoopbackServerFactory.CreateClientAndServerAsync(async uri => + { + using (HttpClient client = CreateHttpClient()) + { + HttpRequestMessage initialMessage = new HttpRequestMessage(HttpMethod.Post, uri) { Version = UseVersion }; + initialMessage.Content = new ThrowingContent(() => new Exception(ExpectedMessage)); + initialMessage.Headers.ExpectContinue = true; + Exception exception = await Assert.ThrowsAsync(() => client.SendAsync(TestAsync, initialMessage)); + Assert.Equal(ExpectedMessage, exception.Message); + + clientFinished.SetResult(true); + } + }, async server => + { + await server.AcceptConnectionAsync(async connection => + { + try + { + await connection.ReadRequestDataAsync(readBody: true); + } + catch { } + await clientFinished.Task; + }); + }); + } + [Fact] public async Task SendAsync_No100ContinueReceived_RequestBodySentEventually() { diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/ThrowingContent.cs b/src/libraries/Common/tests/System/Net/Http/ThrowingContent.cs similarity index 93% rename from src/libraries/System.Net.Http/tests/FunctionalTests/ThrowingContent.cs rename to src/libraries/Common/tests/System/Net/Http/ThrowingContent.cs index aff18aab228780..a6b4c99b7ceaf2 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/ThrowingContent.cs +++ b/src/libraries/Common/tests/System/Net/Http/ThrowingContent.cs @@ -7,7 +7,7 @@ namespace System.Net.Http.Functional.Tests { /// HttpContent that mocks exceptions on serialization. - public class ThrowingContent : HttpContent + public partial class ThrowingContent : HttpContent { private readonly Func _exnFactory; private readonly int _length; diff --git a/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/System.Net.Http.WinHttpHandler.Functional.Tests.csproj b/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/System.Net.Http.WinHttpHandler.Functional.Tests.csproj index 98985ef8fc344b..041e449d88b4a4 100644 --- a/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/System.Net.Http.WinHttpHandler.Functional.Tests.csproj +++ b/src/libraries/System.Net.Http.WinHttpHandler/tests/FunctionalTests/System.Net.Http.WinHttpHandler.Functional.Tests.csproj @@ -129,6 +129,8 @@ Link="Common\System\Net\Http\SchSendAuxRecordHttpTest.cs" /> + - @@ -227,7 +226,9 @@ Link="Common\System\Net\Http\SyncBlockingContent.cs" /> - + + diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/ThrowingContent.netcore.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/ThrowingContent.netcore.cs new file mode 100644 index 00000000000000..39df787a7f299a --- /dev/null +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/ThrowingContent.netcore.cs @@ -0,0 +1,18 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. +// See the LICENSE file in the project root for more information. + +using System.IO; +using System.Threading; +using System.Threading.Tasks; + +namespace System.Net.Http.Functional.Tests +{ + public partial class ThrowingContent : HttpContent + { + protected override void SerializeToStream(Stream stream, TransportContext context, CancellationToken cancellationToken) + { + throw _exnFactory(); + } + } +} From d674782605050f9a33fefce997fc87d9e56b630f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marie=20P=C3=ADchov=C3=A1?= Date: Tue, 7 Jul 2020 16:39:04 +0200 Subject: [PATCH 3/7] Fixed exception propagation from failed sendRequestContentTask. --- .../Http/SocketsHttpHandler/HttpConnection.cs | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 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 d3308d38be39e1..6e3bcb8917de6a 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 @@ -15,8 +15,6 @@ using System.Text; using System.Threading; using System.Threading.Tasks; -using System; -using System.Runtime.ExceptionServices; namespace System.Net.Http { @@ -695,12 +693,19 @@ public async Task SendAsyncCore(HttpRequestMessage request, // hook up a continuation that will log it. if (sendRequestContentTask != null && !sendRequestContentTask.IsCompletedSuccessfully) { - if (sendRequestContentTask.IsFaulted && Volatile.Read(ref _disposed) == 1) + // In case the connection is disposed, it's most probable that + // expect100Continue timer expired and request content sending failed. + // We're awaiting the task to propagate the exception in this case. + if (Volatile.Read(ref _disposed) == 1) { - Exception? sendRequestContentException = sendRequestContentTask.Exception?.InnerException; - if (sendRequestContentException != null) + if (async) { - ExceptionDispatchInfo.Throw(sendRequestContentException); + await sendRequestContentTask.ConfigureAwait(false); + } + else + { + // No way around it here if we want to get the exception from the task. + sendRequestContentTask.GetAwaiter().GetResult(); } } LogExceptions(sendRequestContentTask); From bff243c86ca5e0a9467cbb4a8b9bad6aeb90513e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marie=20P=C3=ADchov=C3=A1?= Date: Tue, 7 Jul 2020 23:04:26 +0200 Subject: [PATCH 4/7] Addressed PR feedback. --- .../Http/SocketsHttpHandler/HttpConnection.cs | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 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 6e3bcb8917de6a..f4c026a9c10f44 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 @@ -320,7 +320,7 @@ private Task WriteHexInt32Async(int value, bool async) public async Task SendAsyncCore(HttpRequestMessage request, bool async, CancellationToken cancellationToken) { - TaskCompletionSource<(bool sendRequestContent, bool timerExpired)>? allowExpect100ToContinue = null; + TaskCompletionSource? allowExpect100ToContinue = null; Task? sendRequestContentTask = null; Debug.Assert(_currentRequest == null, $"Expected null {nameof(_currentRequest)}."); Debug.Assert(RemainingBuffer.Length == 0, "Unexpected data in read buffer"); @@ -456,9 +456,9 @@ public async Task SendAsyncCore(HttpRequestMessage request, // Create a TCS we'll use to block the request content from being sent, and create a timer that's used // as a fail-safe to unblock the request content if we don't hear back from the server in a timely manner. // Then kick off the request. The TCS' result indicates whether content should be sent or not. - allowExpect100ToContinue = new TaskCompletionSource<(bool sendRequestContent, bool timerExpired)>(); + allowExpect100ToContinue = new TaskCompletionSource(); var expect100Timer = new Timer( - tcs => ((TaskCompletionSource<(bool sendRequestContent, bool timerExpired)>)tcs!).TrySetResult((sendRequestContent: true, timerExpired: true)), + tcs => ((TaskCompletionSource)tcs!).TrySetResult(true), allowExpect100ToContinue, _pool.Settings._expect100ContinueTimeout, Timeout.InfiniteTimeSpan); sendRequestContentTask = SendRequestContentWithExpect100ContinueAsync( request, allowExpect100ToContinue.Task, CreateRequestContentStream(request), expect100Timer, async, cancellationToken); @@ -518,7 +518,7 @@ public async Task SendAsyncCore(HttpRequestMessage request, // sending request body (if any). if (allowExpect100ToContinue != null && response.StatusCode == HttpStatusCode.Continue) { - allowExpect100ToContinue.TrySetResult((sendRequestContent: true, timerExpired: false)); + allowExpect100ToContinue.TrySetResult(true); allowExpect100ToContinue = null; } else if (response.StatusCode == HttpStatusCode.SwitchingProtocols) @@ -569,9 +569,9 @@ public async Task SendAsyncCore(HttpRequestMessage request, // to be closed. However, we may have also lost a race condition with the Expect: 100-continue timeout, so if it turns out // we've already started sending the payload (we weren't able to cancel it), then we don't need to force close the connection. // We also must not clone connection if we do NTLM or Negotiate authentication. - allowExpect100ToContinue.TrySetResult((sendRequestContent: false, timerExpired: false)); + allowExpect100ToContinue.TrySetResult(false); - if (!allowExpect100ToContinue.Task.Result.sendRequestContent) // if Result is true, the timeout already expired and we started sending content + if (!allowExpect100ToContinue.Task.Result) // if Result is true, the timeout already expired and we started sending content { _connectionClose = true; } @@ -581,7 +581,7 @@ public async Task SendAsyncCore(HttpRequestMessage request, // For any success status codes, for errors when the request content length is known to be small, // or for session-based authentication challenges, send the payload // (if there is one... if there isn't, Content is null and thus allowExpect100ToContinue is also null, we won't get here). - allowExpect100ToContinue.TrySetResult((sendRequestContent: true, timerExpired: false)); + allowExpect100ToContinue.TrySetResult(true); } } @@ -680,7 +680,7 @@ public async Task SendAsyncCore(HttpRequestMessage request, cancellationRegistration.Dispose(); // Make sure to complete the allowExpect100ToContinue task if it exists. - allowExpect100ToContinue?.TrySetResult((sendRequestContent: false, timerExpired: false)); + allowExpect100ToContinue?.TrySetResult(false); if (NetEventSource.IsEnabled) Trace($"Error sending request: {error}"); @@ -809,12 +809,12 @@ private async ValueTask SendRequestContentAsync(HttpRequestMessage request, Http } private async Task SendRequestContentWithExpect100ContinueAsync( - HttpRequestMessage request, Task<(bool sendRequestContent, bool timerExpired)> allowExpect100ToContinueTask, + HttpRequestMessage request, Task allowExpect100ToContinueTask, HttpContentWriteStream stream, Timer expect100Timer, bool async, CancellationToken cancellationToken) { // Wait until we receive a trigger notification that it's ok to continue sending content. // This will come either when the timer fires or when we receive a response status line from the server. - (bool sendRequestContent, bool timerExpired) = await allowExpect100ToContinueTask.ConfigureAwait(false); + bool sendRequestContent = await allowExpect100ToContinueTask.ConfigureAwait(false); // Clean up the timer; it's no longer needed. expect100Timer.Dispose(); @@ -827,7 +827,7 @@ private async Task SendRequestContentWithExpect100ContinueAsync( { await SendRequestContentAsync(request, stream, async, cancellationToken).ConfigureAwait(false); } - catch (Exception) when (timerExpired) + catch { // Tear down the connection if called from the timer thread because caller's thread will wait for server status line indefinitely // or till HttpClient.Timeout tear the connection itself. From a800632c30e32d2db306bde5c6cd512931304e37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marie=20P=C3=ADchov=C3=A1?= Date: Thu, 9 Jul 2020 12:43:13 +0200 Subject: [PATCH 5/7] Addressed PR feedback. --- .../Common/tests/System/Net/Http/HttpClientHandlerTest.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs index c3c9752910c8fe..aefe20c36b03fe 100644 --- a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs +++ b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs @@ -2150,7 +2150,7 @@ await server.AcceptConnectionAsync(async connection => { await connection.ReadRequestDataAsync(readBody: true); } - catch { } + catch { } // Eat errors from client disconnect. await clientFinished.Task; }); }); From b2ca94ca278b4b9046b30ea1b2d87d321746548f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marie=20P=C3=ADchov=C3=A1?= Date: Sat, 18 Jul 2020 17:06:30 +0200 Subject: [PATCH 6/7] Adressed PR comments --- .../Common/tests/System/Net/Http/HttpClientHandlerTest.cs | 8 +++----- .../Common/tests/System/Net/Http/ThrowingContent.cs | 3 +++ 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs index aefe20c36b03fe..96378f5f1f0776 100644 --- a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs +++ b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs @@ -2128,17 +2128,15 @@ public async Task SendAsync_Expect100Continue_RequestBodyFails_ThrowsContentExce } var clientFinished = new TaskCompletionSource(); - const string ExpectedMessage = "ThrowingContentException"; await LoopbackServerFactory.CreateClientAndServerAsync(async uri => { using (HttpClient client = CreateHttpClient()) { HttpRequestMessage initialMessage = new HttpRequestMessage(HttpMethod.Post, uri) { Version = UseVersion }; - initialMessage.Content = new ThrowingContent(() => new Exception(ExpectedMessage)); + initialMessage.Content = new ThrowingContent(() => new ThrowingContentException()); initialMessage.Headers.ExpectContinue = true; - Exception exception = await Assert.ThrowsAsync(() => client.SendAsync(TestAsync, initialMessage)); - Assert.Equal(ExpectedMessage, exception.Message); + Exception exception = await Assert.ThrowsAsync(() => client.SendAsync(TestAsync, initialMessage)); clientFinished.SetResult(true); } @@ -2151,7 +2149,7 @@ await server.AcceptConnectionAsync(async connection => await connection.ReadRequestDataAsync(readBody: true); } catch { } // Eat errors from client disconnect. - await clientFinished.Task; + await clientFinished.Task.TimeoutAfter(TimeSpan.FromMinutes(2)); }); }); } diff --git a/src/libraries/Common/tests/System/Net/Http/ThrowingContent.cs b/src/libraries/Common/tests/System/Net/Http/ThrowingContent.cs index 9a15f5bb1b459d..8b757e8830ea94 100644 --- a/src/libraries/Common/tests/System/Net/Http/ThrowingContent.cs +++ b/src/libraries/Common/tests/System/Net/Http/ThrowingContent.cs @@ -32,4 +32,7 @@ protected override bool TryComputeLength(out long length) return true; } } + + public class ThrowingContentException : Exception + { } } From 5826443c387676b1b00c75263b28934a98503ac5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marie=20P=C3=ADchov=C3=A1?= Date: Sat, 18 Jul 2020 23:00:54 +0200 Subject: [PATCH 7/7] Fixed throwing proper exception for H/2 as well. --- .../src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs | 3 ++- .../src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs | 2 ++ .../src/System/Net/Http/SocketsHttpHandler/HttpConnection.cs | 2 +- 3 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs index 5432df730991f6..c3cc08d88a8330 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Connection.cs @@ -1734,7 +1734,8 @@ public sealed override async Task SendAsync(HttpRequestMess if (requestBodyTask.IsCompleted || duplex == false || await Task.WhenAny(requestBodyTask, responseHeadersTask).ConfigureAwait(false) == requestBodyTask || - requestBodyTask.IsCompleted) + requestBodyTask.IsCompleted || + http2Stream.SendRequestFinished) { // The sending of the request body completed before receiving all of the request headers (or we're // ok waiting for the request body even if it hasn't completed, e.g. because we're not doing duplex). diff --git a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs index f9cee658c61e7c..5ac829bbe3dc0a 100644 --- a/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs +++ b/src/libraries/System.Net.Http/src/System/Net/Http/SocketsHttpHandler/Http2Stream.cs @@ -145,6 +145,8 @@ public void Initialize(int streamId, int initialWindowSize) public int StreamId { get; private set; } + public bool SendRequestFinished => _requestCompletionState != StreamCompletionState.InProgress; + public HttpResponseMessage GetAndClearResponse() { // Once SendAsync completes, the Http2Stream should no longer hold onto the response message. 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 77aaf3cc098ccc..780212df78062e 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 @@ -457,7 +457,7 @@ public async Task SendAsyncCore(HttpRequestMessage request, // Then kick off the request. The TCS' result indicates whether content should be sent or not. allowExpect100ToContinue = new TaskCompletionSource(); var expect100Timer = new Timer( - static tcs => ((TaskCompletionSource)tcs!).TrySetResult(true), + static s => ((TaskCompletionSource)s!).TrySetResult(true), allowExpect100ToContinue, _pool.Settings._expect100ContinueTimeout, Timeout.InfiniteTimeSpan); sendRequestContentTask = SendRequestContentWithExpect100ContinueAsync( request, allowExpect100ToContinue.Task, CreateRequestContentStream(request), expect100Timer, async, cancellationToken);