From 87a8dcf2a46eaf1e6c62cb31d4929eb105dd8057 Mon Sep 17 00:00:00 2001 From: Phil Asmar Date: Thu, 19 Dec 2024 18:03:43 -0500 Subject: [PATCH 1/2] fix: greedy variable can be any value --- .../Services/ApiGatewayRouteConfigService.cs | 13 +++++++++---- .../Services/ApiGatewayRouteConfigServiceTests.cs | 8 +++++--- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Services/ApiGatewayRouteConfigService.cs b/Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Services/ApiGatewayRouteConfigService.cs index 66f8df4bd..c0020a2c7 100644 --- a/Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Services/ApiGatewayRouteConfigService.cs +++ b/Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Services/ApiGatewayRouteConfigService.cs @@ -142,15 +142,20 @@ private bool IsRouteConfigValid(ApiGatewayRouteConfig routeConfig) return false; } - var occurrences = routeConfig.Path.Split("{proxy+}").Length - 1; - if (occurrences > 1) + var occurrences = routeConfig.Path + .Split('/') + .Where( + x => x.StartsWith("{") && + x.EndsWith("+}")) + .ToList(); + if (occurrences.Count > 1) { _logger.LogError("The route config {Method} {Path} cannot have multiple greedy variables {{proxy+}}.", routeConfig.HttpMethod, routeConfig.Path); return false; } - if (occurrences == 1 && !routeConfig.Path.EndsWith("/{proxy+}")) + if (occurrences.Count == 1 && !routeConfig.Path.EndsWith($"/{occurrences.Last()}")) { _logger.LogError("The route config {Method} {Path} uses a greedy variable {{proxy+}} but does not end with it.", routeConfig.HttpMethod, routeConfig.Path); @@ -412,7 +417,7 @@ private bool IsVariableSegment(string segment) /// true if the segment is a greedy variable segment; false otherwise. private bool IsGreedyVariable(string segment) { - return segment.Equals("{proxy+}", StringComparison.InvariantCultureIgnoreCase); + return segment.StartsWith("{") && segment.EndsWith("+}"); } /// diff --git a/Tools/LambdaTestTool-v2/tests/Amazon.Lambda.TestTool.UnitTests/Services/ApiGatewayRouteConfigServiceTests.cs b/Tools/LambdaTestTool-v2/tests/Amazon.Lambda.TestTool.UnitTests/Services/ApiGatewayRouteConfigServiceTests.cs index 633b04b28..295d1bd38 100644 --- a/Tools/LambdaTestTool-v2/tests/Amazon.Lambda.TestTool.UnitTests/Services/ApiGatewayRouteConfigServiceTests.cs +++ b/Tools/LambdaTestTool-v2/tests/Amazon.Lambda.TestTool.UnitTests/Services/ApiGatewayRouteConfigServiceTests.cs @@ -159,8 +159,10 @@ public void Constructor_LoadsAndParsesListOfConfigs() Assert.Equal("Function2", result2.LambdaResourceName); } - [Fact] - public void CatchAllRouteConfig() + [Theory] + [InlineData("proxy")] + [InlineData("anyvalue")] + public void CatchAllRouteConfig(string variableName) { // Arange var routeConfigs = new List @@ -169,7 +171,7 @@ public void CatchAllRouteConfig() { LambdaResourceName = "F1", HttpMethod = "ANY", - Path = "/{proxy+}" + Path = $"/{{{variableName}+}}" } }; From 4db624d0907162c21c52dea5634cf1a58f38bb42 Mon Sep 17 00:00:00 2001 From: Phil Asmar Date: Fri, 20 Dec 2024 09:32:54 -0500 Subject: [PATCH 2/2] reply to garrett's comments --- .../Services/ApiGatewayRouteConfigService.cs | 27 +++++-------- .../ApiGatewayRouteConfigServiceTests.cs | 39 +++++++++++++++++++ 2 files changed, 49 insertions(+), 17 deletions(-) diff --git a/Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Services/ApiGatewayRouteConfigService.cs b/Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Services/ApiGatewayRouteConfigService.cs index c0020a2c7..63f81e217 100644 --- a/Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Services/ApiGatewayRouteConfigService.cs +++ b/Tools/LambdaTestTool-v2/src/Amazon.Lambda.TestTool/Services/ApiGatewayRouteConfigService.cs @@ -3,6 +3,7 @@ using System.Collections; using System.Text.Json; +using System.Text.RegularExpressions; using Amazon.Lambda.TestTool.Models; using Amazon.Lambda.TestTool.Services.IO; @@ -142,24 +143,16 @@ private bool IsRouteConfigValid(ApiGatewayRouteConfig routeConfig) return false; } - var occurrences = routeConfig.Path - .Split('/') - .Where( - x => x.StartsWith("{") && - x.EndsWith("+}")) - .ToList(); - if (occurrences.Count > 1) + var segments = routeConfig.Path.Trim('/').Split('/'); + foreach (var segment in segments) { - _logger.LogError("The route config {Method} {Path} cannot have multiple greedy variables {{proxy+}}.", - routeConfig.HttpMethod, routeConfig.Path); - return false; - } - - if (occurrences.Count == 1 && !routeConfig.Path.EndsWith($"/{occurrences.Last()}")) - { - _logger.LogError("The route config {Method} {Path} uses a greedy variable {{proxy+}} but does not end with it.", - routeConfig.HttpMethod, routeConfig.Path); - return false; + var regexPattern = "^(\\{[\\w.:-]+\\+?\\}|[a-zA-Z0-9.:_-]+)$"; + if (!Regex.IsMatch(segment, regexPattern)) + { + _logger.LogError("One or more path parts appear to be invalid. Parts are validated against this regular expression: {Regex}", + regexPattern); + return false; + } } return true; diff --git a/Tools/LambdaTestTool-v2/tests/Amazon.Lambda.TestTool.UnitTests/Services/ApiGatewayRouteConfigServiceTests.cs b/Tools/LambdaTestTool-v2/tests/Amazon.Lambda.TestTool.UnitTests/Services/ApiGatewayRouteConfigServiceTests.cs index 295d1bd38..1ac75b9f3 100644 --- a/Tools/LambdaTestTool-v2/tests/Amazon.Lambda.TestTool.UnitTests/Services/ApiGatewayRouteConfigServiceTests.cs +++ b/Tools/LambdaTestTool-v2/tests/Amazon.Lambda.TestTool.UnitTests/Services/ApiGatewayRouteConfigServiceTests.cs @@ -119,6 +119,45 @@ public void GetRouteConfig_ReturnsNullForNonMatchingPath() Assert.Null(result); } + [Theory] + [InlineData("", "", "")] + [InlineData("F1", "", "")] + [InlineData("F1", "GET", "")] + [InlineData("F1", "GET", "//")] + [InlineData("F1", "GET", "/{}")] + [InlineData("F1", "GET", "/{+}")] + [InlineData("F1", "GET", "/{proxy+}+}")] + public void GetRouteConfig_InvalidPath(string lambdaName, string method, string template) + { + // Arrange + var routeConfig = new ApiGatewayRouteConfig + { + LambdaResourceName = lambdaName, + HttpMethod = method, + Path = template + }; + + _mockEnvironmentManager + .Setup(m => m.GetEnvironmentVariables()) + .Returns(new Dictionary + { + { Constants.LambdaConfigEnvironmentVariablePrefix, JsonSerializer.Serialize(routeConfig) } + }); + + // Act + _ = new ApiGatewayRouteConfigService(_mockEnvironmentManager.Object, _mockLogger.Object); + + // Assert + _mockLogger.Verify( + x => x.Log( + LogLevel.Error, + It.IsAny(), + It.Is((v, t) => v.ToString() == $"The route config {method} {template} is not valid. It will be skipped."), + It.IsAny(), + ((Func)It.IsAny())!), + Times.Once); + } + [Fact] public void Constructor_LoadsAndParsesListOfConfigs() {