Fix behavior of StartsWithPrefix

This undoes a behavior change introduced in
7b18d1d3f1.

The intent was to have ClearValidationState do the right thing for a case
where a collection was bound to the empty prefix, and then used again with
TryUpdateModel.

This change was implemented by saying that a key like "[0].Foo" is a match
for the prefix of "Foo". This isn't really right, and it's only
interesting for the ClearValidationState case.

The problem is that we don't know what the keys look like for a
collection. We can assume that they start with [0] but that's not really a
guarantee, it's a guess.

This change fixes the behavior of StartsWithModel, and move the
responsibility for this case back into ClearValidationState.

This change also removes the call to ClearValidationState from
TryUpdateModel. If you need this behavior, then call ClearValidationState
manually. Trying to bind and then re-bind a model object isn't really what
we intend.

src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelStateDictionary.cs

@@ -683,49 +683,30 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
 
             if (prefix.Length == 0)
             {
-                // Everything is prefixed by the empty string
+                // Everything is prefixed by the empty string.
                 return true;
             }
 
-            if (key.Length < prefix.Length)
+            if (prefix.Length > key.Length)
+            {
+                return false; // Not long enough.
+            }
+
+            if (!key.StartsWith(prefix, StringComparison.OrdinalIgnoreCase))
             {
                 return false;
             }
             
-            var subKeyIndex = 0;
+            if (key.Length == prefix.Length)
-            if (!key.StartsWith(prefix, StringComparison.OrdinalIgnoreCase))
             {
-                if (key[0] == '[')
+                // Exact match
-                {
-                    subKeyIndex = key.IndexOf('.') + 1;
-
-                    if (string.Compare(key, subKeyIndex, prefix, 0, prefix.Length, StringComparison.OrdinalIgnoreCase) != 0)
-                    {
-                        return false;
-                    }
-                    else if (prefix.Length == (key.Length - subKeyIndex))
-                    {
-                        // prefix == subKey
-                        return true;
-                    }
-                }
-                else
-                {
-                    return false;
-                }
-            }
-            else if (key.Length == prefix.Length)
-            {
-                // key == prefix
                 return true;
             }
 
-            var charAfterPrefix = key[subKeyIndex + prefix.Length];
+            var charAfterPrefix = key[prefix.Length];
-            switch (charAfterPrefix)
+            if (charAfterPrefix == '.' || charAfterPrefix == '[')
             {
-                case '[':
+                return true;
-                case '.':
-                    return true;
             }
 
             return false;

src/Microsoft.AspNetCore.Mvc.Core/ControllerBase.cs

@@ -1430,20 +1430,11 @@ namespace Microsoft.AspNetCore.Mvc
                 throw new ArgumentNullException(nameof(model));
             }
             
-            var modelName = prefix ?? string.Empty;
-
-            // Clear ModelStateDictionary entries for the model so that it will be re-validated.
-            ModelBindingHelper.ClearValidationStateForModel(
-                model.GetType(),
-                ModelState,
-                MetadataProvider,
-                modelName);
-
             ObjectValidator.Validate(
                 ControllerContext,
                 new CompositeModelValidatorProvider(ControllerContext.ValidatorProviders),
                 validationState: null,
-                prefix: prefix,
+                prefix: prefix ?? string.Empty,
                 model: model);
             return ModelState.IsValid;
         }

src/Microsoft.AspNetCore.Mvc.Core/Internal/ElementalValueProvider.cs

@@ -24,7 +24,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal
 
         public bool ContainsPrefix(string prefix)
         {
-            return PrefixContainer.IsPrefixMatch(prefix, Key);
+            return ModelStateDictionary.StartsWithPrefix(prefix, Key);
         }
 
         public ValueProviderResult GetValue(string key)

src/Microsoft.AspNetCore.Mvc.Core/Internal/PrefixContainer.cs

@@ -173,45 +173,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal
             }
         }
 
-        public static bool IsPrefixMatch(string prefix, string testString)
+        // This is tightly coupled to the definition at ModelStateDictionary.StartsWithPrefix
-        {
-            if (testString == null)
-            {
-                return false;
-            }
-
-            if (prefix.Length == 0)
-            {
-                return true; // shortcut - non-null testString matches empty prefix
-            }
-
-            if (prefix.Length > testString.Length)
-            {
-                return false; // not long enough
-            }
-
-            if (!testString.StartsWith(prefix, StringComparison.OrdinalIgnoreCase))
-            {
-                return false; // prefix doesn't match
-            }
-
-            if (testString.Length == prefix.Length)
-            {
-                return true; // exact match
-            }
-
-            // invariant: testString.Length > prefix.Length
-            switch (testString[prefix.Length])
-            {
-                case '.':
-                case '[':
-                    return true; // known delimiters
-
-                default:
-                    return false; // not known delimiter
-            }
-        }
-
         private int BinarySearch(string prefix)
         {
             var start = 0;

src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/ModelBindingHelper.cs

@@ -513,10 +513,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
             }
 
             var modelMetadata = metadataProvider.GetMetadataForType(modelType);
