Skip to content

Commit e0ee042

Browse files
mikaelm12mikaelm12
authored
SignalR Negotiate protocol versioning (#13389)
1 parent 88dbd1c commit e0ee042

16 files changed

+295
-28
lines changed

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

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,71 @@ public async Task CheckFixedMessage(string protocolName, HttpTransportType trans
114114
}
115115
}
116116

117+
[Fact]
118+
public async Task ServerRejectsClientWithOldProtocol()
119+
{
120+
bool ExpectedError(WriteContext writeContext)
121+
{
122+
return writeContext.LoggerName == typeof(HttpConnection).FullName &&
123+
writeContext.EventId.Name == "ErrorWithNegotiation";
124+
}
125+
126+
var protocol = HubProtocols["json"];
127+
using (StartServer<Startup>(out var server, ExpectedError))
128+
{
129+
var connectionBuilder = new HubConnectionBuilder()
130+
.WithLoggerFactory(LoggerFactory)
131+
.WithUrl(server.Url + "/negotiateProtocolVersion12", HttpTransportType.LongPolling);
132+
connectionBuilder.Services.AddSingleton(protocol);
133+
134+
var connection = connectionBuilder.Build();
135+
136+
try
137+
{
138+
var ex = await Assert.ThrowsAnyAsync<Exception>(() => connection.StartAsync()).OrTimeout();
139+
Assert.Equal("The client requested version '1', but the server does not support this version.", ex.Message);
140+
}
141+
catch (Exception ex)
142+
{
143+
LoggerFactory.CreateLogger<HubConnectionTests>().LogError(ex, "{ExceptionType} from test", ex.GetType().FullName);
144+
throw;
145+
}
146+
finally
147+
{
148+
await connection.DisposeAsync().OrTimeout();
149+
}
150+
}
151+
}
152+
153+
[Fact]
154+
public async Task ClientCanConnectToServerWithLowerMinimumProtocol()
155+
{
156+
var protocol = HubProtocols["json"];
157+
using (StartServer<Startup>(out var server))
158+
{
159+
var connectionBuilder = new HubConnectionBuilder()
160+
.WithLoggerFactory(LoggerFactory)
161+
.WithUrl(server.Url + "/negotiateProtocolVersionNegative", HttpTransportType.LongPolling);
162+
connectionBuilder.Services.AddSingleton(protocol);
163+
164+
var connection = connectionBuilder.Build();
165+
166+
try
167+
{
168+
await connection.StartAsync().OrTimeout();
169+
}
170+
catch (Exception ex)
171+
{
172+
LoggerFactory.CreateLogger<HubConnectionTests>().LogError(ex, "{ExceptionType} from test", ex.GetType().FullName);
173+
throw;
174+
}
175+
finally
176+
{
177+
await connection.DisposeAsync().OrTimeout();
178+
}
179+
}
180+
}
181+
117182
[Theory]
118183
[MemberData(nameof(HubProtocolsAndTransportsAndHubPaths))]
119184
public async Task CanSendAndReceiveMessage(string protocolName, HttpTransportType transportType, string path)

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,16 @@ public void Configure(IApplicationBuilder app)
6969

7070
endpoints.MapHub<TestHub>("/default-nowebsockets", options => options.Transports = HttpTransportType.LongPolling | HttpTransportType.ServerSentEvents);
7171

