SignalR Negotiate protocol versioning

src/SignalR/clients/csharp/Client/test/FunctionalTests/HubConnectionTests.cs

@@ -114,6 +114,71 @@ public async Task CheckFixedMessage(string protocolName, HttpTransportType trans
             }
         }
 
+        [Fact]
+        public async Task ServerRejectsClientWithOldProtocol()
+        {
+            bool ExpectedError(WriteContext writeContext)
+            {
+                return writeContext.LoggerName == typeof(HttpConnection).FullName &&
+                    writeContext.EventId.Name == "ErrorWithNegotiation";
+            }
+
+            var protocol = HubProtocols["json"];
+            using (StartServer<Startup>(out var server, ExpectedError))
+            {
+                var connectionBuilder = new HubConnectionBuilder()
+                    .WithLoggerFactory(LoggerFactory)
+                    .WithUrl(server.Url + "/negotiateProtocolVersion12", HttpTransportType.LongPolling);
+                connectionBuilder.Services.AddSingleton(protocol);
+
+                var connection = connectionBuilder.Build();
+
+                try
+                {
+                    var ex = await Assert.ThrowsAnyAsync<Exception>(() => connection.StartAsync()).OrTimeout();
+                    Assert.Equal("The client requested version '1', but the server does not support this version.", ex.Message);
+                }
+                catch (Exception ex)
+                {
+                    LoggerFactory.CreateLogger<HubConnectionTests>().LogError(ex, "{ExceptionType} from test", ex.GetType().FullName);
+                    throw;
+                }
+                finally
+                {
+                    await connection.DisposeAsync().OrTimeout();
+                }
+            }
+        }
+
+        [Fact]
+        public async Task ClientCanConnectToServerWithLowerMinimumProtocol()
+        {
+            var protocol = HubProtocols["json"];
+            using (StartServer<Startup>(out var server))
+            {
+                var connectionBuilder = new HubConnectionBuilder()
+                    .WithLoggerFactory(LoggerFactory)
+                    .WithUrl(server.Url + "/negotiateProtocolVersionNegative", HttpTransportType.LongPolling);
+                connectionBuilder.Services.AddSingleton(protocol);
+
+                var connection = connectionBuilder.Build();
+
+                try
+                {
+                    await connection.StartAsync().OrTimeout();
+                }
+                catch (Exception ex)
+                {
+                    LoggerFactory.CreateLogger<HubConnectionTests>().LogError(ex, "{ExceptionType} from test", ex.GetType().FullName);
+                    throw;
+                }
+                finally
+                {
+                    await connection.DisposeAsync().OrTimeout();
+                }
+            }
+        }
+
         [Theory]
         [MemberData(nameof(HubProtocolsAndTransportsAndHubPaths))]
         public async Task CanSendAndReceiveMessage(string protocolName, HttpTransportType transportType, string path)

src/SignalR/clients/csharp/Client/test/FunctionalTests/Startup.cs

@@ -69,6 +69,16 @@ public void Configure(IApplicationBuilder app)
 
                 endpoints.MapHub<TestHub>("/default-nowebsockets", options => options.Transports = HttpTransportType.LongPolling | HttpTransportType.ServerSentEvents);
 
