From f68f5b0ee85ab59a5ca94e31cd84fab8de277f68 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Tue, 18 Aug 2020 21:12:52 +1200 Subject: [PATCH] Fix HTTP/2 tests that use HttpClient and H2C (#24981) --- src/Grpc/test/InteropTests/InteropTests.cs | 23 ++--- .../testassets/InteropClient/InteropClient.cs | 18 +++- .../testassets/InteropClient/RunTests.ps1 | 4 +- .../test/testassets/InteropWebsite/Program.cs | 7 +- .../InteropWebsite/TestServiceImpl.cs | 2 +- .../HttpClientHttp2InteropTests.cs | 89 +++++++++---------- 6 files changed, 77 insertions(+), 66 deletions(-) diff --git a/src/Grpc/test/InteropTests/InteropTests.cs b/src/Grpc/test/InteropTests/InteropTests.cs index cc02c4bee2..839292ab4b 100644 --- a/src/Grpc/test/InteropTests/InteropTests.cs +++ b/src/Grpc/test/InteropTests/InteropTests.cs @@ -25,7 +25,7 @@ namespace InteropTests _output = output; } - [Fact(Skip= "https://github.com/dotnet/aspnetcore/issues/24902")] + [Fact] public Task EmptyUnary() => InteropTestCase("empty_unary"); [Fact] @@ -36,20 +36,20 @@ namespace InteropTests [QuarantinedTest] public Task ClientStreaming() => InteropTestCase("client_streaming"); - [Fact(Skip= "https://github.com/dotnet/aspnetcore/issues/24902")] + [Fact] public Task ServerStreaming() => InteropTestCase("server_streaming"); [Fact] [QuarantinedTest("https://github.com/dotnet/aspnetcore/issues/22101")] public Task PingPong() => InteropTestCase("ping_pong"); - [Fact(Skip= "https://github.com/dotnet/aspnetcore/issues/24902")] + [Fact] public Task EmptyStream() => InteropTestCase("empty_stream"); [Fact] public Task CancelAfterBegin() => InteropTestCase("cancel_after_begin"); - [Fact(Skip= "https://github.com/dotnet/aspnetcore/issues/24902")] + [Fact] public Task CancelAfterFirstResponse() => InteropTestCase("cancel_after_first_response"); [Fact] @@ -59,30 +59,31 @@ namespace InteropTests [QuarantinedTest] public Task CustomMetadata() => InteropTestCase("custom_metadata"); - [Fact(Skip= "https://github.com/dotnet/aspnetcore/issues/24902")] + [Fact] public Task StatusCodeAndMessage() => InteropTestCase("status_code_and_message"); - [Fact(Skip= "https://github.com/dotnet/aspnetcore/issues/24902")] + [Fact] public Task SpecialStatusMessage() => InteropTestCase("special_status_message"); - [Fact(Skip= "https://github.com/dotnet/aspnetcore/issues/24902")] + [Fact] public Task UnimplementedService() => InteropTestCase("unimplemented_service"); - [Fact(Skip= "https://github.com/dotnet/aspnetcore/issues/24902")] + [Fact] public Task UnimplementedMethod() => InteropTestCase("unimplemented_method"); [Fact] - [QuarantinedTest] + [QuarantinedTest("Server is getting 'identity' encoding. Will resolve in gRPC project when updated SDK is available.")] public Task ClientCompressedUnary() => InteropTestCase("client_compressed_unary"); - [Fact(Skip= "https://github.com/dotnet/aspnetcore/issues/24902")] + [Fact] + [QuarantinedTest("Server is getting 'identity' encoding. Will resolve in gRPC project when updated SDK is available.")] public Task ClientCompressedStreaming() => InteropTestCase("client_compressed_streaming"); [Fact] [QuarantinedTest] public Task ServerCompressedUnary() => InteropTestCase("server_compressed_unary"); - [Fact(Skip= "https://github.com/dotnet/aspnetcore/issues/24902")] + [Fact] public Task ServerCompressedStreaming() => InteropTestCase("server_compressed_streaming"); private async Task InteropTestCase(string name) diff --git a/src/Grpc/test/testassets/InteropClient/InteropClient.cs b/src/Grpc/test/testassets/InteropClient/InteropClient.cs index 91ce761538..6590d8ae69 100644 --- a/src/Grpc/test/testassets/InteropClient/InteropClient.cs +++ b/src/Grpc/test/testassets/InteropClient/InteropClient.cs @@ -100,6 +100,7 @@ namespace InteropTestsClient { #pragma warning disable CS0618 // Type or member is obsolete loggerOptions.IncludeScopes = true; + loggerOptions.DisableColors = true; #pragma warning restore CS0618 // Type or member is obsolete }); }); @@ -167,7 +168,7 @@ namespace InteropTestsClient httpClientHandler.ClientCertificates.Add(cert); } - var httpClient = new HttpClient(httpClientHandler); + var httpClient = new HttpClient(new VersionPolicyHandler(httpClientHandler)); var channel = GrpcChannel.ForAddress($"{scheme}://{options.ServerHost}:{options.ServerPort}", new GrpcChannelOptions { @@ -179,7 +180,20 @@ namespace InteropTestsClient return new GrpcChannelWrapper(channel); } - private bool IsHttpClient() => string.Equals(options.ClientType, "httpclient", StringComparison.OrdinalIgnoreCase); + // TODO(JamesNK): This type can be removed in the future when Grpc.Net.Client sets VersionPolicy automatically. + // https://github.com/grpc/grpc-dotnet/pull/987 + private class VersionPolicyHandler : DelegatingHandler + { + public VersionPolicyHandler(HttpMessageHandler innerHandler) : base(innerHandler) + { + } + + protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + { + request.VersionPolicy = HttpVersionPolicy.RequestVersionOrHigher; + return base.SendAsync(request, cancellationToken); + } + } private async Task CreateCredentialsAsync(bool? useTestCaOverride = null) { diff --git a/src/Grpc/test/testassets/InteropClient/RunTests.ps1 b/src/Grpc/test/testassets/InteropClient/RunTests.ps1 index faa46d98c1..ae2f270461 100644 --- a/src/Grpc/test/testassets/InteropClient/RunTests.ps1 +++ b/src/Grpc/test/testassets/InteropClient/RunTests.ps1 @@ -1,4 +1,4 @@ -Param +Param ( [bool]$use_tls = $false ) @@ -50,4 +50,4 @@ foreach ($test in $allTests) Write-Host } -Write-Host "Done" -ForegroundColor Cyan \ No newline at end of file +Write-Host "Done" -ForegroundColor Cyan diff --git a/src/Grpc/test/testassets/InteropWebsite/Program.cs b/src/Grpc/test/testassets/InteropWebsite/Program.cs index e160254f5d..c10a4256d4 100644 --- a/src/Grpc/test/testassets/InteropWebsite/Program.cs +++ b/src/Grpc/test/testassets/InteropWebsite/Program.cs @@ -37,7 +37,12 @@ namespace InteropTestsWebsite Host.CreateDefaultBuilder(args) .ConfigureLogging(builder => { - builder.AddConsole(); + builder.AddConsole(loggerOptions => + { +#pragma warning disable CS0618 // Type or member is obsolete + loggerOptions.DisableColors = true; +#pragma warning restore CS0618 // Type or member is obsolete + }); builder.SetMinimumLevel(LogLevel.Trace); }) .ConfigureWebHostDefaults(webBuilder => diff --git a/src/Grpc/test/testassets/InteropWebsite/TestServiceImpl.cs b/src/Grpc/test/testassets/InteropWebsite/TestServiceImpl.cs index 918500cd25..d4cb1f953b 100644 --- a/src/Grpc/test/testassets/InteropWebsite/TestServiceImpl.cs +++ b/src/Grpc/test/testassets/InteropWebsite/TestServiceImpl.cs @@ -135,7 +135,7 @@ namespace Grpc.Testing { // ServerCallContext.RequestHeaders filters out grpc-* headers // Get grpc-encoding from HttpContext instead - var encoding = context.GetHttpContext().Request.Headers.SingleOrDefault(h => h.Key == "grpc-encoding").Value.SingleOrDefault(); + var encoding = context.GetHttpContext().Request.Headers.SingleOrDefault(h => string.Equals(h.Key, "grpc-encoding", StringComparison.OrdinalIgnoreCase)).Value.SingleOrDefault(); if (expectCompressed.Value) { if (encoding == null || encoding == "identity") diff --git a/src/Servers/Kestrel/test/Interop.FunctionalTests/HttpClientHttp2InteropTests.cs b/src/Servers/Kestrel/test/Interop.FunctionalTests/HttpClientHttp2InteropTests.cs index 5592457e80..8f8e57bb26 100644 --- a/src/Servers/Kestrel/test/Interop.FunctionalTests/HttpClientHttp2InteropTests.cs +++ b/src/Servers/Kestrel/test/Interop.FunctionalTests/HttpClientHttp2InteropTests.cs @@ -25,38 +25,27 @@ namespace Interop.FunctionalTests /// /// This tests interop with System.Net.Http.HttpClient (SocketHttpHandler) using HTTP/2 (H2 and H2C) /// - // Attributes are here to avoid testing http. Remove when https://github.com/dotnet/aspnetcore/issues/24902 is resolved. - [OSSkipCondition(OperatingSystems.MacOSX, SkipReason="https://github.com/dotnet/aspnetcore/issues/24902")] - [MinimumOSVersion(OperatingSystems.Windows, WindowsVersions.Win10, SkipReason="https://github.com/dotnet/aspnetcore/issues/24902")] public class HttpClientHttp2InteropTests : LoggedTest { - public HttpClientHttp2InteropTests() - { - // H2C - AppContext.SetSwitch("System.Net.Http.SocketsHttpHandler.Http2UnencryptedSupport", true); - } - public static IEnumerable SupportedSchemes { get { - // Re-add "http" when https://github.com/dotnet/aspnetcore/issues/24902 is resolved. - var list = new List(); + var list = new List() + { + new[] { "http" } + }; + if (Utilities.CurrentPlatformSupportsHTTP2OverTls()) { list.Add(new[] { "https" }); } - else - { - // Here because theory data is checked before class-level attributes are checked. - list.Add(new[] { "Remove when https://github.com/dotnet/aspnetcore/issues/24902 resolved." }); - } return list; } } - [ConditionalTheory] + [Theory] [MemberData(nameof(SupportedSchemes))] public async Task HelloWorld(string scheme) { @@ -77,7 +66,7 @@ namespace Interop.FunctionalTests await host.StopAsync().DefaultTimeout(); } - [ConditionalTheory] + [Theory] [MemberData(nameof(SupportedSchemes))] public async Task Echo(string scheme) { @@ -107,7 +96,7 @@ namespace Interop.FunctionalTests } // Concurrency testing - [ConditionalTheory] + [Theory] [MemberData(nameof(SupportedSchemes))] public async Task MultiplexGet(string scheme) { @@ -155,7 +144,7 @@ namespace Interop.FunctionalTests } // Concurrency testing - [ConditionalTheory] + [Theory] [MemberData(nameof(SupportedSchemes))] public async Task MultiplexEcho(string scheme) { @@ -265,7 +254,7 @@ namespace Interop.FunctionalTests } } - [ConditionalTheory] + [Theory] [MemberData(nameof(SupportedSchemes))] public async Task BidirectionalStreaming(string scheme) { @@ -323,7 +312,7 @@ namespace Interop.FunctionalTests await host.StopAsync().DefaultTimeout(); } - [ConditionalTheory] + [Theory] [MemberData(nameof(SupportedSchemes))] public async Task BidirectionalStreamingMoreClientData(string scheme) { @@ -405,7 +394,7 @@ namespace Interop.FunctionalTests await host.StopAsync().DefaultTimeout(); } - [ConditionalTheory] + [Theory] [MemberData(nameof(SupportedSchemes))] public async Task ReverseEcho(string scheme) { @@ -522,7 +511,7 @@ namespace Interop.FunctionalTests } } - [ConditionalTheory] + [Theory] [MemberData(nameof(SupportedSchemes))] public async Task ResponseTrailersWithoutData(string scheme) { @@ -551,7 +540,7 @@ namespace Interop.FunctionalTests await host.StopAsync().DefaultTimeout(); } - [ConditionalTheory] + [Theory] [MemberData(nameof(SupportedSchemes))] public async Task ResponseTrailersWithData(string scheme) { @@ -587,7 +576,7 @@ namespace Interop.FunctionalTests await host.StopAsync().DefaultTimeout(); } - [ConditionalTheory] + [Theory] [MemberData(nameof(SupportedSchemes))] public async Task ServerReset_BeforeResponse_ClientThrows(string scheme) { @@ -611,7 +600,7 @@ namespace Interop.FunctionalTests await host.StopAsync().DefaultTimeout(); } - [ConditionalTheory] + [Theory] [MemberData(nameof(SupportedSchemes))] public async Task ServerReset_AfterHeaders_ClientBodyThrows(string scheme) { @@ -640,7 +629,7 @@ namespace Interop.FunctionalTests await host.StopAsync().DefaultTimeout(); } - [ConditionalTheory] + [Theory] [MemberData(nameof(SupportedSchemes))] public async Task ServerReset_AfterEndStream_NoError(string scheme) { @@ -667,7 +656,7 @@ namespace Interop.FunctionalTests await host.StopAsync().DefaultTimeout(); } - [ConditionalTheory] + [Theory] [MemberData(nameof(SupportedSchemes))] public async Task ServerReset_AfterTrailers_NoError(string scheme) { @@ -699,7 +688,7 @@ namespace Interop.FunctionalTests await host.StopAsync().DefaultTimeout(); } - [ConditionalTheory] + [Theory] [MemberData(nameof(SupportedSchemes))] [QuarantinedTest] public async Task ServerReset_BeforeRequestBody_ClientBodyThrows(string scheme) @@ -756,7 +745,7 @@ namespace Interop.FunctionalTests await host.StopAsync().DefaultTimeout(); } - [ConditionalTheory] + [Theory] [MemberData(nameof(SupportedSchemes))] public async Task ServerReset_BeforeRequestBodyEnd_ClientBodyThrows(string scheme) { @@ -815,7 +804,7 @@ namespace Interop.FunctionalTests await host.StopAsync().DefaultTimeout(); } - [ConditionalTheory] + [Theory] [MemberData(nameof(SupportedSchemes))] public async Task ClientReset_BeforeRequestData_ReadThrows(string scheme) { @@ -861,7 +850,7 @@ namespace Interop.FunctionalTests await host.StopAsync().DefaultTimeout(); } - [ConditionalTheory] + [Theory] [MemberData(nameof(SupportedSchemes))] public async Task ClientReset_BeforeRequestDataEnd_ReadThrows(string scheme) { @@ -906,7 +895,7 @@ namespace Interop.FunctionalTests await host.StopAsync().DefaultTimeout(); } - [ConditionalTheory] + [Theory] [MemberData(nameof(SupportedSchemes))] public async Task ClientReset_BeforeResponse_ResponseSuppressed(string scheme) { @@ -948,7 +937,7 @@ namespace Interop.FunctionalTests await host.StopAsync().DefaultTimeout(); } - [ConditionalTheory] + [Theory] [MemberData(nameof(SupportedSchemes))] public async Task ClientReset_BeforeEndStream_WritesSuppressed(string scheme) { @@ -988,7 +977,7 @@ namespace Interop.FunctionalTests await host.StopAsync().DefaultTimeout(); } - [ConditionalTheory] + [Theory] [MemberData(nameof(SupportedSchemes))] public async Task ClientReset_BeforeTrailers_TrailersSuppressed(string scheme) { @@ -1029,7 +1018,7 @@ namespace Interop.FunctionalTests await host.StopAsync().DefaultTimeout(); } - [ConditionalTheory] + [Theory] [QuarantinedTest("https://github.com/dotnet/runtime/issues/860")] [MemberData(nameof(SupportedSchemes))] public async Task RequestHeaders_MultipleFrames_Accepted(string scheme) @@ -1082,7 +1071,7 @@ namespace Interop.FunctionalTests await host.StopAsync().DefaultTimeout(); } - [ConditionalTheory] + [Theory] [MemberData(nameof(SupportedSchemes))] public async Task ResponseHeaders_MultipleFrames_Accepted(string scheme) { @@ -1131,7 +1120,7 @@ namespace Interop.FunctionalTests await host.StopAsync().DefaultTimeout(); } - [ConditionalTheory] + [Theory] // Expect this to change when the client implements dynamic request header compression. // Will the client send the first headers before receiving our settings frame? // We'll probably need to ensure the settings changes are ack'd before enforcing them. @@ -1194,7 +1183,7 @@ namespace Interop.FunctionalTests // Settings_HeaderTableSize_CanBeReduced_Client - The client uses the default 4k HPACK dynamic table size and it cannot be changed. // Nor does Kestrel yet support sending dynamic table updates, so there's nothing to test here. https://github.com/dotnet/aspnetcore/issues/4715 - [ConditionalTheory] + [Theory] [QuarantinedTest] [MemberData(nameof(SupportedSchemes))] public async Task Settings_MaxConcurrentStreamsGet_Server(string scheme) @@ -1256,7 +1245,7 @@ namespace Interop.FunctionalTests await host.StopAsync().DefaultTimeout(); } - [ConditionalTheory] + [Theory] [QuarantinedTest] [MemberData(nameof(SupportedSchemes))] public async Task Settings_MaxConcurrentStreamsPost_Server(string scheme) @@ -1320,7 +1309,7 @@ namespace Interop.FunctionalTests // Settings_MaxConcurrentStreams_Client - Neither client or server support Push, nothing to test in this direction. - [ConditionalTheory] + [Theory] [MemberData(nameof(SupportedSchemes))] public async Task Settings_MaxFrameSize_Larger_Server(string scheme) { @@ -1353,7 +1342,7 @@ namespace Interop.FunctionalTests // Settings_MaxFrameSize_Larger_Client - Not configurable - [ConditionalTheory] + [Theory] [QuarantinedTest("https://github.com/dotnet/runtime/issues/860")] [MemberData(nameof(SupportedSchemes))] public async Task Settings_MaxHeaderListSize_Server(string scheme) @@ -1386,7 +1375,7 @@ namespace Interop.FunctionalTests await host.StopAsync().DefaultTimeout(); } - [ConditionalTheory] + [Theory] [MemberData(nameof(SupportedSchemes))] public async Task Settings_MaxHeaderListSize_Client(string scheme) { @@ -1420,7 +1409,7 @@ namespace Interop.FunctionalTests // Settings_InitialWindowSize_Lower_Server - Kestrel does not support lowering the InitialStreamWindowSize below the spec default 64kb. // Settings_InitialWindowSize_Lower_Client - Not configurable. - [ConditionalTheory] + [Theory] [MemberData(nameof(SupportedSchemes))] public async Task Settings_InitialWindowSize_Server(string scheme) { @@ -1462,7 +1451,7 @@ namespace Interop.FunctionalTests await host.StopAsync().DefaultTimeout(); } - [ConditionalTheory] + [Theory] [MemberData(nameof(SupportedSchemes))] public async Task Settings_InitialWindowSize_Client(string scheme) { @@ -1498,7 +1487,7 @@ namespace Interop.FunctionalTests await host.StopAsync().DefaultTimeout(); } - [ConditionalTheory] + [Theory] [MemberData(nameof(SupportedSchemes))] public async Task ConnectionWindowSize_Server(string scheme) { @@ -1557,7 +1546,7 @@ namespace Interop.FunctionalTests // The spec default connection window is 64kb - 1 but the client default is 64Mb (not configurable). // The client restricts streams to 64kb - 1 so we would need to issue 64 * 1024 requests to stress the connection window limit. - [ConditionalTheory] + [Theory] [MemberData(nameof(SupportedSchemes))] public async Task UnicodeRequestHost(string scheme) { @@ -1584,7 +1573,7 @@ namespace Interop.FunctionalTests await host.StopAsync().DefaultTimeout(); } - [ConditionalTheory] + [Theory] [MemberData(nameof(SupportedSchemes))] public async Task UrlEncoding(string scheme) { @@ -1612,6 +1601,7 @@ namespace Interop.FunctionalTests handler.ServerCertificateCustomValidationCallback = HttpClientHandler.DangerousAcceptAnyServerCertificateValidator; var client = new HttpClient(handler); client.DefaultRequestVersion = HttpVersion.Version20; + client.DefaultVersionPolicy = HttpVersionPolicy.RequestVersionExact; return client; } @@ -1620,6 +1610,7 @@ namespace Interop.FunctionalTests return new HttpRequestMessage(method, url) { Version = HttpVersion.Version20, + VersionPolicy = HttpVersionPolicy.RequestVersionExact, Content = content, }; }