Skip to content

Commit 7fdb6d2

Browse files
bergmaniabergmania
authored
Merge pull request #11130 from umbraco/v9/bugfix/dynamic-routing-fixes
UmbracoRouteValueTransformer fixes
2 parents cdae12a + 9306861 commit 7fdb6d2

File tree

4 files changed

+168
-25
lines changed

4 files changed

+168
-25
lines changed

‎src/Umbraco.Tests.UnitTests/Umbraco.Web.Website/Routing/UmbracoRouteValueTransformerTests.cs

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
using Umbraco.Cms.Core;
1515
using Umbraco.Cms.Core.Configuration.Models;
1616
using Umbraco.Cms.Core.Events;
17+
using Umbraco.Cms.Core.Models.PublishedContent;
1718
using Umbraco.Cms.Core.PublishedCache;
1819
using Umbraco.Cms.Core.Routing;
1920
using Umbraco.Cms.Core.Services;
@@ -92,36 +93,36 @@ private IPublishedRouter GetRouter(IPublishedRequest request)
9293
=> Mock.Of<IPublishedRouter>(x => x.RouteRequestAsync(It.IsAny<IPublishedRequestBuilder>(), It.IsAny<RouteRequestOptions>()) == Task.FromResult(request));
9394

9495
[Test]
95-
public async Task Noop_When_Runtime_Level_Not_Run()
96+
public async Task Null_When_Runtime_Level_Not_Run()
9697
{
9798
UmbracoRouteValueTransformer transformer = GetTransformer(
9899
Mock.Of<IUmbracoContextAccessor>(),
99100
Mock.Of<IRuntimeState>());
100101

101102
RouteValueDictionary result = await transformer.TransformAsync(new DefaultHttpContext(), new RouteValueDictionary());
102-
Assert.AreEqual(0, result.Count);
103+
Assert.IsNull(result);
103104
}
104105

105106
[Test]
106-
public async Task Noop_When_No_Umbraco_Context()
107+
public async Task Null_When_No_Umbraco_Context()
107108
{
108109
UmbracoRouteValueTransformer transformer = GetTransformerWithRunState(
109110
Mock.Of<IUmbracoContextAccessor>());
110111

111112
RouteValueDictionary result = await transformer.TransformAsync(new DefaultHttpContext(), new RouteValueDictionary());
112-
Assert.AreEqual(0, result.Count);
113+
Assert.IsNull(result);
113114
}
114115

115116
[Test]
116-
public async Task Noop_When_Not_Document_Request()
117+
public async Task Null_When_Not_Document_Request()
117118
{
118119
var umbracoContext = Mock.Of<IUmbracoContext>();
119120
UmbracoRouteValueTransformer transformer = GetTransformerWithRunState(
120121
Mock.Of<IUmbracoContextAccessor>(x => x.TryGetUmbracoContext(out umbracoContext)),
121122
Mock.Of<IRoutableDocumentFilter>(x => x.IsDocumentRequest(It.IsAny<string>()) == false));
122123

123124
RouteValueDictionary result = await transformer.TransformAsync(new DefaultHttpContext(), new RouteValueDictionary());
124-
Assert.AreEqual(0, result.Count);
125+
Assert.IsNull(result);
125126
}
126127

127128
[Test]
@@ -173,10 +174,10 @@ public async Task Assigns_UmbracoRouteValues_To_HttpContext_Feature()
173174
}
174175

