From ab3dce85fc947b6eb9c925415e41af309876714c Mon Sep 17 00:00:00 2001 From: Andrew Stanton-Nurse Date: Mon, 20 Mar 2017 09:48:15 -0700 Subject: [PATCH] tidy up status codes and transport conflicts (#306) --- SignalR.sln | 2 +- .../Transports/LongPollingTransport.cs | 2 + .../ConnectionTests.cs | 0 .../HubConnectionTests.cs | 0 .../LongPollingTransportTests.cs | 0 ...ft.AspNetCore.SignalR.Client.Tests.csproj} | 0 .../ResponseUtils.cs | 0 .../TaskQueueTests.cs | 0 .../HttpConnectionDispatcherTests.cs | 59 +++++++++++++++++-- .../Microsoft.AspNetCore.Sockets.Tests.csproj | 1 + .../TestWebSocketConnectionFeature.cs | 36 +++++++++++ 11 files changed, 94 insertions(+), 6 deletions(-) rename test/{Microsoft.AspNetCore.Sockets.Client.Tests => Microsoft.AspNetCore.SignalR.Client.Tests}/ConnectionTests.cs (100%) rename test/{Microsoft.AspNetCore.Sockets.Client.Tests => Microsoft.AspNetCore.SignalR.Client.Tests}/HubConnectionTests.cs (100%) rename test/{Microsoft.AspNetCore.Sockets.Client.Tests => Microsoft.AspNetCore.SignalR.Client.Tests}/LongPollingTransportTests.cs (100%) rename test/{Microsoft.AspNetCore.Sockets.Client.Tests/Microsoft.AspNetCore.Client.Tests.csproj => Microsoft.AspNetCore.SignalR.Client.Tests/Microsoft.AspNetCore.SignalR.Client.Tests.csproj} (100%) rename test/{Microsoft.AspNetCore.Sockets.Client.Tests => Microsoft.AspNetCore.SignalR.Client.Tests}/ResponseUtils.cs (100%) rename test/{Microsoft.AspNetCore.Sockets.Client.Tests => Microsoft.AspNetCore.SignalR.Client.Tests}/TaskQueueTests.cs (100%) create mode 100644 test/Microsoft.AspNetCore.Sockets.Tests/TestWebSocketConnectionFeature.cs diff --git a/SignalR.sln b/SignalR.sln index 84642929fc..9ed5bf0f00 100644 --- a/SignalR.sln +++ b/SignalR.sln @@ -47,7 +47,7 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "WebSocketSample", "samples\ EndProject Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Microsoft.AspNetCore.Sockets.Client", "src\Microsoft.AspNetCore.Sockets.Client\Microsoft.AspNetCore.Sockets.Client.csproj", "{623FD372-36DE-41A9-A564-F6040D570DBD}" EndProject -Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Microsoft.AspNetCore.Client.Tests", "test\Microsoft.AspNetCore.Sockets.Client.Tests\Microsoft.AspNetCore.Client.Tests.csproj", "{B19C15A5-F5EA-4CA7-936B-1166ABEE35C4}" +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Microsoft.AspNetCore.SignalR.Client.Tests", "test\Microsoft.AspNetCore.SignalR.Client.Tests\Microsoft.AspNetCore.SignalR.Client.Tests.csproj", "{B19C15A5-F5EA-4CA7-936B-1166ABEE35C4}" EndProject Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Microsoft.AspNetCore.SignalR.Common", "src\Microsoft.AspNetCore.SignalR.Common\Microsoft.AspNetCore.SignalR.Common.csproj", "{E37324FF-6BAF-4243-BA80-7C024CF5F29D}" EndProject diff --git a/src/Microsoft.AspNetCore.Sockets/Transports/LongPollingTransport.cs b/src/Microsoft.AspNetCore.Sockets/Transports/LongPollingTransport.cs index 1904ac5e7c..be13f16415 100644 --- a/src/Microsoft.AspNetCore.Sockets/Transports/LongPollingTransport.cs +++ b/src/Microsoft.AspNetCore.Sockets/Transports/LongPollingTransport.cs @@ -49,6 +49,8 @@ namespace Microsoft.AspNetCore.Sockets.Transports output.Append(MessageFormatter.GetFormatIndicator(messageFormat)); + // We're intentionally not checking cancellation here because we need to drain messages we've got so far, + // but it's too late to emit the 204 required by being cancelled. while (_application.TryRead(out var message)) { _logger.LogDebug("Writing {0} byte message to response", message.Payload.Length); diff --git a/test/Microsoft.AspNetCore.Sockets.Client.Tests/ConnectionTests.cs b/test/Microsoft.AspNetCore.SignalR.Client.Tests/ConnectionTests.cs similarity index 100% rename from test/Microsoft.AspNetCore.Sockets.Client.Tests/ConnectionTests.cs rename to test/Microsoft.AspNetCore.SignalR.Client.Tests/ConnectionTests.cs diff --git a/test/Microsoft.AspNetCore.Sockets.Client.Tests/HubConnectionTests.cs b/test/Microsoft.AspNetCore.SignalR.Client.Tests/HubConnectionTests.cs similarity index 100% rename from test/Microsoft.AspNetCore.Sockets.Client.Tests/HubConnectionTests.cs rename to test/Microsoft.AspNetCore.SignalR.Client.Tests/HubConnectionTests.cs diff --git a/test/Microsoft.AspNetCore.Sockets.Client.Tests/LongPollingTransportTests.cs b/test/Microsoft.AspNetCore.SignalR.Client.Tests/LongPollingTransportTests.cs similarity index 100% rename from test/Microsoft.AspNetCore.Sockets.Client.Tests/LongPollingTransportTests.cs rename to test/Microsoft.AspNetCore.SignalR.Client.Tests/LongPollingTransportTests.cs diff --git a/test/Microsoft.AspNetCore.Sockets.Client.Tests/Microsoft.AspNetCore.Client.Tests.csproj b/test/Microsoft.AspNetCore.SignalR.Client.Tests/Microsoft.AspNetCore.SignalR.Client.Tests.csproj similarity index 100% rename from test/Microsoft.AspNetCore.Sockets.Client.Tests/Microsoft.AspNetCore.Client.Tests.csproj rename to test/Microsoft.AspNetCore.SignalR.Client.Tests/Microsoft.AspNetCore.SignalR.Client.Tests.csproj diff --git a/test/Microsoft.AspNetCore.Sockets.Client.Tests/ResponseUtils.cs b/test/Microsoft.AspNetCore.SignalR.Client.Tests/ResponseUtils.cs similarity index 100% rename from test/Microsoft.AspNetCore.Sockets.Client.Tests/ResponseUtils.cs rename to test/Microsoft.AspNetCore.SignalR.Client.Tests/ResponseUtils.cs diff --git a/test/Microsoft.AspNetCore.Sockets.Client.Tests/TaskQueueTests.cs b/test/Microsoft.AspNetCore.SignalR.Client.Tests/TaskQueueTests.cs similarity index 100% rename from test/Microsoft.AspNetCore.Sockets.Client.Tests/TaskQueueTests.cs rename to test/Microsoft.AspNetCore.SignalR.Client.Tests/TaskQueueTests.cs diff --git a/test/Microsoft.AspNetCore.Sockets.Tests/HttpConnectionDispatcherTests.cs b/test/Microsoft.AspNetCore.Sockets.Tests/HttpConnectionDispatcherTests.cs index 8e12651d9a..7d69685d4c 100644 --- a/test/Microsoft.AspNetCore.Sockets.Tests/HttpConnectionDispatcherTests.cs +++ b/test/Microsoft.AspNetCore.Sockets.Tests/HttpConnectionDispatcherTests.cs @@ -10,6 +10,7 @@ using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Internal; using Microsoft.AspNetCore.SignalR.Tests.Common; using Microsoft.AspNetCore.Sockets.Internal; +using Microsoft.AspNetCore.WebSockets.Internal; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Primitives; @@ -177,16 +178,18 @@ namespace Microsoft.AspNetCore.Sockets.Tests Assert.False(exists); } - [Fact] - public async Task RequestToActiveConnectionId409ForStreamingTransports() + [Theory] + [InlineData("/ws", true)] + [InlineData("/sse", false)] + public async Task RequestToActiveConnectionId409ForStreamingTransports(string path, bool isWebSocketRequest) { var manager = CreateConnectionManager(); var state = manager.CreateConnection(); var dispatcher = new HttpConnectionDispatcher(manager, new LoggerFactory()); - var context1 = MakeRequest("/sse", state); - var context2 = MakeRequest("/sse", state); + var context1 = MakeRequest(path, state, isWebSocketRequest: isWebSocketRequest); + var context2 = MakeRequest(path, state, isWebSocketRequest: isWebSocketRequest); var request1 = dispatcher.ExecuteAsync("", context1); @@ -318,6 +321,34 @@ namespace Microsoft.AspNetCore.Sockets.Tests Assert.False(exists); } + [Fact] + public async Task AttemptingToPollWhileAlreadyPollingReplacesTheCurrentPoll() + { + var manager = CreateConnectionManager(); + var state = manager.CreateConnection(); + + var dispatcher = new HttpConnectionDispatcher(manager, new LoggerFactory()); + + var context1 = MakeRequest("/poll", state); + var task1 = dispatcher.ExecuteAsync("", context1); + var context2 = MakeRequest("/poll", state); + var task2 = dispatcher.ExecuteAsync("", context2); + + // Task 1 should finish when request 2 arrives + await task1.OrTimeout(); + + // Send a message from the app to complete Task 2 + await state.Connection.Transport.Output.WriteAsync(new Message(Encoding.UTF8.GetBytes("Hello, World"), MessageType.Text)); + + await task2.OrTimeout(); + + // Verify the results + Assert.Equal(StatusCodes.Status204NoContent, context1.Response.StatusCode); + Assert.Equal(string.Empty, GetContentAsString(context1.Response.Body)); + Assert.Equal(StatusCodes.Status200OK, context2.Response.StatusCode); + Assert.Equal("T12:T:Hello, World;", GetContentAsString(context2.Response.Body)); + } + [Theory] [InlineData("", "text", "Hello, World", "Hello, World", MessageType.Text)] // Legacy format [InlineData("", "binary", "Hello, World", "Hello, World", MessageType.Binary)] // Legacy format @@ -384,7 +415,7 @@ namespace Microsoft.AspNetCore.Sockets.Tests return messages; } - private static DefaultHttpContext MakeRequest(string path, ConnectionState state, string format = null) where TEndPoint : EndPoint + private static DefaultHttpContext MakeRequest(string path, ConnectionState state, string format = null, bool isWebSocketRequest = false) where TEndPoint : EndPoint { var context = new DefaultHttpContext(); var services = new ServiceCollection(); @@ -399,6 +430,14 @@ namespace Microsoft.AspNetCore.Sockets.Tests } var qs = new QueryCollection(values); context.Request.Query = qs; + context.Response.Body = new MemoryStream(); + + if (isWebSocketRequest) + { + // Add Test WebSocket feature + context.Features.Set(new TestWebSocketConnectionFeature()); + } + return context; } @@ -406,6 +445,16 @@ namespace Microsoft.AspNetCore.Sockets.Tests { return new ConnectionManager(new Logger(new LoggerFactory())); } + + private string GetContentAsString(Stream body) + { + Assert.True(body.CanSeek, "Can't get content of a non-seekable stream"); + body.Seek(0, SeekOrigin.Begin); + using (var reader = new StreamReader(body)) + { + return reader.ReadToEnd(); + } + } } public class BlockingEndPoint : EndPoint diff --git a/test/Microsoft.AspNetCore.Sockets.Tests/Microsoft.AspNetCore.Sockets.Tests.csproj b/test/Microsoft.AspNetCore.Sockets.Tests/Microsoft.AspNetCore.Sockets.Tests.csproj index 4526a3ab10..9e739994a5 100644 --- a/test/Microsoft.AspNetCore.Sockets.Tests/Microsoft.AspNetCore.Sockets.Tests.csproj +++ b/test/Microsoft.AspNetCore.Sockets.Tests/Microsoft.AspNetCore.Sockets.Tests.csproj @@ -21,6 +21,7 @@ + diff --git a/test/Microsoft.AspNetCore.Sockets.Tests/TestWebSocketConnectionFeature.cs b/test/Microsoft.AspNetCore.Sockets.Tests/TestWebSocketConnectionFeature.cs new file mode 100644 index 0000000000..9352cb4494 --- /dev/null +++ b/test/Microsoft.AspNetCore.Sockets.Tests/TestWebSocketConnectionFeature.cs @@ -0,0 +1,36 @@ +using System; +using System.Buffers.Pools; +using System.IO.Pipelines; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.WebSockets.Internal; +using Microsoft.Extensions.WebSockets.Internal; + +namespace Microsoft.AspNetCore.Sockets.Tests +{ + internal class TestWebSocketConnectionFeature : IHttpWebSocketConnectionFeature, IDisposable + { + private PipeFactory _factory = new PipeFactory(ManagedBufferPool.Shared); + + public bool IsWebSocketRequest => true; + + public WebSocketConnection Client { get; private set; } + + public ValueTask AcceptWebSocketConnectionAsync(WebSocketAcceptContext context) + { + var clientToServer = _factory.Create(); + var serverToClient = _factory.Create(); + + var clientSocket = new WebSocketConnection(serverToClient.Reader, clientToServer.Writer); + var serverSocket = new WebSocketConnection(clientToServer.Reader, serverToClient.Writer); + + Client = clientSocket; + return new ValueTask(serverSocket); + } + + public void Dispose() + { + _factory.Dispose(); + } + } +} \ No newline at end of file