Skip to content

Commit 30bf6af

Browse files
author
raghuramn
committed
Bug 333326: HttpControllerDispatcher should not set the controllerName on HttpControllerDescriptor based on controller route value
This commit is just fixing the cache that DefaultHttpControllerSelector uses for ControllerDescriptors. I have opened a seperate bug 391806 to remove the ControllerName property from HttpControllerDescriptor.
1 parent 06f52b8 commit 30bf6af

File tree

3 files changed

+336
-24
lines changed

3 files changed

+336
-24
lines changed

‎src/System.Web.Http/Dispatcher/DefaultHttpControllerSelector.cs‎

Lines changed: 19 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ public DefaultHttpControllerSelector(HttpConfiguration configuration)
3535
{
3636
if (configuration == null)
3737
{
38-
throw Error.ArgumentNull("resolver");
38+
throw Error.ArgumentNull("configuration");
3939
}
4040

4141
_controllerInfoCache = new Lazy<ConcurrentDictionary<string, HttpControllerDescriptor>>(InitializeControllerInfoCache);
@@ -64,29 +64,23 @@ public virtual HttpControllerDescriptor SelectController(HttpRequestMessage requ
6464
}
6565

6666
ICollection<Type> matchingTypes = _controllerTypeCache.GetControllerTypes(controllerName);
67-
switch (matchingTypes.Count)
67+
68+
// ControllerInfoCache is already initialized.
69+
Contract.Assert(matchingTypes.Count != 1);
70+
71+
if (matchingTypes.Count == 0)
6872
{
69-
case 0:
70-
// no matching types
71-
throw new HttpResponseException(request.CreateResponse(
72-
HttpStatusCode.NotFound,
73-
Error.Format(SRResources.DefaultControllerFactory_ControllerNameNotFound, controllerName)));
74-
75-
case 1:
76-
// single matching type
77-
Type match = matchingTypes.First();
78-
79-
// Add controller descriptor to cache
80-
controllerDescriptor = new HttpControllerDescriptor(_configuration, controllerName, match);
81-
_controllerInfoCache.Value.TryAdd(controllerName, controllerDescriptor);
82-
83-
return controllerDescriptor;
84-
85-
default:
86-
// multiple matching types
87-
throw new HttpResponseException(request.CreateResponse(
88-
HttpStatusCode.InternalServerError,
89-
CreateAmbiguousControllerExceptionMessage(request.GetRouteData().Route, controllerName, matchingTypes)));
73+
// no matching types
74+
throw new HttpResponseException(request.CreateResponse(
75+
HttpStatusCode.NotFound,
76+
Error.Format(SRResources.DefaultControllerFactory_ControllerNameNotFound, controllerName)));
77+
}
78+
else
79+
{
80+
// multiple matching types
81+
throw new HttpResponseException(request.CreateResponse(
82+
HttpStatusCode.InternalServerError,
83+
CreateAmbiguousControllerExceptionMessage(request.GetRouteData().Route, controllerName, matchingTypes)));
9084
}
9185
}
9286

