diff --git a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs index 94d04695dc9994416139d62780e5f32c728e25c1..a735b9abee20136b02505e48d70d6818e5c1324d 100644 --- a/src/Http/Http.Extensions/src/RequestDelegateFactory.cs +++ b/src/Http/Http.Extensions/src/RequestDelegateFactory.cs @@ -627,8 +627,7 @@ namespace Microsoft.AspNetCore.Http private static Expression BindParameterFromService(ParameterInfo parameter) { - var nullability = NullabilityContext.Create(parameter); - var isOptional = parameter.HasDefaultValue || nullability.ReadState == NullabilityState.Nullable; + var isOptional = IsOptionalParameter(parameter); return isOptional ? Expression.Call(GetServiceMethod.MakeGenericMethod(parameter.ParameterType), RequestServicesExpr) @@ -637,8 +636,7 @@ namespace Microsoft.AspNetCore.Http private static Expression BindParameterFromValue(ParameterInfo parameter, Expression valueExpression, FactoryContext factoryContext) { - var nullability = NullabilityContext.Create(parameter); - var isOptional = parameter.HasDefaultValue || nullability.ReadState == NullabilityState.Nullable; + var isOptional = IsOptionalParameter(parameter); var argument = Expression.Variable(parameter.ParameterType, $"{parameter.Name}_local"); @@ -671,7 +669,8 @@ namespace Microsoft.AspNetCore.Http } // Allow nullable parameters that don't have a default value - if (nullability.ReadState == NullabilityState.Nullable && !parameter.HasDefaultValue) + var nullability = NullabilityContext.Create(parameter); + if (nullability.ReadState != NullabilityState.NotNull && !parameter.HasDefaultValue) { return valueExpression; } @@ -817,7 +816,7 @@ namespace Microsoft.AspNetCore.Http { // We reference the boundValues array by parameter index here var nullability = NullabilityContext.Create(parameter); - var isOptional = parameter.HasDefaultValue || nullability.ReadState == NullabilityState.Nullable; + var isOptional = IsOptionalParameter(parameter); // Get the BindAsync method var body = TryParseMethodCache.FindBindAsyncMethod(parameter.ParameterType)!; @@ -862,8 +861,7 @@ namespace Microsoft.AspNetCore.Http } } - var nullability = NullabilityContext.Create(parameter); - var isOptional = parameter.HasDefaultValue || nullability.ReadState == NullabilityState.Nullable; + var isOptional = IsOptionalParameter(parameter); factoryContext.JsonRequestBodyType = parameter.ParameterType; factoryContext.AllowEmptyRequestBody = allowEmpty || isOptional; @@ -903,6 +901,19 @@ namespace Microsoft.AspNetCore.Http return Expression.Convert(BodyValueExpr, parameter.ParameterType); } + private static bool IsOptionalParameter(ParameterInfo parameter) + { + // - Parameters representing value or reference types with a default value + // under any nullability context are treated as optional. + // - Value type parameters without a default value in an oblivious + // nullability context are required. + // - Reference type parameters without a default value in an oblivious + // nullability context are optional. + var nullability = NullabilityContext.Create(parameter); + return parameter.HasDefaultValue + || nullability.ReadState != NullabilityState.NotNull; + } + private static MethodInfo GetMethodInfo<T>(Expression<T> expr) { var mc = (MethodCallExpression)expr.Body; diff --git a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs index 00832e35454acf9e5a7912d0f5e2d3489938eae3..5eb0dec95d3202f01592d0f32fb5a71376268c88 100644 --- a/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs +++ b/src/Http/Http.Extensions/test/RequestDelegateFactoryTests.cs @@ -2137,6 +2137,35 @@ namespace Microsoft.AspNetCore.Routing.Internal Assert.Equal(expectedResponse, decodedResponseBody); } + [Theory] + [InlineData(true, "Age: 42")] + [InlineData(false, "Age: ")] + public async Task TreatsUnknownNullabilityAsOptionalForReferenceType(bool provideValue, string expectedResponse) + { + string optionalQueryParam(string age) => $"Age: {age}"; + + var httpContext = new DefaultHttpContext(); + var responseBodyStream = new MemoryStream(); + httpContext.Response.Body = responseBodyStream; + + if (provideValue) + { + httpContext.Request.Query = new QueryCollection(new Dictionary<string, StringValues> + { + ["age"] = "42" + }); + } + + var requestDelegate = RequestDelegateFactory.Create(optionalQueryParam); + + await requestDelegate(httpContext); + + Assert.Equal(200, httpContext.Response.StatusCode); + Assert.False(httpContext.RequestAborted.IsCancellationRequested); + var decodedResponseBody = Encoding.UTF8.GetString(responseBodyStream.ToArray()); + Assert.Equal(expectedResponse, decodedResponseBody); + } + #nullable enable private class Todo : ITodo diff --git a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs index 815d1b2692591d6526bd99b433581a5651a11165..f64d5b0a3b05a8eb4016d96637d2cc99e871d551 100644 --- a/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs +++ b/src/Mvc/Mvc.ApiExplorer/src/EndpointMetadataApiDescriptionProvider.cs @@ -143,7 +143,7 @@ namespace Microsoft.AspNetCore.Mvc.ApiExplorer // Determine the "requiredness" based on nullability, default value or if allowEmpty is set var nullability = NullabilityContext.Create(parameter); - var isOptional = parameter.HasDefaultValue || nullability.ReadState == NullabilityState.Nullable || allowEmpty; + var isOptional = parameter.HasDefaultValue || nullability.ReadState != NullabilityState.NotNull || allowEmpty; return new ApiParameterDescription { diff --git a/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs b/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs index c6d59ce15024ab016c682fcf2d6fbb0de1541c40..439fc686e53894aa9f34edc727c78ea1e5e0a69e 100644 --- a/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs +++ b/src/Mvc/Mvc.ApiExplorer/test/EndpointMetadataApiDescriptionProviderTest.cs @@ -348,7 +348,7 @@ namespace Microsoft.AspNetCore.Mvc.ApiExplorer Assert.Equal(typeof(InferredJsonClass), fromBodyParam.Type); Assert.Equal(typeof(InferredJsonClass), fromBodyParam.ModelMetadata.ModelType); Assert.Equal(BindingSource.Body, fromBodyParam.Source); - Assert.True(fromBodyParam.IsRequired); + Assert.False(fromBodyParam.IsRequired); // Reference type in oblivious nullability context } [Fact] @@ -421,6 +421,27 @@ namespace Microsoft.AspNetCore.Mvc.ApiExplorer Assert.True(apiExplorerSettings.IgnoreApi); } + [Fact] + public void TestParameterIsRequiredForObliviousNullabilityContext() + { + // In an oblivious nullability context, reference type parameters without + // annotations are optional. Value type parameters are always required. + var apiDescription = GetApiDescription((string foo, int bar) => { }); + Assert.Equal(2, apiDescription.ParameterDescriptions.Count); + + var fooParam = apiDescription.ParameterDescriptions[0]; + Assert.Equal(typeof(string), fooParam.Type); + Assert.Equal(typeof(string), fooParam.ModelMetadata.ModelType); + Assert.Equal(BindingSource.Query, fooParam.Source); + Assert.False(fooParam.IsRequired); + + var barParam = apiDescription.ParameterDescriptions[1]; + Assert.Equal(typeof(int), barParam.Type); + Assert.Equal(typeof(int), barParam.ModelMetadata.ModelType); + Assert.Equal(BindingSource.Query, barParam.Source); + Assert.True(barParam.IsRequired); + } + [Fact] public void RespectsProducesProblemExtensionMethod() {