Skip to content

Commit

Permalink
[Blazor] Fixes issues with route precedence (#27907)
Browse files Browse the repository at this point in the history
Description
In 5.0 we introduced two features on Blazor routing that enable users to write routing templates that match paths with variable length segments. These two features are optional parameters {parameter?} and catch all parameters {*catchall}.

Our routing system ordered the routes based on precedence and the (now false) assumption that route templates would only match paths with an equal number of segments.

The implementation that we have worked for naïve scenarios but breaks on more real world scenarios. The change here includes fixes to the way we order the routes in the route table to match the expectations as well as fixes on the route matching algorithm to ensure we match routes with variable number of segments correctly.

Customer Impact
This was reported by customers on #27250

The impact is that a route with {*catchall} will prevent more specific routes like /page/{parameter} from being accessible.

There are no workarounds since precedence is a fundamental behavior of the routing system.

Regression?
No, these Blazor features were initially added in 5.0.

Risk
Low. These two features were just introduced in 5.0 and their usage is not as prevalent as in asp.net core routing. That said, it's important to fix them as otherwise we run the risk of diverting in behavior from asp.net core routing and Blazor routing, which is not something we want to do.

We have functional tests covering the area and we've added a significant amount of unit tests to validate the changes.
  • Loading branch information
javiercn authored Nov 20, 2020
1 parent d14b644 commit 6bb4a3f
Show file tree
Hide file tree
Showing 41 changed files with 2,601 additions and 272 deletions.
2 changes: 2 additions & 0 deletions src/Components/Components/src/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
@@ -1 +1,3 @@
#nullable enable
Microsoft.AspNetCore.Components.Routing.Router.PreferExactMatches.get -> bool
Microsoft.AspNetCore.Components.Routing.Router.PreferExactMatches.set -> void
15 changes: 15 additions & 0 deletions src/Components/Components/src/Routing/IRouteTable.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

namespace Microsoft.AspNetCore.Components.Routing
{
/// <summary>
/// Provides an abstraction over <see cref="RouteTable"/> and <see cref="LegacyRouteMatching.LegacyRouteTable"/>.
/// This is only an internal implementation detail of <see cref="Router"/> and can be removed once
/// the legacy route matching logic is removed.
/// </summary>
internal interface IRouteTable
{
void Route(RouteContext routeContext);
}
}
Original file line number Diff line number Diff line change
@@ -1,20 +1,20 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

namespace Microsoft.AspNetCore.Components.Routing
namespace Microsoft.AspNetCore.Components.LegacyRouteMatching
{
/// <summary>
/// A route constraint that allows the value to be null or parseable as the specified
/// type.
/// </summary>
/// <typeparam name="T">The type to which the value must be parseable.</typeparam>
internal class OptionalTypeRouteConstraint<T> : RouteConstraint
internal class LegacyOptionalTypeRouteConstraint<T> : LegacyRouteConstraint
{
public delegate bool TryParseDelegate(string str, out T result);
public delegate bool LegacyTryParseDelegate(string str, out T result);

private readonly TryParseDelegate _parser;
private readonly LegacyTryParseDelegate _parser;

public OptionalTypeRouteConstraint(TryParseDelegate parser)
public LegacyOptionalTypeRouteConstraint(LegacyTryParseDelegate parser)
{
_parser = parser;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

using System;
using System.Collections.Concurrent;
using System.Globalization;

namespace Microsoft.AspNetCore.Components.LegacyRouteMatching
{
internal abstract class LegacyRouteConstraint
{
// note: the things that prevent this cache from growing unbounded is that
// we're the only caller to this code path, and the fact that there are only
// 8 possible instances that we create.
//
// The values passed in here for parsing are always static text defined in route attributes.
private static readonly ConcurrentDictionary<string, LegacyRouteConstraint> _cachedConstraints
= new ConcurrentDictionary<string, LegacyRouteConstraint>();

public abstract bool Match(string pathSegment, out object? convertedValue);

public static LegacyRouteConstraint Parse(string template, string segment, string constraint)
{
if (string.IsNullOrEmpty(constraint))
{
throw new ArgumentException($"Malformed segment '{segment}' in route '{template}' contains an empty constraint.");
}

if (_cachedConstraints.TryGetValue(constraint, out var cachedInstance))
{
return cachedInstance;
}
else
{
var newInstance = CreateRouteConstraint(constraint);
if (newInstance != null)
{
// We've done to the work to create the constraint now, but it's possible
// we're competing with another thread. GetOrAdd can ensure only a single
// instance is returned so that any extra ones can be GC'ed.
return _cachedConstraints.GetOrAdd(constraint, newInstance);
}
else
{
throw new ArgumentException($"Unsupported constraint '{constraint}' in route '{template}'.");
}
}
}

/// <summary>
/// Creates a structured RouteConstraint object given a string that contains
/// the route constraint. A constraint is the place after the colon in a
/// parameter definition, for example `{age:int?}`.
///
/// If the constraint denotes an optional, this method will return an
/// <see cref="LegacyOptionalTypeRouteConstraint{T}" /> which handles the appropriate checks.
/// </summary>
/// <param name="constraint">String representation of the constraint</param>
/// <returns>Type-specific RouteConstraint object</returns>
private static LegacyRouteConstraint? CreateRouteConstraint(string constraint)
{
switch (constraint)
{
case "bool":
return new LegacyTypeRouteConstraint<bool>(bool.TryParse);
case "bool?":
return new LegacyOptionalTypeRouteConstraint<bool>(bool.TryParse);
case "datetime":
return new LegacyTypeRouteConstraint<DateTime>((string str, out DateTime result)
=> DateTime.TryParse(str, CultureInfo.InvariantCulture, DateTimeStyles.None, out result));
case "datetime?":
return new LegacyOptionalTypeRouteConstraint<DateTime>((string str, out DateTime result)
=> DateTime.TryParse(str, CultureInfo.InvariantCulture, DateTimeStyles.None, out result));
case "decimal":
return new LegacyTypeRouteConstraint<decimal>((string str, out decimal result)
=> decimal.TryParse(str, NumberStyles.Number, CultureInfo.InvariantCulture, out result));
case "decimal?":
return new LegacyOptionalTypeRouteConstraint<decimal>((string str, out decimal result)
=> decimal.TryParse(str, NumberStyles.Number, CultureInfo.InvariantCulture, out result));
case "double":
return new LegacyTypeRouteConstraint<double>((string str, out double result)
=> double.TryParse(str, NumberStyles.Number, CultureInfo.InvariantCulture, out result));
case "double?":
return new LegacyOptionalTypeRouteConstraint<double>((string str, out double result)
=> double.TryParse(str, NumberStyles.Number, CultureInfo.InvariantCulture, out result));
case "float":
return new LegacyTypeRouteConstraint<float>((string str, out float result)
=> float.TryParse(str, NumberStyles.Number, CultureInfo.InvariantCulture, out result));
case "float?":
return new LegacyOptionalTypeRouteConstraint<float>((string str, out float result)
=> float.TryParse(str, NumberStyles.Number, CultureInfo.InvariantCulture, out result));
case "guid":
return new LegacyTypeRouteConstraint<Guid>(Guid.TryParse);
case "guid?":
return new LegacyOptionalTypeRouteConstraint<Guid>(Guid.TryParse);
case "int":
return new LegacyTypeRouteConstraint<int>((string str, out int result)
=> int.TryParse(str, NumberStyles.Integer, CultureInfo.InvariantCulture, out result));
case "int?":
return new LegacyOptionalTypeRouteConstraint<int>((string str, out int result)
=> int.TryParse(str, NumberStyles.Integer, CultureInfo.InvariantCulture, out result));
case "long":
return new LegacyTypeRouteConstraint<long>((string str, out long result)
=> long.TryParse(str, NumberStyles.Integer, CultureInfo.InvariantCulture, out result));
case "long?":
return new LegacyOptionalTypeRouteConstraint<long>((string str, out long result)
=> long.TryParse(str, NumberStyles.Integer, CultureInfo.InvariantCulture, out result));
default:
return null;
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,146 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

#nullable disable warnings

using System;
using System.Collections.Generic;
using System.Diagnostics;

// Avoid referencing the whole Microsoft.AspNetCore.Components.Routing namespace to
// avoid the risk of accidentally relying on the non-legacy types in the legacy fork
using RouteContext = Microsoft.AspNetCore.Components.Routing.RouteContext;

namespace Microsoft.AspNetCore.Components.LegacyRouteMatching
{
[DebuggerDisplay("Handler = {Handler}, Template = {Template}")]
internal class LegacyRouteEntry
{
public LegacyRouteEntry(LegacyRouteTemplate template, Type handler, string[] unusedRouteParameterNames)
{
Template = template;
UnusedRouteParameterNames = unusedRouteParameterNames;
Handler = handler;
}

public LegacyRouteTemplate Template { get; }

public string[] UnusedRouteParameterNames { get; }

public Type Handler { get; }

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.
else if (Template.OptionalSegmentsCount == 0 && Template.Segments.Length != context.Segments.Length)
{
return;
}

// Parameters will be lazily initialized.
Dictionary<string, object> parameters = null;
var numMatchingSegments = 0;
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:
//
// (1) If we are comparing a literal route with a literal template
// and the route is shorter than the template.
// (2) If we are comparing a template where the last value is an optional
// parameter that the route does not provide.
if (i >= context.Segments.Length)
{
// If we are under condition (1) above then we can stop evaluating
// matches on the rest of this template.
if (!segment.IsParameter && !segment.IsOptional)
{
break;
}
}

string pathSegment = null;
if (i < context.Segments.Length)
{
pathSegment = context.Segments[i];
}

if (!segment.Match(pathSegment, out var matchedParameterValue))
{
return;
}
else
{
numMatchingSegments++;
if (segment.IsParameter)
{
parameters ??= new Dictionary<string, object>(StringComparer.Ordinal);
parameters[segment.Value] = matchedParameterValue;
}
}
}

// 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 (!Template.ContainsCatchAllSegment && UnusedRouteParameterNames.Length > 0)
{
parameters ??= new Dictionary<string, object>(StringComparer.Ordinal);
for (var i = 0; i < UnusedRouteParameterNames.Length; i++)
{
parameters[UnusedRouteParameterNames[i]] = null;
}
}

// We track the number of segments in the template that matched
// against this particular route then only select the route that
// matches the most number of segments on the route that was passed.
// This check is an exactness check that favors the more precise of
// two templates in the event that the following route table exists.
// Route 1: /{anythingGoes}
// Route 2: /users/{id:int}
// And the provided route is `/users/1`. We want to choose Route 2
// over Route 1.
// Furthermore, literal routes are preferred over parameterized routes.
// If the two routes below are registered in the route table.
// Route 1: /users/1
// Route 2: /users/{id:int}
// And the provided route is `/users/1`. We want to choose Route 1 over
// Route 2.
var allRouteSegmentsMatch = numMatchingSegments >= context.Segments.Length;
// Checking that all route segments have been matches does not suffice if we are
// comparing literal templates with literal routes. For example, the template
// `/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 (Template.ContainsCatchAllSegment || (allRouteSegmentsMatch && allNonOptionalSegmentsMatch))
{
context.Parameters = parameters;
context.Handler = Handler;
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// Copyright (c) .NET Foundation. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.

// Avoid referencing the whole Microsoft.AspNetCore.Components.Routing namespace to
// avoid the risk of accidentally relying on the non-legacy types in the legacy fork
using RouteContext = Microsoft.AspNetCore.Components.Routing.RouteContext;

namespace Microsoft.AspNetCore.Components.LegacyRouteMatching
{
internal class LegacyRouteTable : Routing.IRouteTable
{
public LegacyRouteTable(LegacyRouteEntry[] routes)
{
Routes = routes;
}

public LegacyRouteEntry[] Routes { get; }

public void Route(RouteContext routeContext)
{
for (var i = 0; i < Routes.Length; i++)
{
Routes[i].Match(routeContext);
if (routeContext.Handler != null)
{
return;
}
}
}
}
}
Loading

0 comments on commit 6bb4a3f

Please sign in to comment.