From 648f0316ba16451134ab0866ef633a3014d07e7e Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Fri, 12 Jul 2019 17:39:52 +0100 Subject: [PATCH] Http2Connection: Wrap InvalidOperationExceptions (dotnet/corefx#39400) * Http2Connection: Wrap InvalidOperationExceptions To achieve parity with the v1.1 implementation. Fixes dotnet/corefx#39295 * do not shutdown connection on server side * prevent disposal of client before server has established connection Commit migrated from https://github.com/dotnet/corefx/commit/68e7999c846131bd8cd43899671fa6a4a7b5da53 --- .../Net/Http/SocketsHttpHandler/Http2Connection.cs | 3 +- .../FunctionalTests/HttpClientHandlerTest.Http2.cs | 57 ++++++++++++++++++++++ .../System.Net.Http.Functional.Tests.csproj | 1 + .../tests/FunctionalTests/ThrowingContent.cs | 32 ++++++++++++ 4 files changed, 92 insertions(+), 1 deletion(-) create mode 100644 src/libraries/System.Net.Http/tests/FunctionalTests/ThrowingContent.cs 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 b9c6a5d..c436745 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 @@ -1600,7 +1600,8 @@ namespace System.Net.Http if (e is IOException || e is ObjectDisposedException || - e is Http2ProtocolException) + e is Http2ProtocolException || + e is InvalidOperationException) { replacementException = new HttpRequestException(SR.net_http_client_execution_error, _abortException ?? e); } diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http2.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http2.cs index 59bda29..a055d0cd 100644 --- a/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http2.cs +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/HttpClientHandlerTest.Http2.cs @@ -1784,6 +1784,63 @@ namespace System.Net.Http.Functional.Tests }); } + [ConditionalFact(nameof(SupportsAlpn))] + public async Task Http2Connection_Should_Wrap_HttpContent_InvalidOperationException() + { + // test for #39295 + var throwingContent = new ThrowingContent(() => new InvalidOperationException()); + + var tcs = new TaskCompletionSource(); + await Http2LoopbackServer.CreateClientAndServerAsync(async url => + { + using (HttpClient client = CreateHttpClient()) + { + var request = new HttpRequestMessage(HttpMethod.Post, url); + request.Version = new Version(2, 0); + request.Content = throwingContent; + + HttpRequestException exn = await Assert.ThrowsAnyAsync(async () => await client.SendAsync(request)); + Assert.IsType(exn.InnerException); + await tcs.Task; // prevent disposal of client until server has completed operations + } + }, + async server => + { + await server.EstablishConnectionAsync(); + tcs.SetResult(false); + }); + } + + [ConditionalFact(nameof(SupportsAlpn))] + public async Task Http2Connection_Should_Not_Wrap_HttpContent_CustomException() + { + // Assert existing HttpConnection behaviour in which custom HttpContent exception types are surfaced as-is + // c.f. https://github.com/dotnet/corefx/issues/39295#issuecomment-510569836 + + var throwingContent = new ThrowingContent(() => new CustomException()); + + var tcs = new TaskCompletionSource(); + await Http2LoopbackServer.CreateClientAndServerAsync(async url => + { + using (HttpClient client = CreateHttpClient()) + { + var request = new HttpRequestMessage(HttpMethod.Post, url); + request.Version = new Version(2, 0); + request.Content = throwingContent; + + await Assert.ThrowsAnyAsync(async () => await client.SendAsync(request)); + await tcs.Task; // prevent disposal of client until server has completed operations + } + }, + async server => + { + await server.EstablishConnectionAsync(); + tcs.SetResult(false); + }); + } + + private class CustomException : Exception { } + [Theory] [InlineData(true)] [InlineData(false)] 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 ee768b4..2b38963 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 @@ -139,6 +139,7 @@ + diff --git a/src/libraries/System.Net.Http/tests/FunctionalTests/ThrowingContent.cs b/src/libraries/System.Net.Http/tests/FunctionalTests/ThrowingContent.cs new file mode 100644 index 0000000..aff18aa --- /dev/null +++ b/src/libraries/System.Net.Http/tests/FunctionalTests/ThrowingContent.cs @@ -0,0 +1,32 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Text; +using System.Threading.Tasks; + +namespace System.Net.Http.Functional.Tests +{ + /// HttpContent that mocks exceptions on serialization. + public class ThrowingContent : HttpContent + { + private readonly Func _exnFactory; + private readonly int _length; + + public ThrowingContent(Func exnFactory, int length = 10) + { + _exnFactory = exnFactory; + _length = length; + } + + protected override Task SerializeToStreamAsync(Stream stream, TransportContext context) + { + throw _exnFactory(); + } + + protected override bool TryComputeLength(out long length) + { + length = _length; + return true; + } + } +} -- 2.7.4