diff --git a/src/Http/Routing/src/Template/TemplateBinder.cs b/src/Http/Routing/src/Template/TemplateBinder.cs index 36cf922a19..d7a93ba0ba 100644 --- a/src/Http/Routing/src/Template/TemplateBinder.cs +++ b/src/Http/Routing/src/Template/TemplateBinder.cs @@ -118,10 +118,9 @@ namespace Microsoft.AspNetCore.Routing.Template var slots = new KeyValuePair[_slots.Length]; Array.Copy(_slots, 0, slots, 0, slots.Length); - // Keeping track of the number of 'values' and 'ambient values' we've processed can be used to avoid doing + // Keeping track of the number of 'values' we've processed can be used to avoid doing // some expensive 'merge' operations later. var valueProcessedCount = 0; - var ambientValueProcessedCount = 0; // Start by copying all of the values out of the 'values' and into the slots. There's no success // case where we *don't* use all of the 'values' so there's no reason not to do this up front @@ -168,11 +167,6 @@ namespace Microsoft.AspNetCore.Routing.Template { ambientValue = null; } - else - { - // Track the count of processed ambient values - this allows a fast path later. - ambientValueProcessedCount++; - } // For now, only check ambient values with required values that don't have a parameter // Ambient values for parameters are processed below @@ -226,15 +220,10 @@ namespace Microsoft.AspNetCore.Routing.Template var parameter = parameters[i]; + // We are copying **all** ambient values if (copyAmbientValues) { hasAmbientValue = ambientValues != null && ambientValues.TryGetValue(key, out ambientValue); - if (hasAmbientValue) - { - // Track the count of processed ambient values - this allows a fast path later. - ambientValueProcessedCount++; - } - if (hasExplicitValue && hasAmbientValue && !RoutePartsEqual(ambientValue, value)) { // Stop copying current values when we find one that doesn't match @@ -261,6 +250,29 @@ namespace Microsoft.AspNetCore.Routing.Template } } + // This might be an ambient value that matches a required value. We want to use these even if we're + // not bulk-copying ambient values. + // + // This comes up in a case like the following: + // ambient-values: { page = "/DeleteUser", area = "Admin", } + // values: { controller = "Home", action = "Index", } + // pattern: {area}/{controller}/{action}/{id?} + // required-values: { area = "Admin", controller = "Home", action = "Index", page = (string)null, } + // + // OR in plain English... when linking from a page in an area to an action in the same area, it should + // be possible to use the area as an ambient value. + if (!copyAmbientValues && _pattern.RequiredValues.TryGetValue(key, out var requiredValue)) + { + hasAmbientValue = ambientValues != null && ambientValues.TryGetValue(key, out ambientValue); + if (hasAmbientValue && RoutePartsEqual(requiredValue, ambientValue)) + { + // Treat this an an explicit value to *force it*. + slots[i] = new KeyValuePair(key, ambientValue); + hasExplicitValue = true; + value = ambientValue; + } + } + // If the parameter is a match, add it to the list of values we will use for URI generation if (hasExplicitValue && !ReferenceEquals(value, SentinullValue.Instance)) { @@ -345,15 +357,13 @@ namespace Microsoft.AspNetCore.Routing.Template // Currently this copy is required because BindValues will mutate the accepted values :( var combinedValues = new RouteValueDictionary(acceptedValues); - if (ambientValueProcessedCount < (ambientValues?.Count ?? 0)) - { - // Add any ambient values that don't match parameters - they need to be visible to constraints - // but they will ignored by link generation. - CopyNonParameterAmbientValues( - ambientValues: ambientValues, - acceptedValues: acceptedValues, - combinedValues: combinedValues); - } + + // Add any ambient values that don't match parameters - they need to be visible to constraints + // but they will ignored by link generation. + CopyNonParameterAmbientValues( + ambientValues: ambientValues, + acceptedValues: acceptedValues, + combinedValues: combinedValues); return new TemplateValuesResult() { diff --git a/src/Http/Routing/test/UnitTests/Template/TemplateBinderTests.cs b/src/Http/Routing/test/UnitTests/Template/TemplateBinderTests.cs index 40de2c08cc..f5f497e4b2 100644 --- a/src/Http/Routing/test/UnitTests/Template/TemplateBinderTests.cs +++ b/src/Http/Routing/test/UnitTests/Template/TemplateBinderTests.cs @@ -1381,6 +1381,40 @@ namespace Microsoft.AspNetCore.Routing.Template.Tests Assert.Equal(expected, boundTemplate); } + // Regression test for aspnet/AspNetCore#4212 + // + // An ambient value should be used to satisfy a required value even if if we're discarding + // ambient values. + [Fact] + public void BindValues_LinkingFromPageToAControllerInAreaWithAmbientArea_Success() + { + // Arrange + var expected = "/Admin/LG2/SomeAction"; + + var template = "{area}/{controller=Home}/{action=Index}/{id?}"; + var defaults = new RouteValueDictionary(); + var ambientValues = new RouteValueDictionary(new { area = "Admin", page = "/LGAnotherPage", id = "17" }); + var explicitValues = new RouteValueDictionary(new { controller = "LG2", action = "SomeAction" }); + var binder = new TemplateBinder( + UrlEncoder.Default, + new DefaultObjectPoolProvider().Create(new UriBuilderContextPooledObjectPolicy()), + RoutePatternFactory.Parse( + template, + defaults, + parameterPolicies: null, + requiredValues: new { area = "Admin", action = "SomeAction", controller = "LG2", page = (string)null }), + defaults, + requiredKeys: new string[] { "area", "action", "controller", "page" }, + parameterPolicies: null); + + // Act + var result = binder.GetValues(ambientValues, explicitValues); + var boundTemplate = binder.BindValues(result.AcceptedValues); + + // Assert + Assert.Equal(expected, boundTemplate); + } + [Fact] public void BindValues_HasUnmatchingAmbientValues_Discard() { diff --git a/src/Mvc/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RazorPagesWithBasePathTest.cs b/src/Mvc/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RazorPagesWithBasePathTest.cs index 1b833e2ce8..4a11189cb2 100644 --- a/src/Mvc/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RazorPagesWithBasePathTest.cs +++ b/src/Mvc/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RazorPagesWithBasePathTest.cs @@ -323,7 +323,7 @@ Hello from page"; var expected = @"Link inside area Link to external area -Link to area action +Link to area action Link to non-area page"; // Act