From 208b87b05462a0f5a517dc161af69b0ac9e19172 Mon Sep 17 00:00:00 2001 From: Darius Letterman Date: Wed, 6 Nov 2024 12:43:20 +0100 Subject: [PATCH] handle catch-all parameters --- .../Http/HttpRouteParser.cs | 58 +++-- .../Http/Segment.cs | 9 +- .../Http/HttpParserTests.cs | 237 +++++++++++++++--- 3 files changed, 261 insertions(+), 43 deletions(-) diff --git a/src/Libraries/Microsoft.Extensions.Telemetry/Http/HttpRouteParser.cs b/src/Libraries/Microsoft.Extensions.Telemetry/Http/HttpRouteParser.cs index c62c50af154..6783785d838 100644 --- a/src/Libraries/Microsoft.Extensions.Telemetry/Http/HttpRouteParser.cs +++ b/src/Libraries/Microsoft.Extensions.Telemetry/Http/HttpRouteParser.cs @@ -7,6 +7,7 @@ using Microsoft.Extensions.Compliance.Classification; using Microsoft.Extensions.Compliance.Redaction; using Microsoft.Extensions.Http.Diagnostics; +using Microsoft.Shared.Diagnostics; namespace Microsoft.Extensions.Http.Diagnostics; @@ -56,16 +57,22 @@ public bool TryExtractParameters( { var startIndex = segment.Start + offset; - string parameterValue; + // If we exceed a length of the http path it means that the appropriate http route + // has optional parameters or parameters with default values, and these parameters + // are omitted in the http path. In this case we return a default value of the + // omitted parameter. + string parameterValue = segment.DefaultValue; + bool isRedacted = false; if (startIndex < httpPathAsSpan.Length) { var parameterContent = segment.Content; var parameterTemplateLength = parameterContent.Length + 2; + var length = httpPathAsSpan.Slice(startIndex).IndexOf(ForwardSlash); - if (length == -1) + if (segment.IsCatchAll || length == -1) { length = httpPathAsSpan.Slice(startIndex).Length; } @@ -75,15 +82,6 @@ public bool TryExtractParameters( parameterValue = GetRedactedParameterValue(httpPathAsSpan, segment, startIndex, length, redactionMode, parametersToRedact, ref isRedacted); } - // If we exceed a length of the http path it means that the appropriate http route - // has optional parameters or parameters with default values, and these parameters - // are omitted in the http path. In this case we return a default value of the - // omitted parameter. - else - { - parameterValue = segment.DefaultValue; - } - httpRouteParameters[index++] = new HttpRouteParameter(segment.ParamName, parameterValue, isRedacted); } } @@ -157,6 +155,8 @@ private static Segment GetParameterSegment(string httpRoute, ref int pos) int start = pos++; int paramNameEnd = PositionNotFound; + int paramNameStart = start + 1; + bool catchAllParamFound = false; int defaultValueStart = PositionNotFound; char ch; @@ -187,13 +187,42 @@ private static Segment GetParameterSegment(string httpRoute, ref int pos) } } + // The segment has '*' catch all parameter. + // When we meet the character it indicates param start position needs to be adjusted, so that we capture 'param' instead of '*param' + // *param can only appear after opening curly brace and position needs to be adjusted only once + else if (!catchAllParamFound && ch == '*' && pos > 0 && httpRoute[pos - 1] == '{') + { + paramNameStart++; + + // Catch all parameters can start with one or two '*' characters. + if (httpRoute[paramNameStart] == '*') + { + paramNameStart++; + } + + catchAllParamFound = true; + } + pos++; } - string content = GetSegmentContent(httpRoute, start + 1, pos); + // Throw an ArgumentException if the segment is a catch-all parameter and not the last segment. + // The current position should be either the end of the route or the second to last position followed by a '/'. + if (catchAllParamFound) + { + bool isLastPosition = pos == httpRoute.Length - 1; + bool isSecondToLastPosition = pos == httpRoute.Length - 2; + + if (!(isLastPosition || (isSecondToLastPosition && httpRoute[pos + 1] == '/'))) + { + Throw.ArgumentException(nameof(httpRoute), "A catch-all parameter must be the last segment in the route."); + } + } + + string content = GetSegmentContent(httpRoute, paramNameStart, pos); string paramName = paramNameEnd == PositionNotFound ? content - : GetSegmentContent(httpRoute, start + 1, paramNameEnd); + : GetSegmentContent(httpRoute, paramNameStart, paramNameEnd); string defaultValue = defaultValueStart == PositionNotFound ? string.Empty : GetSegmentContent(httpRoute, defaultValueStart, pos); @@ -205,7 +234,8 @@ private static Segment GetParameterSegment(string httpRoute, ref int pos) content: content, isParam: true, paramName: paramName, - defaultValue: defaultValue); + defaultValue: defaultValue, + isCatchAll: catchAllParamFound); } private static string GetSegmentContent(string httpRoute, int start, int end) diff --git a/src/Libraries/Microsoft.Extensions.Telemetry/Http/Segment.cs b/src/Libraries/Microsoft.Extensions.Telemetry/Http/Segment.cs index 76f68614ee0..44f7d1b66aa 100644 --- a/src/Libraries/Microsoft.Extensions.Telemetry/Http/Segment.cs +++ b/src/Libraries/Microsoft.Extensions.Telemetry/Http/Segment.cs @@ -24,9 +24,10 @@ internal readonly struct Segment /// If the segment is a param. /// Name of the parameter. /// Default value of the parameter. + /// If the segment is a catch-all parameter. public Segment( int start, int end, string content, bool isParam, - string paramName = "", string defaultValue = "") + string paramName = "", string defaultValue = "", bool isCatchAll = false) { Start = start; End = end; @@ -34,6 +35,7 @@ public Segment( IsParam = isParam; ParamName = paramName; DefaultValue = defaultValue; + IsCatchAll = isCatchAll; } /// @@ -66,6 +68,11 @@ public Segment( /// public string DefaultValue { get; } = string.Empty; + /// + /// Gets a value indicating whether the segment is a catch-all parameter. + /// + public bool IsCatchAll { get; } + internal static bool IsKnownUnredactableParameter(string parameter) => parameter.Equals(ControllerParameter, StringComparison.OrdinalIgnoreCase) || parameter.Equals(ActionParameter, StringComparison.OrdinalIgnoreCase); diff --git a/test/Libraries/Microsoft.Extensions.Telemetry.Tests/Http/HttpParserTests.cs b/test/Libraries/Microsoft.Extensions.Telemetry.Tests/Http/HttpParserTests.cs index 6bee06a7c23..a3e40c53e38 100644 --- a/test/Libraries/Microsoft.Extensions.Telemetry.Tests/Http/HttpParserTests.cs +++ b/test/Libraries/Microsoft.Extensions.Telemetry.Tests/Http/HttpParserTests.cs @@ -377,6 +377,97 @@ public void TryExtractParameters_WhenRouteHasOptionalsAndConstraints_ReturnsExpe ValidateRouteParameter(httpRouteParameters[1], "chatId", "", false); } + [Theory] + [CombinatorialData] + public void TryExtractParameters_WhenRouteHasCatchAllParameter_ReturnsCorrectParameters( + bool routeHasMessageSegment, + bool roundTripSyntax, + HttpRouteParameterRedactionMode redactionMode) + { + bool isRedacted = redactionMode != HttpRouteParameterRedactionMode.None; + string redactedPrefix = isRedacted ? "Redacted:" : string.Empty; + + HttpRouteParser httpParser = CreateHttpRouteParser(); + Dictionary parametersToRedact = new() + { + { "routeId", FakeTaxonomy.PrivateData }, + { "chatId", FakeTaxonomy.PrivateData }, + { "catchAll", FakeTaxonomy.PrivateData }, + }; + + string httpPath = "api/routes/routeId123/chats/chatId123/messages/1/2/3/"; + + var paramName = "*catchAll"; + if (roundTripSyntax) + { + paramName = "**catchAll"; + } + + var expectedValue = "messages/1/2/3/"; + var segment = string.Empty; + if (routeHasMessageSegment) + { + segment = "/messages"; + expectedValue = "1/2/3/"; + } + + string httpRoute = $"api/routes/{{routeId}}/chats/{{chatId}}{segment}/{{{paramName}}}/"; + + var routeSegments = httpParser.ParseRoute(httpRoute); + var httpRouteParameters = new HttpRouteParameter[3]; + var success = httpParser.TryExtractParameters(httpPath, routeSegments, redactionMode, parametersToRedact, ref httpRouteParameters); + + Assert.True(success); + ValidateRouteParameter(httpRouteParameters[0], "routeId", $"{redactedPrefix}routeId123", isRedacted); + ValidateRouteParameter(httpRouteParameters[1], "chatId", $"{redactedPrefix}chatId123", isRedacted); + ValidateRouteParameter(httpRouteParameters[2], "catchAll", $"{redactedPrefix}{expectedValue}", isRedacted); + } + + [Theory] + [CombinatorialData] + public void TryExtractParameters_WhenRouteHasCatchAllParameter_Optional_ReturnsCorrectParameters( + bool routeHasDefaultValue, + bool useRoundTripSyntax, + HttpRouteParameterRedactionMode redactionMode) + { + bool isRedacted = redactionMode != HttpRouteParameterRedactionMode.None; + string redactedPrefix = isRedacted ? "Redacted:" : string.Empty; + + HttpRouteParser httpParser = CreateHttpRouteParser(); + Dictionary parametersToRedact = new() + { + { "routeId", FakeTaxonomy.PrivateData }, + { "chatId", FakeTaxonomy.PrivateData }, + { "catchAll", FakeTaxonomy.PrivateData }, + }; + + var httpPath = "api/routes/routeId123/chats/chatId123"; + + var paramName = "*catchAll"; + if (useRoundTripSyntax) + { + paramName = "**catchAll"; + } + + var expectedValue = string.Empty; + if (routeHasDefaultValue) + { + expectedValue = nameof(routeHasDefaultValue); + paramName += $"={expectedValue}"; + } + + var httpRoute = $"api/routes/{{routeId}}/chats/{{chatId}}/{{{paramName}}}"; + + var routeSegments = httpParser.ParseRoute(httpRoute); + var httpRouteParameters = new HttpRouteParameter[3]; + var success = httpParser.TryExtractParameters(httpPath, routeSegments, redactionMode, parametersToRedact, ref httpRouteParameters); + + Assert.True(success); + ValidateRouteParameter(httpRouteParameters[0], "routeId", $"{redactedPrefix}routeId123", isRedacted); + ValidateRouteParameter(httpRouteParameters[1], "chatId", $"{redactedPrefix}chatId123", isRedacted); + ValidateRouteParameter(httpRouteParameters[2], "catchAll", expectedValue, false); + } + [Fact] public void ParseRoute_WithRouteParameter_ReturnsRouteSegments() { @@ -389,10 +480,10 @@ public void ParseRoute_WithRouteParameter_ReturnsRouteSegments() Assert.Equal(4, routeSegments.Segments.Length); Assert.Equal("api/routes/{routeId}/chats/{chatId}", routeSegments.RouteTemplate); - ValidateRouteSegment(routeSegments.Segments[0], "api/routes/", false, "", "", 0, 11); - ValidateRouteSegment(routeSegments.Segments[1], "routeId", true, "routeId", "", 11, 20); - ValidateRouteSegment(routeSegments.Segments[2], "/chats/", false, "", "", 20, 27); - ValidateRouteSegment(routeSegments.Segments[3], "chatId", true, "chatId", "", 27, 35); + ValidateRouteSegment(routeSegments.Segments[0], ("api/routes/", false, "", "", 0, 11, false)); + ValidateRouteSegment(routeSegments.Segments[1], ("routeId", true, "routeId", "", 11, 20, false)); + ValidateRouteSegment(routeSegments.Segments[2], ("/chats/", false, "", "", 20, 27, false)); + ValidateRouteSegment(routeSegments.Segments[3], ("chatId", true, "chatId", "", 27, 35, false)); // An http route has parameters and ends with text. httpRoute = "/api/routes/{routeId}/chats/{chatId}/messages"; @@ -401,11 +492,11 @@ public void ParseRoute_WithRouteParameter_ReturnsRouteSegments() Assert.Equal(5, routeSegments.Segments.Length); Assert.Equal("api/routes/{routeId}/chats/{chatId}/messages", routeSegments.RouteTemplate); - ValidateRouteSegment(routeSegments.Segments[0], "api/routes/", false, "", "", 0, 11); - ValidateRouteSegment(routeSegments.Segments[1], "routeId", true, "routeId", "", 11, 20); - ValidateRouteSegment(routeSegments.Segments[2], "/chats/", false, "", "", 20, 27); - ValidateRouteSegment(routeSegments.Segments[3], "chatId", true, "chatId", "", 27, 35); - ValidateRouteSegment(routeSegments.Segments[4], "/messages", false, "", "", 35, 44); + ValidateRouteSegment(routeSegments.Segments[0], ("api/routes/", false, "", "", 0, 11, false)); + ValidateRouteSegment(routeSegments.Segments[1], ("routeId", true, "routeId", "", 11, 20, false)); + ValidateRouteSegment(routeSegments.Segments[2], ("/chats/", false, "", "", 20, 27, false)); + ValidateRouteSegment(routeSegments.Segments[3], ("chatId", true, "chatId", "", 27, 35, false)); + ValidateRouteSegment(routeSegments.Segments[4], ("/messages", false, "", "", 35, 44, false)); } [Fact] @@ -419,11 +510,11 @@ public void ParseRoute_WithQueryParameter_ReturnRouteSegmentExcludingQueryParams Assert.Equal(5, routeSegments.Segments.Length); Assert.Equal("api/routes/{routeId}/chats/{chatId}/messages", routeSegments.RouteTemplate); - ValidateRouteSegment(routeSegments.Segments[0], "api/routes/", false, "", "", 0, 11); - ValidateRouteSegment(routeSegments.Segments[1], "routeId", true, "routeId", "", 11, 20); - ValidateRouteSegment(routeSegments.Segments[2], "/chats/", false, "", "", 20, 27); - ValidateRouteSegment(routeSegments.Segments[3], "chatId", true, "chatId", "", 27, 35); - ValidateRouteSegment(routeSegments.Segments[4], "/messages", false, "", "", 35, 44); + ValidateRouteSegment(routeSegments.Segments[0], ("api/routes/", false, "", "", 0, 11, false)); + ValidateRouteSegment(routeSegments.Segments[1], ("routeId", true, "routeId", "", 11, 20, false)); + ValidateRouteSegment(routeSegments.Segments[2], ("/chats/", false, "", "", 20, 27, false)); + ValidateRouteSegment(routeSegments.Segments[3], ("chatId", true, "chatId", "", 27, 35, false)); + ValidateRouteSegment(routeSegments.Segments[4], ("/messages", false, "", "", 35, 44, false)); // Route doesn't start with forward slash, the final result should begin with forward slash. httpRoute = "api/routes/{routeId}/chats/{chatId}/messages?from=7"; @@ -432,11 +523,11 @@ public void ParseRoute_WithQueryParameter_ReturnRouteSegmentExcludingQueryParams Assert.Equal(5, routeSegments.Segments.Length); Assert.Equal("api/routes/{routeId}/chats/{chatId}/messages", routeSegments.RouteTemplate); - ValidateRouteSegment(routeSegments.Segments[0], "api/routes/", false, "", "", 0, 11); - ValidateRouteSegment(routeSegments.Segments[1], "routeId", true, "routeId", "", 11, 20); - ValidateRouteSegment(routeSegments.Segments[2], "/chats/", false, "", "", 20, 27); - ValidateRouteSegment(routeSegments.Segments[3], "chatId", true, "chatId", "", 27, 35); - ValidateRouteSegment(routeSegments.Segments[4], "/messages", false, "", "", 35, 44); + ValidateRouteSegment(routeSegments.Segments[0], ("api/routes/", false, "", "", 0, 11, false)); + ValidateRouteSegment(routeSegments.Segments[1], ("routeId", true, "routeId", "", 11, 20, false)); + ValidateRouteSegment(routeSegments.Segments[2], ("/chats/", false, "", "", 20, 27, false)); + ValidateRouteSegment(routeSegments.Segments[3], ("chatId", true, "chatId", "", 27, 35, false)); + ValidateRouteSegment(routeSegments.Segments[4], ("/messages", false, "", "", 35, 44, false)); } [Fact] @@ -450,14 +541,101 @@ public void ParseRoute_WhenRouteHasDefaultsOptionalsConstraints_ReturnsRouteSegm Assert.Equal(8, routeSegments.Segments.Length); Assert.Equal(httpRoute, routeSegments.RouteTemplate); - ValidateRouteSegment(routeSegments.Segments[0], "api/", false, "", "", 0, 4); - ValidateRouteSegment(routeSegments.Segments[1], "controller=home", true, "controller", "home", 4, 21); - ValidateRouteSegment(routeSegments.Segments[2], "/", false, "", "", 21, 22); - ValidateRouteSegment(routeSegments.Segments[3], "action=index", true, "action", "index", 22, 36); - ValidateRouteSegment(routeSegments.Segments[4], "/", false, "", "", 36, 37); - ValidateRouteSegment(routeSegments.Segments[5], "routeId:int:min(1)", true, "routeId", "", 37, 57); - ValidateRouteSegment(routeSegments.Segments[6], "/", false, "", "", 57, 58); - ValidateRouteSegment(routeSegments.Segments[7], "chatId?", true, "chatId", "", 58, 67); + ValidateRouteSegment(routeSegments.Segments[0], ("api/", false, "", "", 0, 4, false)); + ValidateRouteSegment(routeSegments.Segments[1], ("controller=home", true, "controller", "home", 4, 21, false)); + ValidateRouteSegment(routeSegments.Segments[2], ("/", false, "", "", 21, 22, false)); + ValidateRouteSegment(routeSegments.Segments[3], ("action=index", true, "action", "index", 22, 36, false)); + ValidateRouteSegment(routeSegments.Segments[4], ("/", false, "", "", 36, 37, false)); + ValidateRouteSegment(routeSegments.Segments[5], ("routeId:int:min(1)", true, "routeId", "", 37, 57, false)); + ValidateRouteSegment(routeSegments.Segments[6], ("/", false, "", "", 57, 58, false)); + ValidateRouteSegment(routeSegments.Segments[7], ("chatId?", true, "chatId", "", 58, 67, false)); + } + + [Theory] + [InlineData("api/{controller=home}/{action=index}/{*url}/{invalid}")] + [InlineData("api/{controller=home}/{action=index}/{**url}/{invalid}")] + public void ParseRoute_WhenRouteHasCatchAllParameter_OutOfOrder(string httpRoute) + { + HttpRouteParser httpParser = CreateHttpRouteParser(); + + var exception = Assert.Throws(() => httpParser.ParseRoute(httpRoute)); + + Assert.StartsWith("A catch-all parameter must be the last segment in the route.", exception.Message); + } + + [Theory] + [InlineData("api/{controller=home}/{action=index}/{*url}")] + [InlineData("api/{controller=home}/{action=index}/{*url}/")] + [InlineData("api/{controller=home}/{action=index}/{**url}")] + [InlineData("api/{controller=home}/{action=index}/{**url}/")] + public void ParseRoute_WhenRouteHasCatchAllParameter_InCorrectPosition(string httpRoute) + { + HttpRouteParser httpParser = CreateHttpRouteParser(); + + ParsedRouteSegments routeSegments = httpParser.ParseRoute(httpRoute); + + Assert.Equal(3, routeSegments.ParameterCount); + Assert.Equal(httpRoute, routeSegments.RouteTemplate); + } + + [Theory] + [InlineData("api/{controller=home}/{action=index}/{*url}", 37, 43)] + [InlineData("api/{controller=home}/{action=index}/{**url}", 37, 44)] + public void ParseRoute_WhenRouteHasCatchAllParameter_ReturnsRouteSegments(string httpRoute, int start, int end) + { + HttpRouteParser httpParser = CreateHttpRouteParser(); + + ParsedRouteSegments routeSegments = httpParser.ParseRoute(httpRoute); + + Assert.Equal(6, routeSegments.Segments.Length); + Assert.Equal(httpRoute, routeSegments.RouteTemplate); + + ValidateRouteSegment(routeSegments.Segments[0], ("api/", false, "", "", 0, 4, false)); + ValidateRouteSegment(routeSegments.Segments[1], ("controller=home", true, "controller", "home", 4, 21, false)); + ValidateRouteSegment(routeSegments.Segments[2], ("/", false, "", "", 21, 22, false)); + ValidateRouteSegment(routeSegments.Segments[3], ("action=index", true, "action", "index", 22, 36, false)); + ValidateRouteSegment(routeSegments.Segments[4], ("/", false, "", "", 36, 37, false)); + ValidateRouteSegment(routeSegments.Segments[5], ("url", true, "url", "", start, end, true)); + } + + [Theory] + [InlineData("api/{controller=home}/{action=index}/{*url:int:min(1)}", 37, 54)] + [InlineData("api/{controller=home}/{action=index}/{**url:int:min(1)}", 37, 55)] + public void ParseRoute_WhenRouteHasCatchAllParameterWithRouteConstraint_ReturnsRouteSegments(string httpRoute, int start, int end) + { + HttpRouteParser httpParser = CreateHttpRouteParser(); + + ParsedRouteSegments routeSegments = httpParser.ParseRoute(httpRoute); + + Assert.Equal(6, routeSegments.Segments.Length); + Assert.Equal(httpRoute, routeSegments.RouteTemplate); + + ValidateRouteSegment(routeSegments.Segments[0], ("api/", false, "", "", 0, 4, false)); + ValidateRouteSegment(routeSegments.Segments[1], ("controller=home", true, "controller", "home", 4, 21, false)); + ValidateRouteSegment(routeSegments.Segments[2], ("/", false, "", "", 21, 22, false)); + ValidateRouteSegment(routeSegments.Segments[3], ("action=index", true, "action", "index", 22, 36, false)); + ValidateRouteSegment(routeSegments.Segments[4], ("/", false, "", "", 36, 37, false)); + ValidateRouteSegment(routeSegments.Segments[5], ("url:int:min(1)", true, "url", "", start, end, true)); + } + + [Theory] + [InlineData("api/{controller=home}/{action=index}/{*url:regex(^(web|shared*)$)}", 37, 66)] + [InlineData("api/{controller=home}/{action=index}/{**url:regex(^(web|shared*)$)}", 37, 67)] + public void ParseRoute_WhenRouteHasCatchAllParameterWithRouteConstraintContainingRegexWithStar_ReturnsRouteSegments(string httpRoute, int start, int end) + { + HttpRouteParser httpParser = CreateHttpRouteParser(); + + ParsedRouteSegments routeSegments = httpParser.ParseRoute(httpRoute); + + Assert.Equal(6, routeSegments.Segments.Length); + Assert.Equal(httpRoute, routeSegments.RouteTemplate); + + ValidateRouteSegment(routeSegments.Segments[0], ("api/", false, "", "", 0, 4, false)); + ValidateRouteSegment(routeSegments.Segments[1], ("controller=home", true, "controller", "home", 4, 21, false)); + ValidateRouteSegment(routeSegments.Segments[2], ("/", false, "", "", 21, 22, false)); + ValidateRouteSegment(routeSegments.Segments[3], ("action=index", true, "action", "index", 22, 36, false)); + ValidateRouteSegment(routeSegments.Segments[4], ("/", false, "", "", 36, 37, false)); + ValidateRouteSegment(routeSegments.Segments[5], ("url:regex(^(web|shared*)$)", true, "url", "", start, end, true)); } [Fact] @@ -488,13 +666,16 @@ private static void ValidateRouteParameter( } private static void ValidateRouteSegment( - Segment segment, string content, bool isParam, string paramName, string defaultValue, int start, int end) + Segment segment, (string content, bool isParam, string paramName, string defaultValue, int start, int end, bool isCatchAll) values) { + var (content, isParam, paramName, defaultValue, start, end, isCatchAll) = values; + Assert.Equal(content, segment.Content); Assert.Equal(isParam, segment.IsParam); Assert.Equal(paramName, segment.ParamName); Assert.Equal(defaultValue, segment.DefaultValue); Assert.Equal(start, segment.Start); Assert.Equal(end, segment.End); + Assert.Equal(isCatchAll, segment.IsCatchAll); } }