+                endpoints.MapHub<TestHub>("/negotiateProtocolVersion12", options =>
+                {
+                    options.MinimumProtocolVersion = 12;
+                });
+
+                endpoints.MapHub<TestHub>("/negotiateProtocolVersionNegative", options =>
+                {
+                    options.MinimumProtocolVersion = -1;
+                });
+
                 endpoints.MapGet("/generateJwtToken", context =>
                 {
                     return context.Response.WriteAsync(GenerateJwtToken());

src/SignalR/clients/csharp/Client/test/UnitTests/HttpConnectionTests.ConnectionLifecycle.cs

@@ -359,7 +359,7 @@ public async Task SSEWaitsForResponseToStart()
                     var httpHandler = new TestHttpMessageHandler();
 
                     var connectResponseTcs = new TaskCompletionSource<object>();
-                    httpHandler.OnGet("/?id=00000000-0000-0000-0000-000000000000", async (_, __) =>
+                    httpHandler.OnGet("/?negotiateVersion=1&id=00000000-0000-0000-0000-000000000000", async (_, __) =>
                     {
                         await connectResponseTcs.Task;
                         return ResponseUtils.CreateResponse(HttpStatusCode.Accepted);

src/SignalR/clients/csharp/Client/test/UnitTests/HttpConnectionTests.Negotiate.cs

@@ -50,12 +50,12 @@ public Task ConnectionCannotBeStartedIfNoTransportProvidedByServer()
             }
 
             [Theory]
-            [InlineData("http://fakeuri.org/", "http://fakeuri.org/negotiate")]
-            [InlineData("http://fakeuri.org/?q=1/0", "http://fakeuri.org/negotiate?q=1/0")]
-            [InlineData("http://fakeuri.org?q=1/0", "http://fakeuri.org/negotiate?q=1/0")]
-            [InlineData("http://fakeuri.org/endpoint", "http://fakeuri.org/endpoint/negotiate")]
-            [InlineData("http://fakeuri.org/endpoint/", "http://fakeuri.org/endpoint/negotiate")]
-            [InlineData("http://fakeuri.org/endpoint?q=1/0", "http://fakeuri.org/endpoint/negotiate?q=1/0")]
+            [InlineData("http://fakeuri.org/", "http://fakeuri.org/negotiate?negotiateVersion=1")]
+            [InlineData("http://fakeuri.org/?q=1/0", "http://fakeuri.org/negotiate?q=1/0&negotiateVersion=1")]
+            [InlineData("http://fakeuri.org?q=1/0", "http://fakeuri.org/negotiate?q=1/0&negotiateVersion=1")]
+            [InlineData("http://fakeuri.org/endpoint", "http://fakeuri.org/endpoint/negotiate?negotiateVersion=1")]
+            [InlineData("http://fakeuri.org/endpoint/", "http://fakeuri.org/endpoint/negotiate?negotiateVersion=1")]
+            [InlineData("http://fakeuri.org/endpoint?q=1/0", "http://fakeuri.org/endpoint/negotiate?q=1/0&negotiateVersion=1")]
             public async Task CorrectlyHandlesQueryStringWhenAppendingNegotiateToUrl(string requestedUrl, string expectedNegotiate)
             {
                 var testHttpHandler = new TestHttpMessageHandler(autoNegotiate: false);
@@ -119,6 +119,43 @@ await WithConnectionAsync(
                 Assert.Equal("0rge0d00-0040-0030-0r00-000q00r00e00", connectionId);
             }
 
+            [Fact]
+            public async Task NegotiateCanHaveNewFields()
+            {
+                string connectionId = null;
+
+                var testHttpHandler = new TestHttpMessageHandler(autoNegotiate: false);
+                testHttpHandler.OnNegotiate((request, cancellationToken) => ResponseUtils.CreateResponse(HttpStatusCode.OK,
+                    JsonConvert.SerializeObject(new
+                    {
+                        connectionId = "0rge0d00-0040-0030-0r00-000q00r00e00",
+                        availableTransports = new object[]
+                        {
+                            new
+                            {
+                                transport = "LongPolling",
+                                transferFormats = new[] { "Text" }
+                            },
+                        },
+                        newField = "ignore this",
+                    })));
+                testHttpHandler.OnLongPoll(cancellationToken => ResponseUtils.CreateResponse(HttpStatusCode.NoContent));
+                testHttpHandler.OnLongPollDelete((token) => ResponseUtils.CreateResponse(HttpStatusCode.Accepted));
+
+                using (var noErrorScope = new VerifyNoErrorsScope())
+                {
+                    await WithConnectionAsync(
+                        CreateConnection(testHttpHandler, loggerFactory: noErrorScope.LoggerFactory),
+                        async (connection) =>
+                        {
+                            await connection.StartAsync().OrTimeout();
+                            connectionId = connection.ConnectionId;
+                        });
+                }
+
+                Assert.Equal("0rge0d00-0040-0030-0r00-000q00r00e00", connectionId);
+            }
+
             [Fact]
             public async Task NegotiateThatReturnsUrlGetFollowed()
             {
@@ -172,10 +209,10 @@ await WithConnectionAsync(
                         });
                 }
 
-                Assert.Equal("http://fakeuri.org/negotiate", testHttpHandler.ReceivedRequests[0].RequestUri.ToString());
-                Assert.Equal("https://another.domain.url/chat/negotiate", testHttpHandler.ReceivedRequests[1].RequestUri.ToString());
-                Assert.Equal("https://another.domain.url/chat?id=0rge0d00-0040-0030-0r00-000q00r00e00", testHttpHandler.ReceivedRequests[2].RequestUri.ToString());
-                Assert.Equal("https://another.domain.url/chat?id=0rge0d00-0040-0030-0r00-000q00r00e00", testHttpHandler.ReceivedRequests[3].RequestUri.ToString());
+                Assert.Equal("http://fakeuri.org/negotiate?negotiateVersion=1", testHttpHandler.ReceivedRequests[0].RequestUri.ToString());
+                Assert.Equal("https://another.domain.url/chat/negotiate?negotiateVersion=1", testHttpHandler.ReceivedRequests[1].RequestUri.ToString());
+                Assert.Equal("https://another.domain.url/chat?negotiateVersion=1&id=0rge0d00-0040-0030-0r00-000q00r00e00", testHttpHandler.ReceivedRequests[2].RequestUri.ToString());
+                Assert.Equal("https://another.domain.url/chat?negotiateVersion=1&id=0rge0d00-0040-0030-0r00-000q00r00e00", testHttpHandler.ReceivedRequests[3].RequestUri.ToString());
                 Assert.Equal(5, testHttpHandler.ReceivedRequests.Count);
             }
 
@@ -278,10 +315,10 @@ await WithConnectionAsync(
                         });
                 }
 
-                Assert.Equal("http://fakeuri.org/negotiate", testHttpHandler.ReceivedRequests[0].RequestUri.ToString());
-                Assert.Equal("https://another.domain.url/chat/negotiate", testHttpHandler.ReceivedRequests[1].RequestUri.ToString());
-                Assert.Equal("https://another.domain.url/chat?id=0rge0d00-0040-0030-0r00-000q00r00e00", testHttpHandler.ReceivedRequests[2].RequestUri.ToString());
-                Assert.Equal("https://another.domain.url/chat?id=0rge0d00-0040-0030-0r00-000q00r00e00", testHttpHandler.ReceivedRequests[3].RequestUri.ToString());
+                Assert.Equal("http://fakeuri.org/negotiate?negotiateVersion=1", testHttpHandler.ReceivedRequests[0].RequestUri.ToString());
+                Assert.Equal("https://another.domain.url/chat/negotiate?negotiateVersion=1", testHttpHandler.ReceivedRequests[1].RequestUri.ToString());
+                Assert.Equal("https://another.domain.url/chat?negotiateVersion=1&id=0rge0d00-0040-0030-0r00-000q00r00e00", testHttpHandler.ReceivedRequests[2].RequestUri.ToString());
+                Assert.Equal("https://another.domain.url/chat?negotiateVersion=1&id=0rge0d00-0040-0030-0r00-000q00r00e00", testHttpHandler.ReceivedRequests[3].RequestUri.ToString());
                 // Delete request
                 Assert.Equal(5, testHttpHandler.ReceivedRequests.Count);
             }

