Merge pull request #8160 from dotnet-maestro-bot/merge/release/2.2-to-master

[automated] Merge branch 'release/2.2' => 'master'

src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Validation/ValidationVisitor.cs

@@ -80,6 +80,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Validation
         /// Indicates whether validation of a complex type should be performed if validation fails for any of its children. The default behavior is false.
         /// </summary>
         public bool ValidateComplexTypesIfChildValidationFails { get; set; }
+
         /// <summary>
         /// Validates a object.
         /// </summary>
@@ -105,7 +106,10 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Validation
             if (model == null && key != null && !alwaysValidateAtTopLevel)
             {
                 var entry = ModelState[key];
-                if (entry != null && entry.ValidationState != ModelValidationState.Valid)
+
+                // Rationale: We might see the same model state key for two different objects and want to preserve any
+                // known invalidity.
+                if (entry != null && entry.ValidationState != ModelValidationState.Invalid)
                 {
                     entry.ValidationState = ModelValidationState.Valid;
                 }

test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/ParameterBinderTest.cs

@@ -836,6 +836,60 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
                 });
         }
 
+        // Regression test for aspnet/Mvc#8078. Later parameter should not mark entry as valid.
+        [Fact]
+        public async Task BindModelAsync_ForOverlappingParameters_InValid_WithInValidFirstParameterAndSecondNull()
+        {
+            // Arrange
+            var parameterDescriptor = new ParameterDescriptor
+            {
+                BindingInfo = new BindingInfo
+                {
+                    BinderModelName = "id",
+                },
+                Name = "identifier",
+                ParameterType = typeof(string),
+            };
+
+            var actionContext = GetControllerContext();
+            var modelState = actionContext.ModelState;
+
+            // Mimic ModelStateEntry when first parameter is [FromRoute] int id and request URI is /api/values/notAnInt
+            modelState.SetModelValue("id", "notAnInt", "notAnInt");
+            modelState.AddModelError("id", "This is not valid.");
+
+            var modelMetadataProvider = new TestModelMetadataProvider();
+            var modelMetadata = modelMetadataProvider.GetMetadataForType(typeof(string));
+            var parameterBinder = new ParameterBinder(
+                modelMetadataProvider,
+                Mock.Of<IModelBinderFactory>(),
+                new DefaultObjectValidator(
+                    modelMetadataProvider,
+                    new[] { TestModelValidatorProvider.CreateDefaultProvider() }),
+                _optionsAccessor,
+                NullLoggerFactory.Instance);
+
+            // Mimic result when second parameter is [FromQuery(Name = "id")] string identifier and query is ?id
+            var modelBindingResult = ModelBindingResult.Success(null);
+            var modelBinder = CreateMockModelBinder(modelBindingResult);
+
+            // Act
+            var result = await parameterBinder.BindModelAsync(
+                actionContext,
+                modelBinder,
+                new SimpleValueProvider(),
+                parameterDescriptor,
+                modelMetadata,
+                value: null);
+
+            // Assert
+            Assert.True(result.IsModelSet);
+            Assert.False(modelState.IsValid);
+            var keyValuePair = Assert.Single(modelState);
+            Assert.Equal("id", keyValuePair.Key);
+            Assert.Equal(ModelValidationState.Invalid, keyValuePair.Value.ValidationState);
+        }
+
         private static ControllerContext GetControllerContext()
         {
             var services = new ServiceCollection();