-
-            // Clear ModelStateDictionary entries for the model so that it will be re-validated.
             var modelState = actionContext.ModelState;
-            ClearValidationStateForModel(modelType, modelState, metadataProvider, prefix);
 
             var operationBindingContext = new OperationBindingContext
             {
@@ -659,27 +656,76 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
                 throw new ArgumentNullException(nameof(metadataProvider));
             }
 
-            // If modelkey is empty, we need to iterate through properties (obtained from ModelMetadata) and
+            ClearValidationStateForModel(metadataProvider.GetMetadataForType(modelType), modelstate, modelKey);
-            // clear validation state for all entries in ModelStateDictionary that start with each property name.
+        }
-            // If modelkey is non-empty, clear validation state for all entries in ModelStateDictionary
+
-            // that start with modelKey
+        /// <summary>
+        /// Clears <see cref="ModelStateDictionary"/> entries for <see cref="ModelMetadata"/>.
+        /// </summary>
+        /// <param name="modelMetadata">The <see cref="ModelMetadata"/>.</param>
+        /// <param name="modelKey">The entry to clear. </param>
+        /// <param name="modelMetadataProvider">The <see cref="IModelMetadataProvider"/>.</param>
+        public static void ClearValidationStateForModel(
+            ModelMetadata modelMetadata,
+            ModelStateDictionary modelstate,
+            string modelKey)
+        {
+            if (modelMetadata == null)
+            {
+                throw new ArgumentNullException(nameof(modelMetadata));
+            }
+
+            if (modelstate == null)
+            {
+                throw new ArgumentNullException(nameof(modelstate));
+            }
+
             if (string.IsNullOrEmpty(modelKey))
             {
-                var modelMetadata = metadataProvider.GetMetadataForType(modelType);
+                // If model key is empty, we have to do a best guess to try and clear the appropriate
-                var elementMetadata = modelMetadata.ElementMetadata;
+                // keys. Clearing the empty prefix would clear the state of ALL entries, which might wipe out
-                if (elementMetadata != null)
+                // data from other models.
+                if (modelMetadata.IsEnumerableType)
                 {
-                    modelMetadata = elementMetadata;
+                    // We expect that any key beginning with '[' is an index. We can't just infer the indexes
+                    // used, so we clear all keys that look like <empty prefix -> index>.
+                    //
+                    // In the unlikely case that multiple top-level collections where bound to the empty prefix,
+                    // you're just out of luck.
+                    foreach (var kvp in modelstate)
+                    {
+                        if (kvp.Key.Length > 0 && kvp.Key[0] == '[')
+                        {
+                            // Starts with an indexer
+                            kvp.Value.Errors.Clear();
+                            kvp.Value.ValidationState = ModelValidationState.Unvalidated;
+                        }
+                    }
                 }
-
+                else if (modelMetadata.IsComplexType)
-                foreach (var property in modelMetadata.Properties)
                 {
-                    var childKey = property.BinderModelName ?? property.PropertyName;
+                    foreach (var property in modelMetadata.Properties)
-                    modelstate.ClearValidationState(childKey);
+                    {
+                        modelstate.ClearValidationState(property.BinderModelName ?? property.PropertyName);
+                    }
+                }
+                else
+                {
+                    // Simple types bind to a single entry. So clear the entry with the empty-key, in the
+                    // unlikely event that it has errors.
+                    var entry = modelstate[string.Empty];
+                    if (entry != null)
+                    {
+                        entry.Errors.Clear();
+                        entry.ValidationState = ModelValidationState.Unvalidated;
+                    }
                 }
             }
             else
             {
+                // If model key is non-empty, we just want to clear all keys with that prefix. We expect
+                // model binding to have only used this key (and suffixes) for all entries related to
+                // this model.
                 modelstate.ClearValidationState(modelKey);
             }
         }

test/Microsoft.AspNetCore.Mvc.Abstractions.Test/ModelBinding/ModelStateDictionaryTest.cs

@@ -245,8 +245,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
         [Theory]
         [InlineData("foo")]
         [InlineData("foo.bar")]
