From 97656716c15f8c40e978a6c41e7ba561972e7d9b Mon Sep 17 00:00:00 2001 From: Keah Peters Date: Sun, 28 Apr 2024 19:57:40 +0100 Subject: [PATCH 1/9] Filter illegal header fields --- .../ApiParameterDescriptionExtensions.cs | 12 +++ .../SwaggerGenerator/SwaggerGenerator.cs | 5 +- .../Fixtures/FakeController.cs | 9 ++ .../SwaggerGenerator/SwaggerGeneratorTests.cs | 87 ++++++++++++++++++- .../Controllers/FromHeaderParamsController.cs | 18 ++++ 5 files changed, 127 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 021e2c495e..9b03d20578 100644 --- a/src/Swashbuckle.AspNetCore.SwaggerGen/SwaggerGenerator/ApiParameterDescriptionExtensions.cs +++ b/src/Swashbuckle.AspNetCore.SwaggerGen/SwaggerGenerator/ApiParameterDescriptionExtensions.cs @@ -21,6 +21,13 @@ public static class ApiParameterDescriptionExtensions #endif }; + private static readonly List IllegalHeaderParameters = new List + { + "accept", + "content-type", + "authorization" + }; + public static bool IsRequiredParameter(this ApiParameterDescription apiParameter) { // From the OpenAPI spec: @@ -111,5 +118,10 @@ 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) + { + return apiParameter.Source == BindingSource.Header && IllegalHeaderParameters.Contains(apiParameter.Name, StringComparer.OrdinalIgnoreCase); + } } } diff --git a/src/Swashbuckle.AspNetCore.SwaggerGen/SwaggerGenerator/SwaggerGenerator.cs b/src/Swashbuckle.AspNetCore.SwaggerGen/SwaggerGenerator/SwaggerGenerator.cs index 4c4a6a19c0..6d047c164f 100644 --- a/src/Swashbuckle.AspNetCore.SwaggerGen/SwaggerGenerator/SwaggerGenerator.cs +++ b/src/Swashbuckle.AspNetCore.SwaggerGen/SwaggerGenerator/SwaggerGenerator.cs @@ -255,7 +255,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( @@ -322,7 +322,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 78937d1549..8067084eed 100644 --- a/test/Swashbuckle.AspNetCore.SwaggerGen.Test/Fixtures/FakeController.cs +++ b/test/Swashbuckle.AspNetCore.SwaggerGen.Test/Fixtures/FakeController.cs @@ -53,6 +53,15 @@ public void ActionWithIntParameterWithRequiredAttribute([Required]int param) public void ActionWithIntParameterWithSwaggerIgnoreAttribute([SwaggerIgnore] int param) { } + public void ActionWithAcceptFromHeaderParameter([FromHeader(Name = "Accept")] int accept) + { } + + public void ActionWithContentTypeFromHeaderParameter([FromHeader(Name = "Content-Type")] int param) + { } + + public void ActionWithAuthorizationFromHeaderParameter([FromHeader(Name = "Authorization")] int 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 4fc992006d..07744f63ed 100644 --- a/test/Swashbuckle.AspNetCore.SwaggerGen.Test/SwaggerGenerator/SwaggerGeneratorTests.cs +++ b/test/Swashbuckle.AspNetCore.SwaggerGen.Test/SwaggerGenerator/SwaggerGeneratorTests.cs @@ -2,6 +2,7 @@ using System.Collections.Generic; using System.Linq; using System.Text.Json; +using Microsoft.AspNetCore.Authentication; using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.ApiExplorer; using Microsoft.AspNetCore.Mvc.ModelBinding; @@ -13,8 +14,6 @@ using Swashbuckle.AspNetCore.TestSupport; using Xunit; using System.Threading.Tasks; -using Microsoft.AspNetCore.Server.HttpSys; -using Microsoft.AspNetCore.Authentication; namespace Swashbuckle.AspNetCore.SwaggerGen.Test { @@ -502,6 +501,90 @@ public void GetSwagger_IgnoresParameters_IfActionParameterHasSwaggerIgnoreAttrib Assert.Empty(operation.Parameters); } + [Fact] + public void GetSwagger_IgnoresParameters_IfActionParameterIsIllegalHeaderParameter() + { + var parameter = typeof(FakeController).GetMethod(nameof(FakeController.ActionWithAcceptFromHeaderParameter)).GetParameters()[0]; + + var subject = Subject( + new[] + { + ApiDescriptionFactory.Create( + c => nameof(c.ActionWithAcceptFromHeaderParameter), + groupName: "v1", + httpMethod: "GET", + relativePath: "resource", + parameterDescriptions: new[] + { + new ApiParameterDescription + { + Name = parameter.Name, + Source = BindingSource.Header, + ModelMetadata = ModelMetadataFactory.CreateForParameter(parameter) + } + } + ) + } + ); + + var document = subject.GetSwagger("v1"); + + var operation = document.Paths["/resource"].Operations[OperationType.Get]; + Assert.Empty(operation.Parameters); + } + + [Fact] + public void GetSwagger_GenerateParametersSchemas_IfActionParameterIsIllegalHeaderParameterWithProvidedOpenApiOperation() + { + var methodInfo = typeof(FakeController).GetMethod(nameof(FakeController.ActionWithAcceptFromHeaderParameter)); + var actionDescriptor = new ActionDescriptor + { + EndpointMetadata = new List() + { + new OpenApiOperation + { + OperationId = "OperationIdSetInMetadata", + Parameters = new List() + { + new OpenApiParameter + { + Name = "accept" + } + } + } + }, + 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 = "accept", + 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[0].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 0000000000..cdab581ab5 --- /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(); + } + } +} From 2210c9b9f57400f249a912e6b53440102a4dc705 Mon Sep 17 00:00:00 2001 From: Keah Peters Date: Sun, 28 Apr 2024 20:00:34 +0100 Subject: [PATCH 2/9] Cleanup --- .../Fixtures/FakeController.cs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/test/Swashbuckle.AspNetCore.SwaggerGen.Test/Fixtures/FakeController.cs b/test/Swashbuckle.AspNetCore.SwaggerGen.Test/Fixtures/FakeController.cs index 8067084eed..7ea07516b1 100644 --- a/test/Swashbuckle.AspNetCore.SwaggerGen.Test/Fixtures/FakeController.cs +++ b/test/Swashbuckle.AspNetCore.SwaggerGen.Test/Fixtures/FakeController.cs @@ -56,12 +56,6 @@ public void ActionWithIntParameterWithSwaggerIgnoreAttribute([SwaggerIgnore] int public void ActionWithAcceptFromHeaderParameter([FromHeader(Name = "Accept")] int accept) { } - public void ActionWithContentTypeFromHeaderParameter([FromHeader(Name = "Content-Type")] int param) - { } - - public void ActionWithAuthorizationFromHeaderParameter([FromHeader(Name = "Authorization")] int param) - { } - public void ActionWithObjectParameter(XmlAnnotatedType param) { } From e021c47ff61e1eac5de1f8eecd165f068e2ef423 Mon Sep 17 00:00:00 2001 From: Keah Peters Date: Sun, 28 Apr 2024 23:58:14 +0100 Subject: [PATCH 3/9] Cleanup --- .../Fixtures/FakeController.cs | 2 +- .../SwaggerGenerator/SwaggerGeneratorTests.cs | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/test/Swashbuckle.AspNetCore.SwaggerGen.Test/Fixtures/FakeController.cs b/test/Swashbuckle.AspNetCore.SwaggerGen.Test/Fixtures/FakeController.cs index 7ea07516b1..3a7aa88f56 100644 --- a/test/Swashbuckle.AspNetCore.SwaggerGen.Test/Fixtures/FakeController.cs +++ b/test/Swashbuckle.AspNetCore.SwaggerGen.Test/Fixtures/FakeController.cs @@ -53,7 +53,7 @@ public void ActionWithIntParameterWithRequiredAttribute([Required]int param) public void ActionWithIntParameterWithSwaggerIgnoreAttribute([SwaggerIgnore] int param) { } - public void ActionWithAcceptFromHeaderParameter([FromHeader(Name = "Accept")] int accept) + public void ActionWithAcceptFromHeaderParameter([FromHeader] string accept) { } 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 07744f63ed..8be5d8787d 100644 --- a/test/Swashbuckle.AspNetCore.SwaggerGen.Test/SwaggerGenerator/SwaggerGeneratorTests.cs +++ b/test/Swashbuckle.AspNetCore.SwaggerGen.Test/SwaggerGenerator/SwaggerGeneratorTests.cs @@ -14,6 +14,7 @@ using Swashbuckle.AspNetCore.TestSupport; using Xunit; using System.Threading.Tasks; +using System.Reflection.Metadata; namespace Swashbuckle.AspNetCore.SwaggerGen.Test { @@ -536,6 +537,7 @@ public void GetSwagger_IgnoresParameters_IfActionParameterIsIllegalHeaderParamet [Fact] public void GetSwagger_GenerateParametersSchemas_IfActionParameterIsIllegalHeaderParameterWithProvidedOpenApiOperation() { + var parameter = typeof(FakeController).GetMethod(nameof(FakeController.ActionWithAcceptFromHeaderParameter)).GetParameters()[0]; var methodInfo = typeof(FakeController).GetMethod(nameof(FakeController.ActionWithAcceptFromHeaderParameter)); var actionDescriptor = new ActionDescriptor { @@ -548,7 +550,7 @@ public void GetSwagger_GenerateParametersSchemas_IfActionParameterIsIllegalHeade { new OpenApiParameter { - Name = "accept" + Name = parameter.Name } } } @@ -571,9 +573,9 @@ public void GetSwagger_GenerateParametersSchemas_IfActionParameterIsIllegalHeade { new ApiParameterDescription { - Name = "accept", + Name = parameter.Name, Source = BindingSource.Header, - ModelMetadata = ModelMetadataFactory.CreateForType(typeof(string)) + ModelMetadata = ModelMetadataFactory.CreateForParameter(parameter) } }), } From b779e89e9df6de0538e675704cd8b87c8cdd1e98 Mon Sep 17 00:00:00 2001 From: Keah Peters Date: Mon, 29 Apr 2024 00:41:32 +0100 Subject: [PATCH 4/9] Improve unit tests --- .../Fixtures/FakeController.cs | 6 ++++ .../SwaggerGenerator/SwaggerGeneratorTests.cs | 33 ++++++++++++------- 2 files changed, 27 insertions(+), 12 deletions(-) diff --git a/test/Swashbuckle.AspNetCore.SwaggerGen.Test/Fixtures/FakeController.cs b/test/Swashbuckle.AspNetCore.SwaggerGen.Test/Fixtures/FakeController.cs index 3a7aa88f56..144d4018ea 100644 --- a/test/Swashbuckle.AspNetCore.SwaggerGen.Test/Fixtures/FakeController.cs +++ b/test/Swashbuckle.AspNetCore.SwaggerGen.Test/Fixtures/FakeController.cs @@ -56,6 +56,12 @@ public void ActionWithIntParameterWithSwaggerIgnoreAttribute([SwaggerIgnore] int public void ActionWithAcceptFromHeaderParameter([FromHeader] string accept) { } + public void ActionWithContentTypeFromHeaderParameter([FromHeader(Name = "Content-Type")] string contentType) + { } + + public void ActionWithAuthorizationFromHeaderParameter([FromHeader] string authorization) + { } + 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 8be5d8787d..e45fc771a3 100644 --- a/test/Swashbuckle.AspNetCore.SwaggerGen.Test/SwaggerGenerator/SwaggerGeneratorTests.cs +++ b/test/Swashbuckle.AspNetCore.SwaggerGen.Test/SwaggerGenerator/SwaggerGeneratorTests.cs @@ -3,6 +3,7 @@ using System.Linq; using System.Text.Json; using Microsoft.AspNetCore.Authentication; +using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.ApiExplorer; using Microsoft.AspNetCore.Mvc.ModelBinding; @@ -14,7 +15,7 @@ using Swashbuckle.AspNetCore.TestSupport; using Xunit; using System.Threading.Tasks; -using System.Reflection.Metadata; +using System.Reflection; namespace Swashbuckle.AspNetCore.SwaggerGen.Test { @@ -502,16 +503,20 @@ public void GetSwagger_IgnoresParameters_IfActionParameterHasSwaggerIgnoreAttrib Assert.Empty(operation.Parameters); } - [Fact] - public void GetSwagger_IgnoresParameters_IfActionParameterIsIllegalHeaderParameter() + [Theory] + [InlineData(nameof(FakeController.ActionWithAcceptFromHeaderParameter))] + [InlineData(nameof(FakeController.ActionWithContentTypeFromHeaderParameter))] + [InlineData(nameof(FakeController.ActionWithAuthorizationFromHeaderParameter))] + public void GetSwagger_IgnoresParameters_IfActionParameterIsIllegalHeaderParameter(string action) { - var parameter = typeof(FakeController).GetMethod(nameof(FakeController.ActionWithAcceptFromHeaderParameter)).GetParameters()[0]; + var parameter = typeof(FakeController).GetMethod(action).GetParameters()[0]; + var fromHeaderAttribute = parameter.GetCustomAttribute(); var subject = Subject( new[] { ApiDescriptionFactory.Create( - c => nameof(c.ActionWithAcceptFromHeaderParameter), + c => action, groupName: "v1", httpMethod: "GET", relativePath: "resource", @@ -519,7 +524,7 @@ public void GetSwagger_IgnoresParameters_IfActionParameterIsIllegalHeaderParamet { new ApiParameterDescription { - Name = parameter.Name, + Name = fromHeaderAttribute?.Name ?? parameter.Name, Source = BindingSource.Header, ModelMetadata = ModelMetadataFactory.CreateForParameter(parameter) } @@ -534,11 +539,15 @@ public void GetSwagger_IgnoresParameters_IfActionParameterIsIllegalHeaderParamet Assert.Empty(operation.Parameters); } - [Fact] - public void GetSwagger_GenerateParametersSchemas_IfActionParameterIsIllegalHeaderParameterWithProvidedOpenApiOperation() + [Theory] + [InlineData(nameof(FakeController.ActionWithAcceptFromHeaderParameter))] + [InlineData(nameof(FakeController.ActionWithContentTypeFromHeaderParameter))] + [InlineData(nameof(FakeController.ActionWithAuthorizationFromHeaderParameter))] + public void GetSwagger_GenerateParametersSchemas_IfActionParameterIsIllegalHeaderParameterWithProvidedOpenApiOperation(string action) { - var parameter = typeof(FakeController).GetMethod(nameof(FakeController.ActionWithAcceptFromHeaderParameter)).GetParameters()[0]; - var methodInfo = typeof(FakeController).GetMethod(nameof(FakeController.ActionWithAcceptFromHeaderParameter)); + var parameter = typeof(FakeController).GetMethod(action).GetParameters()[0]; + var fromHeaderAttribute = parameter.GetCustomAttribute(); + var methodInfo = typeof(FakeController).GetMethod(action); var actionDescriptor = new ActionDescriptor { EndpointMetadata = new List() @@ -550,7 +559,7 @@ public void GetSwagger_GenerateParametersSchemas_IfActionParameterIsIllegalHeade { new OpenApiParameter { - Name = parameter.Name + Name = fromHeaderAttribute?.Name ?? parameter.Name, } } } @@ -573,7 +582,7 @@ public void GetSwagger_GenerateParametersSchemas_IfActionParameterIsIllegalHeade { new ApiParameterDescription { - Name = parameter.Name, + Name = fromHeaderAttribute?.Name ?? parameter.Name, Source = BindingSource.Header, ModelMetadata = ModelMetadataFactory.CreateForParameter(parameter) } From 15440eaeb208e610b77c7f5b9df6f9a66f90d9a5 Mon Sep 17 00:00:00 2001 From: Keah Peters Date: Mon, 29 Apr 2024 09:52:03 +0100 Subject: [PATCH 5/9] Cleanup --- .../SwaggerGenerator/SwaggerGeneratorTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/Swashbuckle.AspNetCore.SwaggerGen.Test/SwaggerGenerator/SwaggerGeneratorTests.cs b/test/Swashbuckle.AspNetCore.SwaggerGen.Test/SwaggerGenerator/SwaggerGeneratorTests.cs index e45fc771a3..0c5c4d1ba9 100644 --- a/test/Swashbuckle.AspNetCore.SwaggerGen.Test/SwaggerGenerator/SwaggerGeneratorTests.cs +++ b/test/Swashbuckle.AspNetCore.SwaggerGen.Test/SwaggerGenerator/SwaggerGeneratorTests.cs @@ -2,6 +2,8 @@ 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.Mvc; using Microsoft.AspNetCore.Mvc.Abstractions; @@ -14,8 +16,6 @@ using Swashbuckle.AspNetCore.Swagger; using Swashbuckle.AspNetCore.TestSupport; using Xunit; -using System.Threading.Tasks; -using System.Reflection; namespace Swashbuckle.AspNetCore.SwaggerGen.Test { From 5f84dc19965491666ee0e2015b58b63a7fa7103c Mon Sep 17 00:00:00 2001 From: Keah Peters Date: Mon, 29 Apr 2024 10:05:25 +0100 Subject: [PATCH 6/9] Add missing using --- .../SwaggerGenerator/SwaggerGeneratorTests.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/test/Swashbuckle.AspNetCore.SwaggerGen.Test/SwaggerGenerator/SwaggerGeneratorTests.cs b/test/Swashbuckle.AspNetCore.SwaggerGen.Test/SwaggerGenerator/SwaggerGeneratorTests.cs index 060899c0c4..83f9668bab 100644 --- a/test/Swashbuckle.AspNetCore.SwaggerGen.Test/SwaggerGenerator/SwaggerGeneratorTests.cs +++ b/test/Swashbuckle.AspNetCore.SwaggerGen.Test/SwaggerGenerator/SwaggerGeneratorTests.cs @@ -8,6 +8,7 @@ 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; From 3a2181701508dad4c21fbec303365c39a4dd00f9 Mon Sep 17 00:00:00 2001 From: Keah Peters Date: Mon, 29 Apr 2024 12:50:06 +0100 Subject: [PATCH 7/9] Code review changes --- .../ApiParameterDescriptionExtensions.cs | 13 ++++++---- .../Fixtures/FakeController.cs | 6 ++--- .../SwaggerGenerator/SwaggerGeneratorTests.cs | 25 +++++++++++-------- 3 files changed, 25 insertions(+), 19 deletions(-) diff --git a/src/Swashbuckle.AspNetCore.SwaggerGen/SwaggerGenerator/ApiParameterDescriptionExtensions.cs b/src/Swashbuckle.AspNetCore.SwaggerGen/SwaggerGenerator/ApiParameterDescriptionExtensions.cs index 9b03d20578..583597d92d 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,11 +22,11 @@ public static class ApiParameterDescriptionExtensions #endif }; - private static readonly List IllegalHeaderParameters = new List + private static readonly HashSet IllegalHeaderParameters = new HashSet(StringComparer.OrdinalIgnoreCase) { - "accept", - "content-type", - "authorization" + HeaderNames.Accept, + HeaderNames.ContentType, + HeaderNames.Authorization }; public static bool IsRequiredParameter(this ApiParameterDescription apiParameter) @@ -121,7 +122,9 @@ internal static bool IsFromForm(this ApiParameterDescription apiParameter) internal static bool IsIllegalHeaderParameter(this ApiParameterDescription apiParameter) { - return apiParameter.Source == BindingSource.Header && IllegalHeaderParameters.Contains(apiParameter.Name, StringComparer.OrdinalIgnoreCase); + // Certian 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/test/Swashbuckle.AspNetCore.SwaggerGen.Test/Fixtures/FakeController.cs b/test/Swashbuckle.AspNetCore.SwaggerGen.Test/Fixtures/FakeController.cs index b32a51e321..a25c6f7e58 100644 --- a/test/Swashbuckle.AspNetCore.SwaggerGen.Test/Fixtures/FakeController.cs +++ b/test/Swashbuckle.AspNetCore.SwaggerGen.Test/Fixtures/FakeController.cs @@ -54,13 +54,13 @@ public void ActionWithIntParameterWithRequiredAttribute([Required]int param) public void ActionWithIntParameterWithSwaggerIgnoreAttribute([SwaggerIgnore] int param) { } - public void ActionWithAcceptFromHeaderParameter([FromHeader] string accept) + public void ActionWithAcceptFromHeaderParameter([FromHeader] string accept, string param) { } - public void ActionWithContentTypeFromHeaderParameter([FromHeader(Name = "Content-Type")] string contentType) + public void ActionWithContentTypeFromHeaderParameter([FromHeader(Name = "Content-Type")] string contentType, string param) { } - public void ActionWithAuthorizationFromHeaderParameter([FromHeader] string authorization) + 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 83f9668bab..e10443ba7e 100644 --- a/test/Swashbuckle.AspNetCore.SwaggerGen.Test/SwaggerGenerator/SwaggerGeneratorTests.cs +++ b/test/Swashbuckle.AspNetCore.SwaggerGen.Test/SwaggerGenerator/SwaggerGeneratorTests.cs @@ -510,8 +510,8 @@ public void GetSwagger_IgnoresParameters_IfActionParameterHasSwaggerIgnoreAttrib [InlineData(nameof(FakeController.ActionWithAuthorizationFromHeaderParameter))] public void GetSwagger_IgnoresParameters_IfActionParameterIsIllegalHeaderParameter(string action) { - var parameter = typeof(FakeController).GetMethod(action).GetParameters()[0]; - var fromHeaderAttribute = parameter.GetCustomAttribute(); + var illegalParameter = typeof(FakeController).GetMethod(action).GetParameters()[0]; + var fromHeaderAttribute = illegalParameter.GetCustomAttribute(); var subject = Subject( new[] @@ -525,9 +525,9 @@ public void GetSwagger_IgnoresParameters_IfActionParameterIsIllegalHeaderParamet { new ApiParameterDescription { - Name = fromHeaderAttribute?.Name ?? parameter.Name, + Name = fromHeaderAttribute?.Name ?? illegalParameter.Name, Source = BindingSource.Header, - ModelMetadata = ModelMetadataFactory.CreateForParameter(parameter) + ModelMetadata = ModelMetadataFactory.CreateForParameter(illegalParameter) } } ) @@ -537,7 +537,8 @@ public void GetSwagger_IgnoresParameters_IfActionParameterIsIllegalHeaderParamet var document = subject.GetSwagger("v1"); var operation = document.Paths["/resource"].Operations[OperationType.Get]; - Assert.Empty(operation.Parameters); + var parameter = Assert.Single(operation.Parameters); + Assert.Equal("param", parameter.Name); } [Theory] @@ -546,8 +547,9 @@ public void GetSwagger_IgnoresParameters_IfActionParameterIsIllegalHeaderParamet [InlineData(nameof(FakeController.ActionWithAuthorizationFromHeaderParameter))] public void GetSwagger_GenerateParametersSchemas_IfActionParameterIsIllegalHeaderParameterWithProvidedOpenApiOperation(string action) { - var parameter = typeof(FakeController).GetMethod(action).GetParameters()[0]; - var fromHeaderAttribute = parameter.GetCustomAttribute(); + 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 { @@ -560,7 +562,7 @@ public void GetSwagger_GenerateParametersSchemas_IfActionParameterIsIllegalHeade { new OpenApiParameter { - Name = fromHeaderAttribute?.Name ?? parameter.Name, + Name = illegalParameterName, } } } @@ -583,9 +585,9 @@ public void GetSwagger_GenerateParametersSchemas_IfActionParameterIsIllegalHeade { new ApiParameterDescription { - Name = fromHeaderAttribute?.Name ?? parameter.Name, + Name = illegalParameterName, Source = BindingSource.Header, - ModelMetadata = ModelMetadataFactory.CreateForParameter(parameter) + ModelMetadata = ModelMetadataFactory.CreateForParameter(illegalParameter) } }), } @@ -594,7 +596,8 @@ public void GetSwagger_GenerateParametersSchemas_IfActionParameterIsIllegalHeade var document = subject.GetSwagger("v1"); var operation = document.Paths["/resource"].Operations[OperationType.Get]; - Assert.Null(operation.Parameters[0].Schema); + Assert.Null(operation.Parameters.Single(p => p.Name == illegalParameterName).Schema); + Assert.NotNull(operation.Parameters.Single(p => p.Name == "param").Schema); } [Fact] From 40cb61fa05f38cfd4c07cba87f7f200d59a5555f Mon Sep 17 00:00:00 2001 From: Keah Peters Date: Mon, 29 Apr 2024 13:18:09 +0100 Subject: [PATCH 8/9] Fix failing unit tests --- .../ApiParameterDescriptionExtensions.cs | 6 +++--- .../SwaggerGenerator/SwaggerGeneratorTests.cs | 17 ++++++++++++++++- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/src/Swashbuckle.AspNetCore.SwaggerGen/SwaggerGenerator/ApiParameterDescriptionExtensions.cs b/src/Swashbuckle.AspNetCore.SwaggerGen/SwaggerGenerator/ApiParameterDescriptionExtensions.cs index 583597d92d..e505905e60 100644 --- a/src/Swashbuckle.AspNetCore.SwaggerGen/SwaggerGenerator/ApiParameterDescriptionExtensions.cs +++ b/src/Swashbuckle.AspNetCore.SwaggerGen/SwaggerGenerator/ApiParameterDescriptionExtensions.cs @@ -25,8 +25,8 @@ public static class ApiParameterDescriptionExtensions private static readonly HashSet IllegalHeaderParameters = new HashSet(StringComparer.OrdinalIgnoreCase) { HeaderNames.Accept, - HeaderNames.ContentType, - HeaderNames.Authorization + HeaderNames.Authorization, + HeaderNames.ContentType }; public static bool IsRequiredParameter(this ApiParameterDescription apiParameter) @@ -122,7 +122,7 @@ internal static bool IsFromForm(this ApiParameterDescription apiParameter) internal static bool IsIllegalHeaderParameter(this ApiParameterDescription apiParameter) { - // Certian header parameters are not allowed and should be described using the corresponding OpenAPI keywords + // 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/test/Swashbuckle.AspNetCore.SwaggerGen.Test/SwaggerGenerator/SwaggerGeneratorTests.cs b/test/Swashbuckle.AspNetCore.SwaggerGen.Test/SwaggerGenerator/SwaggerGeneratorTests.cs index e10443ba7e..2d7b76d7cb 100644 --- a/test/Swashbuckle.AspNetCore.SwaggerGen.Test/SwaggerGenerator/SwaggerGeneratorTests.cs +++ b/test/Swashbuckle.AspNetCore.SwaggerGen.Test/SwaggerGenerator/SwaggerGeneratorTests.cs @@ -528,6 +528,11 @@ public void GetSwagger_IgnoresParameters_IfActionParameterIsIllegalHeaderParamet Name = fromHeaderAttribute?.Name ?? illegalParameter.Name, Source = BindingSource.Header, ModelMetadata = ModelMetadataFactory.CreateForParameter(illegalParameter) + }, + new ApiParameterDescription + { + Name = "param", + Source = BindingSource.Header } } ) @@ -563,6 +568,10 @@ public void GetSwagger_GenerateParametersSchemas_IfActionParameterIsIllegalHeade new OpenApiParameter { Name = illegalParameterName, + }, + new OpenApiParameter + { + Name = "param", } } } @@ -588,10 +597,16 @@ public void GetSwagger_GenerateParametersSchemas_IfActionParameterIsIllegalHeade 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"); From a92859bb56361df3a703daaefefa7b1299cf5528 Mon Sep 17 00:00:00 2001 From: Keah Peters Date: Mon, 29 Apr 2024 13:19:00 +0100 Subject: [PATCH 9/9] Cleanup --- .../SwaggerGenerator/SwaggerGeneratorTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/Swashbuckle.AspNetCore.SwaggerGen.Test/SwaggerGenerator/SwaggerGeneratorTests.cs b/test/Swashbuckle.AspNetCore.SwaggerGen.Test/SwaggerGenerator/SwaggerGeneratorTests.cs index 2d7b76d7cb..d448f825dc 100644 --- a/test/Swashbuckle.AspNetCore.SwaggerGen.Test/SwaggerGenerator/SwaggerGeneratorTests.cs +++ b/test/Swashbuckle.AspNetCore.SwaggerGen.Test/SwaggerGenerator/SwaggerGeneratorTests.cs @@ -606,7 +606,7 @@ public void GetSwagger_GenerateParametersSchemas_IfActionParameterIsIllegalHeade } }), } - ); ; + ); var document = subject.GetSwagger("v1");