Skip to content

Commit 51367f6

Browse files
BrennanConroyBrennanConroy
authored
[SignalR] Error if multiple service attributes are on a single parameter (#50248)
1 parent 175f7d7 commit 51367f6

File tree

3 files changed

+84
-29
lines changed

3 files changed

+84
-29
lines changed

‎src/SignalR/server/Core/src/Internal/HubMethodDescriptor.cs‎

Lines changed: 37 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -81,22 +81,53 @@ public HubMethodDescriptor(ObjectMethodExecutor methodExecutor, IServiceProvider
8181
}
8282
else if (p.CustomAttributes.Any())
8383
{
84+
var markedParameter = false;
8485
foreach (var attribute in p.GetCustomAttributes(true))
8586
{
8687
if (attribute is IFromServiceMetadata)
8788
{
88-
return MarkServiceParameter(index);
89+
ThrowIfMarked(markedParameter);
90+
markedParameter = true;
91+
MarkServiceParameter(index);
8992
}
9093
else if (attribute is FromKeyedServicesAttribute keyedServicesAttribute)
9194
{
92-
if (serviceProviderIsService is IServiceProviderIsKeyedService keyedServiceProvider &&
93-
keyedServiceProvider.IsKeyedService(GetServiceType(p.ParameterType), keyedServicesAttribute.Key))
95+
ThrowIfMarked(markedParameter);
96+
markedParameter = true;
97+
98+
if (serviceProviderIsService is IServiceProviderIsKeyedService keyedServiceProvider)
99+
{
100+
if (keyedServiceProvider.IsKeyedService(GetServiceType(p.ParameterType), keyedServicesAttribute.Key))
101+
{
102+
KeyedServiceKeys ??= new List<(int, object)>();
103+
KeyedServiceKeys.Add((index, keyedServicesAttribute.Key));
104+
MarkServiceParameter(index);
105+
}
106+
else
107+
{
108+
throw new InvalidOperationException($"'{p.ParameterType}' is not in DI as a keyed service.");
109+
}
110+
}
111+
else
94112
{
95-
KeyedServiceKeys ??= new List<(int, object)>();
96-
KeyedServiceKeys.Add((index, keyedServicesAttribute.Key));
97-
return MarkServiceParameter(index);
113+
throw new InvalidOperationException($"This service provider doesn't support keyed services.");
98114
}
99115
}
116+
117+
void ThrowIfMarked(bool marked)
118+
{
119+
if (marked)
120+
{
121+
throw new InvalidOperationException(
122+
$"{methodExecutor.MethodInfo.DeclaringType?.Name}.{methodExecutor.MethodInfo.Name}: The {nameof(FromKeyedServicesAttribute)} is not supported on parameters that are also annotated with {nameof(IFromServiceMetadata)}.");
123+
}
124+
}
125+
}
126+
127+
if (markedParameter)
128+
{
129+
// If the parameter is marked because of being a service, we don't want to consider it for method parameters during deserialization
130+
return false;
100131
}
101132
}
102133
else if (serviceProviderIsService?.IsService(GetServiceType(p.ParameterType)) == true)

‎src/SignalR/server/SignalR/test/HubConnectionHandlerTestUtils/Hubs.cs‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1363,12 +1363,6 @@ public async Task<int> ServicesAndParams(int value, [FromService] Service1 servi
13631363
return total + value;
13641364
}
13651365

1366-
public int MultipleSameKeyedServices([FromKeyedServices("service1")] Service1 service, [FromKeyedServices("service1")] Service1 service2)
1367-
{
1368-
Assert.Same(service, service2);
1369-
return 445;
1370-
}
1371-
13721366
public int ServiceWithoutAttribute(Service1 service)
13731367
{
13741368
return 1;
@@ -1391,6 +1385,15 @@ public async Task Stream(ChannelReader<int> channelReader)
13911385
await channelReader.ReadAsync();
13921386
}
13931387
}
1388+
}
1389+
1390+
public class KeyedServicesHub : TestHub
1391+
{
1392+
public int MultipleSameKeyedServices([FromKeyedServices("service1")] Service1 service, [FromKeyedServices("service1")] Service1 service2)
1393+
{
1394+
Assert.Same(service, service2);
1395+
return 445;
1396+
}
13941397

13951398
public int KeyedService([FromKeyedServices("service1")] Service1 service)
13961399
{
@@ -1414,6 +1417,13 @@ public int MultipleKeyedServices([FromKeyedServices("service1")] Service1 servic
14141417
}
14151418
}
14161419

1420+
public class BadServicesHub : Hub
1421+
{
1422+
public void BadMethod([FromKeyedServices("service1")] [FromService] Service1 service)
1423+
{
1424+
}
1425+
}
1426+
14171427
public class TooManyParamsHub : Hub
14181428
{
14191429
public void ManyParams(int a1, string a2, bool a3, float a4, string a5, int a6, int a7, int a8, int a9, int a10, int a11,

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

Lines changed: 31 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -4927,13 +4927,14 @@ public async Task KeyedServiceResolvedIfInDI()
49274927
});
49284928

49294929
provider.AddKeyedScoped<Service1>("service1");
4930+
provider.AddKeyedScoped<Service1>("service2");
49304931
});
4931-
var connectionHandler = serviceProvider.GetService<HubConnectionHandler<ServicesHub>>();
4932+
var connectionHandler = serviceProvider.GetService<HubConnectionHandler<KeyedServicesHub>>();
49324933