-        [InlineData("[0].foo.bar")]
+        [InlineData("foo[bar]")]
-        [InlineData("[0].foo.bar[0]")]
         public void GetFieldValidationState_ReturnsInvalidIfKeyChildContainsErrors(string key)
         {
             // Arrange
@@ -263,8 +262,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
         [Theory]
         [InlineData("foo")]
         [InlineData("foo.bar")]
-        [InlineData("[0].foo.bar")]
+        [InlineData("foo[bar]")]
-        [InlineData("[0].foo.bar[0]")]
         public void GetFieldValidationState_ReturnsValidIfModelStateDoesNotContainErrors(string key)
         {
             // Arrange
@@ -505,9 +503,9 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
         }
 
         [Theory]
+        [InlineData("")]
         [InlineData("user")]
         [InlineData("user.Age")]
-        [InlineData("product")]
         public void GetFieldValidity_ReturnsInvalid_IfAllKeysAreValidatedAndAnyEntryIsInvalid(string key)
         {
             // Arrange
@@ -515,9 +513,26 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
             dictionary["user.Address"] = new ModelStateEntry { ValidationState = ModelValidationState.Valid };
             dictionary["user.Name"] = new ModelStateEntry { ValidationState = ModelValidationState.Valid };
             dictionary.AddModelError("user.Age", "Age is not a valid int");
+
+            // Act
+            var validationState = dictionary.GetFieldValidationState(key);
+
+            // Assert
+            Assert.Equal(ModelValidationState.Invalid, validationState);
+        }
+
+        [Theory]
+        [InlineData("")]
+        [InlineData("[0]")]
+        [InlineData("[0].product")]
+        public void GetFieldValidity_ReturnsInvalid_IfAllKeysAreValidatedAndAnyEntryIsInvalid_Collection(string key)
+        {
+            // Arrange
+            var dictionary = new ModelStateDictionary();
+
             dictionary["[0].product.Name"] = new ModelStateEntry { ValidationState = ModelValidationState.Valid };
             dictionary["[0].product.Age[0]"] = new ModelStateEntry { ValidationState = ModelValidationState.Valid };
-            dictionary.AddModelError("[1].product.Name", "Name is invalid");
+            dictionary.AddModelError("[0].product.Name", "Name is invalid");
 
             // Act
             var validationState = dictionary.GetFieldValidationState(key);

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

@@ -687,10 +687,12 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
         [Theory]
         [InlineData("")]
         [InlineData(null)]
-        public void ClearValidationStateForModel_EmtpyModelKey(string modelKey)
+        public void ClearValidationState_ForComplexTypeModel_EmptyModelKey(string modelKey)
         {
             // Arrange
             var metadataProvider = new EmptyModelMetadataProvider();
+            var modelMetadata = metadataProvider.GetMetadataForType(typeof(Product));
+
             var dictionary = new ModelStateDictionary();
             dictionary["Name"] = new ModelStateEntry { ValidationState = ModelValidationState.Invalid };
             dictionary.AddModelError("Name", "MyProperty invalid.");
@@ -699,12 +701,11 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
             dictionary.AddModelError("Id", "Id is required.");
             dictionary["Category"] = new ModelStateEntry { ValidationState = ModelValidationState.Valid };
 
+            dictionary["Unrelated"] = new ModelStateEntry { ValidationState = ModelValidationState.Invalid };
+            dictionary.AddModelError("Unrelated", "Unrelated is required.");
+
             // Act
-            ModelBindingHelper.ClearValidationStateForModel(
+            ModelBindingHelper.ClearValidationStateForModel(modelMetadata, dictionary, modelKey);
-                typeof(Product),
-                dictionary,
-                metadataProvider,
-                modelKey);
 
             // Assert
             Assert.Equal(0, dictionary["Name"].Errors.Count);
@@ -713,15 +714,48 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
             Assert.Equal(ModelValidationState.Unvalidated, dictionary["Id"].ValidationState);
             Assert.Equal(0, dictionary["Category"].Errors.Count);
             Assert.Equal(ModelValidationState.Unvalidated, dictionary["Category"].ValidationState);
+
+            Assert.Equal(1, dictionary["Unrelated"].Errors.Count);
+            Assert.Equal(ModelValidationState.Invalid, dictionary["Unrelated"].ValidationState);
+        }
+
+        // Not a wholly realistic scenario, but testing it regardless.
+        [Theory]
+        [InlineData("")]
+        [InlineData(null)]
+        public void ClearValidationState_ForSimpleTypeModel_EmptyModelKey(string modelKey)
+        {
+            // Arrange
+            var metadataProvider = new EmptyModelMetadataProvider();
+            var modelMetadata = metadataProvider.GetMetadataForType(typeof(string));
+
+            var dictionary = new ModelStateDictionary();
+            dictionary[string.Empty] = new ModelStateEntry { ValidationState = ModelValidationState.Invalid };
+            dictionary.AddModelError(string.Empty, "MyProperty invalid.");
+
+            dictionary["Unrelated"] = new ModelStateEntry { ValidationState = ModelValidationState.Invalid };
+            dictionary.AddModelError("Unrelated", "Unrelated is required.");
+
+            // Act
+            ModelBindingHelper.ClearValidationStateForModel(modelMetadata, dictionary, modelKey);
+
+            // Assert
+            Assert.Equal(0, dictionary[string.Empty].Errors.Count);
+            Assert.Equal(ModelValidationState.Unvalidated, dictionary[string.Empty].ValidationState);
+
+            Assert.Equal(1, dictionary["Unrelated"].Errors.Count);
+            Assert.Equal(ModelValidationState.Invalid, dictionary["Unrelated"].ValidationState);
         }
 
         [Theory]
         [InlineData("")]
         [InlineData(null)]
-        public void ClearValidationStateForCollectionsModel_EmtpyModelKey(string modelKey)
+        public void ClearValidationState_ForCollectionsModel_EmptyModelKey(string modelKey)
         {
             // Arrange
             var metadataProvider = new EmptyModelMetadataProvider();
+            var modelMetadata = metadataProvider.GetMetadataForType(typeof(List<Product>));
+
             var dictionary = new ModelStateDictionary();
             dictionary["[0].Name"] = new ModelStateEntry { ValidationState = ModelValidationState.Invalid };
             dictionary.AddModelError("[0].Name", "Name invalid.");
@@ -735,12 +769,11 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
             dictionary["[1].Category"] = new ModelStateEntry { ValidationState = ModelValidationState.Invalid };
             dictionary.AddModelError("[1].Category", "Category invalid.");
 
+            dictionary["Unrelated"] = new ModelStateEntry { ValidationState = ModelValidationState.Invalid };
+            dictionary.AddModelError("Unrelated", "Unrelated is required.");
+
             // Act
-            ModelBindingHelper.ClearValidationStateForModel(
+            ModelBindingHelper.ClearValidationStateForModel(modelMetadata, dictionary, modelKey);
-                typeof(List<Product>),
-                dictionary,
-                metadataProvider,
-                modelKey);
 
             // Assert
             Assert.Equal(0, dictionary["[0].Name"].Errors.Count);
@@ -755,6 +788,9 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
             Assert.Equal(ModelValidationState.Unvalidated, dictionary["[1].Id"].ValidationState);
             Assert.Equal(0, dictionary["[1].Category"].Errors.Count);
             Assert.Equal(ModelValidationState.Unvalidated, dictionary["[1].Category"].ValidationState);
+
+            Assert.Equal(1, dictionary["Unrelated"].Errors.Count);
+            Assert.Equal(ModelValidationState.Invalid, dictionary["Unrelated"].ValidationState);
         }
 
         [Theory]
@@ -764,10 +800,11 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
         [InlineData("product.Order[0].Address.Street")]
         [InlineData("product.Category.Name")]
         [InlineData("product.Order")]
-        public void ClearValidationStateForModel_NonEmtpyModelKey(string prefix)
+        public void ClearValidationState_ForComplexModel_NonEmptyModelKey(string prefix)
         {
             // Arrange
-            var metadataProvider = new TestModelMetadataProvider();
+            var metadataProvider = new EmptyModelMetadataProvider();
+            var modelMetadata = metadataProvider.GetMetadataForType(typeof(Product));
 
             var dictionary = new ModelStateDictionary();
             dictionary["product.Name"] = new ModelStateEntry { ValidationState = ModelValidationState.Invalid };
@@ -787,11 +824,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
             dictionary.AddModelError("product.Order[0]", "Order invalid.");
 
             // Act
-            ModelBindingHelper.ClearValidationStateForModel(
+            ModelBindingHelper.ClearValidationStateForModel(modelMetadata, dictionary, prefix);
-                typeof(Product),
-                dictionary,
-                metadataProvider,
-                prefix);
 
             // Assert
             foreach (var entry in dictionary.Keys)

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

@@ -4,7 +4,6 @@
 using System;
 using System.Collections.Generic;
 using System.Globalization;
-using Microsoft.AspNetCore.Mvc.Internal;
 
 namespace Microsoft.AspNetCore.Mvc.ModelBinding.Test
 {
@@ -27,7 +26,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Test
         {
             foreach (string key in Keys)
             {
-                if (PrefixContainer.IsPrefixMatch(prefix, key))
+                if (ModelStateDictionary.StartsWithPrefix(prefix, key))
                 {
                     return true;
                 }