From 7c58653ee619bff5082d935dedaecb87f0b09827 Mon Sep 17 00:00:00 2001 From: Brennan Conroy Date: Fri, 10 Feb 2023 10:16:47 -0800 Subject: [PATCH] Avoid stream ID and client result ID collisions --- src/SignalR/server/Core/src/DefaultHubLifetimeManager.cs | 6 ++++-- .../test/HubConnectionHandlerTests.ClientResult.cs | 8 +++++++- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/src/SignalR/server/Core/src/DefaultHubLifetimeManager.cs b/src/SignalR/server/Core/src/DefaultHubLifetimeManager.cs index 230af87fc33e..48f04e5265d6 100644 --- a/src/SignalR/server/Core/src/DefaultHubLifetimeManager.cs +++ b/src/SignalR/server/Core/src/DefaultHubLifetimeManager.cs @@ -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; @@ -340,7 +339,10 @@ public override async Task InvokeConnectionAsync(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); diff --git a/src/SignalR/server/SignalR/test/HubConnectionHandlerTests.ClientResult.cs b/src/SignalR/server/SignalR/test/HubConnectionHandlerTests.ClientResult.cs index 3b30eb6f6b10..da5078458fa2 100644 --- a/src/SignalR/server/SignalR/test/HubConnectionHandlerTests.ClientResult.cs +++ b/src/SignalR/server/SignalR/test/HubConnectionHandlerTests.ClientResult.cs @@ -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()).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()).DefaultTimeout(); // Hub asks client for a result, this is an invocation message with an ID var invocationMessage = Assert.IsType(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();