From 494c9f1b42d2eb9103a17374238230d6334596e7 Mon Sep 17 00:00:00 2001
From: Shannon <sdeminick@gmail.com>
Date: Fri, 17 Sep 2021 10:29:24 -0600
Subject: [PATCH] UmbracoRouteValueTransformer fixes

---
 .../UmbracoBuilderExtensions.cs               |  2 +-
 .../Routing/UmbracoRouteValueTransformer.cs   | 56 +++++++++++++------
 2 files changed, 41 insertions(+), 17 deletions(-)

diff --git a/src/Umbraco.Web.Website/DependencyInjection/UmbracoBuilderExtensions.cs b/src/Umbraco.Web.Website/DependencyInjection/UmbracoBuilderExtensions.cs
index 62ec5a9921..744bd13388 100644
--- a/src/Umbraco.Web.Website/DependencyInjection/UmbracoBuilderExtensions.cs
+++ b/src/Umbraco.Web.Website/DependencyInjection/UmbracoBuilderExtensions.cs
@@ -38,7 +38,7 @@ namespace Umbraco.Extensions
             builder.Services.AddDataProtection();
             builder.Services.AddAntiforgery();
 
-            builder.Services.AddScoped<UmbracoRouteValueTransformer>();
+            builder.Services.AddSingleton<UmbracoRouteValueTransformer>();
             builder.Services.AddSingleton<IControllerActionSearcher, ControllerActionSearcher>();
             builder.Services.AddSingleton<IUmbracoRouteValuesFactory, UmbracoRouteValuesFactory>();
             builder.Services.AddSingleton<IRoutableDocumentFilter, RoutableDocumentFilter>();
diff --git a/src/Umbraco.Web.Website/Routing/UmbracoRouteValueTransformer.cs b/src/Umbraco.Web.Website/Routing/UmbracoRouteValueTransformer.cs
index 41fd20a69b..9e0b71b61e 100644
--- a/src/Umbraco.Web.Website/Routing/UmbracoRouteValueTransformer.cs
+++ b/src/Umbraco.Web.Website/Routing/UmbracoRouteValueTransformer.cs
@@ -92,31 +92,41 @@ namespace Umbraco.Cms.Web.Website.Routing
             // If we aren't running, then we have nothing to route
             if (_runtime.Level != RuntimeLevel.Run)
             {
-                return values;
+                return null;
             }
             // will be null for any client side requests like JS, etc...
-            if (!_umbracoContextAccessor.TryGetUmbracoContext(out var umbracoContext))
+            if (!_umbracoContextAccessor.TryGetUmbracoContext(out IUmbracoContext umbracoContext))
             {
-                return values;
+                return null;
             }
 
             if (!_routableDocumentFilter.IsDocumentRequest(httpContext.Request.Path))
             {
-                return values;
+                return null;
+            }
+
+            // Don't execute if there are already UmbracoRouteValues assigned.
+            // This can occur if someone else is dynamically routing and in which case we don't want to overwrite
+            // the routing work being done there.
+            UmbracoRouteValues umbracoRouteValues = httpContext.Features.Get<UmbracoRouteValues>();
+            if (umbracoRouteValues != null)
+            {
+                return null;
             }
 
             // Check if there is no existing content and return the no content controller
             if (!umbracoContext.Content.HasContent())
             {
-                values[ControllerToken] = ControllerExtensions.GetControllerName<RenderNoContentController>();
-                values[ActionToken] = nameof(RenderNoContentController.Index);
-
-                return values;
+                return new RouteValueDictionary
+                {
+                    [ControllerToken] = ControllerExtensions.GetControllerName<RenderNoContentController>(),
+                    [ActionToken] = nameof(RenderNoContentController.Index)
+                };
             }
 
             IPublishedRequest publishedRequest = await RouteRequestAsync(httpContext, umbracoContext);
 
-            UmbracoRouteValues umbracoRouteValues = await _routeValuesFactory.CreateAsync(httpContext, publishedRequest);
+            umbracoRouteValues = await _routeValuesFactory.CreateAsync(httpContext, publishedRequest);
 
             // Store the route values as a httpcontext feature
             httpContext.Features.Set(umbracoRouteValues);
@@ -125,16 +135,27 @@ namespace Umbraco.Cms.Web.Website.Routing
             PostedDataProxyInfo postedInfo = GetFormInfo(httpContext, values);
             if (postedInfo != null)
             {
-                return HandlePostedValues(postedInfo, httpContext, values);
+                return HandlePostedValues(postedInfo, httpContext);
             }
 
-            values[ControllerToken] = umbracoRouteValues.ControllerName;
+            // 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_
+            // We should apparenlty not be modified these values.
+            // So we create new ones.
+            var newValues = new RouteValueDictionary
+            {
+                [ControllerToken] = umbracoRouteValues.ControllerName
+            };
             if (string.IsNullOrWhiteSpace(umbracoRouteValues.ActionName) == false)
             {
-                values[ActionToken] = umbracoRouteValues.ActionName;
+                newValues[ActionToken] = umbracoRouteValues.ActionName;
             }
 
-            return values;
+            // NOTE: If we are never returning null it means that it is not possible for another
+            // DynamicRouteValueTransformer to execute to set the route values. This one will
+            // always win even if it is a 404 because we manage all 404s via Umbraco and 404
+            // handlers.
+
+            return newValues;
         }
 
         private async Task<IPublishedRequest> RouteRequestAsync(HttpContext httpContext, IUmbracoContext umbracoContext)
@@ -196,11 +217,14 @@ namespace Umbraco.Cms.Web.Website.Routing
             };
         }
 
-        private RouteValueDictionary HandlePostedValues(PostedDataProxyInfo postedInfo, HttpContext httpContext, RouteValueDictionary values)
+        private RouteValueDictionary HandlePostedValues(PostedDataProxyInfo postedInfo, HttpContext httpContext)
         {
             // set the standard route values/tokens
-            values[ControllerToken] = postedInfo.ControllerName;
-            values[ActionToken] = postedInfo.ActionName;
+            var values = new RouteValueDictionary
+            {
+                [ControllerToken] = postedInfo.ControllerName,
+                [ActionToken] = postedInfo.ActionName
+            };
 
             ControllerActionDescriptor surfaceControllerDescriptor = _controllerActionSearcher.Find<SurfaceController>(httpContext, postedInfo.ControllerName, postedInfo.ActionName);