From 2809c58c0a00629592e5626f9714e38a77b3303a Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Wed, 21 Aug 2024 11:46:23 +0800 Subject: [PATCH 1/5] Add HTTP version configuration to GrpcChannelOptions --- perf/benchmarkapps/GrpcClient/Program.cs | 25 +++------------ src/Grpc.Net.Client.Web/GrpcWebHandler.cs | 12 +++---- src/Grpc.Net.Client/GrpcChannel.cs | 8 +++++ src/Grpc.Net.Client/GrpcChannelOptions.cs | 30 +++++++++++++++++- src/Grpc.Net.Client/Internal/GrpcCall.cs | 4 +-- .../Balancer/ConnectionTests.cs | 19 ++---------- .../Client/ClientFactoryTests.cs | 21 +++---------- .../Infrastructure/GrpcHttpHelper.cs | 5 ++- testassets/Shared/InteropClient.cs | 31 ++++--------------- 9 files changed, 66 insertions(+), 89 deletions(-) diff --git a/perf/benchmarkapps/GrpcClient/Program.cs b/perf/benchmarkapps/GrpcClient/Program.cs index 9517079bb..88275c5f9 100644 --- a/perf/benchmarkapps/GrpcClient/Program.cs +++ b/perf/benchmarkapps/GrpcClient/Program.cs @@ -1,4 +1,4 @@ -#region Copyright notice and license +#region Copyright notice and license // Copyright 2019 The gRPC Authors // @@ -497,13 +497,14 @@ private static ChannelBase CreateChannel(string target) HttpMessageHandler httpMessageHandler = httpClientHandler; + Version? versionOverride = null; if (_options.Protocol == "h3") { // Stop gRPC channel from creating TCP socket. httpClientHandler.ConnectCallback = (context, cancellationToken) => throw new InvalidOperationException("Should never be called for H3."); // Force H3 on all requests. - httpMessageHandler = new Http3DelegatingHandler(httpMessageHandler); + versionOverride = new Version(3, 0); } return GrpcChannel.ForAddress(address, new GrpcChannelOptions @@ -513,7 +514,8 @@ private static ChannelBase CreateChannel(string target) #else HttpClient = new HttpClient(httpMessageHandler), #endif - LoggerFactory = _loggerFactory + LoggerFactory = _loggerFactory, + HttpVersion = versionOverride }); } } @@ -759,21 +761,4 @@ private static bool IsCallCountExceeded() { return _options.CallCount != null && _callsStarted > _options.CallCount; } - - private class Http3DelegatingHandler : DelegatingHandler - { - private static readonly Version Http3Version = new Version(3, 0); - - public Http3DelegatingHandler(HttpMessageHandler innerHandler) - { - InnerHandler = innerHandler; - } - - protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) - { - request.Version = Http3Version; - request.VersionPolicy = HttpVersionPolicy.RequestVersionExact; - return base.SendAsync(request, cancellationToken); - } - } } diff --git a/src/Grpc.Net.Client.Web/GrpcWebHandler.cs b/src/Grpc.Net.Client.Web/GrpcWebHandler.cs index a4d4ac821..7a5089ff2 100644 --- a/src/Grpc.Net.Client.Web/GrpcWebHandler.cs +++ b/src/Grpc.Net.Client.Web/GrpcWebHandler.cs @@ -1,4 +1,4 @@ -#region Copyright notice and license +#region Copyright notice and license // Copyright 2019 The gRPC Authors // @@ -145,13 +145,13 @@ private async Task SendAsyncCore(HttpRequestMessage request } #if NET5_0_OR_GREATER else if (request.RequestUri?.Scheme == Uri.UriSchemeHttps - && request.VersionPolicy == HttpVersionPolicy.RequestVersionOrHigher + && request.VersionPolicy == HttpVersionPolicy.RequestVersionExact && request.Version == System.Net.HttpVersion.Version20) { - // If no explicit HttpVersion is set and the request is using TLS then default to HTTP/1.1. - // HTTP/1.1 together with HttpVersionPolicy.RequestVersionOrHigher it will be compatible - // with all endpoints. - request.Version = System.Net.HttpVersion.Version11; + // If no explicit HttpVersion is set and the request is using TLS then change the version policy + // to allow for HTTP/1.1. HttpVersionPolicy.RequestVersionOrLower it will be compatible + // with HTTP/1.1 and HTTP/2. + request.VersionPolicy = HttpVersionPolicy.RequestVersionOrLower; } #endif #if NETSTANDARD2_0 diff --git a/src/Grpc.Net.Client/GrpcChannel.cs b/src/Grpc.Net.Client/GrpcChannel.cs index e84459040..e046854a1 100644 --- a/src/Grpc.Net.Client/GrpcChannel.cs +++ b/src/Grpc.Net.Client/GrpcChannel.cs @@ -79,6 +79,10 @@ public sealed class GrpcChannel : ChannelBase, IDisposable internal Dictionary CompressionProviders { get; } internal string MessageAcceptEncoding { get; } internal bool Disposed { get; private set; } + internal Version HttpVersion { get; } +#if NET5_0_OR_GREATER + internal HttpVersionPolicy HttpVersionPolicy { get; } +#endif #if SUPPORT_LOAD_BALANCING // Load balancing @@ -175,6 +179,10 @@ internal GrpcChannel(Uri address, GrpcChannelOptions channelOptions) : base(addr RetryThrottling = serviceConfig.RetryThrottling != null ? CreateChannelRetryThrottling(serviceConfig.RetryThrottling) : null; _serviceConfigMethods = CreateServiceConfigMethods(serviceConfig); } + HttpVersion = channelOptions.HttpVersion ?? GrpcProtocolConstants.Http2Version; +#if NET5_0_OR_GREATER + HttpVersionPolicy = channelOptions.HttpVersionPolicy ?? HttpVersionPolicy.RequestVersionExact; +#endif // Non-HTTP addresses (e.g. dns:///custom-hostname) usually specify a path instead of an authority. // Only log about a path being present if HTTP or HTTPS. diff --git a/src/Grpc.Net.Client/GrpcChannelOptions.cs b/src/Grpc.Net.Client/GrpcChannelOptions.cs index f08fc7773..893e0adfa 100644 --- a/src/Grpc.Net.Client/GrpcChannelOptions.cs +++ b/src/Grpc.Net.Client/GrpcChannelOptions.cs @@ -1,4 +1,4 @@ -#region Copyright notice and license +#region Copyright notice and license // Copyright 2019 The gRPC Authors // @@ -287,6 +287,34 @@ public TimeSpan InitialReconnectBackoff /// public IServiceProvider? ServiceProvider { get; set; } + /// + /// Gets or sets the HTTP version to use when making gRPC calls. + /// + /// When a is specified the value will be set on + /// as gRPC calls are made. Changing this property allows the HTTP version of gRPC calls to + /// be overridden. + /// + /// + /// A null value doesn't override the HTTP version of gRPC calls. Defaults to 2.0. + /// + /// + public Version? HttpVersion { get; set; } + +#if NET5_0_OR_GREATER + /// + /// Gets or sets the HTTP policy to use when making gRPC calls. + /// + /// The policy determins how is interpreted when the final HTTP version is + /// negotiated with the server. Changing this property allows the HTTP version of gRPC calls to + /// be overridden. + /// + /// + /// A null value doesn't override the HTTP policy of gRPC calls. Defaults to . + /// + /// + public HttpVersionPolicy? HttpVersionPolicy { get; set; } +#endif + internal T ResolveService(T defaultValue) { return (T?)ServiceProvider?.GetService(typeof(T)) ?? defaultValue; diff --git a/src/Grpc.Net.Client/Internal/GrpcCall.cs b/src/Grpc.Net.Client/Internal/GrpcCall.cs index 36f126a43..3e4653575 100644 --- a/src/Grpc.Net.Client/Internal/GrpcCall.cs +++ b/src/Grpc.Net.Client/Internal/GrpcCall.cs @@ -957,9 +957,9 @@ private void SetWriter(HttpRequestMessage message, HttpContentClientStreamWriter private HttpRequestMessage CreateHttpRequestMessage(TimeSpan? timeout) { var message = new HttpRequestMessage(HttpMethod.Post, _grpcMethodInfo.CallUri); - message.Version = GrpcProtocolConstants.Http2Version; + message.Version = Channel.HttpVersion; #if NET5_0_OR_GREATER - message.VersionPolicy = HttpVersionPolicy.RequestVersionOrHigher; + message.VersionPolicy = Channel.HttpVersionPolicy; #endif // Set raw headers on request using name/values. Typed headers allocate additional objects. diff --git a/test/FunctionalTests/Balancer/ConnectionTests.cs b/test/FunctionalTests/Balancer/ConnectionTests.cs index 9476b5126..13c6aa78a 100644 --- a/test/FunctionalTests/Balancer/ConnectionTests.cs +++ b/test/FunctionalTests/Balancer/ConnectionTests.cs @@ -310,13 +310,14 @@ async Task UnaryMethod(HelloRequest request, ServerCallContext conte } } }; - var grpcWebHandler = new GrpcWebHandler(GrpcWebMode.GrpcWeb, new RequestVersionHandler(socketsHttpHandler)); + var grpcWebHandler = new GrpcWebHandler(GrpcWebMode.GrpcWeb, socketsHttpHandler); var channel = GrpcChannel.ForAddress("static:///localhost", new GrpcChannelOptions { LoggerFactory = LoggerFactory, HttpHandler = grpcWebHandler, ServiceProvider = services.BuildServiceProvider(), - Credentials = new SslCredentials() + Credentials = new SslCredentials(), + HttpVersionPolicy = HttpVersionPolicy.RequestVersionExact }); var client = TestClientFactory.Create(channel, endpoint1.Method); @@ -562,20 +563,6 @@ Task UnaryMethod(HelloRequest request, ServerCallContext context) Assert.AreEqual("Balancer", reply.Message); } - private class RequestVersionHandler : DelegatingHandler - { - public RequestVersionHandler(HttpMessageHandler innerHandler) : base(innerHandler) - { - } - - protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) - { - request.VersionPolicy = HttpVersionPolicy.RequestVersionExact; - - return base.SendAsync(request, cancellationToken); - } - } - [Test] public async Task SocketSendsBytes_BeforeCall_ReplaysBufferedData() { diff --git a/test/FunctionalTests/Client/ClientFactoryTests.cs b/test/FunctionalTests/Client/ClientFactoryTests.cs index 59f0991c8..bcfc500aa 100644 --- a/test/FunctionalTests/Client/ClientFactoryTests.cs +++ b/test/FunctionalTests/Client/ClientFactoryTests.cs @@ -104,7 +104,11 @@ Task UnaryCall(HelloRequest request, ServerCallContext context) { return TestClientFactory.Create(invoker, method); }) - .AddHttpMessageHandler(() => new Http3Handler()) + .ConfigureChannel(options => + { + options.HttpVersion = HttpVersion.Version30; + options.HttpVersionPolicy = HttpVersionPolicy.RequestVersionExact; + }) .ConfigurePrimaryHttpMessageHandler(() => { return new SocketsHttpHandler @@ -125,20 +129,5 @@ Task UnaryCall(HelloRequest request, ServerCallContext context) // Assert Assert.AreEqual("Hello world", response1.Message); } - - private class Http3Handler : DelegatingHandler - { - public Http3Handler() { } - public Http3Handler(HttpMessageHandler innerHandler) : base(innerHandler) { } - - protected override Task SendAsync( - HttpRequestMessage request, CancellationToken cancellationToken) - { - request.Version = HttpVersion.Version30; - request.VersionPolicy = HttpVersionPolicy.RequestVersionExact; - - return base.SendAsync(request, cancellationToken); - } - } #endif } diff --git a/test/FunctionalTests/Infrastructure/GrpcHttpHelper.cs b/test/FunctionalTests/Infrastructure/GrpcHttpHelper.cs index bfcfe9c41..f631e498b 100644 --- a/test/FunctionalTests/Infrastructure/GrpcHttpHelper.cs +++ b/test/FunctionalTests/Infrastructure/GrpcHttpHelper.cs @@ -1,4 +1,4 @@ -#region Copyright notice and license +#region Copyright notice and license // Copyright 2019 The gRPC Authors // @@ -16,7 +16,6 @@ #endregion - namespace Grpc.AspNetCore.FunctionalTests.Infrastructure; public static class GrpcHttpHelper @@ -26,7 +25,7 @@ public static HttpRequestMessage Create(string url, HttpMethod? method = null) var request = new HttpRequestMessage(method ?? HttpMethod.Post, url); request.Version = new Version(2, 0); #if NET5_0_OR_GREATER - request.VersionPolicy = HttpVersionPolicy.RequestVersionOrHigher; + request.VersionPolicy = HttpVersionPolicy.RequestVersionExact; #endif return request; diff --git a/testassets/Shared/InteropClient.cs b/testassets/Shared/InteropClient.cs index 34e98184d..8ea989dc6 100644 --- a/testassets/Shared/InteropClient.cs +++ b/testassets/Shared/InteropClient.cs @@ -120,18 +120,17 @@ private async Task HttpClientCreateChannel() httpMessageHandler = CreateWinHttpHandler(); } + Version? versionOverride = null; if (!string.IsNullOrEmpty(options.GrpcWebMode) && !string.Equals(options.GrpcWebMode, "None", StringComparison.OrdinalIgnoreCase)) { var mode = (GrpcWebMode)Enum.Parse(typeof(GrpcWebMode), options.GrpcWebMode); - httpMessageHandler = new GrpcWebHandler(mode, httpMessageHandler) - { - HttpVersion = new Version(1, 1) - }; + httpMessageHandler = new GrpcWebHandler(mode, httpMessageHandler); + versionOverride = new Version(1, 1); } if (options.UseHttp3) { #if NET6_0_OR_GREATER - httpMessageHandler = new Http3DelegatingHandler(httpMessageHandler); + versionOverride = new Version(3, 0); #else throw new Exception("HTTP/3 requires .NET 6 or later."); #endif @@ -141,31 +140,13 @@ private async Task HttpClientCreateChannel() { Credentials = credentials, HttpHandler = httpMessageHandler, - LoggerFactory = loggerFactory + LoggerFactory = loggerFactory, + HttpVersion = versionOverride }); return new GrpcChannelWrapper(channel); } -#if NET6_0_OR_GREATER - private class Http3DelegatingHandler : DelegatingHandler - { - private static readonly Version Http3Version = new Version(3, 0); - - public Http3DelegatingHandler(HttpMessageHandler innerHandler) - { - InnerHandler = innerHandler; - } - - protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) - { - request.Version = Http3Version; - request.VersionPolicy = HttpVersionPolicy.RequestVersionExact; - return base.SendAsync(request, cancellationToken); - } - } -#endif - private static WinHttpHandler CreateWinHttpHandler() { #pragma warning disable CA1416 // Validate platform compatibility From 543c7281e5f5d44a9b1f95087be2f7cd3719f813 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Wed, 21 Aug 2024 13:24:52 +0800 Subject: [PATCH 2/5] Update --- src/Grpc.Net.Client.Web/GrpcWebHandler.cs | 6 ++ .../Balancer/ConnectionTests.cs | 15 +++- .../Balancer/PickFirstBalancerTests.cs | 15 ---- .../Web/Client/ConnectionTests.cs | 86 +++++++++++++------ .../Web/GrpcWebFunctionalTestBase.cs | 4 +- .../GrpcWebHandlerTests.cs | 31 ++++++- 6 files changed, 108 insertions(+), 49 deletions(-) diff --git a/src/Grpc.Net.Client.Web/GrpcWebHandler.cs b/src/Grpc.Net.Client.Web/GrpcWebHandler.cs index 7a5089ff2..677431c83 100644 --- a/src/Grpc.Net.Client.Web/GrpcWebHandler.cs +++ b/src/Grpc.Net.Client.Web/GrpcWebHandler.cs @@ -44,6 +44,7 @@ public sealed class GrpcWebHandler : DelegatingHandler /// be overridden. /// /// + [Obsolete("HttpVersion is obsolete and will be removed in a future release. Use GrpcChannelOptions.HttpVersion and GrpcChannelOptions.HttpVersionPolicy instead.")] public Version? HttpVersion { get; set; } /// @@ -136,13 +137,18 @@ private async Task SendAsyncCore(HttpRequestMessage request // https://github.com/mono/mono/issues/18718 request.SetOption(WebAssemblyEnableStreamingResponseKey, true); +#pragma warning disable CS0618 // Type or member is obsolete if (HttpVersion != null) { // This doesn't guarantee that the specified version is used. Some handlers will ignore it. // For example, version in the browser always negotiated by the browser and HttpClient // uses what the browser has negotiated. request.Version = HttpVersion; +#if NET5_0_OR_GREATER + request.VersionPolicy = HttpVersionPolicy.RequestVersionOrHigher; +#endif } +#pragma warning restore CS0618 // Type or member is obsolete #if NET5_0_OR_GREATER else if (request.RequestUri?.Scheme == Uri.UriSchemeHttps && request.VersionPolicy == HttpVersionPolicy.RequestVersionExact diff --git a/test/FunctionalTests/Balancer/ConnectionTests.cs b/test/FunctionalTests/Balancer/ConnectionTests.cs index 13c6aa78a..7b1454038 100644 --- a/test/FunctionalTests/Balancer/ConnectionTests.cs +++ b/test/FunctionalTests/Balancer/ConnectionTests.cs @@ -317,13 +317,13 @@ async Task UnaryMethod(HelloRequest request, ServerCallContext conte HttpHandler = grpcWebHandler, ServiceProvider = services.BuildServiceProvider(), Credentials = new SslCredentials(), - HttpVersionPolicy = HttpVersionPolicy.RequestVersionExact + HttpVersionPolicy = HttpVersionPolicy.RequestVersionOrHigher }); var client = TestClientFactory.Create(channel, endpoint1.Method); // Act - grpcWebHandler.HttpVersion = new Version(1, 1); + SetHandlerHttpVersion(grpcWebHandler, new Version(1, 1)); var http11CallTasks = new List>(); for (int i = 0; i < 10; i++) { @@ -353,7 +353,7 @@ await TestHelpers.AssertIsTrueRetryAsync(() => } // Act - grpcWebHandler.HttpVersion = new Version(2, 0); + SetHandlerHttpVersion(grpcWebHandler, new Version(2, 0)); var http2CallTasks = new List>(); for (int i = 0; i < 10; i++) { @@ -390,7 +390,7 @@ await TestHelpers.AssertIsTrueRetryAsync(() => return activeStreams.Count == 10; }, "Wait for HTTP/2 connection to end."); - grpcWebHandler.HttpVersion = new Version(1, 1); + SetHandlerHttpVersion(grpcWebHandler, new Version(1, 1)); await Task.Delay(1000); @@ -413,6 +413,13 @@ await TestHelpers.AssertIsTrueRetryAsync(() => Assert.AreEqual(new DnsEndPoint("127.0.0.1", endpoint2.Address.Port), activeStreams[0].EndPoint); } + private static void SetHandlerHttpVersion(GrpcWebHandler handler, Version version) + { +#pragma warning disable CS0618 // Type or member is obsolete + handler.HttpVersion = version; +#pragma warning restore CS0618 // Type or member is obsolete + } + #if NET7_0_OR_GREATER [Test] public async Task Active_UnaryCall_HostOverride_Success() diff --git a/test/FunctionalTests/Balancer/PickFirstBalancerTests.cs b/test/FunctionalTests/Balancer/PickFirstBalancerTests.cs index 23c542491..fe820fe4a 100644 --- a/test/FunctionalTests/Balancer/PickFirstBalancerTests.cs +++ b/test/FunctionalTests/Balancer/PickFirstBalancerTests.cs @@ -47,21 +47,6 @@ namespace Grpc.AspNetCore.FunctionalTests.Balancer; [TestFixture] public class PickFirstBalancerTests : FunctionalTestBase { - private GrpcChannel CreateGrpcWebChannel(TestServerEndpointName endpointName, Version? version) - { - var grpcWebHandler = new GrpcWebHandler(GrpcWebMode.GrpcWeb); - grpcWebHandler.HttpVersion = version; - - var httpClient = Fixture.CreateClient(endpointName, grpcWebHandler); - var channel = GrpcChannel.ForAddress(httpClient.BaseAddress!, new GrpcChannelOptions - { - HttpClient = httpClient, - LoggerFactory = LoggerFactory - }); - - return channel; - } - [Test] public async Task UnaryCall_CallAfterConnectionTimeout_Success() { diff --git a/test/FunctionalTests/Web/Client/ConnectionTests.cs b/test/FunctionalTests/Web/Client/ConnectionTests.cs index 84135c57b..f544f029e 100644 --- a/test/FunctionalTests/Web/Client/ConnectionTests.cs +++ b/test/FunctionalTests/Web/Client/ConnectionTests.cs @@ -16,6 +16,7 @@ #endregion +using System.Net.Http; using Grpc.AspNetCore.FunctionalTests.Infrastructure; using Grpc.Core; using Grpc.Gateway.Testing; @@ -31,51 +32,80 @@ public class ConnectionTests : FunctionalTestBase private HttpClient CreateGrpcWebClient(TestServerEndpointName endpointName, Version? version) { GrpcWebHandler grpcWebHandler = new GrpcWebHandler(GrpcWebMode.GrpcWeb); +#pragma warning disable CS0618 // Type or member is obsolete grpcWebHandler.HttpVersion = version; +#pragma warning restore CS0618 // Type or member is obsolete return Fixture.CreateClient(endpointName, grpcWebHandler); } - private GrpcChannel CreateGrpcWebChannel(TestServerEndpointName endpointName, Version? version) + private GrpcChannel CreateGrpcWebChannel(TestServerEndpointName endpointName, Version? version, bool setVersionOnHandler) { - var httpClient = CreateGrpcWebClient(endpointName, version); - var channel = GrpcChannel.ForAddress(httpClient.BaseAddress!, new GrpcChannelOptions + var options = new GrpcChannelOptions { - HttpClient = httpClient, LoggerFactory = LoggerFactory - }); - - return channel; + }; + if (setVersionOnHandler) + { + options.HttpClient = CreateGrpcWebClient(endpointName, version: null); + if (version != null) + { + options.HttpVersion = version; + options.HttpVersionPolicy = HttpVersionPolicy.RequestVersionOrHigher; + } + return GrpcChannel.ForAddress(options.HttpClient.BaseAddress!, options); + } + else + { + options.HttpClient = CreateGrpcWebClient(endpointName, version); + return GrpcChannel.ForAddress(options.HttpClient.BaseAddress!, options); + } } - [TestCase(TestServerEndpointName.Http1, "2.0", false)] - [TestCase(TestServerEndpointName.Http1, "1.1", true)] - [TestCase(TestServerEndpointName.Http1, null, false)] - [TestCase(TestServerEndpointName.Http2, "2.0", true)] - [TestCase(TestServerEndpointName.Http2, "1.1", false)] - [TestCase(TestServerEndpointName.Http2, null, true)] + private static IEnumerable ConnectionTestData() + { + yield return new TestCaseData(TestServerEndpointName.Http1, "2.0", false); + yield return new TestCaseData(TestServerEndpointName.Http1, "1.1", true); + yield return new TestCaseData(TestServerEndpointName.Http1, null, false); + yield return new TestCaseData(TestServerEndpointName.Http2, "2.0", true); + yield return new TestCaseData(TestServerEndpointName.Http2, "1.1", false); + yield return new TestCaseData(TestServerEndpointName.Http2, null, true); #if NET5_0_OR_GREATER - // Specifing HTTP/2 doesn't work when the server is using TLS with HTTP/1.1 - // Caused by using HttpVersionPolicy.RequestVersionOrHigher setting - [TestCase(TestServerEndpointName.Http1WithTls, "2.0", false)] + // Specifing HTTP/2 doesn't work when the server is using TLS with HTTP/1.1 + // Caused by using HttpVersionPolicy.RequestVersionOrHigher setting + yield return new TestCaseData(TestServerEndpointName.Http1WithTls, "2.0", false); #else - [TestCase(TestServerEndpointName.Http1WithTls, "2.0", true)] + yield return new TestCaseData(TestServerEndpointName.Http1WithTls, "2.0", true); #endif - [TestCase(TestServerEndpointName.Http1WithTls, "1.1", true)] - [TestCase(TestServerEndpointName.Http1WithTls, null, true)] - [TestCase(TestServerEndpointName.Http2WithTls, "2.0", true)] + yield return new TestCaseData(TestServerEndpointName.Http1WithTls, "1.1", true); + yield return new TestCaseData(TestServerEndpointName.Http1WithTls, null, true); + yield return new TestCaseData(TestServerEndpointName.Http2WithTls, "2.0", true); #if NET5_0_OR_GREATER - // Specifing HTTP/1.1 does work when the server is using TLS with HTTP/2 - // Caused by using HttpVersionPolicy.RequestVersionOrHigher setting - [TestCase(TestServerEndpointName.Http2WithTls, "1.1", true)] + // Specifing HTTP/1.1 does work when the server is using TLS with HTTP/2 + // Caused by using HttpVersionPolicy.RequestVersionOrHigher setting + yield return new TestCaseData(TestServerEndpointName.Http2WithTls, "1.1", true); #else - [TestCase(TestServerEndpointName.Http2WithTls, "1.1", false)] + yield return new TestCaseData(TestServerEndpointName.Http2WithTls, "1.1", false); #endif - [TestCase(TestServerEndpointName.Http2WithTls, null, true)] + yield return new TestCaseData(TestServerEndpointName.Http2WithTls, null, true); #if NET7_0_OR_GREATER - [TestCase(TestServerEndpointName.Http3WithTls, null, true)] + yield return new TestCaseData(TestServerEndpointName.Http3WithTls, null, true); #endif - public async Task SendValidRequest_WithConnectionOptions(TestServerEndpointName endpointName, string? version, bool success) + } + + [TestCaseSource(nameof(ConnectionTestData))] + public async Task SendValidRequest_WithConnectionOptionsOnHandler(TestServerEndpointName endpointName, string? version, bool success) + { + await SendRequestWithConnectionOptionsCore(endpointName, version, success, setVersionOnHandler: true); + } + + [TestCaseSource(nameof(ConnectionTestData))] + public async Task SendValidRequest_WithConnectionOptionsOnChannel(TestServerEndpointName endpointName, string? version, bool success) + { + await SendRequestWithConnectionOptionsCore(endpointName, version, success, setVersionOnHandler: false); + } + + private async Task SendRequestWithConnectionOptionsCore(TestServerEndpointName endpointName, string? version, bool success, bool setVersionOnHandler) { #if NET6_0_OR_GREATER if (endpointName == TestServerEndpointName.Http3WithTls && @@ -92,7 +122,7 @@ public async Task SendValidRequest_WithConnectionOptions(TestServerEndpointName // Arrage Version.TryParse(version, out var v); - var channel = CreateGrpcWebChannel(endpointName, v); + var channel = CreateGrpcWebChannel(endpointName, v, setVersionOnHandler); var client = new EchoService.EchoServiceClient(channel); diff --git a/test/FunctionalTests/Web/GrpcWebFunctionalTestBase.cs b/test/FunctionalTests/Web/GrpcWebFunctionalTestBase.cs index 5fa37fd20..4e6cabc5e 100644 --- a/test/FunctionalTests/Web/GrpcWebFunctionalTestBase.cs +++ b/test/FunctionalTests/Web/GrpcWebFunctionalTestBase.cs @@ -1,4 +1,4 @@ -#region Copyright notice and license +#region Copyright notice and license // Copyright 2019 The gRPC Authors // @@ -88,7 +88,9 @@ protected HttpClient CreateGrpcWebClient() var mode = GrpcTestMode == GrpcTestMode.GrpcWeb ? GrpcWebMode.GrpcWeb : GrpcWebMode.GrpcWebText; grpcWebHandler = new GrpcWebHandler(mode) { +#pragma warning disable CS0618 // Type or member is obsolete HttpVersion = protocol +#pragma warning restore CS0618 // Type or member is obsolete }; } diff --git a/test/Grpc.Net.Client.Web.Tests/GrpcWebHandlerTests.cs b/test/Grpc.Net.Client.Web.Tests/GrpcWebHandlerTests.cs index 237bf591f..895cc77b1 100644 --- a/test/Grpc.Net.Client.Web.Tests/GrpcWebHandlerTests.cs +++ b/test/Grpc.Net.Client.Web.Tests/GrpcWebHandlerTests.cs @@ -54,8 +54,9 @@ public async Task HttpVersion_Unset_HttpRequestMessageVersionUnchanged() Assert.AreEqual(GrpcWebProtocolConstants.Http2Version, response.Version); } +#pragma warning disable CS0618 // Type or member is obsolete [Test] - public async Task HttpVersion_Set_HttpRequestMessageVersionChanged() + public async Task HttpVersion_SetOnHandler_HttpRequestMessageVersionChanged() { // Arrange var request = new HttpRequestMessage @@ -81,6 +82,34 @@ public async Task HttpVersion_Set_HttpRequestMessageVersionChanged() Assert.AreEqual(HttpVersion.Version11, testHttpHandler.Request!.Version); Assert.AreEqual(GrpcWebProtocolConstants.Http2Version, response.Version); } +#pragma warning restore CS0618 // Type or member is obsolete + + [Test] + public async Task HttpVersion_SetOnMessage_HttpRequestMessageVersionChanged() + { + // Arrange + var request = new HttpRequestMessage + { + Version = HttpVersion.Version11, + Content = new ByteArrayContent(Array.Empty()) + { + Headers = { ContentType = new MediaTypeHeaderValue("application/grpc") } + } + }; + var testHttpHandler = new TestHttpHandler(); + var grpcWebHandler = new GrpcWebHandler(GrpcWebMode.GrpcWeb) + { + InnerHandler = testHttpHandler + }; + var messageInvoker = new HttpMessageInvoker(grpcWebHandler); + + // Act + var response = await messageInvoker.SendAsync(request, CancellationToken.None); + + // Assert + Assert.AreEqual(HttpVersion.Version11, testHttpHandler.Request!.Version); + Assert.AreEqual(GrpcWebProtocolConstants.Http2Version, response.Version); + } [Test] public async Task GrpcWebMode_GrpcWebText_AcceptHeaderAdded() From d6045258eed5ffc0a1ab2a4ca147f86f5f0357ff Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Wed, 21 Aug 2024 14:03:25 +0800 Subject: [PATCH 3/5] Fix --- .../Balancer/ConnectionTests.cs | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/test/FunctionalTests/Balancer/ConnectionTests.cs b/test/FunctionalTests/Balancer/ConnectionTests.cs index 7b1454038..952a1b49d 100644 --- a/test/FunctionalTests/Balancer/ConnectionTests.cs +++ b/test/FunctionalTests/Balancer/ConnectionTests.cs @@ -310,14 +310,13 @@ async Task UnaryMethod(HelloRequest request, ServerCallContext conte } } }; - var grpcWebHandler = new GrpcWebHandler(GrpcWebMode.GrpcWeb, socketsHttpHandler); + var grpcWebHandler = new GrpcWebHandler(GrpcWebMode.GrpcWeb, new RequestVersionHandler(socketsHttpHandler)); var channel = GrpcChannel.ForAddress("static:///localhost", new GrpcChannelOptions { LoggerFactory = LoggerFactory, HttpHandler = grpcWebHandler, ServiceProvider = services.BuildServiceProvider(), - Credentials = new SslCredentials(), - HttpVersionPolicy = HttpVersionPolicy.RequestVersionOrHigher + Credentials = new SslCredentials() }); var client = TestClientFactory.Create(channel, endpoint1.Method); @@ -570,6 +569,20 @@ Task UnaryMethod(HelloRequest request, ServerCallContext context) Assert.AreEqual("Balancer", reply.Message); } + private class RequestVersionHandler : DelegatingHandler + { + public RequestVersionHandler(HttpMessageHandler innerHandler) : base(innerHandler) + { + } + + protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + { + request.VersionPolicy = HttpVersionPolicy.RequestVersionExact; + + return base.SendAsync(request, cancellationToken); + } + } + [Test] public async Task SocketSendsBytes_BeforeCall_ReplaysBufferedData() { From 5109726d128bb9de4473870a7c586603bfa11fbb Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Wed, 21 Aug 2024 14:07:15 +0800 Subject: [PATCH 4/5] Improve comment --- src/Grpc.Net.Client/GrpcChannelOptions.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/Grpc.Net.Client/GrpcChannelOptions.cs b/src/Grpc.Net.Client/GrpcChannelOptions.cs index 893e0adfa..b7adebe86 100644 --- a/src/Grpc.Net.Client/GrpcChannelOptions.cs +++ b/src/Grpc.Net.Client/GrpcChannelOptions.cs @@ -304,9 +304,10 @@ public TimeSpan InitialReconnectBackoff /// /// Gets or sets the HTTP policy to use when making gRPC calls. /// - /// The policy determins how is interpreted when the final HTTP version is - /// negotiated with the server. Changing this property allows the HTTP version of gRPC calls to - /// be overridden. + /// When a is specified the value will be set on + /// as gRPC calls are made. The policy determines how is interpreted when + /// the final HTTP version is negotiated with the server. Changing this property allows the HTTP + /// version of gRPC calls to be overridden. /// /// /// A null value doesn't override the HTTP policy of gRPC calls. Defaults to . From f5ff25930bb1c128fd93df5283e17d824a976a90 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Thu, 22 Aug 2024 07:33:34 +0800 Subject: [PATCH 5/5] Update src/Grpc.Net.Client.Web/GrpcWebHandler.cs Co-authored-by: Andrew Casey --- src/Grpc.Net.Client.Web/GrpcWebHandler.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Grpc.Net.Client.Web/GrpcWebHandler.cs b/src/Grpc.Net.Client.Web/GrpcWebHandler.cs index 677431c83..136f73e83 100644 --- a/src/Grpc.Net.Client.Web/GrpcWebHandler.cs +++ b/src/Grpc.Net.Client.Web/GrpcWebHandler.cs @@ -44,7 +44,11 @@ public sealed class GrpcWebHandler : DelegatingHandler /// be overridden. /// /// +#if NET5_0_OR_GREATER [Obsolete("HttpVersion is obsolete and will be removed in a future release. Use GrpcChannelOptions.HttpVersion and GrpcChannelOptions.HttpVersionPolicy instead.")] +#else + [Obsolete("HttpVersion is obsolete and will be removed in a future release. Use GrpcChannelOptions.HttpVersion instead.")] +#endif public Version? HttpVersion { get; set; } ///