Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Better support for HTTP/2 in SignalR .NET client #42817

Merged
merged 7 commits into from
Aug 22, 2022
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/Shared/SignalR/FunctionalTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -36,9 +37,9 @@ private Func<WriteContext, bool> ResolveExpectedErrorsFilter(Func<WriteContext,
};
}

public Task<InProcessTestServer<T>> StartServer<T>(Func<WriteContext, bool> expectedErrorsFilter = null) where T : class
public Task<InProcessTestServer<T>> StartServer<T>(Func<WriteContext, bool> expectedErrorsFilter = null, Action<KestrelServerOptions> configureKestrelServerOptions = null) where T : class
{
var disposable = base.StartVerifiableLog(ResolveExpectedErrorsFilter(expectedErrorsFilter));
return InProcessTestServer<T>.StartServer(LoggerFactory, disposable);
return InProcessTestServer<T>.StartServer(LoggerFactory, configureKestrelServerOptions, disposable);
}
}
13 changes: 8 additions & 5 deletions src/Shared/SignalR/InProcessTestServer.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -38,6 +39,7 @@ public class InProcessTestServer<TStartup> : InProcessTestServer
private IHostApplicationLifetime _lifetime;
private readonly IDisposable _logToken;
private readonly IDisposable _extraDisposable;
private readonly Action<KestrelServerOptions> _configureKestrelServerOptions;

private readonly LogSinkProvider _logSinkProvider;
private string _url;
Expand All @@ -52,19 +54,20 @@ internal override event Action<LogRecord> ServerLogged

public override string Url => _url;

public static async Task<InProcessTestServer<TStartup>> StartServer(ILoggerFactory loggerFactory, IDisposable disposable = null)
public static async Task<InProcessTestServer<TStartup>> StartServer(ILoggerFactory loggerFactory, Action<KestrelServerOptions> configureKestrelServerOptions = null, IDisposable disposable = null)
{
var server = new InProcessTestServer<TStartup>(loggerFactory, disposable);
var server = new InProcessTestServer<TStartup>(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<KestrelServerOptions> configureKestrelServerOptions, IDisposable disposable)
{
_configureKestrelServerOptions = configureKestrelServerOptions;
_extraDisposable = disposable;
_logSinkProvider = new LogSinkProvider();

Expand Down Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,25 +1,21 @@
// 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;
using Microsoft.AspNetCore.SignalR.Test.Internal;
using Microsoft.AspNetCore.SignalR.Tests;
using Microsoft.AspNetCore.Testing;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Logging.Testing;
using Xunit;

namespace Microsoft.AspNetCore.SignalR.Client.FunctionalTests;

Expand Down Expand Up @@ -1736,6 +1732,129 @@ 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<Startup>(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.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<string>(nameof(TestHub.Echo), "Foo").DefaultTimeout();
Assert.Equal("Foo", echoResponse);
}
catch (Exception ex)
{
LoggerFactory.CreateLogger<HubConnectionTests>().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]
[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;
await using (var server = await StartServer<Startup>(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.WebSockets, options =>
{
options.HttpMessageHandlerFactory = h =>
{
((HttpClientHandler)h).ServerCertificateCustomValidationCallback = (_, _, _, _) => true;
return h;
};
options.WebSocketConfiguration = o =>
{
o.HttpVersion = HttpVersion.Version20;
o.HttpVersionPolicy = HttpVersionPolicy.RequestVersionExact;
halter73 marked this conversation as resolved.
Show resolved Hide resolved
};
options.AccessTokenProvider = () =>
{
accessTokenCallCount++;
return Task.FromResult("test");
};
})
.Build();
try
{
await hubConnection.StartAsync().DefaultTimeout();
var headerResponse = await hubConnection.InvokeAsync<string[]>(nameof(TestHub.GetHeaderValues), new string[] { "Authorization" }).DefaultTimeout();
Assert.Single(headerResponse);
Assert.Equal("Bearer test", headerResponse[0]);
}
catch (Exception ex)
{
LoggerFactory.CreateLogger<HubConnectionTests>().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()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,9 @@
<Reference Include="Microsoft.AspNetCore.TestHost" />
<Reference Include="System.Reactive.Linq" />
</ItemGroup>

<ItemGroup>
<Compile Include="$(SignalRSharedSourceRoot)TestCertificates.cs" Link="TestCertificates.cs" />
<Content Include="$(SignalRSharedSourceRoot)*.pfx" LinkBase="TestCertificates" CopyToOutputDirectory="PreserveNewest" />
</ItemGroup>
</Project>
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -461,9 +462,10 @@ private async Task<NegotiationResponse> 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 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;
halter73 marked this conversation as resolved.
Show resolved Hide resolved
#endif
#if NET5_0_OR_GREATER
request.Options.Set(new HttpRequestOptionsKey<bool>("IsNegotiate"), true);
#else
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,8 @@ public bool? UseDefaultCredentials
/// <remarks>
/// This delegate is invoked after headers from <see cref="Headers"/> and the access token from <see cref="AccessTokenProvider"/>
/// has been applied.
/// <para />
/// If <c>ClientWebSocketOptions.HttpVersion</c> is set to <c>2.0</c> or higher, some options like <see cref="ClientWebSocketOptions.Cookies"/> will not be applied. Instead use <see cref="Cookies"/> or the corresponding option on <see cref="HttpConnectionOptions"/>.
halter73 marked this conversation as resolved.
Show resolved Hide resolved
/// </remarks>
[UnsupportedOSPlatform("browser")]
public Action<ClientWebSocketOptions>? WebSocketConfiguration
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ public AccessTokenHttpMessageHandler(HttpMessageHandler inner, HttpConnection ht

protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
{
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
Expand All @@ -31,31 +31,36 @@ protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage
#endif
)
{
isNegotiate = true;
shouldRetry = false;
_accessToken = await _httpConnection.GetAccessTokenAsync().ConfigureAwait(false);
}

if (!string.IsNullOrEmpty(_accessToken))
{
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);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ protected override async Task<HttpResponseMessage> 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!);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,11 @@ 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);

#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);

Expand Down
Loading