From 9779aebafeff921ea42f9580d04eb60390effaa2 Mon Sep 17 00:00:00 2001 From: Brennan Conroy Date: Tue, 19 Jul 2022 11:08:08 -0700 Subject: [PATCH 1/7] Better support for HTTP/2 in SignalR .NET client --- .../src/HttpConnection.cs | 3 --- .../src/Internal/DefaultTransportFactory.cs | 2 +- .../src/Internal/LoggingHttpMessageHandler.cs | 2 +- .../src/Internal/SendUtils.cs | 2 -- .../src/Internal/WebSocketsTransport.cs | 11 ++++++++++- .../SignalR/test/WebSocketsTransportTests.cs | 18 +++++++++--------- 6 files changed, 21 insertions(+), 17 deletions(-) diff --git a/src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnection.cs b/src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnection.cs index 293ac92d71e5..0aac5de51fdc 100644 --- a/src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnection.cs +++ b/src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnection.cs @@ -461,9 +461,6 @@ private async Task NegotiateAsync(Uri url, HttpClient httpC using (var request = new HttpRequestMessage(HttpMethod.Post, uri)) { - // Corefx changed the default version and High Sierra curlhandler tries to upgrade request - request.Version = new Version(1, 1); - #if NET5_0_OR_GREATER request.Options.Set(new HttpRequestOptionsKey("IsNegotiate"), true); #else diff --git a/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/DefaultTransportFactory.cs b/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/DefaultTransportFactory.cs index 2c7315a744ee..d40836493827 100644 --- a/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/DefaultTransportFactory.cs +++ b/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/DefaultTransportFactory.cs @@ -37,7 +37,7 @@ public ITransport CreateTransport(HttpTransportType availableServerTransports) { try { - return new WebSocketsTransport(_httpConnectionOptions, _loggerFactory, _accessTokenProvider); + return new WebSocketsTransport(_httpConnectionOptions, _loggerFactory, _accessTokenProvider, _httpClient); } catch (PlatformNotSupportedException ex) { diff --git a/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/LoggingHttpMessageHandler.cs b/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/LoggingHttpMessageHandler.cs index 33988562a9bd..5be883dfdc70 100644 --- a/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/LoggingHttpMessageHandler.cs +++ b/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/LoggingHttpMessageHandler.cs @@ -30,7 +30,7 @@ protected override async Task SendAsync(HttpRequestMessage var response = await base.SendAsync(request, cancellationToken).ConfigureAwait(false); - if (!response.IsSuccessStatusCode) + if (!response.IsSuccessStatusCode && response.StatusCode != HttpStatusCode.SwitchingProtocols) { Log.UnsuccessfulHttpResponse(_logger, response.StatusCode, request.Method, request.RequestUri!); } diff --git a/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/SendUtils.cs b/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/SendUtils.cs index 25a050398454..2ad92f7d279f 100644 --- a/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/SendUtils.cs +++ b/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/SendUtils.cs @@ -40,8 +40,6 @@ public static async Task SendMessages(Uri sendUrl, IDuplexPipe application, Http // Send them in a single post var request = new HttpRequestMessage(HttpMethod.Post, sendUrl); - // Corefx changed the default version and High Sierra curlhandler tries to upgrade request - request.Version = new Version(1, 1); request.Content = new ReadOnlySequenceContent(buffer); diff --git a/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/WebSocketsTransport.cs b/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/WebSocketsTransport.cs index 31641345b798..0a11be4b3985 100644 --- a/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/WebSocketsTransport.cs +++ b/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/WebSocketsTransport.cs @@ -4,6 +4,7 @@ using System; using System.Diagnostics; using System.IO.Pipelines; +using System.Net.Http; using System.Net.WebSockets; using System.Runtime.InteropServices; using System.Text.Encodings.Web; @@ -24,6 +25,7 @@ internal sealed partial class WebSocketsTransport : ITransport private readonly TimeSpan _closeTimeout; private volatile bool _aborted; private readonly HttpConnectionOptions _httpConnectionOptions; + private readonly HttpClient? _httpClient; private IDuplexPipe? _transport; @@ -33,7 +35,7 @@ internal sealed partial class WebSocketsTransport : ITransport public PipeWriter Output => _transport!.Output; - public WebSocketsTransport(HttpConnectionOptions httpConnectionOptions, ILoggerFactory loggerFactory, Func> accessTokenProvider) + public WebSocketsTransport(HttpConnectionOptions httpConnectionOptions, ILoggerFactory loggerFactory, Func> accessTokenProvider, HttpClient? httpClient) { _logger = (loggerFactory ?? NullLoggerFactory.Instance).CreateLogger(); _httpConnectionOptions = httpConnectionOptions ?? new HttpConnectionOptions(); @@ -43,6 +45,8 @@ public WebSocketsTransport(HttpConnectionOptions httpConnectionOptions, ILoggerF // We were given an updated delegate from the HttpConnection and we are updating what we have in httpOptions // options itself is copied object of user's options _httpConnectionOptions.AccessTokenProvider = accessTokenProvider; + + _httpClient = httpClient; } private async ValueTask DefaultWebSocketFactory(WebSocketConnectionContext context, CancellationToken cancellationToken) @@ -138,7 +142,12 @@ private async ValueTask DefaultWebSocketFactory(WebSocketConnectionCo try { +#if NET7_0_OR_GREATER + // TODO: Pass _httpClient here once https://github.com/dotnet/runtime/issues/72476 and https://github.com/dotnet/runtime/issues/72301 are resolved + await webSocket.ConnectAsync(url, invoker: null, cancellationToken).ConfigureAwait(false); +#else await webSocket.ConnectAsync(url, cancellationToken).ConfigureAwait(false); +#endif } catch { diff --git a/src/SignalR/server/SignalR/test/WebSocketsTransportTests.cs b/src/SignalR/server/SignalR/test/WebSocketsTransportTests.cs index 2afa3c2f0305..550a69bee106 100644 --- a/src/SignalR/server/SignalR/test/WebSocketsTransportTests.cs +++ b/src/SignalR/server/SignalR/test/WebSocketsTransportTests.cs @@ -40,7 +40,7 @@ public async Task HttpOptionsSetOntoWebSocketOptions() await using (var server = await StartServer()) { - var webSocketsTransport = new WebSocketsTransport(httpConnectionOptions: httpOptions, loggerFactory: null, accessTokenProvider: null); + var webSocketsTransport = new WebSocketsTransport(httpConnectionOptions: httpOptions, loggerFactory: null, accessTokenProvider: null, httpClient: null); Assert.NotNull(webSocketsTransport); // we need to open a connection so it would apply httpOptions to webSocketOptions @@ -77,7 +77,7 @@ public async Task HttpOptionsWebSocketFactoryIsUsed() return ValueTask.FromResult(webSocketMock.Object); }; - var webSocketsTransport = new WebSocketsTransport(httpConnectionOptions: httpOptions, loggerFactory: null, accessTokenProvider: null); + var webSocketsTransport = new WebSocketsTransport(httpConnectionOptions: httpOptions, loggerFactory: null, accessTokenProvider: null, httpClient: null); await webSocketsTransport.StartAsync(new Uri("http://FakeEndpot.com/echo"), TransferFormat.Binary).DefaultTimeout(); await webSocketsTransport.StopAsync().DefaultTimeout(); @@ -91,7 +91,7 @@ public async Task WebSocketsTransportStopsSendAndReceiveLoopsWhenTransportIsStop { await using (var server = await StartServer()) { - var webSocketsTransport = new WebSocketsTransport(httpConnectionOptions: null, loggerFactory: LoggerFactory, accessTokenProvider: null); + var webSocketsTransport = new WebSocketsTransport(httpConnectionOptions: null, loggerFactory: LoggerFactory, accessTokenProvider: null, httpClient: null); await webSocketsTransport.StartAsync(new Uri(server.WebSocketsUrl + "/echo"), TransferFormat.Binary).DefaultTimeout(); await webSocketsTransport.StopAsync().DefaultTimeout(); @@ -105,7 +105,7 @@ public async Task WebSocketsTransportSendsUserAgent() { await using (var server = await StartServer()) { - var webSocketsTransport = new WebSocketsTransport(httpConnectionOptions: null, loggerFactory: LoggerFactory, accessTokenProvider: null); + var webSocketsTransport = new WebSocketsTransport(httpConnectionOptions: null, loggerFactory: LoggerFactory, accessTokenProvider: null, httpClient: null); await webSocketsTransport.StartAsync(new Uri(server.WebSocketsUrl + "/httpheader"), TransferFormat.Binary).DefaultTimeout(); @@ -136,7 +136,7 @@ public async Task WebSocketsTransportSendsXRequestedWithHeader() { await using (var server = await StartServer()) { - var webSocketsTransport = new WebSocketsTransport(httpConnectionOptions: null, loggerFactory: LoggerFactory, accessTokenProvider: null); + var webSocketsTransport = new WebSocketsTransport(httpConnectionOptions: null, loggerFactory: LoggerFactory, accessTokenProvider: null, httpClient: null); await webSocketsTransport.StartAsync(new Uri(server.WebSocketsUrl + "/httpheader"), TransferFormat.Binary).DefaultTimeout(); @@ -159,7 +159,7 @@ public async Task WebSocketsTransportStopsWhenConnectionChannelClosed() { await using (var server = await StartServer()) { - var webSocketsTransport = new WebSocketsTransport(httpConnectionOptions: null, loggerFactory: LoggerFactory, accessTokenProvider: null); + var webSocketsTransport = new WebSocketsTransport(httpConnectionOptions: null, loggerFactory: LoggerFactory, accessTokenProvider: null, httpClient: null); await webSocketsTransport.StartAsync(new Uri(server.WebSocketsUrl + "/echo"), TransferFormat.Binary); webSocketsTransport.Output.Complete(); @@ -175,7 +175,7 @@ public async Task WebSocketsTransportStopsWhenConnectionClosedByTheServer(Transf { await using (var server = await StartServer()) { - var webSocketsTransport = new WebSocketsTransport(httpConnectionOptions: null, loggerFactory: LoggerFactory, accessTokenProvider: null); + var webSocketsTransport = new WebSocketsTransport(httpConnectionOptions: null, loggerFactory: LoggerFactory, accessTokenProvider: null, httpClient: null); await webSocketsTransport.StartAsync(new Uri(server.WebSocketsUrl + "/echoAndClose"), transferFormat); await webSocketsTransport.Output.WriteAsync(new byte[] { 0x42 }); @@ -197,7 +197,7 @@ public async Task WebSocketsTransportSetsTransferFormat(TransferFormat transferF { await using (var server = await StartServer()) { - var webSocketsTransport = new WebSocketsTransport(httpConnectionOptions: null, loggerFactory: LoggerFactory, accessTokenProvider: null); + var webSocketsTransport = new WebSocketsTransport(httpConnectionOptions: null, loggerFactory: LoggerFactory, accessTokenProvider: null, httpClient: null); await webSocketsTransport.StartAsync(new Uri(server.WebSocketsUrl + "/echo"), transferFormat).DefaultTimeout(); @@ -215,7 +215,7 @@ public async Task WebSocketsTransportThrowsForInvalidTransferFormat(TransferForm { using (StartVerifiableLog()) { - var webSocketsTransport = new WebSocketsTransport(httpConnectionOptions: null, LoggerFactory, accessTokenProvider: null); + var webSocketsTransport = new WebSocketsTransport(httpConnectionOptions: null, LoggerFactory, accessTokenProvider: null, httpClient: null); var exception = await Assert.ThrowsAsync(() => webSocketsTransport.StartAsync(new Uri("http://fakeuri.org"), transferFormat)); From 21b5175cda2f212e1384d369d91d27684bcdb17a Mon Sep 17 00:00:00 2001 From: Brennan Conroy Date: Mon, 15 Aug 2022 17:58:36 -0700 Subject: [PATCH 2/7] progress --- src/Shared/SignalR/FunctionalTestBase.cs | 5 +- src/Shared/SignalR/InProcessTestServer.cs | 13 ++-- .../FunctionalTests/HubConnectionTests.cs | 59 +++++++++++++++++-- .../src/HttpConnection.cs | 5 ++ .../Internal/AccessTokenHttpMessageHandler.cs | 25 ++++++++ .../src/Internal/SendUtils.cs | 5 ++ .../src/Internal/WebSocketsTransport.cs | 22 +++++-- 7 files changed, 116 insertions(+), 18 deletions(-) diff --git a/src/Shared/SignalR/FunctionalTestBase.cs b/src/Shared/SignalR/FunctionalTestBase.cs index 6b3f99125484..fe96a5368762 100644 --- a/src/Shared/SignalR/FunctionalTestBase.cs +++ b/src/Shared/SignalR/FunctionalTestBase.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Threading.Tasks; +using Microsoft.AspNetCore.Server.Kestrel.Core; using Microsoft.Extensions.Logging.Testing; namespace Microsoft.AspNetCore.SignalR.Tests; @@ -36,9 +37,9 @@ private Func ResolveExpectedErrorsFilter(Func> StartServer(Func expectedErrorsFilter = null) where T : class + public Task> StartServer(Func expectedErrorsFilter = null, Action configureKestrelServerOptions = null) where T : class { var disposable = base.StartVerifiableLog(ResolveExpectedErrorsFilter(expectedErrorsFilter)); - return InProcessTestServer.StartServer(LoggerFactory, disposable); + return InProcessTestServer.StartServer(LoggerFactory, configureKestrelServerOptions, disposable); } } diff --git a/src/Shared/SignalR/InProcessTestServer.cs b/src/Shared/SignalR/InProcessTestServer.cs index 296d72640ee5..1dd6ccfdb236 100644 --- a/src/Shared/SignalR/InProcessTestServer.cs +++ b/src/Shared/SignalR/InProcessTestServer.cs @@ -11,6 +11,7 @@ using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Hosting.Server; using Microsoft.AspNetCore.Hosting.Server.Features; +using Microsoft.AspNetCore.Server.Kestrel.Core; using Microsoft.AspNetCore.Testing; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Hosting; @@ -38,6 +39,7 @@ public class InProcessTestServer : InProcessTestServer private IHostApplicationLifetime _lifetime; private readonly IDisposable _logToken; private readonly IDisposable _extraDisposable; + private readonly Action _configureKestrelServerOptions; private readonly LogSinkProvider _logSinkProvider; private string _url; @@ -52,19 +54,20 @@ internal override event Action ServerLogged public override string Url => _url; - public static async Task> StartServer(ILoggerFactory loggerFactory, IDisposable disposable = null) + public static async Task> StartServer(ILoggerFactory loggerFactory, Action configureKestrelServerOptions = null, IDisposable disposable = null) { - var server = new InProcessTestServer(loggerFactory, disposable); + var server = new InProcessTestServer(loggerFactory, configureKestrelServerOptions, disposable); await server.StartServerInner(); return server; } - private InProcessTestServer() : this(loggerFactory: null, null) + private InProcessTestServer() : this(loggerFactory: null, null, null) { } - private InProcessTestServer(ILoggerFactory loggerFactory, IDisposable disposable) + private InProcessTestServer(ILoggerFactory loggerFactory, Action configureKestrelServerOptions, IDisposable disposable) { + _configureKestrelServerOptions = configureKestrelServerOptions; _extraDisposable = disposable; _logSinkProvider = new LogSinkProvider(); @@ -99,7 +102,7 @@ private async Task StartServerInner() .SetMinimumLevel(LogLevel.Trace) .AddProvider(new ForwardingLoggerProvider(_loggerFactory))) .UseStartup(typeof(TStartup)) - .UseKestrel() + .UseKestrel(o => _configureKestrelServerOptions?.Invoke(o)) .UseUrls(url) .UseContentRoot(Directory.GetCurrentDirectory()); }).Build(); diff --git a/src/SignalR/clients/csharp/Client/test/FunctionalTests/HubConnectionTests.cs b/src/SignalR/clients/csharp/Client/test/FunctionalTests/HubConnectionTests.cs index 59bb41e5d3f6..50bbab657ac0 100644 --- a/src/SignalR/clients/csharp/Client/test/FunctionalTests/HubConnectionTests.cs +++ b/src/SignalR/clients/csharp/Client/test/FunctionalTests/HubConnectionTests.cs @@ -1,16 +1,12 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System; -using System.Collections.Generic; -using System.Linq; using System.Net; using System.Net.Http; using System.Text.Json; -using System.Threading; using System.Threading.Channels; -using System.Threading.Tasks; using Microsoft.AspNetCore.Connections; +using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Http.Connections; using Microsoft.AspNetCore.Http.Connections.Client; using Microsoft.AspNetCore.SignalR.Protocol; @@ -19,7 +15,6 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Testing; -using Xunit; namespace Microsoft.AspNetCore.SignalR.Client.FunctionalTests; @@ -1736,6 +1731,58 @@ public async Task WebSocketOptionsAreApplied() } } + [ConditionalFact] + [WebSocketsSupportedCondition] + public async Task WebSocketsCanConnectOverHttp2() + { + await using (var server = await StartServer(configureKestrelServerOptions: o => + { + o.ConfigureEndpointDefaults(o2 => + { + o2.Protocols = Server.Kestrel.Core.HttpProtocols.Http2; + o2.UseHttps(); + }); + })) + { + var hubConnection = new HubConnectionBuilder() + .WithLoggerFactory(LoggerFactory) + .WithUrl(server.Url + "/default", HttpTransportType.WebSockets, options => + { + options.HttpMessageHandlerFactory = h => + { + ((HttpClientHandler)h).ServerCertificateCustomValidationCallback = (_, _, _, _) => true; + return h; + }; + options.WebSocketConfiguration = o => + { + o.HttpVersion = HttpVersion.Version20; + o.HttpVersionPolicy = HttpVersionPolicy.RequestVersionExact; + }; + }) + .Build(); + try + { + await hubConnection.StartAsync().DefaultTimeout(); + var echoResponse = await hubConnection.InvokeAsync(nameof(TestHub.Echo), "Foo").DefaultTimeout(); + Assert.Equal("Foo", echoResponse); + } + catch (Exception ex) + { + LoggerFactory.CreateLogger().LogError(ex, "{ExceptionType} from test", ex.GetType().FullName); + throw; + } + finally + { + await hubConnection.DisposeAsync().DefaultTimeout(); + } + } + + // Triple check that the WebSocket ran over HTTP/2, also verify the negotiate was HTTP/2 + Assert.Contains(TestSink.Writes, context => context.Message.Contains("Request starting HTTP/2 POST")); + Assert.Contains(TestSink.Writes, context => context.Message.Contains("Request starting HTTP/2 CONNECT")); + Assert.Contains(TestSink.Writes, context => context.Message.Contains("Request finished HTTP/2 CONNECT")); + } + [ConditionalFact] [WebSocketsSupportedCondition] public async Task CookiesFromNegotiateAreAppliedToWebSockets() diff --git a/src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnection.cs b/src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnection.cs index 0aac5de51fdc..d430cc2dda9c 100644 --- a/src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnection.cs +++ b/src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnection.cs @@ -6,6 +6,7 @@ using System.Diagnostics.CodeAnalysis; using System.IO.Pipelines; using System.Linq; +using System.Net; using System.Net.Http; using System.Net.WebSockets; using System.Threading; @@ -461,6 +462,10 @@ private async Task NegotiateAsync(Uri url, HttpClient httpC using (var request = new HttpRequestMessage(HttpMethod.Post, uri)) { +#if NETSTANDARD2_1_OR_GREATER || NET7_0_OR_GREATER + // HttpClient gracefully falls back to HTTP/1.1, so it's fine to set the preferred version to a higher version + request.Version = HttpVersion.Version20; +#endif #if NET5_0_OR_GREATER request.Options.Set(new HttpRequestOptionsKey("IsNegotiate"), true); #else diff --git a/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/AccessTokenHttpMessageHandler.cs b/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/AccessTokenHttpMessageHandler.cs index eca0aed7893d..86122c6844ff 100644 --- a/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/AccessTokenHttpMessageHandler.cs +++ b/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/AccessTokenHttpMessageHandler.cs @@ -1,9 +1,12 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System; using System.Net; using System.Net.Http; using System.Net.Http.Headers; +using System.Net.WebSockets; +using System.Text.Encodings.Web; using System.Threading; using System.Threading.Tasks; @@ -21,6 +24,12 @@ public AccessTokenHttpMessageHandler(HttpMessageHandler inner, HttpConnection ht protected override async Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) { + var isWebSocket = false; + if (request.RequestUri?.Scheme.StartsWith("ws", System.StringComparison.InvariantCultureIgnoreCase) is true) + { + isWebSocket = true; + } + var isNegotiate = false; if (string.IsNullOrEmpty(_accessToken) || // Negotiate redirects likely will have a new access token so let's always grab a (potentially) new access token on negotiate @@ -37,6 +46,22 @@ protected override async Task SendAsync(HttpRequestMessage if (!string.IsNullOrEmpty(_accessToken)) { + if (isWebSocket) + { + // We can't use request headers in the browser, so instead append the token as a query string in that case + if (OperatingSystem.IsBrowser()) + { + var accessTokenEncoded = UrlEncoder.Default.Encode(_accessToken); + accessTokenEncoded = "access_token=" + accessTokenEncoded; + request.RequestUri = Utils.AppendQueryString(request.RequestUri!, accessTokenEncoded); + } + else + { +#pragma warning disable CA1416 // Analyzer bug + request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", _accessToken); +#pragma warning restore CA1416 // Analyzer bug + } + } request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", _accessToken); } diff --git a/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/SendUtils.cs b/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/SendUtils.cs index 2ad92f7d279f..eae54a7550b4 100644 --- a/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/SendUtils.cs +++ b/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/SendUtils.cs @@ -41,6 +41,11 @@ public static async Task SendMessages(Uri sendUrl, IDuplexPipe application, Http // Send them in a single post var request = new HttpRequestMessage(HttpMethod.Post, sendUrl); +#if NETSTANDARD2_1_OR_GREATER || NET7_0_OR_GREATER + // HttpClient gracefully falls back to HTTP/1.1, so it's fine to set the preferred version to a higher version + request.Version = HttpVersion.Version20; +#endif + request.Content = new ReadOnlySequenceContent(buffer); // ResponseHeadersRead instructs SendAsync to return once headers are read diff --git a/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/WebSocketsTransport.cs b/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/WebSocketsTransport.cs index 0a11be4b3985..353ea8d01a2f 100644 --- a/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/WebSocketsTransport.cs +++ b/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/WebSocketsTransport.cs @@ -4,6 +4,7 @@ using System; using System.Diagnostics; using System.IO.Pipelines; +using System.Net; using System.Net.Http; using System.Net.WebSockets; using System.Runtime.InteropServices; @@ -119,7 +120,11 @@ private async ValueTask DefaultWebSocketFactory(WebSocketConnectionCo } } - if (_httpConnectionOptions.AccessTokenProvider != null) + if (_httpConnectionOptions.AccessTokenProvider != null +#if NET7_0_OR_GREATER + && webSocket.Options.HttpVersion < HttpVersion.Version20 +#endif + ) { var accessToken = await _httpConnectionOptions.AccessTokenProvider().ConfigureAwait(false); if (!string.IsNullOrWhiteSpace(accessToken)) @@ -143,11 +148,18 @@ private async ValueTask DefaultWebSocketFactory(WebSocketConnectionCo try { #if NET7_0_OR_GREATER - // TODO: Pass _httpClient here once https://github.com/dotnet/runtime/issues/72476 and https://github.com/dotnet/runtime/issues/72301 are resolved - await webSocket.ConnectAsync(url, invoker: null, cancellationToken).ConfigureAwait(false); -#else - await webSocket.ConnectAsync(url, cancellationToken).ConfigureAwait(false); + // Only share the HttpClient if the user opts-in to HTTP/2 (or higher) + // This is because there is some non-obvious behavior changes when passing in an invoker to ConnectAsync + // and there isn't really any benefit to sharing the HttpClient in HTTP/1.1 + if (webSocket.Options.HttpVersion > HttpVersion.Version11) + { + await webSocket.ConnectAsync(url, invoker: _httpClient, cancellationToken).ConfigureAwait(false); + } + else #endif + { + await webSocket.ConnectAsync(url, cancellationToken).ConfigureAwait(false); + } } catch { From 6a0119db4e934fcad98ba6c00b4b978538ce2a29 Mon Sep 17 00:00:00 2001 From: Brennan Conroy Date: Tue, 16 Aug 2022 14:43:59 -0700 Subject: [PATCH 3/7] cleanup --- .../FunctionalTests/HubConnectionTests.cs | 61 +++++++++++++++++++ .../src/HttpConnectionOptions.cs | 2 + .../Internal/AccessTokenHttpMessageHandler.cs | 52 +++++----------- .../src/Internal/WebSocketsTransport.cs | 3 +- 4 files changed, 80 insertions(+), 38 deletions(-) diff --git a/src/SignalR/clients/csharp/Client/test/FunctionalTests/HubConnectionTests.cs b/src/SignalR/clients/csharp/Client/test/FunctionalTests/HubConnectionTests.cs index 50bbab657ac0..ab9a2162a86a 100644 --- a/src/SignalR/clients/csharp/Client/test/FunctionalTests/HubConnectionTests.cs +++ b/src/SignalR/clients/csharp/Client/test/FunctionalTests/HubConnectionTests.cs @@ -1783,6 +1783,67 @@ public async Task WebSocketsCanConnectOverHttp2() Assert.Contains(TestSink.Writes, context => context.Message.Contains("Request finished HTTP/2 CONNECT")); } + [ConditionalFact] + [WebSocketsSupportedCondition] + public async Task WebSocketsWithAccessTokenOverHttp2() + { + var accessTokenCallCount = 0; + await using (var server = await StartServer(configureKestrelServerOptions: o => + { + o.ConfigureEndpointDefaults(o2 => + { + o2.Protocols = Server.Kestrel.Core.HttpProtocols.Http2; + o2.UseHttps(); + }); + })) + { + var hubConnection = new HubConnectionBuilder() + .WithLoggerFactory(LoggerFactory) + .WithUrl(server.Url + "/default", HttpTransportType.WebSockets, options => + { + options.HttpMessageHandlerFactory = h => + { + ((HttpClientHandler)h).ServerCertificateCustomValidationCallback = (_, _, _, _) => true; + return h; + }; + options.WebSocketConfiguration = o => + { + o.HttpVersion = HttpVersion.Version20; + o.HttpVersionPolicy = HttpVersionPolicy.RequestVersionExact; + }; + options.AccessTokenProvider = () => + { + accessTokenCallCount++; + return Task.FromResult("test"); + }; + }) + .Build(); + try + { + await hubConnection.StartAsync().DefaultTimeout(); + var headerResponse = await hubConnection.InvokeAsync(nameof(TestHub.GetHeaderValues), new string[] { "Authorization" }).DefaultTimeout(); + Assert.Single(headerResponse); + Assert.Equal("Bearer test", headerResponse[0]); + } + catch (Exception ex) + { + LoggerFactory.CreateLogger().LogError(ex, "{ExceptionType} from test", ex.GetType().FullName); + throw; + } + finally + { + await hubConnection.DisposeAsync().DefaultTimeout(); + } + } + + Assert.Equal(1, accessTokenCallCount); + + // Triple check that the WebSocket ran over HTTP/2, also verify the negotiate was HTTP/2 + Assert.Contains(TestSink.Writes, context => context.Message.Contains("Request starting HTTP/2 POST")); + Assert.Contains(TestSink.Writes, context => context.Message.Contains("Request starting HTTP/2 CONNECT")); + Assert.Contains(TestSink.Writes, context => context.Message.Contains("Request finished HTTP/2 CONNECT")); + } + [ConditionalFact] [WebSocketsSupportedCondition] public async Task CookiesFromNegotiateAreAppliedToWebSockets() diff --git a/src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnectionOptions.cs b/src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnectionOptions.cs index 73156af340a0..c81d15186c6b 100644 --- a/src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnectionOptions.cs +++ b/src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnectionOptions.cs @@ -262,6 +262,8 @@ public bool? UseDefaultCredentials /// /// This delegate is invoked after headers from and the access token from /// has been applied. + /// + /// If ClientWebSocketOptions.HttpVersion is set to 2.0 or higher, some options like will not be applied. Instead use or the corresponding option on . /// [UnsupportedOSPlatform("browser")] public Action? WebSocketConfiguration diff --git a/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/AccessTokenHttpMessageHandler.cs b/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/AccessTokenHttpMessageHandler.cs index 86122c6844ff..329cf0780c36 100644 --- a/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/AccessTokenHttpMessageHandler.cs +++ b/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/AccessTokenHttpMessageHandler.cs @@ -1,12 +1,9 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System; using System.Net; using System.Net.Http; using System.Net.Http.Headers; -using System.Net.WebSockets; -using System.Text.Encodings.Web; using System.Threading; using System.Threading.Tasks; @@ -24,13 +21,7 @@ public AccessTokenHttpMessageHandler(HttpMessageHandler inner, HttpConnection ht protected override async Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) { - var isWebSocket = false; - if (request.RequestUri?.Scheme.StartsWith("ws", System.StringComparison.InvariantCultureIgnoreCase) is true) - { - isWebSocket = true; - } - - var isNegotiate = false; + var shouldRetry = true; if (string.IsNullOrEmpty(_accessToken) || // Negotiate redirects likely will have a new access token so let's always grab a (potentially) new access token on negotiate #if NET5_0_OR_GREATER @@ -40,47 +31,36 @@ protected override async Task SendAsync(HttpRequestMessage #endif ) { - isNegotiate = true; + shouldRetry = false; _accessToken = await _httpConnection.GetAccessTokenAsync().ConfigureAwait(false); } - if (!string.IsNullOrEmpty(_accessToken)) - { - if (isWebSocket) - { - // We can't use request headers in the browser, so instead append the token as a query string in that case - if (OperatingSystem.IsBrowser()) - { - var accessTokenEncoded = UrlEncoder.Default.Encode(_accessToken); - accessTokenEncoded = "access_token=" + accessTokenEncoded; - request.RequestUri = Utils.AppendQueryString(request.RequestUri!, accessTokenEncoded); - } - else - { -#pragma warning disable CA1416 // Analyzer bug - request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", _accessToken); -#pragma warning restore CA1416 // Analyzer bug - } - } - request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", _accessToken); - } + SetAccessToken(_accessToken, request); var result = await base.SendAsync(request, cancellationToken).ConfigureAwait(false); // retry once with a new token on auth failure - if (!isNegotiate && result.StatusCode is HttpStatusCode.Unauthorized) + if (shouldRetry && result.StatusCode is HttpStatusCode.Unauthorized) { HttpConnection.Log.RetryAccessToken(_httpConnection._logger, result.StatusCode); result.Dispose(); _accessToken = await _httpConnection.GetAccessTokenAsync().ConfigureAwait(false); - if (!string.IsNullOrEmpty(_accessToken)) - { - request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", _accessToken); - } + SetAccessToken(_accessToken, request); + // Retrying the request relies on any HttpContent being non-disposable. // Currently this is true, the only HttpContent we send is type ReadOnlySequenceContent which is used by SSE and LongPolling for sending an already buffered byte[] result = await base.SendAsync(request, cancellationToken).ConfigureAwait(false); } return result; } + + private static void SetAccessToken(string? accessToken, HttpRequestMessage request) + { + if (!string.IsNullOrEmpty(accessToken)) + { + // Don't need to worry about WebSockets and browser because this code path will not be hit in the browser case + // ClientWebSocketOptions.HttpVersion isn't settable in the browser + request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", accessToken); + } + } } diff --git a/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/WebSocketsTransport.cs b/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/WebSocketsTransport.cs index 353ea8d01a2f..2dd87407d632 100644 --- a/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/WebSocketsTransport.cs +++ b/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/WebSocketsTransport.cs @@ -126,6 +126,7 @@ private async ValueTask DefaultWebSocketFactory(WebSocketConnectionCo #endif ) { + // Apply access token logic when using HTTP/1.1 because we don't use the AccessTokenHttpMessageHandler via HttpClient unless the user specifies HTTP/2.0 or higher var accessToken = await _httpConnectionOptions.AccessTokenProvider().ConfigureAwait(false); if (!string.IsNullOrWhiteSpace(accessToken)) { @@ -138,9 +139,7 @@ private async ValueTask DefaultWebSocketFactory(WebSocketConnectionCo } else { -#pragma warning disable CA1416 // Analyzer bug webSocket.Options.SetRequestHeader("Authorization", $"Bearer {accessToken}"); -#pragma warning restore CA1416 // Analyzer bug } } } From f3be040522a5af87c98f75728ac24005d945e816 Mon Sep 17 00:00:00 2001 From: Brennan Conroy Date: Thu, 18 Aug 2022 10:37:34 -0700 Subject: [PATCH 4/7] test cert --- .../FunctionalTests/HubConnectionTests.cs | 9 ++++ ...Core.SignalR.Client.FunctionalTests.csproj | 5 ++ .../clients/ts/FunctionalTests/Program.cs | 34 +------------ .../SignalR.Client.FunctionalTestApp.csproj | 5 ++ src/SignalR/common/Shared/TestCertificates.cs | 45 ++++++++++++++++++ .../Shared}/testCert.pfx | Bin .../Shared}/testCertECC.pfx | Bin 7 files changed, 66 insertions(+), 32 deletions(-) create mode 100644 src/SignalR/common/Shared/TestCertificates.cs rename src/SignalR/{clients/ts/FunctionalTests => common/Shared}/testCert.pfx (100%) rename src/SignalR/{clients/ts/FunctionalTests => common/Shared}/testCertECC.pfx (100%) diff --git a/src/SignalR/clients/csharp/Client/test/FunctionalTests/HubConnectionTests.cs b/src/SignalR/clients/csharp/Client/test/FunctionalTests/HubConnectionTests.cs index ab9a2162a86a..86caa9e02693 100644 --- a/src/SignalR/clients/csharp/Client/test/FunctionalTests/HubConnectionTests.cs +++ b/src/SignalR/clients/csharp/Client/test/FunctionalTests/HubConnectionTests.cs @@ -10,6 +10,7 @@ using Microsoft.AspNetCore.Http.Connections; using Microsoft.AspNetCore.Http.Connections.Client; using Microsoft.AspNetCore.SignalR.Protocol; +using Microsoft.AspNetCore.SignalR.Test.Internal; using Microsoft.AspNetCore.SignalR.Tests; using Microsoft.AspNetCore.Testing; using Microsoft.Extensions.DependencyInjection; @@ -1742,6 +1743,10 @@ public async Task WebSocketsCanConnectOverHttp2() o2.Protocols = Server.Kestrel.Core.HttpProtocols.Http2; o2.UseHttps(); }); + o.ConfigureHttpsDefaults(httpsOptions => + { + httpsOptions.ServerCertificate = TestCertificateHelper.GetTestCert(); + }); })) { var hubConnection = new HubConnectionBuilder() @@ -1795,6 +1800,10 @@ public async Task WebSocketsWithAccessTokenOverHttp2() o2.Protocols = Server.Kestrel.Core.HttpProtocols.Http2; o2.UseHttps(); }); + o.ConfigureHttpsDefaults(httpsOptions => + { + httpsOptions.ServerCertificate = TestCertificateHelper.GetTestCert(); + }); })) { var hubConnection = new HubConnectionBuilder() diff --git a/src/SignalR/clients/csharp/Client/test/FunctionalTests/Microsoft.AspNetCore.SignalR.Client.FunctionalTests.csproj b/src/SignalR/clients/csharp/Client/test/FunctionalTests/Microsoft.AspNetCore.SignalR.Client.FunctionalTests.csproj index 603ed72970c0..558f4c5a7522 100644 --- a/src/SignalR/clients/csharp/Client/test/FunctionalTests/Microsoft.AspNetCore.SignalR.Client.FunctionalTests.csproj +++ b/src/SignalR/clients/csharp/Client/test/FunctionalTests/Microsoft.AspNetCore.SignalR.Client.FunctionalTests.csproj @@ -14,4 +14,9 @@ + + + + + diff --git a/src/SignalR/clients/ts/FunctionalTests/Program.cs b/src/SignalR/clients/ts/FunctionalTests/Program.cs index b7aace408507..11e3a84f6c14 100644 --- a/src/SignalR/clients/ts/FunctionalTests/Program.cs +++ b/src/SignalR/clients/ts/FunctionalTests/Program.cs @@ -1,8 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System.Security.Cryptography.X509Certificates; -using Microsoft.Win32; +using Microsoft.AspNetCore.SignalR.Test.Internal; namespace FunctionalTests; @@ -41,36 +40,7 @@ public static Task Main(string[] args) { options.ConfigureHttpsDefaults(httpsOptions => { - bool useRSA = false; - if (OperatingSystem.IsWindows()) - { - // Detect Win10+ - var key = Registry.LocalMachine.OpenSubKey(@"SOFTWARE\Microsoft\Windows NT\CurrentVersion"); - var major = key.GetValue("CurrentMajorVersionNumber") as int?; - var minor = key.GetValue("CurrentMinorVersionNumber") as int?; - - if (major.HasValue && minor.HasValue) - { - useRSA = true; - } - } - else - { - useRSA = true; - } - - if (useRSA) - { - // RSA cert, won't work on Windows 8.1 & Windows 2012 R2 using HTTP2, and ECC won't work in some Node environments - var certPath = Path.Combine(Directory.GetCurrentDirectory(), "testCert.pfx"); - httpsOptions.ServerCertificate = new X509Certificate2(certPath, "testPassword"); - } - else - { - // ECC cert, works on Windows 8.1 & Windows 2012 R2 using HTTP2 - var certPath = Path.Combine(Directory.GetCurrentDirectory(), "testCertECC.pfx"); - httpsOptions.ServerCertificate = new X509Certificate2(certPath, "testPassword"); - } + httpsOptions.ServerCertificate = TestCertificateHelper.GetTestCert(); }); }) .UseContentRoot(Directory.GetCurrentDirectory()) diff --git a/src/SignalR/clients/ts/FunctionalTests/SignalR.Client.FunctionalTestApp.csproj b/src/SignalR/clients/ts/FunctionalTests/SignalR.Client.FunctionalTestApp.csproj index 52d66ace46fb..39769bda9741 100644 --- a/src/SignalR/clients/ts/FunctionalTests/SignalR.Client.FunctionalTestApp.csproj +++ b/src/SignalR/clients/ts/FunctionalTests/SignalR.Client.FunctionalTestApp.csproj @@ -36,6 +36,11 @@ + + + + + diff --git a/src/SignalR/common/Shared/TestCertificates.cs b/src/SignalR/common/Shared/TestCertificates.cs new file mode 100644 index 000000000000..b452291f82ef --- /dev/null +++ b/src/SignalR/common/Shared/TestCertificates.cs @@ -0,0 +1,45 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Reflection; +using System.Security.Cryptography.X509Certificates; +using Microsoft.Win32; + +namespace Microsoft.AspNetCore.SignalR.Test.Internal; + +internal static class TestCertificateHelper +{ + internal static X509Certificate2 GetTestCert() + { + bool useRSA = false; + if (OperatingSystem.IsWindows()) + { + // Detect Win10+ + var key = Registry.LocalMachine.OpenSubKey(@"SOFTWARE\Microsoft\Windows NT\CurrentVersion"); + var major = key.GetValue("CurrentMajorVersionNumber") as int?; + var minor = key.GetValue("CurrentMinorVersionNumber") as int?; + + if (major.HasValue && minor.HasValue) + { + useRSA = true; + } + } + else + { + useRSA = true; + } + + if (useRSA) + { + // RSA cert, won't work on Windows 8.1 & Windows 2012 R2 using HTTP2, and ECC won't work in some Node environments + var certPath = Path.Combine(Path.GetDirectoryName(Assembly.GetCallingAssembly().Location), "TestCertificates", "testCert.pfx"); + return new X509Certificate2(certPath, "testPassword"); + } + else + { + // ECC cert, works on Windows 8.1 & Windows 2012 R2 using HTTP2 + var certPath = Path.Combine(Path.GetDirectoryName(Assembly.GetCallingAssembly().Location), "TestCertificates", "testCertECC.pfx"); + return new X509Certificate2(certPath, "testPassword"); + } + } +} diff --git a/src/SignalR/clients/ts/FunctionalTests/testCert.pfx b/src/SignalR/common/Shared/testCert.pfx similarity index 100% rename from src/SignalR/clients/ts/FunctionalTests/testCert.pfx rename to src/SignalR/common/Shared/testCert.pfx diff --git a/src/SignalR/clients/ts/FunctionalTests/testCertECC.pfx b/src/SignalR/common/Shared/testCertECC.pfx similarity index 100% rename from src/SignalR/clients/ts/FunctionalTests/testCertECC.pfx rename to src/SignalR/common/Shared/testCertECC.pfx From c3ebac334ca84b3445a2a280925191507cfd466f Mon Sep 17 00:00:00 2001 From: Brennan Conroy Date: Thu, 18 Aug 2022 20:16:55 -0700 Subject: [PATCH 5/7] skip on osx --- .../csharp/Client/test/FunctionalTests/HubConnectionTests.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/SignalR/clients/csharp/Client/test/FunctionalTests/HubConnectionTests.cs b/src/SignalR/clients/csharp/Client/test/FunctionalTests/HubConnectionTests.cs index 86caa9e02693..42f63a697615 100644 --- a/src/SignalR/clients/csharp/Client/test/FunctionalTests/HubConnectionTests.cs +++ b/src/SignalR/clients/csharp/Client/test/FunctionalTests/HubConnectionTests.cs @@ -1734,6 +1734,7 @@ public async Task WebSocketOptionsAreApplied() [ConditionalFact] [WebSocketsSupportedCondition] + [OSSkipCondition(OperatingSystems.MacOSX, SkipReason = "HTTP/2 over TLS is not supported on macOS due to missing ALPN support.")] public async Task WebSocketsCanConnectOverHttp2() { await using (var server = await StartServer(configureKestrelServerOptions: o => @@ -1790,6 +1791,7 @@ public async Task WebSocketsCanConnectOverHttp2() [ConditionalFact] [WebSocketsSupportedCondition] + [OSSkipCondition(OperatingSystems.MacOSX, SkipReason = "HTTP/2 over TLS is not supported on macOS due to missing ALPN support.")] public async Task WebSocketsWithAccessTokenOverHttp2() { var accessTokenCallCount = 0; From 2a6bd1870fe2193a0465b8f8b373ae56f1a56a25 Mon Sep 17 00:00:00 2001 From: Brennan Conroy Date: Fri, 19 Aug 2022 14:23:11 -0700 Subject: [PATCH 6/7] default http/2 for LP and SSE --- .../src/Internal/LongPollingTransport.cs | 7 ++++++- .../src/Internal/ServerSentEventsTransport.cs | 8 +++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/LongPollingTransport.cs b/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/LongPollingTransport.cs index adbb15d2e4f0..27e13a6a4c58 100644 --- a/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/LongPollingTransport.cs +++ b/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/LongPollingTransport.cs @@ -148,7 +148,12 @@ private async Task Poll(Uri pollUrl, CancellationToken cancellationToken) { while (!cancellationToken.IsCancellationRequested) { - var request = new HttpRequestMessage(HttpMethod.Get, pollUrl); + var request = new HttpRequestMessage(HttpMethod.Get, pollUrl) + { +#if NETSTANDARD2_1_OR_GREATER || NETCOREAPP + Version = HttpVersion.Version20, +#endif + }; HttpResponseMessage response; diff --git a/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/ServerSentEventsTransport.cs b/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/ServerSentEventsTransport.cs index 33d09e1ca12d..1efb5f7349db 100644 --- a/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/ServerSentEventsTransport.cs +++ b/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/ServerSentEventsTransport.cs @@ -4,6 +4,7 @@ using System; using System.Diagnostics; using System.IO.Pipelines; +using System.Net; using System.Net.Http; using System.Net.Http.Headers; using System.Threading; @@ -54,7 +55,12 @@ public async Task StartAsync(Uri url, TransferFormat transferFormat, Cancellatio Log.StartTransport(_logger, transferFormat); - var request = new HttpRequestMessage(HttpMethod.Get, url); + var request = new HttpRequestMessage(HttpMethod.Get, url) + { +#if NETSTANDARD2_1_OR_GREATER || NETCOREAPP + Version = HttpVersion.Version20, +#endif + }; request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("text/event-stream")); HttpResponseMessage? response = null; From 546b414e19a15d8fbed0f01ec5c46823111ac2ef Mon Sep 17 00:00:00 2001 From: Brennan Conroy Date: Fri, 19 Aug 2022 16:25:06 -0700 Subject: [PATCH 7/7] smoke test other transports --- .../FunctionalTests/HubConnectionTests.cs | 193 ++++++++++++++++++ .../Client/test/FunctionalTests/Hubs.cs | 5 + .../src/Internal/LongPollingTransport.cs | 15 +- 3 files changed, 211 insertions(+), 2 deletions(-) diff --git a/src/SignalR/clients/csharp/Client/test/FunctionalTests/HubConnectionTests.cs b/src/SignalR/clients/csharp/Client/test/FunctionalTests/HubConnectionTests.cs index 42f63a697615..d61f72b72bc4 100644 --- a/src/SignalR/clients/csharp/Client/test/FunctionalTests/HubConnectionTests.cs +++ b/src/SignalR/clients/csharp/Client/test/FunctionalTests/HubConnectionTests.cs @@ -2246,6 +2246,199 @@ public async Task CanBlockOnAsyncOperationsWithOneAtATimeSynchronizationContext( } } + [ConditionalFact] + [OSSkipCondition(OperatingSystems.MacOSX, SkipReason = "HTTP/2 over TLS is not supported on macOS due to missing ALPN support.")] + public async Task LongPollingUsesHttp2ByDefault() + { + await using (var server = await StartServer(configureKestrelServerOptions: o => + { + o.ConfigureEndpointDefaults(o2 => + { + o2.Protocols = Server.Kestrel.Core.HttpProtocols.Http1AndHttp2; + o2.UseHttps(); + }); + o.ConfigureHttpsDefaults(httpsOptions => + { + httpsOptions.ServerCertificate = TestCertificateHelper.GetTestCert(); + }); + })) + { + var hubConnection = new HubConnectionBuilder() + .WithLoggerFactory(LoggerFactory) + .WithUrl(server.Url + "/default", HttpTransportType.LongPolling, o => o.HttpMessageHandlerFactory = h => + { + ((HttpClientHandler)h).ServerCertificateCustomValidationCallback = (_, _, _, _) => true; + return h; + }) + .Build(); + try + { + await hubConnection.StartAsync().DefaultTimeout(); + var httpProtocol = await hubConnection.InvokeAsync(nameof(TestHub.GetHttpProtocol)).DefaultTimeout(); + + Assert.Equal("HTTP/2", httpProtocol); + } + catch (Exception ex) + { + LoggerFactory.CreateLogger().LogError(ex, "{ExceptionType} from test", ex.GetType().FullName); + throw; + } + finally + { + await hubConnection.DisposeAsync().DefaultTimeout(); + } + } + + // negotiate is HTTP2 + Assert.Contains(TestSink.Writes, context => context.Message.Contains("Request starting HTTP/2 POST") && context.Message.Contains("/negotiate?")); + + // LongPolling polls and sends are HTTP2 + Assert.Contains(TestSink.Writes, context => context.Message.Contains("Request starting HTTP/2 POST") && context.Message.Contains("?id=")); + Assert.Contains(TestSink.Writes, context => context.Message.Contains("Request finished HTTP/2 GET") && context.Message.Contains("?id=")); + + // LongPolling delete is HTTP2 + Assert.Contains(TestSink.Writes, context => context.Message.Contains("Request finished HTTP/2 DELETE") && context.Message.Contains("?id=")); + } + + [ConditionalFact] + [OSSkipCondition(OperatingSystems.MacOSX, SkipReason = "HTTP/2 over TLS is not supported on macOS due to missing ALPN support.")] + public async Task LongPollingWorksWithHttp2OnlyEndpoint() + { + await using (var server = await StartServer(configureKestrelServerOptions: o => + { + o.ConfigureEndpointDefaults(o2 => + { + o2.Protocols = Server.Kestrel.Core.HttpProtocols.Http2; + o2.UseHttps(); + }); + o.ConfigureHttpsDefaults(httpsOptions => + { + httpsOptions.ServerCertificate = TestCertificateHelper.GetTestCert(); + }); + })) + { + var hubConnection = new HubConnectionBuilder() + .WithLoggerFactory(LoggerFactory) + .WithUrl(server.Url + "/default", HttpTransportType.LongPolling, o => o.HttpMessageHandlerFactory = h => + { + ((HttpClientHandler)h).ServerCertificateCustomValidationCallback = (_, _, _, _) => true; + return h; + }) + .Build(); + try + { + await hubConnection.StartAsync().DefaultTimeout(); + var httpProtocol = await hubConnection.InvokeAsync(nameof(TestHub.GetHttpProtocol)).DefaultTimeout(); + + Assert.Equal("HTTP/2", httpProtocol); + } + catch (Exception ex) + { + LoggerFactory.CreateLogger().LogError(ex, "{ExceptionType} from test", ex.GetType().FullName); + throw; + } + finally + { + await hubConnection.DisposeAsync().DefaultTimeout(); + } + } + } + + [ConditionalFact] + [OSSkipCondition(OperatingSystems.MacOSX, SkipReason = "HTTP/2 over TLS is not supported on macOS due to missing ALPN support.")] + public async Task ServerSentEventsUsesHttp2ByDefault() + { + await using (var server = await StartServer(configureKestrelServerOptions: o => + { + o.ConfigureEndpointDefaults(o2 => + { + o2.Protocols = Server.Kestrel.Core.HttpProtocols.Http1AndHttp2; + o2.UseHttps(); + }); + o.ConfigureHttpsDefaults(httpsOptions => + { + httpsOptions.ServerCertificate = TestCertificateHelper.GetTestCert(); + }); + })) + { + var hubConnection = new HubConnectionBuilder() + .WithLoggerFactory(LoggerFactory) + .WithUrl(server.Url + "/default", HttpTransportType.ServerSentEvents, o => o.HttpMessageHandlerFactory = h => + { + ((HttpClientHandler)h).ServerCertificateCustomValidationCallback = (_, _, _, _) => true; + return h; + }) + .Build(); + try + { + await hubConnection.StartAsync().DefaultTimeout(); + var httpProtocol = await hubConnection.InvokeAsync(nameof(TestHub.GetHttpProtocol)).DefaultTimeout(); + + Assert.Equal("HTTP/2", httpProtocol); + } + catch (Exception ex) + { + LoggerFactory.CreateLogger().LogError(ex, "{ExceptionType} from test", ex.GetType().FullName); + throw; + } + finally + { + await hubConnection.DisposeAsync().DefaultTimeout(); + } + } + + // negotiate is HTTP2 + Assert.Contains(TestSink.Writes, context => context.Message.Contains("Request starting HTTP/2 POST") && context.Message.Contains("/negotiate?")); + + // ServerSentEvents eventsource and sendsos are HTTP2 + Assert.Contains(TestSink.Writes, context => context.Message.Contains("Request starting HTTP/2 POST") && context.Message.Contains("?id=")); + Assert.Contains(TestSink.Writes, context => context.Message.Contains("Request finished HTTP/2 GET") && context.Message.Contains("?id=")); + } + + [ConditionalFact] + [OSSkipCondition(OperatingSystems.MacOSX, SkipReason = "HTTP/2 over TLS is not supported on macOS due to missing ALPN support.")] + public async Task ServerSentEventsWorksWithHttp2OnlyEndpoint() + { + await using (var server = await StartServer(configureKestrelServerOptions: o => + { + o.ConfigureEndpointDefaults(o2 => + { + o2.Protocols = Server.Kestrel.Core.HttpProtocols.Http2; + o2.UseHttps(); + }); + o.ConfigureHttpsDefaults(httpsOptions => + { + httpsOptions.ServerCertificate = TestCertificateHelper.GetTestCert(); + }); + })) + { + var hubConnection = new HubConnectionBuilder() + .WithLoggerFactory(LoggerFactory) + .WithUrl(server.Url + "/default", HttpTransportType.ServerSentEvents, o => o.HttpMessageHandlerFactory = h => + { + ((HttpClientHandler)h).ServerCertificateCustomValidationCallback = (_, _, _, _) => true; + return h; + }) + .Build(); + try + { + await hubConnection.StartAsync().DefaultTimeout(); + var httpProtocol = await hubConnection.InvokeAsync(nameof(TestHub.GetHttpProtocol)).DefaultTimeout(); + + Assert.Equal("HTTP/2", httpProtocol); + } + catch (Exception ex) + { + LoggerFactory.CreateLogger().LogError(ex, "{ExceptionType} from test", ex.GetType().FullName); + throw; + } + finally + { + await hubConnection.DisposeAsync().DefaultTimeout(); + } + } + } + private class OneAtATimeSynchronizationContext : SynchronizationContext, IAsyncDisposable { private readonly Channel<(SendOrPostCallback, object)> _taskQueue = Channel.CreateUnbounded<(SendOrPostCallback, object)>(); diff --git a/src/SignalR/clients/csharp/Client/test/FunctionalTests/Hubs.cs b/src/SignalR/clients/csharp/Client/test/FunctionalTests/Hubs.cs index a58a49be2367..0b270d5e4a4b 100644 --- a/src/SignalR/clients/csharp/Client/test/FunctionalTests/Hubs.cs +++ b/src/SignalR/clients/csharp/Client/test/FunctionalTests/Hubs.cs @@ -96,6 +96,11 @@ public string GetActiveTransportName() return Context.Features.Get().TransportType.ToString(); } + public string GetHttpProtocol() + { + return Context.GetHttpContext()?.Request?.Protocol ?? "unknown"; + } + public async Task CallWithUnserializableObject() { await Clients.All.SendAsync("Foo", Unserializable.Create()); diff --git a/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/LongPollingTransport.cs b/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/LongPollingTransport.cs index 27e13a6a4c58..2a5ca460cf54 100644 --- a/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/LongPollingTransport.cs +++ b/src/SignalR/clients/csharp/Http.Connections.Client/src/Internal/LongPollingTransport.cs @@ -50,7 +50,12 @@ public async Task StartAsync(Uri url, TransferFormat transferFormat, Cancellatio // Make initial long polling request // Server uses first long polling request to finish initializing connection and it returns without data - var request = new HttpRequestMessage(HttpMethod.Get, url); + var request = new HttpRequestMessage(HttpMethod.Get, url) + { +#if NETSTANDARD2_1_OR_GREATER || NETCOREAPP + Version = HttpVersion.Version20, +#endif + }; using (var response = await _httpClient.SendAsync(request, cancellationToken).ConfigureAwait(false)) { response.EnsureSuccessStatusCode(); @@ -232,7 +237,13 @@ private async Task SendDeleteRequest(Uri url) try { Log.SendingDeleteRequest(_logger, url); - var response = await _httpClient.DeleteAsync(url).ConfigureAwait(false); + var request = new HttpRequestMessage(HttpMethod.Delete, url) + { +#if NETSTANDARD2_1_OR_GREATER || NETCOREAPP + Version = HttpVersion.Version20, +#endif + }; + var response = await _httpClient.SendAsync(request).ConfigureAwait(false); if (response.StatusCode == HttpStatusCode.NotFound) {