Răsfoiți Sursa

Fix OpenAPI generator crash on routes starting with '~' (#64408)

* Initial plan

* Add RoutePattern field to ApiDescription and update OpenApi extension to use it

Co-authored-by: captainsafia <[email protected]>

* Add integration test for routes starting with tilde

Co-authored-by: captainsafia <[email protected]>

* Add test for routes starting with tilde from MVC controller actions

Co-authored-by: captainsafia <[email protected]>

* Simplify RoutePattern assignment and add tests for ~/health pattern

Co-authored-by: halter73 <[email protected]>

---------

Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: captainsafia <[email protected]>
Co-authored-by: Safia Abdalla <[email protected]>
Co-authored-by: halter73 <[email protected]>
Copilot 1 lună în urmă
părinte
comite
caa56b215d

+ 6 - 0
src/Mvc/Mvc.Abstractions/src/ApiExplorer/ApiDescription.cs

@@ -3,6 +3,7 @@
 
 using System.Diagnostics;
 using Microsoft.AspNetCore.Mvc.Abstractions;
+using Microsoft.AspNetCore.Routing.Patterns;
 
 namespace Microsoft.AspNetCore.Mvc.ApiExplorer;
 
@@ -42,6 +43,11 @@ public class ApiDescription
     /// </summary>
     public string? RelativePath { get; set; }
 
+    /// <summary>
+    /// Gets or sets the route pattern for this api.
+    /// </summary>
+    public RoutePattern? RoutePattern { get; set; }
+
     /// <summary>
     /// Gets the list of possible formats for a request.
     /// </summary>

+ 1 - 0
src/Mvc/Mvc.Abstractions/src/Microsoft.AspNetCore.Mvc.Abstractions.csproj

@@ -28,6 +28,7 @@ Microsoft.AspNetCore.Mvc.IActionResult</Description>
   </ItemGroup>
 
   <ItemGroup>
+    <Reference Include="Microsoft.AspNetCore.Routing" />
     <Reference Include="Microsoft.AspNetCore.Http.Abstractions" />
     <Reference Include="Microsoft.AspNetCore.Http.Features" />
     <Reference Include="Microsoft.AspNetCore.Routing.Abstractions" />

+ 2 - 0
src/Mvc/Mvc.Abstractions/src/PublicAPI.Unshipped.txt

@@ -1 +1,3 @@
 #nullable enable
+Microsoft.AspNetCore.Mvc.ApiExplorer.ApiDescription.RoutePattern.get -> Microsoft.AspNetCore.Routing.Patterns.RoutePattern?
+Microsoft.AspNetCore.Mvc.ApiExplorer.ApiDescription.RoutePattern.set -> void

+ 4 - 0
src/Mvc/Mvc.ApiExplorer/src/DefaultApiDescriptionProvider.cs

@@ -12,6 +12,7 @@ using Microsoft.AspNetCore.Mvc.Infrastructure;
 using Microsoft.AspNetCore.Mvc.ModelBinding;
 using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata;
 using Microsoft.AspNetCore.Routing;
+using Microsoft.AspNetCore.Routing.Patterns;
 using Microsoft.AspNetCore.Routing.Template;
 using Microsoft.Extensions.Options;
 
@@ -104,6 +105,9 @@ public class DefaultApiDescriptionProvider : IApiDescriptionProvider
             GroupName = groupName,
             HttpMethod = httpMethod,
             RelativePath = GetRelativePath(parsedTemplate),
+            RoutePattern = action.AttributeRouteInfo?.Template is not null
+                ? RoutePatternFactory.Parse(action.AttributeRouteInfo.Template)
+                : null,
         };
 
         var templateParameters = parsedTemplate?.Parameters?.ToList() ?? new List<TemplatePart>();

+ 1 - 0
src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs

@@ -102,6 +102,7 @@ internal sealed class EndpointMetadataApiDescriptionProvider : IApiDescriptionPr
             HttpMethod = httpMethod,
             GroupName = routeEndpoint.Metadata.GetMetadata<IEndpointGroupNameMetadata>()?.EndpointGroupName,
             RelativePath = routeEndpoint.RoutePattern.RawText?.TrimStart('/'),
+            RoutePattern = routeEndpoint.RoutePattern,
             ActionDescriptor = new ActionDescriptor
             {
                 DisplayName = routeEndpoint.DisplayName,

+ 2 - 0
src/Mvc/Mvc.ApiExplorer/src/PublicAPI.Unshipped.txt

@@ -1 +1,3 @@
 #nullable enable
+Microsoft.AspNetCore.Mvc.ApiExplorer.ApiDescription.RoutePattern.get -> Microsoft.AspNetCore.Routing.Patterns.RoutePattern? (forwarded, contained in Microsoft.AspNetCore.Mvc.Abstractions)
+Microsoft.AspNetCore.Mvc.ApiExplorer.ApiDescription.RoutePattern.set -> void (forwarded, contained in Microsoft.AspNetCore.Mvc.Abstractions)

+ 1 - 1
src/OpenApi/src/Extensions/ApiDescriptionExtensions.cs

@@ -49,7 +49,7 @@ internal static class ApiDescriptionExtensions
             return "/";
         }
         var strippedRoute = new StringBuilder();
