Code review changes

domaindrivendev · Apr 29, 2024 · 3a21817 · 3a21817
1 parent 5f84dc1
commit 3a21817
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 19 deletions.
diff --git 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<string> IllegalHeaderParameters = new List<string>
+        private static readonly HashSet<string> IllegalHeaderParameters = new HashSet<string>(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
@@ -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
@@ -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<FromHeaderAttribute>();
+            var illegalParameter = typeof(FakeController).GetMethod(action).GetParameters()[0];
+            var fromHeaderAttribute = illegalParameter.GetCustomAttribute<FromHeaderAttribute>();
 
             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<FromHeaderAttribute>();
+            var illegalParameter = typeof(FakeController).GetMethod(action).GetParameters()[0];
+            var fromHeaderAttribute = illegalParameter.GetCustomAttribute<FromHeaderAttribute>();
+            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]