src/SignalR/clients/csharp/Client/test/UnitTests/TestHttpMessageHandler.cs

@@ -1,3 +1,6 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
+
 using System;
 using System.Collections.Generic;
 using System.Net;
@@ -117,7 +120,7 @@ public static TestHttpMessageHandler CreateDefault()
             });
             testHttpMessageHandler.OnRequest((request, next, cancellationToken) =>
             {
-                if (request.Method.Equals(HttpMethod.Delete) && request.RequestUri.PathAndQuery.StartsWith("/?id="))
+                if (request.Method.Equals(HttpMethod.Delete) && request.RequestUri.PathAndQuery.Contains("&id="))
                 {
                     deleteCts.Cancel();
                     return Task.FromResult(ResponseUtils.CreateResponse(HttpStatusCode.Accepted));

src/SignalR/clients/csharp/Http.Connections.Client/src/HttpConnection.cs

@@ -26,6 +26,7 @@ public partial class HttpConnection : ConnectionContext, IConnectionInherentKeep
         // Not configurable on purpose, high enough that if we reach here, it's likely
         // a buggy server
         private static readonly int _maxRedirects = 100;
+        private static readonly int _protocolVersionNumber = 1;
         private static readonly Task<string> _noAccessToken = Task.FromResult<string>(null);
 
         private static readonly TimeSpan HttpClientTimeout = TimeSpan.FromSeconds(120);
@@ -428,8 +429,9 @@ private async Task<NegotiationResponse> NegotiateAsync(Uri url, HttpClient httpC
                     urlBuilder.Path += "/";
                 }
                 urlBuilder.Path += "negotiate";
+                var uri = Utils.AppendQueryString(urlBuilder.Uri, $"negotiateVersion={_protocolVersionNumber}");
 
-                using (var request = new HttpRequestMessage(HttpMethod.Post, urlBuilder.Uri))
+                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);
@@ -466,7 +468,7 @@ private static Uri CreateConnectUrl(Uri url, string connectionId)
                 throw new FormatException("Invalid connection id.");
             }
 
-            return Utils.AppendQueryString(url, "id=" + connectionId);
+            return Utils.AppendQueryString(url, $"negotiateVersion={_protocolVersionNumber}&id=" + connectionId);
         }
 
         private async Task StartTransport(Uri connectUrl, HttpTransportType transportType, TransferFormat transferFormat, CancellationToken cancellationToken)

src/SignalR/common/Http.Connections.Common/ref/Microsoft.AspNetCore.Http.Connections.Common.netcoreapp.cs

