Treat reference type parameters in oblivious nullability context as optional (#35563)

* Treat parameters in oblivious nullability context as optional

* Only apply fix for reference types

* Update optionality check in API descriptor

* Update check in BindAsync and Mvc.ApiExplorer test

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;

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

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
             {

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()
         {