Skip to content

Commit

Permalink
Avoid stream ID and client result ID collisions (#46591)
Browse files Browse the repository at this point in the history
Co-authored-by: Brennan Conroy <brecon@microsoft.com>
  • Loading branch information
github-actions[bot] and BrennanConroy committed Feb 14, 2023
1 parent 4e5df96 commit 691a715
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 3 deletions.
6 changes: 4 additions & 2 deletions src/SignalR/server/Core/src/DefaultHubLifetimeManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

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

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

using var _ = CancellationTokenUtils.CreateLinkedToken(cancellationToken,
connection.ConnectionAborted, out var linkedToken);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -453,11 +453,17 @@ public async Task ClientResultInUploadStreamingMethodWorks()
{
var connectionHandlerTask = await client.ConnectAsync(connectionHandler).DefaultTimeout();

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

// Hub asks client for a result, this is an invocation message with an ID
var invocationMessage = Assert.IsType<InvocationMessage>(await client.ReadAsync().DefaultTimeout());
Assert.NotNull(invocationMessage.InvocationId);
// This check isn't really needed except we want to make sure the regression test mentioned above is still testing the expected scenario
Assert.Equal("s1", invocationMessage.InvocationId);

var res = 4 + ((long)invocationMessage.Arguments[0]);
await client.SendHubMessageAsync(CompletionMessage.WithResult(invocationMessage.InvocationId, res)).DefaultTimeout();

Expand Down

0 comments on commit 691a715

Please sign in to comment.