-        var routePattern = RoutePatternFactory.Parse(apiDescription.RelativePath);
+        var routePattern = apiDescription.RoutePattern ?? RoutePatternFactory.Parse(apiDescription.RelativePath);
         for (var i = 0; i < routePattern.PathSegments.Count; i++)
         {
             strippedRoute.Append('/');

+ 25 - 0
src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Extensions/ApiDescriptionExtensionsTests.cs

@@ -3,6 +3,7 @@
 
 using System.Net.Http;
 using Microsoft.AspNetCore.Mvc.ApiExplorer;
+using Microsoft.AspNetCore.Routing.Patterns;
 
 public class ApiDescriptionExtensionsTests
 {
@@ -31,6 +32,30 @@ public class ApiDescriptionExtensionsTests
         Assert.Equal(expectedItemPath, itemPath);
     }
 
+    [Theory]
+    [InlineData("/~health", "~health", "/~health")]
+    [InlineData("/~api/todos", "~api/todos", "/~api/todos")]
+    [InlineData("/~api/todos/{id}", "~api/todos/{id}", "/~api/todos/{id}")]
+    [InlineData("~/health", "health", "/health")]
+    [InlineData("~/api/todos", "api/todos", "/api/todos")]
+    [InlineData("~/api/todos/{id}", "api/todos/{id}", "/api/todos/{id}")]
+    public void MapRelativePathToItemPath_WithRoutePattern_HandlesRoutesThatStartWithTilde(string rawPattern, string relativePath, string expectedItemPath)
+    {
+        // Arrange
+        var routePattern = RoutePatternFactory.Parse(rawPattern);
+        var apiDescription = new ApiDescription
+        {
+            RelativePath = relativePath,
+            RoutePattern = routePattern
+        };
+
+        // Act
+        var itemPath = apiDescription.MapRelativePathToItemPath();
+
+        // Assert
+        Assert.Equal(expectedItemPath, itemPath);
+    }
+
     public static class HttpMethodTestData
     {
         public static IEnumerable<object[]> TestCases => new List<object[]>

+ 91 - 0
src/OpenApi/test/Microsoft.AspNetCore.OpenApi.Tests/Services/OpenApiDocumentService/OpenApiDocumentServiceTests.Paths.cs

@@ -3,6 +3,7 @@
 
 using System.Net.Http;
 using Microsoft.AspNetCore.Builder;
+using Microsoft.AspNetCore.Mvc;
 using Microsoft.AspNetCore.OpenApi;
 using Microsoft.AspNetCore.Routing;
 
@@ -170,4 +171,94 @@ public partial class OpenApiDocumentServiceTests : OpenApiDocumentServiceTestBas
             );
         });
     }
+
+    [Fact]
+    public async Task GetOpenApiPaths_HandlesRoutesStartingWithTilde()
+    {
+        // Arrange
+        var builder = CreateBuilder();
+
+        // Act
+        builder.MapGet("/~health", () => "Healthy");
+        builder.MapGet("/~api/todos", () => { });
+        builder.MapGet("/~api/todos/{id}", () => { });
+        builder.MapGet("~/health2", () => "Healthy2");
+
+        // Assert
+        await VerifyOpenApiDocument(builder, document =>
+        {
+            Assert.Collection(document.Paths.OrderBy(p => p.Key),
+                path =>
+                {
+                    Assert.Equal("/~api/todos", path.Key);
+                    Assert.Single(path.Value.Operations);
+                    Assert.Contains(HttpMethod.Get, path.Value.Operations);
+                },
+                path =>
+                {
+                    Assert.Equal("/~api/todos/{id}", path.Key);
+                    Assert.Single(path.Value.Operations);
+                    Assert.Contains(HttpMethod.Get, path.Value.Operations);
+                },
+                path =>
+                {
+                    Assert.Equal("/~health", path.Key);
+                    Assert.Single(path.Value.Operations);
+                    Assert.Contains(HttpMethod.Get, path.Value.Operations);
+                },
+                path =>
+                {
+                    Assert.Equal("/health2", path.Key);
+                    Assert.Single(path.Value.Operations);
+                    Assert.Contains(HttpMethod.Get, path.Value.Operations);
+                }
+            );
+        });
+    }
+
+    [Fact]
+    public async Task GetOpenApiPaths_HandlesRoutesStartingWithTilde_MvcAction()
+    {
+        // Arrange
+        var action = CreateActionDescriptor(nameof(ActionWithTildeRoute));
+
+        // Assert
+        await VerifyOpenApiDocument(action, document =>
+        {
+            Assert.Collection(document.Paths.OrderBy(p => p.Key),
+                path =>
+                {
+                    Assert.Equal("/~health", path.Key);
+                    Assert.Single(path.Value.Operations);
+                    Assert.Contains(HttpMethod.Get, path.Value.Operations);
+                }
+            );
+        });
+    }
+
+    [Fact]
+    public async Task GetOpenApiPaths_HandlesRoutesStartingWithTildeBeforeSlash_MvcAction()
+    {
+        // Arrange
+        var action = CreateActionDescriptor(nameof(ActionWithTildeBeforeSlashRoute));
+
+        // Assert
+        await VerifyOpenApiDocument(action, document =>
+        {
+            Assert.Collection(document.Paths.OrderBy(p => p.Key),
+                path =>
+                {
+                    Assert.Equal("/health", path.Key);
+                    Assert.Single(path.Value.Operations);
+                    Assert.Contains(HttpMethod.Get, path.Value.Operations);
+                }
+            );
+        });
+    }
+
+    [Route("/~health")]
+    private void ActionWithTildeRoute() { }
+
+    [Route("~/health")]
+    private void ActionWithTildeBeforeSlashRoute() { }
 }