From e79035ce88a8b52ef9102ff968567e36c4ba8c38 Mon Sep 17 00:00:00 2001 From: mohab ashraf Date: Fri, 25 Apr 2025 05:00:32 +0300 Subject: [PATCH 1/4] Fix Dependency Injection Conflict with Null Parameter in SignalR Hub Methods --- .../Core/src/Internal/DefaultHubDispatcher.cs | 11 ++++++----- .../HubConnectionHandlerTestUtils/Hubs.cs | 5 +++++ .../HubConnectionHandlerTests.cs | 19 +++++++++++++++++-- 3 files changed, 28 insertions(+), 7 deletions(-) diff --git a/src/SignalR/server/Core/src/Internal/DefaultHubDispatcher.cs b/src/SignalR/server/Core/src/Internal/DefaultHubDispatcher.cs index 24c675a5aa8f..b6d023520ad9 100644 --- a/src/SignalR/server/Core/src/Internal/DefaultHubDispatcher.cs +++ b/src/SignalR/server/Core/src/Internal/DefaultHubDispatcher.cs @@ -726,6 +726,11 @@ private void ReplaceArguments(HubMethodDescriptor descriptor, HubMethodInvocatio var hubInvocationArgumentPointer = 0; for (var parameterPointer = 0; parameterPointer < arguments.Length; parameterPointer++) { + if (descriptor.IsServiceArgument(parameterPointer)) + { + arguments[parameterPointer] = descriptor.GetService(scope.ServiceProvider, parameterPointer, descriptor.OriginalParameterTypes[parameterPointer]); + continue; + } if (hubMethodInvocationMessage.Arguments?.Length > hubInvocationArgumentPointer && (hubMethodInvocationMessage.Arguments[hubInvocationArgumentPointer] == null || descriptor.OriginalParameterTypes[parameterPointer].IsAssignableFrom(hubMethodInvocationMessage.Arguments[hubInvocationArgumentPointer]?.GetType()))) @@ -741,10 +746,6 @@ private void ReplaceArguments(HubMethodDescriptor descriptor, HubMethodInvocatio cts = CancellationTokenSource.CreateLinkedTokenSource(connection.ConnectionAborted); arguments[parameterPointer] = cts.Token; } - else if (descriptor.IsServiceArgument(parameterPointer)) - { - arguments[parameterPointer] = descriptor.GetService(scope.ServiceProvider, parameterPointer, descriptor.OriginalParameterTypes[parameterPointer]); - } else if (isStreamCall && ReflectionHelper.IsStreamingType(descriptor.OriginalParameterTypes[parameterPointer], mustBeDirectType: true)) { Log.StartingParameterStream(_logger, hubMethodInvocationMessage.StreamIds![streamPointer]); @@ -896,4 +897,4 @@ private static void SetActivityError(Activity? activity, Exception ex) activity?.SetTag("error.type", ex.GetType().FullName); activity?.SetStatus(ActivityStatusCode.Error); } -} +} \ No newline at end of file diff --git a/src/SignalR/server/SignalR/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTestUtils/Hubs.cs b/src/SignalR/server/SignalR/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTestUtils/Hubs.cs index 091ffabaad9f..46ce9f5f79b9 100644 --- a/src/SignalR/server/SignalR/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTestUtils/Hubs.cs +++ b/src/SignalR/server/SignalR/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTestUtils/Hubs.cs @@ -1381,6 +1381,11 @@ public async Task ServicesAndParams(int value, [FromService] Service1 servi } return total + value; } + + public int ServiceWithStringAttribute([FromService] Service1 service, string value) + { + return 115; + } public int ServiceWithoutAttribute(Service1 service) { diff --git a/src/SignalR/server/SignalR/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTests.cs b/src/SignalR/server/SignalR/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTests.cs index b6e37225ef4e..524ed0d60e6d 100644 --- a/src/SignalR/server/SignalR/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTests.cs +++ b/src/SignalR/server/SignalR/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTests.cs @@ -4679,6 +4679,23 @@ public async Task HubMethodCanInjectService() Assert.True(Assert.IsType(res.Result)); } } + + [Fact] + public async Task HubMethodCanInjectServiceWithNullParameter() + { + var serviceProvider = HubConnectionHandlerTestUtils.CreateServiceProvider(provider => + { + provider.AddSingleton(); + }); + var connectionHandler = serviceProvider.GetService>(); + + using (var client = new TestClient()) + { + var connectionHandlerTask = await client.ConnectAsync(connectionHandler).DefaultTimeout(); + var res = await client.InvokeAsync(nameof(ServicesHub.ServiceWithStringAttribute),(string)null ).DefaultTimeout(); + Assert.Equal(115L, res.Result); + } + } [Fact] public async Task HubMethodCanInjectMultipleServices() @@ -4764,7 +4781,6 @@ public async Task ServiceNotResolvedWithoutAttribute_WithSettingDisabledGlobally Assert.Equal("Failed to invoke 'ServiceWithoutAttribute' due to an error on the server. InvalidDataException: Invocation provides 0 argument(s) but target expects 1.", res.Error); } } - [Fact] public async Task ServiceResolvedWithoutAttribute() { @@ -4785,7 +4801,6 @@ public async Task ServiceResolvedWithoutAttribute() Assert.Equal(1L, res.Result); } } - [Fact] public async Task ServiceResolvedForIEnumerableParameter() { From 9eba599053538130c81b1a33355e7f6ebad7de41 Mon Sep 17 00:00:00 2001 From: Mohab Ashraf <63811158+MohabASHRAF-byte@users.noreply.github.com> Date: Fri, 25 Apr 2025 16:54:34 +0300 Subject: [PATCH 2/4] Update src/SignalR/server/Core/src/Internal/DefaultHubDispatcher.cs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Remove extra space Co-authored-by: Günther Foidl --- src/SignalR/server/Core/src/Internal/DefaultHubDispatcher.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SignalR/server/Core/src/Internal/DefaultHubDispatcher.cs b/src/SignalR/server/Core/src/Internal/DefaultHubDispatcher.cs index b6d023520ad9..6efffea9d432 100644 --- a/src/SignalR/server/Core/src/Internal/DefaultHubDispatcher.cs +++ b/src/SignalR/server/Core/src/Internal/DefaultHubDispatcher.cs @@ -897,4 +897,4 @@ private static void SetActivityError(Activity? activity, Exception ex) activity?.SetTag("error.type", ex.GetType().FullName); activity?.SetStatus(ActivityStatusCode.Error); } -} \ No newline at end of file +} From fa64be48a3fe96bc6a34b0a0c628ae4558bbc97d Mon Sep 17 00:00:00 2001 From: mohab ashraf Date: Sat, 26 Apr 2025 18:42:48 +0300 Subject: [PATCH 3/4] fix populate the synthetic parms first & fix style --- .../Core/src/Internal/DefaultHubDispatcher.cs | 43 +++++++++---------- .../HubConnectionHandlerTestUtils/Hubs.cs | 2 +- .../HubConnectionHandlerTests.cs | 3 +- 3 files changed, 23 insertions(+), 25 deletions(-) diff --git a/src/SignalR/server/Core/src/Internal/DefaultHubDispatcher.cs b/src/SignalR/server/Core/src/Internal/DefaultHubDispatcher.cs index 6efffea9d432..eeb2bea1bdfa 100644 --- a/src/SignalR/server/Core/src/Internal/DefaultHubDispatcher.cs +++ b/src/SignalR/server/Core/src/Internal/DefaultHubDispatcher.cs @@ -726,40 +726,37 @@ private void ReplaceArguments(HubMethodDescriptor descriptor, HubMethodInvocatio var hubInvocationArgumentPointer = 0; for (var parameterPointer = 0; parameterPointer < arguments.Length; parameterPointer++) { + // populate the synthetic arguments first if (descriptor.IsServiceArgument(parameterPointer)) { arguments[parameterPointer] = descriptor.GetService(scope.ServiceProvider, parameterPointer, descriptor.OriginalParameterTypes[parameterPointer]); - continue; } - if (hubMethodInvocationMessage.Arguments?.Length > hubInvocationArgumentPointer && + else if (descriptor.OriginalParameterTypes[parameterPointer] == typeof(CancellationToken)) + { + cts = CancellationTokenSource.CreateLinkedTokenSource(connection.ConnectionAborted); + arguments[parameterPointer] = cts.Token; + } + else if (isStreamCall && ReflectionHelper.IsStreamingType(descriptor.OriginalParameterTypes[parameterPointer], mustBeDirectType: true)) + { + Log.StartingParameterStream(_logger, hubMethodInvocationMessage.StreamIds![streamPointer]); + var itemType = descriptor.StreamingParameters![streamPointer]; + arguments[parameterPointer] = connection.StreamTracker.AddStream(hubMethodInvocationMessage.StreamIds[streamPointer], + itemType, descriptor.OriginalParameterTypes[parameterPointer]); + + streamPointer++; + } + else if (hubMethodInvocationMessage.Arguments?.Length > hubInvocationArgumentPointer && (hubMethodInvocationMessage.Arguments[hubInvocationArgumentPointer] == null || descriptor.OriginalParameterTypes[parameterPointer].IsAssignableFrom(hubMethodInvocationMessage.Arguments[hubInvocationArgumentPointer]?.GetType()))) { // The types match so it isn't a synthetic argument, just copy it into the arguments array arguments[parameterPointer] = hubMethodInvocationMessage.Arguments[hubInvocationArgumentPointer]; hubInvocationArgumentPointer++; - } + } else { - if (descriptor.OriginalParameterTypes[parameterPointer] == typeof(CancellationToken)) - { - cts = CancellationTokenSource.CreateLinkedTokenSource(connection.ConnectionAborted); - arguments[parameterPointer] = cts.Token; - } - else if (isStreamCall && ReflectionHelper.IsStreamingType(descriptor.OriginalParameterTypes[parameterPointer], mustBeDirectType: true)) - { - Log.StartingParameterStream(_logger, hubMethodInvocationMessage.StreamIds![streamPointer]); - var itemType = descriptor.StreamingParameters![streamPointer]; - arguments[parameterPointer] = connection.StreamTracker.AddStream(hubMethodInvocationMessage.StreamIds[streamPointer], - itemType, descriptor.OriginalParameterTypes[parameterPointer]); - - streamPointer++; - } - else - { - // This should never happen - Debug.Assert(false, $"Failed to bind argument of type '{descriptor.OriginalParameterTypes[parameterPointer].Name}' for hub method '{descriptor.MethodExecutor.MethodInfo.Name}'."); - } + // This should never happen + Debug.Assert(false, $"Failed to bind argument of type '{descriptor.OriginalParameterTypes[parameterPointer].Name}' for hub method '{descriptor.MethodExecutor.MethodInfo.Name}'."); } } } @@ -897,4 +894,4 @@ private static void SetActivityError(Activity? activity, Exception ex) activity?.SetTag("error.type", ex.GetType().FullName); activity?.SetStatus(ActivityStatusCode.Error); } -} +} \ No newline at end of file diff --git a/src/SignalR/server/SignalR/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTestUtils/Hubs.cs b/src/SignalR/server/SignalR/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTestUtils/Hubs.cs index 46ce9f5f79b9..90fabf90265b 100644 --- a/src/SignalR/server/SignalR/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTestUtils/Hubs.cs +++ b/src/SignalR/server/SignalR/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTestUtils/Hubs.cs @@ -1469,4 +1469,4 @@ public override async Task OnConnectedAsync() await Clients.Client(id).SendAsync("Test", 1); } } -} +} \ No newline at end of file diff --git a/src/SignalR/server/SignalR/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTests.cs b/src/SignalR/server/SignalR/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTests.cs index 524ed0d60e6d..afec34285b39 100644 --- a/src/SignalR/server/SignalR/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTests.cs +++ b/src/SignalR/server/SignalR/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTests.cs @@ -4680,6 +4680,7 @@ public async Task HubMethodCanInjectService() } } + // Regression test for https://github.com/dotnet/aspnetcore/issues/61491 [Fact] public async Task HubMethodCanInjectServiceWithNullParameter() { @@ -5474,4 +5475,4 @@ public static async Task> ReadAllAsync(this IAsyncEnumerable Date: Mon, 28 Apr 2025 09:30:49 -0700 Subject: [PATCH 4/4] Apply suggestions from code review --- src/SignalR/server/Core/src/Internal/DefaultHubDispatcher.cs | 2 +- .../HubConnectionHandlerTests.cs | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/SignalR/server/Core/src/Internal/DefaultHubDispatcher.cs b/src/SignalR/server/Core/src/Internal/DefaultHubDispatcher.cs index eeb2bea1bdfa..5e914b31e29e 100644 --- a/src/SignalR/server/Core/src/Internal/DefaultHubDispatcher.cs +++ b/src/SignalR/server/Core/src/Internal/DefaultHubDispatcher.cs @@ -752,7 +752,7 @@ private void ReplaceArguments(HubMethodDescriptor descriptor, HubMethodInvocatio // The types match so it isn't a synthetic argument, just copy it into the arguments array arguments[parameterPointer] = hubMethodInvocationMessage.Arguments[hubInvocationArgumentPointer]; hubInvocationArgumentPointer++; - } + } else { // This should never happen diff --git a/src/SignalR/server/SignalR/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTests.cs b/src/SignalR/server/SignalR/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTests.cs index afec34285b39..486f6c53ea02 100644 --- a/src/SignalR/server/SignalR/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTests.cs +++ b/src/SignalR/server/SignalR/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTests.cs @@ -4782,6 +4782,7 @@ public async Task ServiceNotResolvedWithoutAttribute_WithSettingDisabledGlobally Assert.Equal("Failed to invoke 'ServiceWithoutAttribute' due to an error on the server. InvalidDataException: Invocation provides 0 argument(s) but target expects 1.", res.Error); } } + [Fact] public async Task ServiceResolvedWithoutAttribute() { @@ -4802,6 +4803,7 @@ public async Task ServiceResolvedWithoutAttribute() Assert.Equal(1L, res.Result); } } + [Fact] public async Task ServiceResolvedForIEnumerableParameter() {