72+
endpoints.MapHub<TestHub>("/negotiateProtocolVersion12", options =>
73+
{
74+
options.MinimumProtocolVersion = 12;
75+
});
76+
77+
endpoints.MapHub<TestHub>("/negotiateProtocolVersionNegative", options =>
78+
{
79+
options.MinimumProtocolVersion = -1;
80+
});
81+
7282
endpoints.MapGet("/generateJwtToken", context =>
7383
{
7484
return context.Response.WriteAsync(GenerateJwtToken());

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -359,7 +359,7 @@ public async Task SSEWaitsForResponseToStart()
359359
var httpHandler = new TestHttpMessageHandler();
360360

361361
var connectResponseTcs = new TaskCompletionSource<object>();
362-
httpHandler.OnGet("/?id=00000000-0000-0000-0000-000000000000", async (_, __) =>
362+
httpHandler.OnGet("/?negotiateVersion=1&id=00000000-0000-0000-0000-000000000000", async (_, __) =>
363363
{
364364
await connectResponseTcs.Task;
365365
return ResponseUtils.CreateResponse(HttpStatusCode.Accepted);

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

Lines changed: 51 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -50,12 +50,12 @@ public Task ConnectionCannotBeStartedIfNoTransportProvidedByServer()
5050
}
5151

5252
[Theory]
53-
[InlineData("http://fakeuri.org/", "http://fakeuri.org/negotiate")]
54-
[InlineData("http://fakeuri.org/?q=1/0", "http://fakeuri.org/negotiate?q=1/0")]
55-
[InlineData("http://fakeuri.org?q=1/0", "http://fakeuri.org/negotiate?q=1/0")]
56-
[InlineData("http://fakeuri.org/endpoint", "http://fakeuri.org/endpoint/negotiate")]
57-
[InlineData("http://fakeuri.org/endpoint/", "http://fakeuri.org/endpoint/negotiate")]
58-
[InlineData("http://fakeuri.org/endpoint?q=1/0", "http://fakeuri.org/endpoint/negotiate?q=1/0")]
53+
[InlineData("http://fakeuri.org/", "http://fakeuri.org/negotiate?negotiateVersion=1")]
54+
[InlineData("http://fakeuri.org/?q=1/0", "http://fakeuri.org/negotiate?q=1/0&negotiateVersion=1")]
55+
[InlineData("http://fakeuri.org?q=1/0", "http://fakeuri.org/negotiate?q=1/0&negotiateVersion=1")]
56+
[InlineData("http://fakeuri.org/endpoint", "http://fakeuri.org/endpoint/negotiate?negotiateVersion=1")]
57+
[InlineData("http://fakeuri.org/endpoint/", "http://fakeuri.org/endpoint/negotiate?negotiateVersion=1")]
58+
[InlineData("http://fakeuri.org/endpoint?q=1/0", "http://fakeuri.org/endpoint/negotiate?q=1/0&negotiateVersion=1")]
5959
public async Task CorrectlyHandlesQueryStringWhenAppendingNegotiateToUrl(string requestedUrl, string expectedNegotiate)
6060
{
6161
var testHttpHandler = new TestHttpMessageHandler(autoNegotiate: false);
@@ -119,6 +119,43 @@ await WithConnectionAsync(
119119
Assert.Equal("0rge0d00-0040-0030-0r00-000q00r00e00", connectionId);
120120
}
121121

122+
[Fact]
123+
public async Task NegotiateCanHaveNewFields()
124+
{
125+
string connectionId = null;
126+
127+
var testHttpHandler = new TestHttpMessageHandler(autoNegotiate: false);
128+
testHttpHandler.OnNegotiate((request, cancellationToken) => ResponseUtils.CreateResponse(HttpStatusCode.OK,
129+
JsonConvert.SerializeObject(new
130+
{
131+
connectionId = "0rge0d00-0040-0030-0r00-000q00r00e00",
132+
availableTransports = new object[]
133+
{
134+
new
135+
{
136+
transport = "LongPolling",
137+
transferFormats = new[] { "Text" }
138+
},
139+
},
140+
newField = "ignore this",
141+
})));
142+
testHttpHandler.OnLongPoll(cancellationToken => ResponseUtils.CreateResponse(HttpStatusCode.NoContent));
143+
testHttpHandler.OnLongPollDelete((token) => ResponseUtils.CreateResponse(HttpStatusCode.Accepted));
144+
145+
using (var noErrorScope = new VerifyNoErrorsScope())
146+
{
147+
await WithConnectionAsync(
148+
CreateConnection(testHttpHandler, loggerFactory: noErrorScope.LoggerFactory),
149+
async (connection) =>
150+
{
151+
await connection.StartAsync().OrTimeout();
152+
connectionId = connection.ConnectionId;
153+
});
154+
}
155+
156+
Assert.Equal("0rge0d00-0040-0030-0r00-000q00r00e00", connectionId);
157+
}
158+
122159
[Fact]
123160
public async Task NegotiateThatReturnsUrlGetFollowed()
124161
{
@@ -172,10 +209,10 @@ await WithConnectionAsync(
172209
});
173210
}
174211

175-
Assert.Equal("http://fakeuri.org/negotiate", testHttpHandler.ReceivedRequests[0].RequestUri.ToString());
176-
Assert.Equal("https://another.domain.url/chat/negotiate", testHttpHandler.ReceivedRequests[1].RequestUri.ToString());
177-
Assert.Equal("https://another.domain.url/chat?id=0rge0d00-0040-0030-0r00-000q00r00e00", testHttpHandler.ReceivedRequests[2].RequestUri.ToString());
178-
Assert.Equal("https://another.domain.url/chat?id=0rge0d00-0040-0030-0r00-000q00r00e00", testHttpHandler.ReceivedRequests[3].RequestUri.ToString());
212+
Assert.Equal("http://fakeuri.org/negotiate?negotiateVersion=1", testHttpHandler.ReceivedRequests[0].RequestUri.ToString());
213+
Assert.Equal("https://another.domain.url/chat/negotiate?negotiateVersion=1", testHttpHandler.ReceivedRequests[1].RequestUri.ToString());
214+
Assert.Equal("https://another.domain.url/chat?negotiateVersion=1&id=0rge0d00-0040-0030-0r00-000q00r00e00", testHttpHandler.ReceivedRequests[2].RequestUri.ToString());
215+
Assert.Equal("https://another.domain.url/chat?negotiateVersion=1&id=0rge0d00-0040-0030-0r00-000q00r00e00", testHttpHandler.ReceivedRequests[3].RequestUri.ToString());
179216
Assert.Equal(5, testHttpHandler.ReceivedRequests.Count);
180217
}
181218

@@ -278,10 +315,10 @@ await WithConnectionAsync(
278315
});
279316
}
280317

281-
Assert.Equal("http://fakeuri.org/negotiate", testHttpHandler.ReceivedRequests[0].RequestUri.ToString());
282-
Assert.Equal("https://another.domain.url/chat/negotiate", testHttpHandler.ReceivedRequests[1].RequestUri.ToString());
283-
Assert.Equal("https://another.domain.url/chat?id=0rge0d00-0040-0030-0r00-000q00r00e00", testHttpHandler.ReceivedRequests[2].RequestUri.ToString());
284-
Assert.Equal("https://another.domain.url/chat?id=0rge0d00-0040-0030-0r00-000q00r00e00", testHttpHandler.ReceivedRequests[3].RequestUri.ToString());
318+
Assert.Equal("http://fakeuri.org/negotiate?negotiateVersion=1", testHttpHandler.ReceivedRequests[0].RequestUri.ToString());
319+
Assert.Equal("https://another.domain.url/chat/negotiate?negotiateVersion=1", testHttpHandler.ReceivedRequests[1].RequestUri.ToString());
320+
Assert.Equal("https://another.domain.url/chat?negotiateVersion=1&id=0rge0d00-0040-0030-0r00-000q00r00e00", testHttpHandler.ReceivedRequests[2].RequestUri.ToString());
321+
Assert.Equal("https://another.domain.url/chat?negotiateVersion=1&id=0rge0d00-0040-0030-0r00-000q00r00e00", testHttpHandler.ReceivedRequests[3].RequestUri.ToString());
285322
// Delete request
286323
Assert.Equal(5, testHttpHandler.ReceivedRequests.Count);
287324
}

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
// Copyright (c) .NET Foundation. All rights reserved.
2+
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
3+
14
using System;
25
using System.Collections.Generic;
36
using System.Net;
@@ -117,7 +120,7 @@ public static TestHttpMessageHandler CreateDefault()
117120
});
118121
testHttpMessageHandler.OnRequest((request, next, cancellationToken) =>
119122
{
120-
if (request.Method.Equals(HttpMethod.Delete) && request.RequestUri.PathAndQuery.StartsWith("/?id="))
123+
if (request.Method.Equals(HttpMethod.Delete) && request.RequestUri.PathAndQuery.Contains("&id="))
121124
{
122125
deleteCts.Cancel();
123126
return Task.FromResult(ResponseUtils.CreateResponse(HttpStatusCode.Accepted));

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ public partial class HttpConnection : ConnectionContext, IConnectionInherentKeep
2626
// Not configurable on purpose, high enough that if we reach here, it's likely
2727
// a buggy server
2828
private static readonly int _maxRedirects = 100;
29+
private static readonly int _protocolVersionNumber = 1;
2930
private static readonly Task<string> _noAccessToken = Task.FromResult<string>(null);
3031

3132
private static readonly TimeSpan HttpClientTimeout = TimeSpan.FromSeconds(120);
@@ -428,8 +429,9 @@ private async Task<NegotiationResponse> NegotiateAsync(Uri url, HttpClient httpC
428429
urlBuilder.Path += "/";
429430
}
430431
urlBuilder.Path += "negotiate";
432+
var uri = Utils.AppendQueryString(urlBuilder.Uri, $"negotiateVersion={_protocolVersionNumber}");
431433

432-
using (var request = new HttpRequestMessage(HttpMethod.Post, urlBuilder.Uri))
434+
using (var request = new HttpRequestMessage(HttpMethod.Post, uri))
433435
{
434436
// Corefx changed the default version and High Sierra curlhandler tries to upgrade request
435437
request.Version = new Version(1, 1);
@@ -466,7 +468,7 @@ private static Uri CreateConnectUrl(Uri url, string connectionId)
466468
throw new FormatException("Invalid connection id.");
467469
}
468470

469-
return Utils.AppendQueryString(url, "id=" + connectionId);
471+
return Utils.AppendQueryString(url, $"negotiateVersion={_protocolVersionNumber}&id=" + connectionId);
470472
}
471473

472474
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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,5 +36,6 @@ public NegotiationResponse() { }
3636
public string ConnectionId { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
3737
public string Error { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
3838
public string Url { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
39+
public int Version { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
3940
}
4041
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,5 +36,6 @@ public NegotiationResponse() { }
3636
public string ConnectionId { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
3737
public string Error { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
3838
public string Url { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
39+
public int Version { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
3940
}
4041
}

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ public static class NegotiateProtocol
2727
private static JsonEncodedText TransferFormatsPropertyNameBytes = JsonEncodedText.Encode(TransferFormatsPropertyName);
2828
private const string ErrorPropertyName = "error";
2929
private static JsonEncodedText ErrorPropertyNameBytes = JsonEncodedText.Encode(ErrorPropertyName);
30+
private const string NegotiateVersionPropertyName = "negotiateVersion";
31+
private static JsonEncodedText NegotiateVersionPropertyNameBytes = JsonEncodedText.Encode(NegotiateVersionPropertyName);
3032

3133
// Use C#7.3's ReadOnlySpan<byte> optimization for static data https://vcsjones.com/2019/02/01/csharp-readonly-span-bytes-static/
3234
// Used to detect ASP.NET SignalR Server connection attempt
@@ -41,6 +43,19 @@ public static void WriteResponse(NegotiationResponse response, IBufferWriter<byt
4143
var writer = reusableWriter.GetJsonWriter();
4244
writer.WriteStartObject();
4345

46+
// If we already have an error its due to a protocol version incompatibility.
47+
// We can just write the error and complete the JSON object and return.
48+
if (!string.IsNullOrEmpty(response.Error))
49+
{
50+
writer.WriteString(ErrorPropertyNameBytes, response.Error);
51+
writer.WriteEndObject();
52+
writer.Flush();
53+
Debug.Assert(writer.CurrentDepth == 0);
54+
return;
55+
}
56+
57+
writer.WriteNumber(NegotiateVersionPropertyNameBytes, response.Version);
58+
4459
if (!string.IsNullOrEmpty(response.Url))
4560
{
4661
writer.WriteString(UrlPropertyNameBytes, response.Url);
@@ -116,6 +131,7 @@ public static NegotiationResponse ParseResponse(ReadOnlySpan<byte> content)
116131
string accessToken = null;
117132
List<AvailableTransport> availableTransports = null;
118133
string error = null;
134+
int version = 0;
119135

120136
var completed = false;
121137
while (!completed && reader.CheckRead())
@@ -135,6 +151,10 @@ public static NegotiationResponse ParseResponse(ReadOnlySpan<byte> content)
135151
{
136152
connectionId = reader.ReadAsString(ConnectionIdPropertyName);
137153
}
154+
else if (reader.ValueTextEquals(NegotiateVersionPropertyNameBytes.EncodedUtf8Bytes))
155+
{
156+
version = reader.ReadAsInt32(NegotiateVersionPropertyName).GetValueOrDefault();
157+
}
138158
else if (reader.ValueTextEquals(AvailableTransportsPropertyNameBytes.EncodedUtf8Bytes))
139159
{
140160
reader.CheckRead();
@@ -195,6 +215,7 @@ public static NegotiationResponse ParseResponse(ReadOnlySpan<byte> content)
195215
AccessToken = accessToken,
196216
AvailableTransports = availableTransports,
197217
Error = error,
218+
Version = version
198219
};
199220
}
200221
catch (Exception ex)

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ public class NegotiationResponse
1010
public string Url { get; set; }
1111
public string AccessToken { get; set; }
1212
public string ConnectionId { get; set; }
13+
public int Version { get; set; }
1314
public IList<AvailableTransport> AvailableTransports { get; set; }
1415
public string Error { get; set; }
1516
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ public HttpConnectionDispatcherOptions() { }
5353
public long ApplicationMaxBufferSize { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
5454
public System.Collections.Generic.IList<Microsoft.AspNetCore.Authorization.IAuthorizeData> AuthorizationData { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } }
5555
public Microsoft.AspNetCore.Http.Connections.LongPollingOptions LongPolling { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } }
56+
public int MinimumProtocolVersion { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
5657
public long TransportMaxBufferSize { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
5758
public Microsoft.AspNetCore.Http.Connections.HttpTransportType Transports { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
5859
public Microsoft.AspNetCore.Http.Connections.WebSocketOptions WebSockets { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } }

0 commit comments

Comments
 (0)