Dependency Injection Conflict with Null Parameter in SignalR Hub Methods

[pranjalg-dew]

Is there an existing issue for this?

• I have searched the existing issues

Describe the bug

I am trying to inject a service into a hub method using Dependency Injection (DI). I have explicitly specified the [FromServices] attribute before the parameter that should be resolved from DI in the hub method.

Currently, I have multiple hub methods that start with a service parameter. Recently, I encountered an issue with such an arrangement. When I pass a null value for the parameter following the service parameter, the service parameter fails to resolve its dependency.

It seems that the hub method is trying to resolve dependencies from both the client message and the DI container. Consequently, placing the injecting parameter first may lead to improper binding of the null value.

I also tried using the DisableImplicitFromServicesParameters option, but the same issue persists.

Then, I tried placing the service parameter last, and it worked fine.

Expected Behavior

When explicitly marked with the [FromServices] attribute, the service parameter should reliably resolve its dependency from the Dependency Injection (DI) container, regardless of the order of parameters in the hub method.

Steps To Reproduce

1. Clone the sample project repository from https://github.com/pranjalg-dew/AspNetCore-SignalR-BugRepro.
2. Build and run the project locally.
3. Open the web page hosted in the repository which can send values to the SignalR endpoint.
4. Keep textbox empty and click on send message button.
5. Observe the resolution of the [FromServices] IMyService service parameter. Note that the service parameter is null.

Exceptions (if any)

No response

.NET Version

No response

Anything else?

No response

[BrennanConroy]

Bug in how we inject synthetic arguments. Workaround is to place arguments from services as the last arguments in the method.

While iterating over the method arguments, we check if a value from the client is null and assign it to the current argument position.

aspnetcore/src/SignalR/server/Core/src/Internal/DefaultHubDispatcher.cs

Line 730 in a94ba4b

(hubMethodInvocationMessage.Arguments[hubInvocationArgumentPointer] == null ||

We should probably reorganize this method and check IsServiceArgument first

aspnetcore/src/SignalR/server/Core/src/Internal/DefaultHubDispatcher.cs

Line 744 in a94ba4b

else if (descriptor.IsServiceArgument(parameterPointer))

[dotnet-policy-service]

Looks like this issue has been identified as a candidate for community contribution. If you're considering sending a PR for this issue, look for the Summary Comment link in the issue description. That comment has been left by an engineer on our team to help you get started with handling this issue. You can learn more about our Help Wanted process here

[MohabASHRAF-byte]

Hello, I'd be interested in contributing to this issue—may I take it on as my first contribution to the project? @BrennanConroy

[BrennanConroy]

Sure! I also forgot to mention that we should make sure to have a regression test, it'd likely go near

aspnetcore/src/SignalR/server/SignalR/test/Microsoft.AspNetCore.SignalR.Tests/HubConnectionHandlerTests.cs

Line 4769 in bb7c393

public async Task ServiceResolvedWithoutAttribute()