@@ -133,13 +127,14 @@ private static string CreateAmbiguousControllerExceptionMessage(IHttpRoute route
133127

134128
private ConcurrentDictionary<string, HttpControllerDescriptor> InitializeControllerInfoCache()
135129
{
136-
var result = new ConcurrentDictionary<string, HttpControllerDescriptor>();
130+
var result = new ConcurrentDictionary<string, HttpControllerDescriptor>(StringComparer.OrdinalIgnoreCase);
137131
var duplicateControllers = new HashSet<string>();
138132
Dictionary<string, ILookup<string, Type>> controllerTypeGroups = _controllerTypeCache.Cache;
139133

140134
foreach (KeyValuePair<string, ILookup<string, Type>> controllerTypeGroup in controllerTypeGroups)
141135
{
142136
string controllerName = controllerTypeGroup.Key;
137+
143138
foreach (IGrouping<string, Type> controllerTypesGroupedByNs in controllerTypeGroup.Value)
144139
{
145140
foreach (Type controllerType in controllerTypesGroupedByNs)

‎test/System.Web.Http.Test/Dispatcher/DefaultHttpControllerSelectorTest.cs‎

Lines changed: 316 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,316 @@
1+
using System.Collections.ObjectModel;
2+
using System.Linq;
3+
using System.Net;
4+
using System.Net.Http;
5+
using System.Web.Http.Controllers;
6+
using System.Web.Http.Hosting;
7+
using System.Web.Http.Routing;
8+
using Moq;
9+
using Xunit;
10+
using Xunit.Extensions;
11+
using Assert = Microsoft.TestCommon.AssertEx;
12+
13+
namespace System.Web.Http.Dispatcher
14+
{
15+
public class DefaultHttpControllerSelectorTest
16+
{
17+
[Fact]
18+
public void Constructor_Throws_NullConfiguration()
19+
{
20+
Assert.ThrowsArgumentNull(
21+
() => new DefaultHttpControllerSelector(configuration: null),
22+
"configuration");
23+
}
24+
25+
[Theory]
26+
[InlineData("controller", "abc")]
27+
[InlineData("Controller", "123")]
28+
[InlineData("ControLler", "123")]
29+
[InlineData("CONTROLLER", "ABC")]
30+
public void GetControllerName_PicksControllerNameFromRouteData(string controllerKeyName, string controllerName)
31+
{
32+
// Arrange
33+
HttpRequestMessage request = new HttpRequestMessage();
34+
IHttpRouteData routeData = GetRouteData();
35+
routeData.Values[controllerKeyName] = controllerName;
36+
request.Properties[HttpPropertyKeys.HttpRouteDataKey] = routeData;
37+
DefaultHttpControllerSelector selector = new DefaultHttpControllerSelector(new HttpConfiguration());
38+
39+
// Act
40+
string selectedControllerName = selector.GetControllerName(request);
41+
42+
// Assert
43+
Assert.Equal(controllerName, selectedControllerName);
44+
}
45+
46+
[Fact]
47+
public void GetControllerName_PicksNull_NoRouteData()
48+
{
49+
// Arrange
50+
HttpRequestMessage request = new HttpRequestMessage();
51+
DefaultHttpControllerSelector selector = new DefaultHttpControllerSelector(new HttpConfiguration());
52+
53+
// Act
54+
string selectedControllerName = selector.GetControllerName(request);
55+
56+
// Assert
57+
Assert.Null(selectedControllerName);
58+
}
59+
60+
[Fact]
61+
public void GetControllerName_PicksNull_EmptyRouteData()
62+
{
63+
// Arrange
64+
HttpRequestMessage request = new HttpRequestMessage();
65+
request.Properties[HttpPropertyKeys.HttpRouteDataKey] = GetRouteData();
66+
DefaultHttpControllerSelector selector = new DefaultHttpControllerSelector(new HttpConfiguration());
67+
68+
// Act
69+
string selectedControllerName = selector.GetControllerName(request);
70+
71+
// Assert
72+
Assert.Null(selectedControllerName);
73+
}
74+
75+
[Fact]
76+
public void DefaultHttpControllerSelector_Uses_IAssemblyResolverAndIHttpControllerTypeResolver()
77+
{
78+
// Arrange
79+
HttpConfiguration configuration = new HttpConfiguration();
80+
Mock<IAssembliesResolver> assemblyResolver = new Mock<IAssembliesResolver>();
81+
Mock<IHttpControllerTypeResolver> controllerTypeResolver = new Mock<IHttpControllerTypeResolver>();
82+
configuration.ServiceResolver.SetService(typeof(IAssembliesResolver), assemblyResolver.Object);
83+
configuration.ServiceResolver.SetService(typeof(IHttpControllerTypeResolver), controllerTypeResolver.Object);
84+
85+
controllerTypeResolver.Setup(c => c.GetControllerTypes(assemblyResolver.Object)).Returns(new Collection<Type> { GetMockControllerType("Sample") }).Verifiable();
86+
DefaultHttpControllerSelector selector = new DefaultHttpControllerSelector(configuration);
87+
88+
HttpRequestMessage request = new HttpRequestMessage();
89+
IHttpRouteData routeData = GetRouteData();
90+
routeData.Values["controller"] = "Sample";
91+
request.Properties[HttpPropertyKeys.HttpRouteDataKey] = routeData;
92+
93+
// Act
94+
selector.SelectController(request);
95+
96+
// Assert
97+
controllerTypeResolver.Verify();
98+
}
99+
100+
[Theory]
101+
[InlineData("Sample")]
102+
[InlineData("SAmple")]
103+
[InlineData("SAMPLE")]
104+
public void SelectController_IsCaseInsensitive(string controllerTypeName)
105+
{
106+
HttpConfiguration configuration = new HttpConfiguration();
107+
Mock<IHttpControllerTypeResolver> controllerTypeResolver = new Mock<IHttpControllerTypeResolver>();
108+
configuration.ServiceResolver.SetService(typeof(IHttpControllerTypeResolver), controllerTypeResolver.Object);
109+
110+
Type controllerType = GetMockControllerType("Sample");
111+
controllerTypeResolver
112+
.Setup(c => c.GetControllerTypes(It.IsAny<IAssembliesResolver>()))
113+
.Returns(new Collection<Type> { controllerType });
114+
115+
HttpRequestMessage request = new HttpRequestMessage();
116+
IHttpRouteData routeData = GetRouteData();
117+
routeData.Values["controller"] = controllerTypeName;
118+
request.Properties[HttpPropertyKeys.HttpRouteDataKey] = routeData;
119+
120+
DefaultHttpControllerSelector selector = new DefaultHttpControllerSelector(configuration);
121+
122+
// Act
123+
HttpControllerDescriptor descriptor = selector.SelectController(request);
124+
125+
// Assert
126+
Assert.IsType(typeof(HttpControllerDescriptor), descriptor);
127+
Assert.Equal(controllerType, descriptor.ControllerType);
128+
}
129+
130+
[Fact]
131+
public void SelectController_DoesNotCreateNewInstances_ForSameController()
132+
{
133+
HttpConfiguration configuration = new HttpConfiguration();
134+
Mock<IHttpControllerTypeResolver> controllerTypeResolver = new Mock<IHttpControllerTypeResolver>();
135+
configuration.ServiceResolver.SetService(typeof(IHttpControllerTypeResolver), controllerTypeResolver.Object);
136+
137+
Type controllerType = GetMockControllerType("Sample");
138+
controllerTypeResolver
139+
.Setup(c => c.GetControllerTypes(It.IsAny<IAssembliesResolver>()))
140+
.Returns(new Collection<Type> { controllerType });
141+
142+
HttpRequestMessage request = new HttpRequestMessage();
143+
IHttpRouteData routeData = GetRouteData();
144+
routeData.Values["controller"] = "Sample";
145+
request.Properties[HttpPropertyKeys.HttpRouteDataKey] = routeData;
146+
147+
DefaultHttpControllerSelector selector = new DefaultHttpControllerSelector(configuration);
148+
149+
// Act
150+
HttpControllerDescriptor descriptor1 = selector.SelectController(request);
151+
HttpControllerDescriptor descriptor2 = selector.SelectController(request);
152+
153+
// Assert
154+
Assert.ReferenceEquals(descriptor1, descriptor2);
155+
}
156+
157+
[Fact]
158+
public void SelectController_DoesNotCreateNewInstances_ForSameController_DiferentCasedControllerName()
159+
{
160+
HttpConfiguration configuration = new HttpConfiguration();
161+
Mock<IHttpControllerTypeResolver> controllerTypeResolver = new Mock<IHttpControllerTypeResolver>();
162+
configuration.ServiceResolver.SetService(typeof(IHttpControllerTypeResolver), controllerTypeResolver.Object);
163+
164+
Type controllerType = GetMockControllerType("Sample");
165+
controllerTypeResolver
166+
.Setup(c => c.GetControllerTypes(It.IsAny<IAssembliesResolver>()))
167+
.Returns(new Collection<Type> { controllerType });
168+
169+
HttpRequestMessage request1 = new HttpRequestMessage();
170+
IHttpRouteData routeData1 = GetRouteData();
171+
routeData1.Values["controller"] = "Sample";
172+
request1.Properties[HttpPropertyKeys.HttpRouteDataKey] = routeData1;
173+
174+
HttpRequestMessage request2 = new HttpRequestMessage();
175+
IHttpRouteData routeData2 = GetRouteData();
176+
routeData2.Values["controller"] = "SaMPle";
177+
request2.Properties[HttpPropertyKeys.HttpRouteDataKey] = routeData2;
178+
179+
DefaultHttpControllerSelector selector = new DefaultHttpControllerSelector(configuration);
180+
181+
// Act
182+
HttpControllerDescriptor descriptor1 = selector.SelectController(request1);
183+
HttpControllerDescriptor descriptor2 = selector.SelectController(request2);
184+
185+
// Assert
186+
Assert.ReferenceEquals(descriptor1, descriptor2);
187+
}
188+
189+
[Fact]
190+
public void SelectController_Throws_NullRequest()
191+
{
192+
DefaultHttpControllerSelector selector = new DefaultHttpControllerSelector(new HttpConfiguration());
193+
194+
Assert.ThrowsArgumentNull(
195+
() => selector.SelectController(request: null),
196+
"request");
197+
}
198+
199+
[Fact]
200+
public void SelectController_Throws_NotFound_NoControllerInRouteData()
201+
{
202+
HttpConfiguration configuration = new HttpConfiguration();
203+
HttpRequestMessage request = new HttpRequestMessage();
204+
request.Properties[HttpPropertyKeys.HttpRouteDataKey] = GetRouteData();
205+
206+
DefaultHttpControllerSelector selector = new DefaultHttpControllerSelector(configuration);
207+
208+
// Act
209+
var ex = Assert.Throws<HttpResponseException>(
210+
() => selector.SelectController(request));
211+
212+
// Assert
213+
Assert.Equal(HttpStatusCode.NotFound, ex.Response.StatusCode);
214+
}
215+
216+
[Fact]
217+
public void SelectController_Throws_NotFound_NoRouteData()
218+
{
219+
HttpConfiguration configuration = new HttpConfiguration();
220+
HttpRequestMessage request = new HttpRequestMessage();
221+
222+
DefaultHttpControllerSelector selector = new DefaultHttpControllerSelector(configuration);
223+
224+
// Act
225+
var ex = Assert.Throws<HttpResponseException>(
226+
() => selector.SelectController(request));
227+
228+
// Assert
229+
Assert.Equal(HttpStatusCode.NotFound, ex.Response.StatusCode);
230+
}
231+
232+
233+
[Fact]
234+
public void SelectController_Throws_NotFound_NoMatchingControllerType()
235+
{
236+
HttpConfiguration configuration = new HttpConfiguration();
237+
Mock<IHttpControllerTypeResolver> controllerTypeResolver = new Mock<IHttpControllerTypeResolver>();
238+
configuration.ServiceResolver.SetService(typeof(IHttpControllerTypeResolver), controllerTypeResolver.Object);
239+
240+
controllerTypeResolver
241+
.Setup(c => c.GetControllerTypes(It.IsAny<IAssembliesResolver>()))
242+
.Returns(new Collection<Type>());
243+
244+
HttpRequestMessage request = new HttpRequestMessage();
245+
IHttpRouteData routeData1 = GetRouteData();
246+
routeData1.Values["controller"] = "Sample";
247+
request.Properties[HttpPropertyKeys.HttpRouteDataKey] = routeData1;
248+
request.Properties[HttpPropertyKeys.HttpConfigurationKey] = configuration;
249+
250+
DefaultHttpControllerSelector selector = new DefaultHttpControllerSelector(configuration);
251+
252+
// Act
253+
var ex = Assert.Throws<HttpResponseException>(
254+
() => selector.SelectController(request));
255+
256+
// Assert
257+
Assert.Equal(HttpStatusCode.NotFound, ex.Response.StatusCode);
258+
Assert.Equal("\"No type was found that matches the controller named 'Sample'.\"", ex.Response.Content.ReadAsStringAsync().Result);
259+
}
260+
261+
[Fact]
262+
public void SelectController_Throws_DuplicateController()
263+
{
264+
HttpConfiguration configuration = new HttpConfiguration();
265+
Mock<IHttpControllerTypeResolver> controllerTypeResolver = new Mock<IHttpControllerTypeResolver>();
266+
configuration.ServiceResolver.SetService(typeof(IHttpControllerTypeResolver), controllerTypeResolver.Object);
267+
268+
controllerTypeResolver
269+
.Setup(c => c.GetControllerTypes(It.IsAny<IAssembliesResolver>()))
270+
.Returns(new Collection<Type> { GetMockControllerType("Sample"), GetMockControllerType("SampLe"), GetMockControllerType("SAmpLE") });
271+
272+
HttpRequestMessage request = new HttpRequestMessage();
273+
IHttpRouteData routeData1 = GetRouteData();
274+
routeData1.Values["controller"] = "Sample";
275+
request.Properties[HttpPropertyKeys.HttpRouteDataKey] = routeData1;
276+
request.Properties[HttpPropertyKeys.HttpConfigurationKey] = configuration;
277+
278+
DefaultHttpControllerSelector selector = new DefaultHttpControllerSelector(configuration);
279+
280+
// Act
281+
var ex = Assert.Throws<HttpResponseException>(
282+
() => selector.SelectController(request));
283+
284+
// Assert
285+
Assert.Equal(HttpStatusCode.InternalServerError, ex.Response.StatusCode);
286+
string response = ex.Response.Content.ReadAsAsync<string>().Result;
287+
Assert.Contains(
288+
"Multiple types were found that match the controller named 'Sample'. This can happen if the route that services this request ('') found multiple controllers defined with the same name but differing namespaces, which is not supported.\r\n\r\nThe request for 'Sample' has found the following matching controllers:",
289+
response);
290+
291+
var duplicateControllers = response.Split(':')[1].Split('\n').Select(str => str.Trim());
292+
Assert.Contains("FullSampleController", duplicateControllers);
293+
Assert.Contains("FullSampLeController", duplicateControllers);
294+
Assert.Contains("FullSAmpLEController", duplicateControllers);
295+
}
296+
297+
private static IHttpRouteData GetRouteData()
298+
{
299+
IHttpRoute mockRoute = new Mock<IHttpRoute>().Object;
300+
HttpRouteData routeData = new HttpRouteData(mockRoute);
301+
302+
return routeData;
303+
}
304+
305+
private static Type GetMockControllerType(string controllerName)
306+
{
307+
Mock<Type> mockType = new Mock<Type>();
308+
309+
mockType.Setup(t => t.Name).Returns(controllerName + "Controller");
310+
mockType.Setup(t => t.FullName).Returns("Full" + controllerName + "Controller");
311+
mockType.Setup(t => t.GetCustomAttributes(typeof(HttpControllerConfigurationAttribute), It.IsAny<bool>()))
312+
.Returns(new HttpControllerConfigurationAttribute[0]);
313+
return mockType.Object;
314+
}
315+
}
316+
}

‎test/System.Web.Http.Test/System.Web.Http.Test.csproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@
8181
<Compile Include="Controllers\HttpConfigurationTest.cs" />
8282
<Compile Include="Controllers\VoidResultConverterTest.cs" />
8383
<Compile Include="DictionaryExtensionsTest.cs" />
84+
<Compile Include="Dispatcher\DefaultHttpControllerSelectorTest.cs" />
8485
<Compile Include="Filters\HttpFilterCollectionTest.cs" />
8586
<Compile Include="HttpResponseMessageExtensionsTest.cs" />
8687
<Compile Include="HttpResponseExceptionTest.cs" />

0 commit comments

Comments
 (0)