@@ -36,5 +36,6 @@ public NegotiationResponse() { }
         public string ConnectionId { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
         public string Error { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
         public string Url { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
+        public int Version { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
     }
 }

src/SignalR/common/Http.Connections.Common/ref/Microsoft.AspNetCore.Http.Connections.Common.netstandard2.0.cs

@@ -36,5 +36,6 @@ public NegotiationResponse() { }
         public string ConnectionId { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
         public string Error { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
         public string Url { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
+        public int Version { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
     }
 }

src/SignalR/common/Http.Connections.Common/src/NegotiateProtocol.cs

@@ -27,6 +27,8 @@ public static class NegotiateProtocol
         private static JsonEncodedText TransferFormatsPropertyNameBytes = JsonEncodedText.Encode(TransferFormatsPropertyName);
         private const string ErrorPropertyName = "error";
         private static JsonEncodedText ErrorPropertyNameBytes = JsonEncodedText.Encode(ErrorPropertyName);
+        private const string NegotiateVersionPropertyName = "negotiateVersion";
+        private static JsonEncodedText NegotiateVersionPropertyNameBytes = JsonEncodedText.Encode(NegotiateVersionPropertyName);
 
         // Use C#7.3's ReadOnlySpan<byte> optimization for static data https://vcsjones.com/2019/02/01/csharp-readonly-span-bytes-static/
         // Used to detect ASP.NET SignalR Server connection attempt
@@ -41,6 +43,19 @@ public static void WriteResponse(NegotiationResponse response, IBufferWriter<byt
                 var writer = reusableWriter.GetJsonWriter();
                 writer.WriteStartObject();
 
+                // If we already have an error its due to a protocol version incompatibility.
+                // We can just write the error and complete the JSON object and return.
+                if (!string.IsNullOrEmpty(response.Error))
+                {
+                    writer.WriteString(ErrorPropertyNameBytes, response.Error);
+                    writer.WriteEndObject();
+                    writer.Flush();
+                    Debug.Assert(writer.CurrentDepth == 0);
+                    return;
+                }
+
+                writer.WriteNumber(NegotiateVersionPropertyNameBytes, response.Version);
+
                 if (!string.IsNullOrEmpty(response.Url))
                 {
                     writer.WriteString(UrlPropertyNameBytes, response.Url);
@@ -116,6 +131,7 @@ public static NegotiationResponse ParseResponse(ReadOnlySpan<byte> content)
                 string accessToken = null;
                 List<AvailableTransport> availableTransports = null;
                 string error = null;
+                int version = 0;
 
                 var completed = false;
                 while (!completed && reader.CheckRead())
@@ -135,6 +151,10 @@ public static NegotiationResponse ParseResponse(ReadOnlySpan<byte> content)
                             {
                                 connectionId = reader.ReadAsString(ConnectionIdPropertyName);
                             }
+                            else if (reader.ValueTextEquals(NegotiateVersionPropertyNameBytes.EncodedUtf8Bytes))
+                            {
+                                version = reader.ReadAsInt32(NegotiateVersionPropertyName).GetValueOrDefault();
+                            }
                             else if (reader.ValueTextEquals(AvailableTransportsPropertyNameBytes.EncodedUtf8Bytes))
                             {
                                 reader.CheckRead();
@@ -195,6 +215,7 @@ public static NegotiationResponse ParseResponse(ReadOnlySpan<byte> content)
                     AccessToken = accessToken,
                     AvailableTransports = availableTransports,
                     Error = error,
+                    Version = version
                 };
             }
             catch (Exception ex)

src/SignalR/common/Http.Connections.Common/src/NegotiationResponse.cs

@@ -10,6 +10,7 @@ public class NegotiationResponse
         public string Url { get; set; }
         public string AccessToken { get; set; }
         public string ConnectionId { get; set; }
+        public int Version { get; set; }
         public IList<AvailableTransport> AvailableTransports { get; set; }
         public string Error { get; set; }
     }

src/SignalR/common/Http.Connections/ref/Microsoft.AspNetCore.Http.Connections.netcoreapp.cs

@@ -53,6 +53,7 @@ public HttpConnectionDispatcherOptions() { }
         public long ApplicationMaxBufferSize { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
         public System.Collections.Generic.IList<Microsoft.AspNetCore.Authorization.IAuthorizeData> AuthorizationData { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } }
         public Microsoft.AspNetCore.Http.Connections.LongPollingOptions LongPolling { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } }
+        public int MinimumProtocolVersion { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
         public long TransportMaxBufferSize { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
         public Microsoft.AspNetCore.Http.Connections.HttpTransportType Transports { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
         public Microsoft.AspNetCore.Http.Connections.WebSocketOptions WebSockets { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } }