Skip to content

Commit 691a715

Browse files
Avoid stream ID and client result ID collisions (#46591)
Co-authored-by: Brennan Conroy <brecon@microsoft.com>
1 parent 4e5df96 commit 691a715

File tree

2 files changed

+11
-3
lines changed

2 files changed

+11
-3
lines changed

src/SignalR/server/Core/src/DefaultHubLifetimeManager.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33

44
using System.Collections.Concurrent;
55
using System.Diagnostics.CodeAnalysis;
6-
using System.Globalization;
76
using System.Linq;
87
using Microsoft.AspNetCore.SignalR.Internal;
98
using Microsoft.AspNetCore.SignalR.Protocol;
@@ -340,7 +339,10 @@ public override async Task<T> InvokeConnectionAsync<T>(string connectionId, stri
340339
throw new IOException($"Connection '{connectionId}' does not exist.");
341340
}
342341

343-
var invocationId = Interlocked.Increment(ref _lastInvocationId).ToString(NumberFormatInfo.InvariantInfo);
342+
var id = Interlocked.Increment(ref _lastInvocationId);
343+
// prefix the client result ID with 's' for server, so that it won't conflict with other CompletionMessage's from the client
344+
// e.g. Stream IDs when completing
345+
var invocationId = $"s{id}";
344346

345347
using var _ = CancellationTokenUtils.CreateLinkedToken(cancellationToken,
346348
connection.ConnectionAborted, out var linkedToken);

src/SignalR/server/SignalR/test/HubConnectionHandlerTests.ClientResult.cs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -453,11 +453,17 @@ public async Task ClientResultInUploadStreamingMethodWorks()
453453
{
454454
var connectionHandlerTask = await client.ConnectAsync(connectionHandler).DefaultTimeout();
455455

456-
var invocationId = await client.BeginUploadStreamAsync("1", nameof(MethodHub.GetClientResultWithStream), new[] { "id" }, Array.Empty<object>()).DefaultTimeout();
456+
// Regression test: Use 1 as the stream ID as this is the first ID the server would use for invocation IDs it generates
457+
// We want to make sure the client result completion doesn't accidentally complete the stream
458+
var streamId = "1";
459+
var invocationId = await client.BeginUploadStreamAsync("1", nameof(MethodHub.GetClientResultWithStream), new[] { streamId }, Array.Empty<object>()).DefaultTimeout();
457460

458461
// Hub asks client for a result, this is an invocation message with an ID
459462
var invocationMessage = Assert.IsType<InvocationMessage>(await client.ReadAsync().DefaultTimeout());
460463
Assert.NotNull(invocationMessage.InvocationId);
464+
// This check isn't really needed except we want to make sure the regression test mentioned above is still testing the expected scenario
465+
Assert.Equal("s1", invocationMessage.InvocationId);
466+
461467
var res = 4 + ((long)invocationMessage.Arguments[0]);
462468
await client.SendHubMessageAsync(CompletionMessage.WithResult(invocationMessage.InvocationId, res)).DefaultTimeout();
463469

0 commit comments

Comments
 (0)