From: Marie Píchová <11718369+ManickaP@users.noreply.github.com> Date: Tue, 28 Jul 2020 14:58:43 +0000 (+0200) Subject: Expect 100 Continue Hang (#38774) X-Git-Tag: submit/tizen/20210909.063632~6384 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=7a6ca80bcde1d133dccba2c2ab361cf76c0832c9;p=platform%2Fupstream%2Fdotnet%2Fruntime.git Expect 100 Continue Hang (#38774) Inside sendRequestContentTask recognizes that it's invoked from a timer thread and if request content fails, unblocks SendAsyncCore and eventually propagates the exception from the content to the outside. Fixes the issue for H2 as well. Fixes #36717 --- diff --git a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs index 2aa3c26..687fa61 100644 --- a/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs +++ b/src/libraries/Common/tests/System/Net/Http/HttpClientHandlerTest.cs @@ -2117,6 +2117,45 @@ namespace System.Net.Http.Functional.Tests } [Fact] + public async Task SendAsync_Expect100Continue_RequestBodyFails_ThrowsContentException() + { + if (IsWinHttpHandler && UseVersion >= HttpVersion20.Value) + { + return; + } + if (!TestAsync && UseVersion >= HttpVersion20.Value) + { + return; + } + + var clientFinished = new TaskCompletionSource(); + + await LoopbackServerFactory.CreateClientAndServerAsync(async uri => + { + using (HttpClient client = CreateHttpClient()) + { + HttpRequestMessage initialMessage = new HttpRequestMessage(HttpMethod.Post, uri) { Version = UseVersion }; + initialMessage.Content = new ThrowingContent(() => new ThrowingContentException()); + initialMessage.Headers.ExpectContinue = true; + Exception exception = await Assert.ThrowsAsync(() => client.SendAsync(TestAsync, initialMessage)); + + clientFinished.SetResult(true); + } + }, async server => + { + await server.AcceptConnectionAsync(async connection => + { + try + { + await connection.ReadRequestDataAsync(readBody: true); + } + catch { } // Eat errors from client disconnect. + await clientFinished.Task.TimeoutAfter(TimeSpan.FromMinutes(2)); + }); + }); + } + + [Fact] public async Task SendAsync_No100ContinueReceived_RequestBodySentEventually() { if (IsWinHttpHandler && UseVersion >= HttpVersion20.Value) diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/ThrowingContent.cs b/src/libraries/Common/tests/System/Net/Http/ThrowingContent.cs similarity index 88% rename from src/libraries/System.Net.Http/tests/FunctionalTests/ThrowingContent.cs rename to src/libraries/Common/tests/System/Net/Http/ThrowingContent.cs index 2765705..8b757e8 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/ThrowingContent.cs +++ b/src/libraries/Common/tests/System/Net/Http/ThrowingContent.cs @@ -10,7 +10,7 @@ using System.Threading.Tasks; 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; @@ -32,4 +32,7 @@ namespace System.Net.Http.Functional.Tests return true; } } + + public class ThrowingContentException : Exception + { } } 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 b1dbc73..1d0dcec 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" /> + _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 4a9485a..780212d 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 @@ -692,6 +692,21 @@ namespace System.Net.Http // hook up a continuation that will log it. if (sendRequestContentTask != null && !sendRequestContentTask.IsCompletedSuccessfully) { + // 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) + { + if (async) + { + 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); } @@ -793,7 +808,8 @@ namespace System.Net.Http } private async Task SendRequestContentWithExpect100ContinueAsync( - HttpRequestMessage request, Task allowExpect100ToContinueTask, HttpContentWriteStream stream, Timer expect100Timer, bool async, CancellationToken cancellationToken) + 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. @@ -806,7 +822,17 @@ namespace System.Net.Http if (sendRequestContent) { if (NetEventSource.Log.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 + { + // 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 { diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/System.Net.Http.Functional.Tests.csproj b/src/libraries/System.Net.Http/tests/FunctionalTests/System.Net.Http.Functional.Tests.csproj index dc5bc00..2e00a89 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/System.Net.Http.Functional.Tests.csproj +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/System.Net.Http.Functional.Tests.csproj @@ -179,7 +179,6 @@ - @@ -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 0000000..8f4fd47 --- /dev/null +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/ThrowingContent.netcore.cs @@ -0,0 +1,17 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +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(); + } + } +}