From 05409d5ce09c22fdb60465f4e4fec5092923ee1f Mon Sep 17 00:00:00 2001 From: Keah Peters Date: Mon, 29 Apr 2024 14:19:55 +0100 Subject: [PATCH] Filter illegal header fields (#2842) Filter out illegal header fields. --- .../ApiParameterDescriptionExtensions.cs | 15 +++ .../SwaggerGenerator/SwaggerGenerator.cs | 5 +- .../Fixtures/FakeController.cs | 9 ++ .../SwaggerGenerator/SwaggerGeneratorTests.cs | 117 +++++++++++++++++- .../Controllers/FromHeaderParamsController.cs | 18 +++ 5 files changed, 160 insertions(+), 4 deletions(-) create mode 100644 test/WebSites/Basic/Controllers/FromHeaderParamsController.cs diff --git a/src/Swashbuckle.AspNetCore.SwaggerGen/SwaggerGenerator/ApiParameterDescriptionExtensions.cs b/src/Swashbuckle.AspNetCore.SwaggerGen/SwaggerGenerator/ApiParameterDescriptionExtensions.cs index 021e2c495..e505905e6 100644 --- a/src/Swashbuckle.AspNetCore.SwaggerGen/SwaggerGenerator/ApiParameterDescriptionExtensions.cs +++ b/src/Swashbuckle.AspNetCore.SwaggerGen/SwaggerGenerator/ApiParameterDescriptionExtensions.cs @@ -7,6 +7,7 @@ using Microsoft.AspNetCore.Mvc.ApiExplorer; using Microsoft.AspNetCore.Mvc.Controllers; using Microsoft.AspNetCore.Mvc.ModelBinding; +using Microsoft.Net.Http.Headers; namespace Swashbuckle.AspNetCore.SwaggerGen { @@ -21,6 +22,13 @@ public static class ApiParameterDescriptionExtensions #endif }; + private static readonly HashSet IllegalHeaderParameters = new HashSet(StringComparer.OrdinalIgnoreCase) + { + HeaderNames.Accept, + HeaderNames.Authorization, + HeaderNames.ContentType + }; + public static bool IsRequiredParameter(this ApiParameterDescription apiParameter) { // From the OpenAPI spec: @@ -111,5 +119,12 @@ internal static bool IsFromForm(this ApiParameterDescription apiParameter) return (source == BindingSource.Form || source == BindingSource.FormFile) || (elementType != null && typeof(IFormFile).IsAssignableFrom(elementType)); } + + internal static bool IsIllegalHeaderParameter(this ApiParameterDescription apiParameter) + { + // Certain header parameters are not allowed and should be described using the corresponding OpenAPI keywords + // https://swagger.io/docs/specification/describing-parameters/#header-parameters + return apiParameter.Source == BindingSource.Header && IllegalHeaderParameters.Contains(apiParameter.Name); + } } } diff --git a/src/Swashbuckle.AspNetCore.SwaggerGen/SwaggerGenerator/SwaggerGenerator.cs b/src/Swashbuckle.AspNetCore.SwaggerGen/SwaggerGenerator/SwaggerGenerator.cs index 0cfa1c418..88b46d4a2 100644 --- a/src/Swashbuckle.AspNetCore.SwaggerGen/SwaggerGenerator/SwaggerGenerator.cs +++ b/src/Swashbuckle.AspNetCore.SwaggerGen/SwaggerGenerator/SwaggerGenerator.cs @@ -269,7 +269,7 @@ private OpenApiOperation GenerateOpenApiOperationFromMetadata(ApiDescription api // Schemas will be generated via Swashbuckle by default. foreach (var parameter in operation.Parameters) { - var apiParameter = apiDescription.ParameterDescriptions.SingleOrDefault(desc => desc.Name == parameter.Name && !desc.IsFromBody() && !desc.IsFromForm()); + var apiParameter = apiDescription.ParameterDescriptions.SingleOrDefault(desc => desc.Name == parameter.Name && !desc.IsFromBody() && !desc.IsFromForm() && !desc.IsIllegalHeaderParameter()); if (apiParameter is not null) { parameter.Schema = GenerateSchema( @@ -342,7 +342,8 @@ private IList GenerateParameters(ApiDescription apiDescription return (!apiParam.IsFromBody() && !apiParam.IsFromForm()) && (!apiParam.CustomAttributes().OfType().Any()) && (!apiParam.CustomAttributes().OfType().Any()) - && (apiParam.ModelMetadata == null || apiParam.ModelMetadata.IsBindingAllowed); + && (apiParam.ModelMetadata == null || apiParam.ModelMetadata.IsBindingAllowed) + && !apiParam.IsIllegalHeaderParameter(); }); return applicableApiParameters diff --git a/test/Swashbuckle.AspNetCore.SwaggerGen.Test/Fixtures/FakeController.cs b/test/Swashbuckle.AspNetCore.SwaggerGen.Test/Fixtures/FakeController.cs index 856bbf457..a25c6f7e5 100644 --- a/test/Swashbuckle.AspNetCore.SwaggerGen.Test/Fixtures/FakeController.cs +++ b/test/Swashbuckle.AspNetCore.SwaggerGen.Test/Fixtures/FakeController.cs @@ -54,6 +54,15 @@ public void ActionWithIntParameterWithRequiredAttribute([Required]int param) public void ActionWithIntParameterWithSwaggerIgnoreAttribute([SwaggerIgnore] int param) { } + public void ActionWithAcceptFromHeaderParameter([FromHeader] string accept, string param) + { } + + public void ActionWithContentTypeFromHeaderParameter([FromHeader(Name = "Content-Type")] string contentType, string param) + { } + + public void ActionWithAuthorizationFromHeaderParameter([FromHeader] string authorization, string param) + { } + public void ActionWithObjectParameter(XmlAnnotatedType param) { } diff --git a/test/Swashbuckle.AspNetCore.SwaggerGen.Test/SwaggerGenerator/SwaggerGeneratorTests.cs b/test/Swashbuckle.AspNetCore.SwaggerGen.Test/SwaggerGenerator/SwaggerGeneratorTests.cs index e2d5e187c..d448f825d 100644 --- a/test/Swashbuckle.AspNetCore.SwaggerGen.Test/SwaggerGenerator/SwaggerGeneratorTests.cs +++ b/test/Swashbuckle.AspNetCore.SwaggerGen.Test/SwaggerGenerator/SwaggerGeneratorTests.cs @@ -1,14 +1,16 @@ -using System; +using System; using System.Collections.Generic; using System.Linq; using System.Text.Json; using System.Threading.Tasks; +using System.Reflection; using Microsoft.AspNetCore.Authentication; -using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.ApiExplorer; using Microsoft.AspNetCore.Mvc.Controllers; using Microsoft.AspNetCore.Mvc.ModelBinding; +using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Routing; using Microsoft.OpenApi.Any; using Microsoft.OpenApi.Models; @@ -502,6 +504,117 @@ public void GetSwagger_IgnoresParameters_IfActionParameterHasSwaggerIgnoreAttrib Assert.Empty(operation.Parameters); } + [Theory] + [InlineData(nameof(FakeController.ActionWithAcceptFromHeaderParameter))] + [InlineData(nameof(FakeController.ActionWithContentTypeFromHeaderParameter))] + [InlineData(nameof(FakeController.ActionWithAuthorizationFromHeaderParameter))] + public void GetSwagger_IgnoresParameters_IfActionParameterIsIllegalHeaderParameter(string action) + { + var illegalParameter = typeof(FakeController).GetMethod(action).GetParameters()[0]; + var fromHeaderAttribute = illegalParameter.GetCustomAttribute(); + + var subject = Subject( + new[] + { + ApiDescriptionFactory.Create( + c => action, + groupName: "v1", + httpMethod: "GET", + relativePath: "resource", + parameterDescriptions: new[] + { + new ApiParameterDescription + { + Name = fromHeaderAttribute?.Name ?? illegalParameter.Name, + Source = BindingSource.Header, + ModelMetadata = ModelMetadataFactory.CreateForParameter(illegalParameter) + }, + new ApiParameterDescription + { + Name = "param", + Source = BindingSource.Header + } + } + ) + } + ); + + var document = subject.GetSwagger("v1"); + + var operation = document.Paths["/resource"].Operations[OperationType.Get]; + var parameter = Assert.Single(operation.Parameters); + Assert.Equal("param", parameter.Name); + } + + [Theory] + [InlineData(nameof(FakeController.ActionWithAcceptFromHeaderParameter))] + [InlineData(nameof(FakeController.ActionWithContentTypeFromHeaderParameter))] + [InlineData(nameof(FakeController.ActionWithAuthorizationFromHeaderParameter))] + public void GetSwagger_GenerateParametersSchemas_IfActionParameterIsIllegalHeaderParameterWithProvidedOpenApiOperation(string action) + { + var illegalParameter = typeof(FakeController).GetMethod(action).GetParameters()[0]; + var fromHeaderAttribute = illegalParameter.GetCustomAttribute(); + var illegalParameterName = fromHeaderAttribute?.Name ?? illegalParameter.Name; + var methodInfo = typeof(FakeController).GetMethod(action); + var actionDescriptor = new ActionDescriptor + { + EndpointMetadata = new List() + { + new OpenApiOperation + { + OperationId = "OperationIdSetInMetadata", + Parameters = new List() + { + new OpenApiParameter + { + Name = illegalParameterName, + }, + new OpenApiParameter + { + Name = "param", + } + } + } + }, + RouteValues = new Dictionary + { + ["controller"] = methodInfo.DeclaringType.Name.Replace("Controller", string.Empty) + } + }; + var subject = Subject( + apiDescriptions: new[] + { + ApiDescriptionFactory.Create( + actionDescriptor, + methodInfo, + groupName: "v1", + httpMethod: "GET", + relativePath: "resource", + parameterDescriptions: new[] + { + new ApiParameterDescription + { + Name = illegalParameterName, + Source = BindingSource.Header, + ModelMetadata = ModelMetadataFactory.CreateForParameter(illegalParameter) + }, + new ApiParameterDescription + { + Name = "param", + Source = BindingSource.Header, + ModelMetadata = ModelMetadataFactory.CreateForType(typeof(string)) + } + }), + } + ); + + var document = subject.GetSwagger("v1"); + + var operation = document.Paths["/resource"].Operations[OperationType.Get]; + Assert.Null(operation.Parameters.Single(p => p.Name == illegalParameterName).Schema); + Assert.NotNull(operation.Parameters.Single(p => p.Name == "param").Schema); + } + [Fact] public void GetSwagger_SetsParameterRequired_IfApiParameterIsBoundToPath() { diff --git a/test/WebSites/Basic/Controllers/FromHeaderParamsController.cs b/test/WebSites/Basic/Controllers/FromHeaderParamsController.cs new file mode 100644 index 000000000..cdab581ab --- /dev/null +++ b/test/WebSites/Basic/Controllers/FromHeaderParamsController.cs @@ -0,0 +1,18 @@ +using Microsoft.AspNetCore.Mvc; + +namespace Basic.Controllers +{ + [Produces("application/json")] + public class FromHeaderParamsController + { + [HttpGet("country/validate")] + public IActionResult Get( + [FromHeader]string accept, + [FromHeader(Name = "Content-Type")] string contentType, + [FromHeader] string authorization, + [FromQuery] string country) + { + return new NoContentResult(); + } + } +}