49334934
using (var client = new TestClient())
49344935
{
49354936
var connectionHandlerTask = await client.ConnectAsync(connectionHandler).DefaultTimeout();
4936-
var res = await client.InvokeAsync(nameof(ServicesHub.KeyedService)).DefaultTimeout();
4937+
var res = await client.InvokeAsync(nameof(KeyedServicesHub.KeyedService)).DefaultTimeout();
49374938
Assert.Equal(43L, res.Result);
49384939
}
49394940
}
@@ -4949,13 +4950,14 @@ public async Task HubMethodCanInjectKeyedServiceWithOtherParameters()
49494950
});
49504951

49514952
provider.AddKeyedScoped<Service1>("service1");
4953+
provider.AddKeyedScoped<Service1>("service2");
49524954
});
4953-
var connectionHandler = serviceProvider.GetService<HubConnectionHandler<ServicesHub>>();
4955+
var connectionHandler = serviceProvider.GetService<HubConnectionHandler<KeyedServicesHub>>();
49544956

49554957
using (var client = new TestClient())
49564958
{
49574959
var connectionHandlerTask = await client.ConnectAsync(connectionHandler).DefaultTimeout();
4958-
var res = await client.InvokeAsync(nameof(ServicesHub.KeyedServiceWithParam), 91).DefaultTimeout();
4960+
var res = await client.InvokeAsync(nameof(KeyedServicesHub.KeyedServiceWithParam), 91).DefaultTimeout();
49594961
Assert.Equal(1183L, res.Result);
49604962
}
49614963
}
@@ -4971,14 +4973,15 @@ public async Task HubMethodCanInjectKeyedServiceWithNonKeyedService()
49714973
});
49724974

49734975
provider.AddKeyedScoped<Service1>("service1");
4976+
provider.AddKeyedScoped<Service1>("service2");
49744977
provider.AddScoped<Service2>();
49754978
});
4976-
var connectionHandler = serviceProvider.GetService<HubConnectionHandler<ServicesHub>>();
4979+
var connectionHandler = serviceProvider.GetService<HubConnectionHandler<KeyedServicesHub>>();
49774980

49784981
using (var client = new TestClient())
49794982
{
49804983
var connectionHandlerTask = await client.ConnectAsync(connectionHandler).DefaultTimeout();
4981-
var res = await client.InvokeAsync(nameof(ServicesHub.KeyedServiceNonKeyedService)).DefaultTimeout();
4984+
var res = await client.InvokeAsync(nameof(KeyedServicesHub.KeyedServiceNonKeyedService)).DefaultTimeout();
49824985
Assert.Equal(11L, res.Result);
49834986
}
49844987
}
@@ -4996,12 +4999,12 @@ public async Task MultipleKeyedServicesResolved()
49964999
provider.AddKeyedScoped<Service1>("service1");
49975000
provider.AddKeyedScoped<Service1>("service2");
49985001
});
4999-
var connectionHandler = serviceProvider.GetService<HubConnectionHandler<ServicesHub>>();
5002+
var connectionHandler = serviceProvider.GetService<HubConnectionHandler<KeyedServicesHub>>();
50005003

