From 0889a62250e56066055d45a6fce99413d3f48d56 Mon Sep 17 00:00:00 2001 From: Zachary Becknell <zbecknell@users.noreply.github.com> Date: Fri, 17 Jul 2020 20:44:24 -0400 Subject: [PATCH] Fix #18539 - add Blazor catch-all route parameter (#24038) * Fix #18539 - add Blazor catch-all route parameter * Add E2E tests for catch-all parameter * Adjust E2E test for catch-all params * Remove ** scenarios for catch-all params * Fix typo causing test failure --- .../Components/src/Routing/RouteEntry.cs | 26 ++++++-- .../Components/src/Routing/RouteTemplate.cs | 3 + .../Components/src/Routing/TemplateParser.cs | 10 ++- .../Components/src/Routing/TemplateSegment.cs | 51 ++++++++++++--- .../test/Routing/RouteTableFactoryTests.cs | 40 ++++++++++++ .../test/Routing/TemplateParserTests.cs | 62 ++++++++++++++++++- .../test/E2ETest/Tests/RoutingTest.cs | 11 ++++ .../RouterTest/WithCatchAllParameter.razor | 7 +++ 8 files changed, 193 insertions(+), 17 deletions(-) create mode 100644 src/Components/test/testassets/BasicTestApp/RouterTest/WithCatchAllParameter.razor diff --git a/src/Components/Components/src/Routing/RouteEntry.cs b/src/Components/Components/src/Routing/RouteEntry.cs index e21517daf12..e18b71f0ef9 100644 --- a/src/Components/Components/src/Routing/RouteEntry.cs +++ b/src/Components/Components/src/Routing/RouteEntry.cs @@ -29,10 +29,20 @@ namespace Microsoft.AspNetCore.Components.Routing internal void Match(RouteContext context) { + string? catchAllValue = null; + + // If this template contains a catch-all parameter, we can concatenate the pathSegments + // at and beyond the catch-all segment's position. For example: + // Template: /foo/bar/{*catchAll} + // PathSegments: /foo/bar/one/two/three + if (Template.ContainsCatchAllSegment && context.Segments.Length >= Template.Segments.Length) + { + catchAllValue = string.Join('/', context.Segments[Range.StartAt(Template.Segments.Length - 1)]); + } // If there are no optional segments on the route and the length of the route // and the template do not match, then there is no chance of this matching and // we can bail early. - if (Template.OptionalSegmentsCount == 0 && Template.Segments.Length != context.Segments.Length) + else if (Template.OptionalSegmentsCount == 0 && Template.Segments.Length != context.Segments.Length) { return; } @@ -43,7 +53,15 @@ namespace Microsoft.AspNetCore.Components.Routing for (var i = 0; i < Template.Segments.Length; i++) { var segment = Template.Segments[i]; - + + if (segment.IsCatchAll) + { + numMatchingSegments += 1; + parameters ??= new Dictionary<string, object>(StringComparer.Ordinal); + parameters[segment.Value] = catchAllValue; + break; + } + // If the template contains more segments than the path, then // we may need to break out of this for-loop. This can happen // in one of two cases: @@ -86,7 +104,7 @@ namespace Microsoft.AspNetCore.Components.Routing // In addition to extracting parameter values from the URL, each route entry // also knows which other parameters should be supplied with null values. These // are parameters supplied by other route entries matching the same handler. - if (UnusedRouteParameterNames.Length > 0) + if (!Template.ContainsCatchAllSegment && UnusedRouteParameterNames.Length > 0) { parameters ??= new Dictionary<string, object>(StringComparer.Ordinal); for (var i = 0; i < UnusedRouteParameterNames.Length; i++) @@ -116,7 +134,7 @@ namespace Microsoft.AspNetCore.Components.Routing // `/this/is/a/template` and the route `/this/`. In that case, we want to ensure // that all non-optional segments have matched as well. var allNonOptionalSegmentsMatch = numMatchingSegments >= (Template.Segments.Length - Template.OptionalSegmentsCount); - if (allRouteSegmentsMatch && allNonOptionalSegmentsMatch) + if (Template.ContainsCatchAllSegment || (allRouteSegmentsMatch && allNonOptionalSegmentsMatch)) { context.Parameters = parameters; context.Handler = Handler; diff --git a/src/Components/Components/src/Routing/RouteTemplate.cs b/src/Components/Components/src/Routing/RouteTemplate.cs index 6f4541e896a..eb37454f6f6 100644 --- a/src/Components/Components/src/Routing/RouteTemplate.cs +++ b/src/Components/Components/src/Routing/RouteTemplate.cs @@ -15,6 +15,7 @@ namespace Microsoft.AspNetCore.Components.Routing TemplateText = templateText; Segments = segments; OptionalSegmentsCount = segments.Count(template => template.IsOptional); + ContainsCatchAllSegment = segments.Any(template => template.IsCatchAll); } public string TemplateText { get; } @@ -22,5 +23,7 @@ namespace Microsoft.AspNetCore.Components.Routing public TemplateSegment[] Segments { get; } public int OptionalSegmentsCount { get; } + + public bool ContainsCatchAllSegment { get; } } } diff --git a/src/Components/Components/src/Routing/TemplateParser.cs b/src/Components/Components/src/Routing/TemplateParser.cs index 3f9d5e1459c..41bc8ede28d 100644 --- a/src/Components/Components/src/Routing/TemplateParser.cs +++ b/src/Components/Components/src/Routing/TemplateParser.cs @@ -12,15 +12,15 @@ namespace Microsoft.AspNetCore.Components.Routing // The class in here just takes care of parsing a route and extracting // simple parameters from it. // Some differences with ASP.NET Core routes are: - // * We don't support catch all parameter segments. // * We don't support complex segments. // The things that we support are: // * Literal path segments. (Like /Path/To/Some/Page) // * Parameter path segments (Like /Customer/{Id}/Orders/{OrderId}) + // * Catch-all parameters (Like /blog/{*slug}) internal class TemplateParser { public static readonly char[] InvalidParameterNameCharacters = - new char[] { '*', '{', '}', '=', '.' }; + new char[] { '{', '}', '=', '.' }; internal static RouteTemplate ParseTemplate(string template) { @@ -80,6 +80,12 @@ namespace Microsoft.AspNetCore.Components.Routing for (int i = 0; i < templateSegments.Length; i++) { var currentSegment = templateSegments[i]; + + if (currentSegment.IsCatchAll && i != templateSegments.Length - 1) + { + throw new InvalidOperationException($"Invalid template '{template}'. A catch-all parameter can only appear as the last segment of the route template."); + } + if (!currentSegment.IsParameter) { continue; diff --git a/src/Components/Components/src/Routing/TemplateSegment.cs b/src/Components/Components/src/Routing/TemplateSegment.cs index ee2e58ceb44..c4d35199519 100644 --- a/src/Components/Components/src/Routing/TemplateSegment.cs +++ b/src/Components/Components/src/Routing/TemplateSegment.cs @@ -12,34 +12,48 @@ namespace Microsoft.AspNetCore.Components.Routing { IsParameter = isParameter; + IsCatchAll = segment.StartsWith('*'); + + if (IsCatchAll) + { + // Only one '*' currently allowed + Value = segment.Substring(1); + + var invalidCharacter = Value.IndexOf('*'); + if (Value.IndexOf('*') != -1) + { + throw new InvalidOperationException($"Invalid template '{template}'. A catch-all parameter may only have one '*' at the beginning of the segment."); + } + } + else + { + Value = segment; + } + // Process segments that are not parameters or do not contain // a token separating a type constraint. - if (!isParameter || segment.IndexOf(':') < 0) + if (!isParameter || Value.IndexOf(':') < 0) { // Set the IsOptional flag to true for segments that contain // a parameter with no type constraints but optionality set // via the '?' token. - if (segment.IndexOf('?') == segment.Length - 1) + if (Value.IndexOf('?') == Value.Length - 1) { IsOptional = true; - Value = segment.Substring(0, segment.Length - 1); + Value = Value.Substring(0, Value.Length - 1); } // If the `?` optional marker shows up in the segment but not at the very end, // then throw an error. - else if (segment.IndexOf('?') >= 0 && segment.IndexOf('?') != segment.Length - 1) + else if (Value.IndexOf('?') >= 0 && Value.IndexOf('?') != Value.Length - 1) { throw new ArgumentException($"Malformed parameter '{segment}' in route '{template}'. '?' character can only appear at the end of parameter name."); } - else - { - Value = segment; - } - + Constraints = Array.Empty<RouteConstraint>(); } else { - var tokens = segment.Split(':'); + var tokens = Value.Split(':'); if (tokens[0].Length == 0) { throw new ArgumentException($"Malformed parameter '{segment}' in route '{template}' has no name before the constraints list."); @@ -54,6 +68,21 @@ namespace Microsoft.AspNetCore.Components.Routing .Select(token => RouteConstraint.Parse(template, segment, token)) .ToArray(); } + + if (IsParameter) + { + if (IsOptional && IsCatchAll) + { + throw new InvalidOperationException($"Invalid segment '{segment}' in route '{template}'. A catch-all parameter cannot be marked optional."); + } + + // Moving the check for this here instead of TemplateParser so we can allow catch-all. + // We checked for '*' up above specifically for catch-all segments, this one checks for all others + if (Value.IndexOf('*') != -1) + { + throw new InvalidOperationException($"Invalid template '{template}'. The character '*' in parameter segment '{{{segment}}}' is not allowed."); + } + } } // The value of the segment. The exact text to match when is a literal. @@ -64,6 +93,8 @@ namespace Microsoft.AspNetCore.Components.Routing public bool IsOptional { get; } + public bool IsCatchAll { get; } + public RouteConstraint[] Constraints { get; } public bool Match(string pathSegment, out object? matchedParameterValue) diff --git a/src/Components/Components/test/Routing/RouteTableFactoryTests.cs b/src/Components/Components/test/Routing/RouteTableFactoryTests.cs index f8893aa0a46..3a4887fba09 100644 --- a/src/Components/Components/test/Routing/RouteTableFactoryTests.cs +++ b/src/Components/Components/test/Routing/RouteTableFactoryTests.cs @@ -226,6 +226,23 @@ namespace Microsoft.AspNetCore.Components.Test.Routing Assert.Single(context.Parameters, p => p.Key == "parameter" && (string)p.Value == expectedValue); } + [Theory] + [InlineData("/blog/value1", "value1")] + [InlineData("/blog/value1/foo%20bar", "value1/foo bar")] + public void CanMatchCatchAllParameterTemplate(string path, string expectedValue) + { + // Arrange + var routeTable = new TestRouteTableBuilder().AddRoute("/blog/{*parameter}").Build(); + var context = new RouteContext(path); + + // Act + routeTable.Route(context); + + // Assert + Assert.NotNull(context.Handler); + Assert.Single(context.Parameters, p => p.Key == "parameter" && (string)p.Value == expectedValue); + } + [Fact] public void CanMatchTemplateWithMultipleParameters() { @@ -247,6 +264,29 @@ namespace Microsoft.AspNetCore.Components.Test.Routing Assert.Equal(expectedParameters, context.Parameters); } + + [Fact] + public void CanMatchTemplateWithMultipleParametersAndCatchAllParameter() + { + // Arrange + var routeTable = new TestRouteTableBuilder().AddRoute("/{some}/awesome/{route}/with/{*catchAll}").Build(); + var context = new RouteContext("/an/awesome/path/with/some/catch/all/stuff"); + + var expectedParameters = new Dictionary<string, object> + { + ["some"] = "an", + ["route"] = "path", + ["catchAll"] = "some/catch/all/stuff" + }; + + // Act + routeTable.Route(context); + + // Assert + Assert.NotNull(context.Handler); + Assert.Equal(expectedParameters, context.Parameters); + } + public static IEnumerable<object[]> CanMatchParameterWithConstraintCases() => new object[][] { new object[] { "/{value:bool}", "/true", true }, diff --git a/src/Components/Components/test/Routing/TemplateParserTests.cs b/src/Components/Components/test/Routing/TemplateParserTests.cs index aba3f0496a2..1cd8ab88bf4 100644 --- a/src/Components/Components/test/Routing/TemplateParserTests.cs +++ b/src/Components/Components/test/Routing/TemplateParserTests.cs @@ -83,6 +83,45 @@ namespace Microsoft.AspNetCore.Components.Routing Assert.Equal(expected, actual, RouteTemplateTestComparer.Instance); } + [Fact] + public void Parse_SingleCatchAllParameter() + { + // Arrange + var expected = new ExpectedTemplateBuilder().Parameter("p"); + + // Act + var actual = TemplateParser.ParseTemplate("{*p}"); + + // Assert + Assert.Equal(expected, actual, RouteTemplateTestComparer.Instance); + } + + [Fact] + public void Parse_MixedLiteralAndCatchAllParameter() + { + // Arrange + var expected = new ExpectedTemplateBuilder().Literal("awesome").Literal("wow").Parameter("p"); + + // Act + var actual = TemplateParser.ParseTemplate("awesome/wow/{*p}"); + + // Assert + Assert.Equal(expected, actual, RouteTemplateTestComparer.Instance); + } + + [Fact] + public void Parse_MixedLiteralParameterAndCatchAllParameter() + { + // Arrange + var expected = new ExpectedTemplateBuilder().Literal("awesome").Parameter("p1").Parameter("p2"); + + // Act + var actual = TemplateParser.ParseTemplate("awesome/{p1}/{*p2}"); + + // Assert + Assert.Equal(expected, actual, RouteTemplateTestComparer.Instance); + } + [Fact] public void InvalidTemplate_WithRepeatedParameter() { @@ -113,7 +152,8 @@ namespace Microsoft.AspNetCore.Components.Routing } [Theory] - [InlineData("{*}", "Invalid template '{*}'. The character '*' in parameter segment '{*}' is not allowed.")] + // * is only allowed at beginning for catch-all parameters + [InlineData("{p*}", "Invalid template '{p*}'. The character '*' in parameter segment '{p*}' is not allowed.")] [InlineData("{{}", "Invalid template '{{}'. The character '{' in parameter segment '{{}' is not allowed.")] [InlineData("{}}", "Invalid template '{}}'. The character '}' in parameter segment '{}}' is not allowed.")] [InlineData("{=}", "Invalid template '{=}'. The character '=' in parameter segment '{=}' is not allowed.")] @@ -166,6 +206,26 @@ namespace Microsoft.AspNetCore.Components.Routing Assert.Equal(expectedMessage, ex.Message); } + [Fact] + public void InvalidTemplate_CatchAllParamWithMultipleAsterisks() + { + var ex = Assert.Throws<InvalidOperationException>(() => TemplateParser.ParseTemplate("/test/{a}/{**b}")); + + var expectedMessage = "Invalid template '/test/{a}/{**b}'. A catch-all parameter may only have one '*' at the beginning of the segment."; + + Assert.Equal(expectedMessage, ex.Message); + } + + [Fact] + public void InvalidTemplate_CatchAllParamNotLast() + { + var ex = Assert.Throws<InvalidOperationException>(() => TemplateParser.ParseTemplate("/test/{*a}/{b}")); + + var expectedMessage = "Invalid template 'test/{*a}/{b}'. A catch-all parameter can only appear as the last segment of the route template."; + + Assert.Equal(expectedMessage, ex.Message); + } + [Fact] public void InvalidTemplate_BadOptionalCharacterPosition() { diff --git a/src/Components/test/E2ETest/Tests/RoutingTest.cs b/src/Components/test/E2ETest/Tests/RoutingTest.cs index 3f3f3920ff9..2b0c708097d 100644 --- a/src/Components/test/E2ETest/Tests/RoutingTest.cs +++ b/src/Components/test/E2ETest/Tests/RoutingTest.cs @@ -109,6 +109,17 @@ namespace Microsoft.AspNetCore.Components.E2ETest.Tests Assert.Equal(expected, app.FindElement(By.Id("test-info")).Text); } + [Fact] + public void CanArriveAtPageWithCatchAllParameter() + { + SetUrlViaPushState("/WithCatchAllParameter/life/the/universe/and/everything%20%3D%2042"); + + var app = Browser.MountTestComponent<TestRouter>(); + var expected = $"The answer: life/the/universe/and/everything = 42."; + + Assert.Equal(expected, app.FindElement(By.Id("test-info")).Text); + } + [Fact] public void CanArriveAtNonDefaultPage() { diff --git a/src/Components/test/testassets/BasicTestApp/RouterTest/WithCatchAllParameter.razor b/src/Components/test/testassets/BasicTestApp/RouterTest/WithCatchAllParameter.razor new file mode 100644 index 00000000000..58928d407c1 --- /dev/null +++ b/src/Components/test/testassets/BasicTestApp/RouterTest/WithCatchAllParameter.razor @@ -0,0 +1,7 @@ +@page "/WithCatchAllParameter/{*theAnswer}" +<div id="test-info">The answer: @TheAnswer.</div> + +@code +{ + [Parameter] public string TheAnswer { get; set; } +} -- GitLab