Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 1 addition & 7 deletions src/Http/Routing/src/Patterns/RoutePattern.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics;
using System.Linq;
using Microsoft.AspNetCore.Routing.Template;

namespace Microsoft.AspNetCore.Routing.Patterns;
Expand Down Expand Up @@ -35,8 +34,6 @@ internal static bool IsRequiredValueAny(object? value)
return object.ReferenceEquals(RequiredValueAny, value);
}

private const string SeparatorString = "/";

internal RoutePattern(
string? rawText,
IReadOnlyDictionary<string, object?> defaults,
Expand Down Expand Up @@ -158,10 +155,7 @@ internal RoutePattern(
// 1. RoutePattern debug string.
// 2. Default IRouteDiagnosticsMetadata value.
// 3. RouteEndpoint display name.
internal string DebuggerToString()
{
return RawText ?? string.Join(SeparatorString, PathSegments.Select(s => s.DebuggerToString()));
}
internal string DebuggerToString() => RoutePatternDebugStringFormatter.Format(this);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation Recommended

4. Metrics tag cardinality 🐞 Bug

• The default IRouteDiagnosticsMetadata.Route is used as a metrics tag; substituting RequiredValues
  (controller/action names) changes the tag from a generic template to a per-endpoint value.
• This increases metric label cardinality (bounded by number of endpoints, but can be large in big
  MVC apps) and may increase memory/CPU cost in metrics pipelines/backends.
Agent Prompt
### Issue description
`IRouteDiagnosticsMetadata.Route` is used as a metrics tag. With required-value substitution, the value becomes more endpoint-specific (e.g., controller/action names), increasing tag cardinality.

### Issue Context
This is a trade-off: better per-endpoint readability vs higher cardinality. Cardinality is bounded by endpoint count but can be large for big apps.

### Fix Focus Areas
- src/Http/Routing/src/Patterns/RoutePattern.cs[154-159]
- src/Http/Routing/src/RouteEndpointBuilder.cs[108-112]
- src/Http/Routing/src/EndpointRoutingMiddleware.cs[129-133]
- src/Http/Routing/src/Patterns/RoutePatternDebugStringFormatter.cs[13-45]

### Implementation notes
Options to consider:
- Keep the substituted formatter for `DisplayName`, but use `RoutePattern.RawText` (or a template-only formatter) for default `IRouteDiagnosticsMetadata.Route`.
- Alternatively add a second API (e.g., `GetDiagnosticsRouteString()` vs `GetDebuggerDisplayString()`) and use the lower-cardinality one for metrics.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


[DebuggerDisplay("{DebuggerToString(),nq}")]
private sealed class RequiredValueAnySentinal
Expand Down
103 changes: 103 additions & 0 deletions src/Http/Routing/src/Patterns/RoutePatternDebugStringFormatter.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics.CodeAnalysis;

namespace Microsoft.AspNetCore.Routing.Patterns;

internal static class RoutePatternDebugStringFormatter
{
private const char Separator = '/';
private const string SeparatorString = "/";

public static string Format(RoutePattern pattern)
{
// If there are no required values that match parameters, use the simple approach
if (pattern.RawText is { Length: > 0 } rawText && !HasMatchingRequiredValues(pattern))
{
return rawText;
}

// Build the string replacing parameters with their required values when available
var segments = new string[pattern.PathSegments.Count];
for (var i = 0; i < pattern.PathSegments.Count; i++)
{
var segment = pattern.PathSegments[i];
var segmentString = GetSegmentDebuggerToString(pattern, segment);
segments[i] = segmentString;
}

var result = string.Join(SeparatorString, segments);

// Preserve leading slash from raw text
if (pattern.RawText is { Length: > 0 } rt && rt[0] == Separator)
{
result = Separator + result;
}
Comment on lines +32 to +36

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation Recommended

2. Drops tilde-slash prefix 🐞 Bug

• When RequiredValues match parameters, RoutePatternDebugStringFormatter rebuilds the string from
  segments and only re-adds a leading '/' (not "~/").
• For patterns whose RawText starts with "~/" (supported by RoutePatternParser), this means the
  returned debug string loses that prefix only in the rebuild/substitution path.
Agent Prompt
### Issue description
When required-value substitution triggers the rebuild path, `RoutePatternDebugStringFormatter.Format` only preserves a leading `/`. Templates starting with `~/` will lose that prefix in the returned debug string.

### Issue Context
`RoutePatternParser` allows `~/` and strips it for parsing, but `RoutePattern.RawText` retains the original input. The formatter should preserve the same prefix semantics as the original raw template, even when rebuilding.

### Fix Focus Areas
- src/Http/Routing/src/Patterns/RoutePatternDebugStringFormatter.cs[13-45]
- src/Http/Routing/src/Patterns/RoutePatternParser.cs[457-472]
- src/Http/Routing/test/UnitTests/Patterns/RoutePatternDebugStringFormatterTest.cs[197-230]

### Implementation notes
- Add handling like:
  - if `RawText` starts with `"~/"`, prefix the result with `"~/"`
  - else if `RawText` starts with `/`, prefix with `/`
- Add unit test coverage for `template="~/{controller}/{action}"` with required values causing substitution.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


// Return "/" for empty results
if (result.Length == 0)
{
return SeparatorString;
}

return result;
}

private static bool HasMatchingRequiredValues(RoutePattern pattern)
{
if (pattern.RequiredValues.Count == 0)
{
return false;
}

for (var i = 0; i < pattern.Parameters.Count; i++)
{
if (TryGetRequiredValue(pattern, pattern.Parameters[i].Name, out _))
{
return true;
}
}

return false;
}

private static string GetSegmentDebuggerToString(RoutePattern pattern, RoutePatternPathSegment segment)
{
// Simple segment with single parameter that has a required value - just return the required value
if (segment.IsSimple && segment.Parts[0] is RoutePatternParameterPart parameter)
{
if (TryGetRequiredValue(pattern, parameter.Name, out var requiredValue))
{
return requiredValue;
}
return parameter.DebuggerToString();
}

// For complex segments, build the string part by part
var parts = new string[segment.Parts.Count];
for (var i = 0; i < segment.Parts.Count; i++)
{
var part = segment.Parts[i];
parts[i] = part is RoutePatternParameterPart paramPart && TryGetRequiredValue(pattern, paramPart.Name, out var value)
? value
: part.DebuggerToString();
}

return string.Join(string.Empty, parts);
}

private static bool TryGetRequiredValue(RoutePattern pattern, string parameterName, [NotNullWhen(true)] out string? value)
{
if (pattern.RequiredValues.TryGetValue(parameterName, out var requiredValue) &&
!RoutePattern.IsRequiredValueAny(requiredValue) &&
requiredValue?.ToString() is { Length: > 0 } v)
{
value = v;
return true;
}
Comment on lines +92 to +98

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remediation Recommended

3. Required value culture drift 🐞 Bug

• Required value substitution uses requiredValue.ToString(), which is culture-dependent for
  non-string values and may produce different endpoint display names/route metadata across
  environments.
• Routing already normalizes values with Convert.ToString(..., CultureInfo.InvariantCulture)
  elsewhere; using object.ToString() here is inconsistent and can also surface unexpected ToString()
  exceptions into endpoint construction.
Agent Prompt
### Issue description
`TryGetRequiredValue` uses `requiredValue?.ToString()` which is culture-dependent for non-strings and can make endpoint display names and route diagnostics metadata differ across environments.

### Issue Context
Routing already treats non-string values as route strings using `Convert.ToString(..., CultureInfo.InvariantCulture)` (see `RouteValueEqualityComparer`).

### Fix Focus Areas
- src/Http/Routing/src/Patterns/RoutePatternDebugStringFormatter.cs[90-101]
- src/Http/Routing/src/RouteValueEqualityComparer.cs[12-35]

### Implementation notes
- Add `using System.Globalization;`
- Replace `requiredValue?.ToString()` with invariant conversion:
  - `var v = requiredValue as string ?? Convert.ToString(requiredValue, CultureInfo.InvariantCulture);`
- Keep the existing `{ Length: > 0 }` check and RequiredValueAny filtering.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


value = null;
return false;
}
}
17 changes: 15 additions & 2 deletions src/Http/Routing/src/Patterns/RoutePatternParameterPart.cs
Original file line number Diff line number Diff line change
Expand Up @@ -99,10 +99,23 @@ internal override string DebuggerToString()
foreach (var constraint in ParameterPolicies)
{
builder.Append(':');
builder.Append(constraint.ParameterPolicy);
if (constraint.Content is not null)
{
builder.Append(constraint.Content);
}
else if (constraint.ParameterPolicy is Constraints.RegexRouteConstraint regexConstraint)
{
builder.Append("regex(");
builder.Append(regexConstraint.Constraint.ToString());
builder.Append(')');
}
Comment on lines +106 to +111

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action Required

1. Regex compiled in debug 🐞 Bug

• RoutePatternParameterPart.DebuggerToString now formats RegexRouteConstraint by reading
  regexConstraint.Constraint, which forces the lazily-created Regex instance to be created/compiled.
• DebuggerToString is executed during endpoint construction for both RouteEndpoint.DisplayName and
  default IRouteDiagnosticsMetadata, so this can regress startup performance for apps using
  regex-based constraints/policies.
Agent Prompt
### Issue description
`RoutePatternParameterPart.DebuggerToString()` accesses `RegexRouteConstraint.Constraint` to print the regex pattern. This forces lazy regex creation/compilation during endpoint build, regressing startup performance and undermining the explicit lazy-init design of `RegexRouteConstraint`.

### Issue Context
`DebuggerToString()` is used beyond the debugger: it feeds `RouteEndpoint.DisplayName` and default `IRouteDiagnosticsMetadata`.

### Fix Focus Areas
- src/Http/Routing/src/Patterns/RoutePatternParameterPart.cs[99-116]
- src/Http/Routing/src/Constraints/RegexRouteConstraint.cs[41-76]
- src/Http/Routing/src/Patterns/RoutePatternFactory.cs[946-963]

### Implementation notes
- If the policy came from text, prefer emitting that text (`constraint.Content`) without materializing the regex.
- For `RegexRouteConstraint` instances created from strings (e.g., `Constraint(object)` converting strings to `RegexRouteConstraint`), consider preserving the original pattern string in the policy reference (or enhancing `RegexRouteConstraint` to expose it) so debug formatting does not require compiling the regex.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

else if (constraint.ParameterPolicy is not null)
{
builder.Append(constraint.ParameterPolicy);
}
}

if (Default != null)
if (Default is not null)
{
builder.Append('=');
builder.Append(Default);
Expand Down
Loading