175176
[Test]
176-
public async Task Assigns_Values_To_RouteValueDictionary()
177+
public async Task Assigns_Values_To_RouteValueDictionary_When_Content()
177178
{
178179
IUmbracoContext umbracoContext = GetUmbracoContext(true);
179-
IPublishedRequest request = Mock.Of<IPublishedRequest>();
180+
IPublishedRequest request = Mock.Of<IPublishedRequest>(x => x.PublishedContent == Mock.Of<IPublishedContent>());
180181
UmbracoRouteValues routeValues = GetRouteValues(request);
181182

182183
UmbracoRouteValueTransformer transformer = GetTransformerWithRunState(
@@ -190,6 +191,23 @@ public async Task Assigns_Values_To_RouteValueDictionary()
190191
Assert.AreEqual(routeValues.ActionName, result[ActionToken]);
191192
}
192193

194+
[Test]
195+
public async Task Returns_Null_RouteValueDictionary_When_No_Content()
196+
{
197+
IUmbracoContext umbracoContext = GetUmbracoContext(true);
198+
IPublishedRequest request = Mock.Of<IPublishedRequest>(x => x.PublishedContent == null);
199+
UmbracoRouteValues routeValues = GetRouteValues(request);
200+
201+
UmbracoRouteValueTransformer transformer = GetTransformerWithRunState(
202+
Mock.Of<IUmbracoContextAccessor>(x => x.TryGetUmbracoContext(out umbracoContext)),
203+
router: GetRouter(request),
204+
routeValuesFactory: GetRouteValuesFactory(request));
205+
206+
RouteValueDictionary result = await transformer.TransformAsync(new DefaultHttpContext(), new RouteValueDictionary());
207+
208+
Assert.IsNull(result);
209+
}
210+
193211
private class TestController : RenderController
194212
{
195213
public TestController(ILogger<TestController> logger, ICompositeViewEngine compositeViewEngine, IUmbracoContextAccessor umbracoContextAccessor)

‎src/Umbraco.Web.Website/DependencyInjection/UmbracoBuilderExtensions.cs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
using Microsoft.AspNetCore.Mvc;
2+
using Microsoft.AspNetCore.Routing;
23
using Microsoft.Extensions.DependencyInjection;
4+
using Microsoft.Extensions.DependencyInjection.Extensions;
35
using Microsoft.Extensions.Options;
46
using Umbraco.Cms.Core.DependencyInjection;
57
using Umbraco.Cms.Infrastructure.DependencyInjection;
@@ -11,6 +13,7 @@
1113
using Umbraco.Cms.Web.Website.Models;
1214
using Umbraco.Cms.Web.Website.Routing;
1315
using Umbraco.Cms.Web.Website.ViewEngines;
16+
using static Microsoft.Extensions.DependencyInjection.ServiceDescriptor;
1417

1518
namespace Umbraco.Extensions
1619
{
@@ -38,8 +41,9 @@ public static IUmbracoBuilder AddWebsite(this IUmbracoBuilder builder)
3841
builder.Services.AddDataProtection();
3942
builder.Services.AddAntiforgery();
4043

41-
builder.Services.AddScoped<UmbracoRouteValueTransformer>();
44+
builder.Services.AddSingleton<UmbracoRouteValueTransformer>();
4245
builder.Services.AddSingleton<IControllerActionSearcher, ControllerActionSearcher>();
46+
builder.Services.TryAddEnumerable(Singleton<MatcherPolicy, NotFoundSelectorPolicy>());
4347
builder.Services.AddSingleton<IUmbracoRouteValuesFactory, UmbracoRouteValuesFactory>();
4448
builder.Services.AddSingleton<IRoutableDocumentFilter, RoutableDocumentFilter>();
4549

‎src/Umbraco.Web.Website/Routing/NotFoundSelectorPolicy.cs

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using System.Linq;
4+
using System.Threading.Tasks;
5+
using Microsoft.AspNetCore.Http;
6+
using Microsoft.AspNetCore.Mvc;
7+
using Microsoft.AspNetCore.Mvc.Controllers;
8+
using Microsoft.AspNetCore.Routing;
9+
using Microsoft.AspNetCore.Routing.Matching;
10+
using Umbraco.Cms.Core.Routing;
11+
using Umbraco.Cms.Web.Common.Controllers;
12+
using Umbraco.Cms.Web.Common.Routing;
13+
14+
namespace Umbraco.Cms.Web.Website.Routing
15+
{
16+
/// <summary>
17+
/// Used to handle 404 routes that haven't been handled by the end user
18+
/// </summary>
19+
internal class NotFoundSelectorPolicy : MatcherPolicy, IEndpointSelectorPolicy
20+
{
21+
private readonly Lazy<Endpoint> _notFound;
22+
private readonly EndpointDataSource _endpointDataSource;
23+
24+
public NotFoundSelectorPolicy(EndpointDataSource endpointDataSource)
25+
{
26+
_notFound = new Lazy<Endpoint>(GetNotFoundEndpoint);
27+
_endpointDataSource = endpointDataSource;
28+
}
29+
30+
// return the endpoint for the RenderController.Index action.
31+
private Endpoint GetNotFoundEndpoint()
32+
{
33+
Endpoint e = _endpointDataSource.Endpoints.First(x =>
34+
{
35+
// return the endpoint for the RenderController.Index action.
36+
ControllerActionDescriptor descriptor = x.Metadata?.GetMetadata<ControllerActionDescriptor>();
37+
return descriptor?.ControllerTypeInfo == typeof(RenderController)
38+
&& descriptor?.ActionName == nameof(RenderController.Index);
39+
});
40+
return e;
41+
}
42+
43+
public override int Order => 0;
44+
45+
public bool AppliesToEndpoints(IReadOnlyList<Endpoint> endpoints)
46+
{
47+
// Don't apply this filter to any endpoint group that is a controller route
48+
// i.e. only dynamic routes.
49+
foreach (Endpoint endpoint in endpoints)
50+
{
51+
ControllerAttribute controller = endpoint.Metadata?.GetMetadata<ControllerAttribute>();
52+
if (controller != null)
53+
{
54+
return false;
55+
}
56+
}
57+
58+
// then ensure this is only applied if all endpoints are IDynamicEndpointMetadata
59+
return endpoints.All(x => x.Metadata?.GetMetadata<IDynamicEndpointMetadata>() != null);
60+
}
61+
62+
public Task ApplyAsync(HttpContext httpContext, CandidateSet candidates)
63+
{
64+
if (AllInvalid(candidates))
65+
{
66+
UmbracoRouteValues umbracoRouteValues = httpContext.Features.Get<UmbracoRouteValues>();
67+
if (umbracoRouteValues?.PublishedRequest != null
68+
&& !umbracoRouteValues.PublishedRequest.HasPublishedContent()
69+
&& umbracoRouteValues.PublishedRequest.ResponseStatusCode == StatusCodes.Status404NotFound)
70+
{
71+
// not found/404
72+
httpContext.SetEndpoint(_notFound.Value);
73+
}
74+
}
75+
76+
return Task.CompletedTask;
77+
}
78+
79+
private static bool AllInvalid(CandidateSet candidates)
80+
{
81+
for (int i = 0; i < candidates.Count; i++)
82+
{
83+
if (candidates.IsValidCandidate(i))
84+
{
85+
return false;
86+
}
87+
}
88+
return true;
89+
}
90+
}
91+
}

‎src/Umbraco.Web.Website/Routing/UmbracoRouteValueTransformer.cs

Lines changed: 46 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828

2929
namespace Umbraco.Cms.Web.Website.Routing
3030
{
31+
3132
/// <summary>
3233
/// The route value transformer for Umbraco front-end routes
3334
/// </summary>
@@ -92,31 +93,41 @@ public override async ValueTask<RouteValueDictionary> TransformAsync(HttpContext
9293
// If we aren't running, then we have nothing to route
9394
if (_runtime.Level != RuntimeLevel.Run)
9495
{
95-
return values;
96+
return null;
9697
}
9798
// will be null for any client side requests like JS, etc...
98-
if (!_umbracoContextAccessor.TryGetUmbracoContext(out var umbracoContext))
99+
if (!_umbracoContextAccessor.TryGetUmbracoContext(out IUmbracoContext umbracoContext))
99100
{
100-
return values;
101+
return null;
101102
}
102103

103104
if (!_routableDocumentFilter.IsDocumentRequest(httpContext.Request.Path))
104105
{
105-
return values;
106+
return null;
107+
}
108+
109+
// Don't execute if there are already UmbracoRouteValues assigned.
110+
// This can occur if someone else is dynamically routing and in which case we don't want to overwrite
111+
// the routing work being done there.
112+
UmbracoRouteValues umbracoRouteValues = httpContext.Features.Get<UmbracoRouteValues>();
113+
if (umbracoRouteValues != null)
114+
{
115+
return null;
106116
}
107117

108118
// Check if there is no existing content and return the no content controller
109119
if (!umbracoContext.Content.HasContent())
110120
{
111-
values[ControllerToken] = ControllerExtensions.GetControllerName<RenderNoContentController>();
112-
values[ActionToken] = nameof(RenderNoContentController.Index);
113-
114-
return values;
121+
return new RouteValueDictionary
122+
{
123+
[ControllerToken] = ControllerExtensions.GetControllerName<RenderNoContentController>(),
124+
[ActionToken] = nameof(RenderNoContentController.Index)
125+
};
115126
}
116127

117128
IPublishedRequest publishedRequest = await RouteRequestAsync(httpContext, umbracoContext);
118129

119-
UmbracoRouteValues umbracoRouteValues = await _routeValuesFactory.CreateAsync(httpContext, publishedRequest);
130+
umbracoRouteValues = await _routeValuesFactory.CreateAsync(httpContext, publishedRequest);
120131

121132
// Store the route values as a httpcontext feature
122133
httpContext.Features.Set(umbracoRouteValues);
@@ -125,16 +136,32 @@ public override async ValueTask<RouteValueDictionary> TransformAsync(HttpContext
125136
PostedDataProxyInfo postedInfo = GetFormInfo(httpContext, values);
126137
if (postedInfo != null)
127138
{
128-
return HandlePostedValues(postedInfo, httpContext, values);
139+
return HandlePostedValues(postedInfo, httpContext);
129140
}
130141

131-
values[ControllerToken] = umbracoRouteValues.ControllerName;
142+
if (!umbracoRouteValues?.PublishedRequest?.HasPublishedContent() ?? false)
143+
{
144+
// No content was found, not by any registered 404 handlers and
145+
// not by the IContentLastChanceFinder. In this case we want to return
146+
// our default 404 page but we cannot return route values now because
147+
// it's possible that a developer is handling dynamic routes too.
148+
// Our 404 page will be handled with the NotFoundSelectorPolicy
149+
return null;
150+
}
151+
152+
// See https://docs.microsoft.com/en-us/dotnet/api/microsoft.aspnetcore.mvc.routing.dynamicroutevaluetransformer.transformasync?view=aspnetcore-5.0#Microsoft_AspNetCore_Mvc_Routing_DynamicRouteValueTransformer_TransformAsync_Microsoft_AspNetCore_Http_HttpContext_Microsoft_AspNetCore_Routing_RouteValueDictionary_
153+
// We should apparenlty not be modified these values.
154+
// So we create new ones.
155+
var newValues = new RouteValueDictionary
156+
{
157+
[ControllerToken] = umbracoRouteValues.ControllerName
158+
};
132159
if (string.IsNullOrWhiteSpace(umbracoRouteValues.ActionName) == false)
133160
{
134-
values[ActionToken] = umbracoRouteValues.ActionName;
161+
newValues[ActionToken] = umbracoRouteValues.ActionName;
135162
}
136163

137-
return values;
164+
return newValues;
138165
}
139166

140167
private async Task<IPublishedRequest> RouteRequestAsync(HttpContext httpContext, IUmbracoContext umbracoContext)
@@ -196,11 +223,14 @@ private PostedDataProxyInfo GetFormInfo(HttpContext httpContext, RouteValueDicti
196223
};
197224
}
198225

199-
private RouteValueDictionary HandlePostedValues(PostedDataProxyInfo postedInfo, HttpContext httpContext, RouteValueDictionary values)
226+
private RouteValueDictionary HandlePostedValues(PostedDataProxyInfo postedInfo, HttpContext httpContext)
200227
{
201228
// set the standard route values/tokens
202-
values[ControllerToken] = postedInfo.ControllerName;
203-
values[ActionToken] = postedInfo.ActionName;
229+
var values = new RouteValueDictionary
230+
{
231+
[ControllerToken] = postedInfo.ControllerName,
232+
[ActionToken] = postedInfo.ActionName
233+
};
204234

205235
ControllerActionDescriptor surfaceControllerDescriptor = _controllerActionSearcher.Find<SurfaceController>(httpContext, postedInfo.ControllerName, postedInfo.ActionName);
206236

0 commit comments

Comments
 (0)