50015004
using (var client = new TestClient())
50025005
{
50035006
var connectionHandlerTask = await client.ConnectAsync(connectionHandler).DefaultTimeout();
5004-
var res = await client.InvokeAsync(nameof(ServicesHub.MultipleKeyedServices)).DefaultTimeout();
5007+
var res = await client.InvokeAsync(nameof(KeyedServicesHub.MultipleKeyedServices)).DefaultTimeout();
50055008
Assert.Equal(45L, res.Result);
50065009
}
50075010
}
@@ -5017,19 +5020,20 @@ public async Task MultipleKeyedServicesWithSameNameResolved()
50175020
});
50185021

50195022
provider.AddKeyedScoped<Service1>("service1");
5023+
provider.AddKeyedScoped<Service1>("service2");
50205024
});
5021-
var connectionHandler = serviceProvider.GetService<HubConnectionHandler<ServicesHub>>();
5025+
var connectionHandler = serviceProvider.GetService<HubConnectionHandler<KeyedServicesHub>>();
50225026

50235027
using (var client = new TestClient())
50245028
{
50255029
var connectionHandlerTask = await client.ConnectAsync(connectionHandler).DefaultTimeout();
5026-
var res = await client.InvokeAsync(nameof(ServicesHub.MultipleSameKeyedServices)).DefaultTimeout();
5030+
var res = await client.InvokeAsync(nameof(KeyedServicesHub.MultipleSameKeyedServices)).DefaultTimeout();
50275031
Assert.Equal(445L, res.Result);
50285032
}
50295033
}
50305034

50315035
[Fact]
5032-
public async Task KeyedServiceNotResolvedIfNotInDI()
5036+
public void KeyedServiceNotResolvedIfNotInDI()
50335037
{
50345038
var serviceProvider = HubConnectionHandlerTestUtils.CreateServiceProvider(provider =>
50355039
{
@@ -5038,14 +5042,24 @@ public async Task KeyedServiceNotResolvedIfNotInDI()
50385042
options.EnableDetailedErrors = true;
50395043
});
50405044
});
5041-
var connectionHandler = serviceProvider.GetService<HubConnectionHandler<ServicesHub>>();
5045+
var ex = Assert.Throws<InvalidOperationException>(() => serviceProvider.GetService<HubConnectionHandler<KeyedServicesHub>>());
5046+
Assert.Equal("'Microsoft.AspNetCore.SignalR.Tests.Service1' is not in DI as a keyed service.", ex.Message);
5047+
}
50425048

5043-
using (var client = new TestClient())
5049+
[Fact]
5050+
public void KeyedServiceAndFromServiceOnSameParameterInvalidWithKeyedServiceInDI()
5051+
{
5052+
var serviceProvider = HubConnectionHandlerTestUtils.CreateServiceProvider(provider =>
50445053
{
5045-
var connectionHandlerTask = await client.ConnectAsync(connectionHandler).DefaultTimeout();
5046-
var res = await client.InvokeAsync(nameof(ServicesHub.KeyedService)).DefaultTimeout();
5047-
Assert.Equal("Failed to invoke 'KeyedService' due to an error on the server. InvalidDataException: Invocation provides 0 argument(s) but target expects 1.", res.Error);
5048-
}
5054+
provider.AddSignalR(options =>
5055+
{
5056+
options.EnableDetailedErrors = true;
5057+
});
5058+
5059+
provider.AddKeyedScoped<Service1>("service1");
5060+
});
5061+
var ex = Assert.Throws<InvalidOperationException>(() => serviceProvider.GetService<HubConnectionHandler<BadServicesHub>>());
5062+
Assert.Equal("BadServicesHub.BadMethod: The FromKeyedServicesAttribute is not supported on parameters that are also annotated with IFromServiceMetadata.", ex.Message);
50495063
}
50505064

50515065
[Fact]

0 commit comments

Comments
 (0)