From 3f84eee116b1e277347868d4ad5555c91ba55a9e Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Thu, 22 Mar 2018 12:35:31 +1300 Subject: [PATCH 1/3] Detect availability of web sockets on client and server (#1682) --- client-ts/signalr/spec/HttpConnection.spec.ts | 46 +++++++++++++++++++ client-ts/signalr/src/HttpConnection.ts | 9 +++- .../HttpConnection.Log.cs | 8 ++++ .../HttpConnection.cs | 29 ++++++++++++ .../HttpConnectionDispatcher.cs | 20 ++++++-- .../HttpConnectionDispatcherTests.cs | 26 +++++++++++ 6 files changed, 132 insertions(+), 6 deletions(-) diff --git a/client-ts/signalr/spec/HttpConnection.spec.ts b/client-ts/signalr/spec/HttpConnection.spec.ts index 5006bce951..1cd521798f 100644 --- a/client-ts/signalr/spec/HttpConnection.spec.ts +++ b/client-ts/signalr/spec/HttpConnection.spec.ts @@ -269,6 +269,52 @@ describe("HttpConnection", () => { } }); + it("does not select ServerSentEvents transport when not available in environment", async (done) => { + const serverSentEventsTransport = { transport: "ServerSentEvents", transferFormats: [ "Text" ] }; + + const options: IHttpConnectionOptions = { + ...commonOptions, + httpClient: new TestHttpClient() + .on("POST", (r) => ({ connectionId: "42", availableTransports: [serverSentEventsTransport] })), + } as IHttpConnectionOptions; + + const connection = new HttpConnection("http://tempuri.org", options); + + try { + await connection.start(TransferFormat.Text); + fail(); + done(); + } catch (e) { + // ServerSentEvents is only transport returned from server but is not selected + // because there is no support in the environment, leading to the following error + expect(e.message).toBe("Unable to initialize any of the available transports."); + done(); + } + }); + + it("does not select WebSockets transport when not available in environment", async (done) => { + const webSocketsTransport = { transport: "WebSockets", transferFormats: [ "Text" ] }; + + const options: IHttpConnectionOptions = { + ...commonOptions, + httpClient: new TestHttpClient() + .on("POST", (r) => ({ connectionId: "42", availableTransports: [webSocketsTransport] })), + } as IHttpConnectionOptions; + + const connection = new HttpConnection("http://tempuri.org", options); + + try { + await connection.start(TransferFormat.Text); + fail(); + done(); + } catch (e) { + // WebSockets is only transport returned from server but is not selected + // because there is no support in the environment, leading to the following error + expect(e.message).toBe("Unable to initialize any of the available transports."); + done(); + } + }); + describe(".constructor", () => { it("throws if no Url is provided", async () => { // Force TypeScript to let us call the constructor incorrectly :) diff --git a/client-ts/signalr/src/HttpConnection.ts b/client-ts/signalr/src/HttpConnection.ts index 974a4bdb08..614e51b416 100644 --- a/client-ts/signalr/src/HttpConnection.ts +++ b/client-ts/signalr/src/HttpConnection.ts @@ -160,8 +160,13 @@ export class HttpConnection implements IConnection { const transferFormats = endpoint.transferFormats.map((s) => TransferFormat[s]); if (!requestedTransport || transport === requestedTransport) { if (transferFormats.indexOf(requestedTransferFormat) >= 0) { - this.logger.log(LogLevel.Trace, `Selecting transport '${TransportType[transport]}'`); - return transport; + if ((transport === TransportType.WebSockets && typeof WebSocket === "undefined") || + (transport === TransportType.ServerSentEvents && typeof EventSource === "undefined")) { + this.logger.log(LogLevel.Trace, `Skipping transport '${TransportType[transport]}' because it is not supported in your environment.'`); + } else { + this.logger.log(LogLevel.Trace, `Selecting transport '${TransportType[transport]}'`); + return transport; + } } else { this.logger.log(LogLevel.Trace, `Skipping transport '${TransportType[transport]}' because it does not support the requested transfer format '${TransferFormat[requestedTransferFormat]}'.`); } diff --git a/src/Microsoft.AspNetCore.Sockets.Client.Http/HttpConnection.Log.cs b/src/Microsoft.AspNetCore.Sockets.Client.Http/HttpConnection.Log.cs index 3d02162943..00dc83ffdb 100644 --- a/src/Microsoft.AspNetCore.Sockets.Client.Http/HttpConnection.Log.cs +++ b/src/Microsoft.AspNetCore.Sockets.Client.Http/HttpConnection.Log.cs @@ -98,6 +98,9 @@ namespace Microsoft.AspNetCore.Sockets.Client private static readonly Action _transportFailed = LoggerMessage.Define(LogLevel.Debug, new EventId(29, "TransportFailed"), "Skipping transport {TransportName} because it failed to initialize."); + private static readonly Action _webSocketsNotSupportedByOperatingSystem = + LoggerMessage.Define(LogLevel.Debug, new EventId(30, "WebSocketsNotSupportedByOperatingSystem"), "Skipping WebSockets because they are not supported by the operating system."); + public static void HttpConnectionStarting(ILogger logger) { _httpConnectionStarting(logger, null); @@ -260,6 +263,11 @@ namespace Microsoft.AspNetCore.Sockets.Client _transportFailed(logger, transport.ToString(), ex); } } + + public static void WebSocketsNotSupportedByOperatingSystem(ILogger logger) + { + _webSocketsNotSupportedByOperatingSystem(logger, null); + } } } } diff --git a/src/Microsoft.AspNetCore.Sockets.Client.Http/HttpConnection.cs b/src/Microsoft.AspNetCore.Sockets.Client.Http/HttpConnection.cs index 227c0e619f..3adf223633 100644 --- a/src/Microsoft.AspNetCore.Sockets.Client.Http/HttpConnection.cs +++ b/src/Microsoft.AspNetCore.Sockets.Client.Http/HttpConnection.cs @@ -8,6 +8,7 @@ using System.IO; using System.IO.Pipelines; using System.Linq; using System.Net.Http; +using System.Runtime.InteropServices; using System.Threading; using System.Threading.Tasks; using Microsoft.AspNetCore.Http.Features; @@ -25,6 +26,7 @@ namespace Microsoft.AspNetCore.Sockets.Client public partial class HttpConnection : IConnection { private static readonly TimeSpan HttpClientTimeout = TimeSpan.FromSeconds(120); + private static readonly Version Windows8Version = new Version(6, 2); private readonly ILoggerFactory _loggerFactory; private readonly ILogger _logger; @@ -203,6 +205,7 @@ namespace Microsoft.AspNetCore.Sockets.Client var connectUrl = Url; if (_requestedTransportType == TransportType.WebSockets) { + // if we're running on Windows 7 this could throw because the OS does not support web sockets Log.StartingTransport(_logger, _requestedTransportType, connectUrl); await StartTransport(connectUrl, _requestedTransportType, transferFormat); } @@ -233,6 +236,12 @@ namespace Microsoft.AspNetCore.Sockets.Client continue; } + if (transportType == TransportType.WebSockets && !IsWebSocketsSupported()) + { + Log.WebSocketsNotSupportedByOperatingSystem(_logger); + continue; + } + try { if ((transportType & _requestedTransportType) == 0) @@ -711,6 +720,26 @@ namespace Microsoft.AspNetCore.Sockets.Client } } + private static bool IsWebSocketsSupported() + { +#if NETCOREAPP2_1 + // .NET Core 2.1 and above has a managed implementation + return true; +#else + bool isWindows = RuntimeInformation.IsOSPlatform(OSPlatform.Windows); + if (!isWindows) + { + // Assume other OSes have websockets + return true; + } + else + { + // Windows 8 and above has websockets + return Environment.OSVersion.Version >= Windows8Version; + } +#endif + } + // Internal because it's used by logging to avoid ToStringing prematurely. internal enum ConnectionState { diff --git a/src/Microsoft.AspNetCore.Sockets.Http/HttpConnectionDispatcher.cs b/src/Microsoft.AspNetCore.Sockets.Http/HttpConnectionDispatcher.cs index 25f239877a..f09cad93a2 100644 --- a/src/Microsoft.AspNetCore.Sockets.Http/HttpConnectionDispatcher.cs +++ b/src/Microsoft.AspNetCore.Sockets.Http/HttpConnectionDispatcher.cs @@ -368,7 +368,7 @@ namespace Microsoft.AspNetCore.Sockets logScope.ConnectionId = connection.ConnectionId; // Get the bytes for the connection id - var negotiateResponseBuffer = Encoding.UTF8.GetBytes(GetNegotiatePayload(connection.ConnectionId, options)); + var negotiateResponseBuffer = Encoding.UTF8.GetBytes(GetNegotiatePayload(connection.ConnectionId, context, options)); Log.NegotiationRequest(_logger); @@ -377,7 +377,7 @@ namespace Microsoft.AspNetCore.Sockets return context.Response.Body.WriteAsync(negotiateResponseBuffer, 0, negotiateResponseBuffer.Length); } - private static string GetNegotiatePayload(string connectionId, HttpSocketOptions options) + private static string GetNegotiatePayload(string connectionId, HttpContext context, HttpSocketOptions options) { var sb = new StringBuilder(); using (var jsonWriter = new JsonTextWriter(new StringWriter(sb))) @@ -387,18 +387,25 @@ namespace Microsoft.AspNetCore.Sockets jsonWriter.WriteValue(connectionId); jsonWriter.WritePropertyName("availableTransports"); jsonWriter.WriteStartArray(); - if ((options.Transports & TransportType.WebSockets) != 0) + + if (ServerHasWebSockets(context.Features)) { - WriteTransport(jsonWriter, nameof(TransportType.WebSockets), TransferFormat.Text | TransferFormat.Binary); + if ((options.Transports & TransportType.WebSockets) != 0) + { + WriteTransport(jsonWriter, nameof(TransportType.WebSockets), TransferFormat.Text | TransferFormat.Binary); + } } + if ((options.Transports & TransportType.ServerSentEvents) != 0) { WriteTransport(jsonWriter, nameof(TransportType.ServerSentEvents), TransferFormat.Text); } + if ((options.Transports & TransportType.LongPolling) != 0) { WriteTransport(jsonWriter, nameof(TransportType.LongPolling), TransferFormat.Text | TransferFormat.Binary); } + jsonWriter.WriteEndArray(); jsonWriter.WriteEndObject(); } @@ -406,6 +413,11 @@ namespace Microsoft.AspNetCore.Sockets return sb.ToString(); } + private static bool ServerHasWebSockets(IFeatureCollection features) + { + return features.Get() != null; + } + private static void WriteTransport(JsonWriter writer, string transportName, TransferFormat supportedTransferFormats) { writer.WriteStartObject(); diff --git a/test/Microsoft.AspNetCore.Sockets.Tests/HttpConnectionDispatcherTests.cs b/test/Microsoft.AspNetCore.Sockets.Tests/HttpConnectionDispatcherTests.cs index b129825ba4..c6cb016cb8 100644 --- a/test/Microsoft.AspNetCore.Sockets.Tests/HttpConnectionDispatcherTests.cs +++ b/test/Microsoft.AspNetCore.Sockets.Tests/HttpConnectionDispatcherTests.cs @@ -157,6 +157,7 @@ namespace Microsoft.AspNetCore.Sockets.Tests var dispatcher = new HttpConnectionDispatcher(manager, loggerFactory); var context = new DefaultHttpContext(); context.Features.Set(new ResponseFeature()); + context.Features.Set(new TestWebSocketConnectionFeature()); var services = new ServiceCollection(); services.AddSingleton(); services.AddOptions(); @@ -1225,6 +1226,31 @@ namespace Microsoft.AspNetCore.Sockets.Tests } } + [Fact] + public async Task NegotiateDoesNotReturnWebSocketsWhenNotAvailable() + { + using (StartLog(out var loggerFactory, LogLevel.Debug)) + { + var manager = CreateConnectionManager(loggerFactory); + var dispatcher = new HttpConnectionDispatcher(manager, loggerFactory); + var context = new DefaultHttpContext(); + context.Features.Set(new ResponseFeature()); + var services = new ServiceCollection(); + services.AddSingleton(); + services.AddOptions(); + var ms = new MemoryStream(); + context.Request.Path = "/foo"; + context.Request.Method = "POST"; + context.Response.Body = ms; + await dispatcher.ExecuteNegotiateAsync(context, new HttpSocketOptions { Transports = TransportType.WebSockets }); + + var negotiateResponse = JsonConvert.DeserializeObject(Encoding.UTF8.GetString(ms.ToArray())); + var availableTransports = (JArray)negotiateResponse["availableTransports"]; + + Assert.Empty(availableTransports); + } + } + private class RejectHandler : TestAuthenticationHandler { protected override bool ShouldAccept => false; From b111c91cb0c6e165b8e002dcbef500bd7c908afc Mon Sep 17 00:00:00 2001 From: David Fowler Date: Thu, 22 Mar 2018 08:47:06 -0700 Subject: [PATCH 2/3] Don't copy the array for incoming msgpack reads (#1686) * Don't copy the array for incoming msgpack reads - Don't use ToArray on the already sliced msgpack data. - Turns out msgpack is self describing enough to not require the count, it just needs the buffer and start offset. --- .../Internal/Protocol/MessagePackHubProtocol.cs | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.AspNetCore.SignalR.Protocols.MsgPack/Internal/Protocol/MessagePackHubProtocol.cs b/src/Microsoft.AspNetCore.SignalR.Protocols.MsgPack/Internal/Protocol/MessagePackHubProtocol.cs index 046df1cad1..8b01cbd3e3 100644 --- a/src/Microsoft.AspNetCore.SignalR.Protocols.MsgPack/Internal/Protocol/MessagePackHubProtocol.cs +++ b/src/Microsoft.AspNetCore.SignalR.Protocols.MsgPack/Internal/Protocol/MessagePackHubProtocol.cs @@ -3,8 +3,10 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.IO; using System.Runtime.ExceptionServices; +using System.Runtime.InteropServices; using Microsoft.AspNetCore.Protocols; using Microsoft.AspNetCore.SignalR.Internal.Formatters; using Microsoft.AspNetCore.Sockets; @@ -41,15 +43,18 @@ namespace Microsoft.AspNetCore.SignalR.Internal.Protocol { while (BinaryMessageParser.TryParseMessage(ref input, out var payload)) { - messages.Add(ParseMessage(payload.ToArray(), binder)); + var isArray = MemoryMarshal.TryGetArray(payload, out var arraySegment); + // This will never be false unless we started using un-managed buffers + Debug.Assert(isArray); + messages.Add(ParseMessage(arraySegment.Array, arraySegment.Offset, binder)); } return messages.Count > 0; } - private static HubMessage ParseMessage(byte[] input, IInvocationBinder binder) + private static HubMessage ParseMessage(byte[] input, int startOffset, IInvocationBinder binder) { - using (var unpacker = Unpacker.Create(input)) + using (var unpacker = Unpacker.Create(input, startOffset)) { _ = ReadArrayLength(unpacker, "elementCount"); From b5c46f35b3550936655c94c9ae8925d1fd4256be Mon Sep 17 00:00:00 2001 From: Mikael Mengistu Date: Thu, 22 Mar 2018 19:03:48 +0000 Subject: [PATCH 3/3] Check for actual start in SSE (#1681) --- .../ServerSentEventsTransport.cs | 22 ++++++-- ...HttpConnectionTests.ConnectionLifecycle.cs | 53 +++++++++++++++++++ 2 files changed, 71 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.AspNetCore.Sockets.Client.Http/ServerSentEventsTransport.cs b/src/Microsoft.AspNetCore.Sockets.Client.Http/ServerSentEventsTransport.cs index f8784756e2..8d819d6cbd 100644 --- a/src/Microsoft.AspNetCore.Sockets.Client.Http/ServerSentEventsTransport.cs +++ b/src/Microsoft.AspNetCore.Sockets.Client.Http/ServerSentEventsTransport.cs @@ -54,8 +54,9 @@ namespace Microsoft.AspNetCore.Sockets.Client Log.StartTransport(_logger, transferFormat); + var startTcs = new TaskCompletionSource(TaskContinuationOptions.RunContinuationsAsynchronously); var sendTask = SendUtils.SendMessages(url, _application, _httpClient, _httpOptions, _transportCts, _logger); - var receiveTask = OpenConnection(_application, url, _transportCts.Token); + var receiveTask = OpenConnection(_application, url, startTcs, _transportCts.Token); Running = Task.WhenAll(sendTask, receiveTask).ContinueWith(t => { @@ -66,17 +67,30 @@ namespace Microsoft.AspNetCore.Sockets.Client return t; }).Unwrap(); - return Task.CompletedTask; + return startTcs.Task; } - private async Task OpenConnection(IDuplexPipe application, Uri url, CancellationToken cancellationToken) + private async Task OpenConnection(IDuplexPipe application, Uri url, TaskCompletionSource startTcs, CancellationToken cancellationToken) { Log.StartReceive(_logger); var request = new HttpRequestMessage(HttpMethod.Get, url); SendUtils.PrepareHttpRequest(request, _httpOptions); request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("text/event-stream")); - var response = await _httpClient.SendAsync(request, HttpCompletionOption.ResponseHeadersRead, cancellationToken); + + HttpResponseMessage response; + try + { + response = await _httpClient.SendAsync(request, HttpCompletionOption.ResponseHeadersRead, cancellationToken); + response.EnsureSuccessStatusCode(); + startTcs.TrySetResult(null); + } + catch (Exception ex) + { + Log.TransportStopping(_logger); + startTcs.TrySetException(ex); + return; + } using (var stream = await response.Content.ReadAsStreamAsync()) { diff --git a/test/Microsoft.AspNetCore.SignalR.Client.Tests/HttpConnectionTests.ConnectionLifecycle.cs b/test/Microsoft.AspNetCore.SignalR.Client.Tests/HttpConnectionTests.ConnectionLifecycle.cs index db2cb3879b..bff6c5b0af 100644 --- a/test/Microsoft.AspNetCore.SignalR.Client.Tests/HttpConnectionTests.ConnectionLifecycle.cs +++ b/test/Microsoft.AspNetCore.SignalR.Client.Tests/HttpConnectionTests.ConnectionLifecycle.cs @@ -370,6 +370,59 @@ namespace Microsoft.AspNetCore.SignalR.Client.Tests }); } + [Fact] + public async Task SSEWontStartIfSuccessfulConnectionIsNotEstablished() + { + using (StartLog(out var loggerFactory)) + { + var httpHandler = new TestHttpMessageHandler(); + + httpHandler.OnGet("/?id=00000000-0000-0000-0000-000000000000", (_, __) => + { + return Task.FromResult(ResponseUtils.CreateResponse(HttpStatusCode.InternalServerError)); + }); + + var sse = new ServerSentEventsTransport(new HttpClient(httpHandler)); + + await WithConnectionAsync( + CreateConnection(httpHandler, loggerFactory: loggerFactory, url: null, transport: sse), + async (connection, closed) => + { + await Assert.ThrowsAsync( + () => connection.StartAsync(TransferFormat.Text).OrTimeout()); + }); + } + } + + [Fact] + public async Task SSEWaitsForResponseToStart() + { + using (StartLog(out var loggerFactory)) + { + var httpHandler = new TestHttpMessageHandler(); + + var connectResponseTcs = new TaskCompletionSource(); + httpHandler.OnGet("/?id=00000000-0000-0000-0000-000000000000", async (_, __) => + { + await connectResponseTcs.Task; + return ResponseUtils.CreateResponse(HttpStatusCode.Accepted); + }); + + var sse = new ServerSentEventsTransport(new HttpClient(httpHandler)); + + await WithConnectionAsync( + CreateConnection(httpHandler, loggerFactory: loggerFactory, url: null, transport: sse), + async (connection, closed) => + { + var startTask = connection.StartAsync(TransferFormat.Text).OrTimeout(); + Assert.False(connectResponseTcs.Task.IsCompleted); + Assert.False(startTask.IsCompleted); + connectResponseTcs.TrySetResult(null); + await startTask; + }); + } + } + [Fact] public async Task TransportIsStoppedWhenConnectionIsStopped() {