Add `null` check for `ModelStateEntry.Children`

- #4989

src/Microsoft.AspNetCore.Mvc.ViewFeatures/Internal/ValidationHelpers.cs

@@ -3,7 +3,6 @@
 
 using System.Collections.Generic;
 using System.Diagnostics;
-using System.Linq;
 using Microsoft.AspNetCore.Mvc.ModelBinding;
 
 namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
@@ -90,7 +89,7 @@ namespace Microsoft.AspNetCore.Mvc.ViewFeatures.Internal
             ModelMetadata metadata,
             List<ModelStateEntry> orderedModelStateEntries)
         {
-            if (metadata.ElementMetadata != null)
+            if (metadata.ElementMetadata != null && modelStateEntry.Children != null)
             {
                 foreach (var indexEntry in modelStateEntry.Children)
                 {

test/Microsoft.AspNetCore.Mvc.TagHelpers.Test/TestableHtmlGenerator.cs

@@ -65,8 +65,21 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers
             IHtmlGenerator htmlGenerator,
             IModelMetadataProvider metadataProvider)
         {
-            var actionContext = new ActionContext(new DefaultHttpContext(), new RouteData(), new ActionDescriptor());
+            return GetViewContext(model, htmlGenerator, metadataProvider, modelState: new ModelStateDictionary());
-            var viewData = new ViewDataDictionary(metadataProvider)
+        }
+
+        public static ViewContext GetViewContext(
+            object model,
+            IHtmlGenerator htmlGenerator,
+            IModelMetadataProvider metadataProvider,
+            ModelStateDictionary modelState)
+        {
+            var actionContext = new ActionContext(
+                new DefaultHttpContext(),
+                new RouteData(),
+                new ActionDescriptor(),
+                modelState);
+            var viewData = new ViewDataDictionary(metadataProvider, modelState)
             {
                 Model = model,
             };

test/Microsoft.AspNetCore.Mvc.TagHelpers.Test/ValidationSummaryTagHelperTest.cs

@@ -23,14 +23,153 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers
 {
     public class ValidationSummaryTagHelperTest
     {
-        [Fact]
+        public static TheoryData<ModelStateDictionary> ProcessAsync_GeneratesExpectedOutput_WithNoErrorsData
-        public async Task ProcessAsync_GeneratesExpectedOutput()
+        {
+            get
+            {
+                var emptyModelState = new ModelStateDictionary();
+
+                var modelState = new ModelStateDictionary();
+                SetValidModelState(modelState);
+
+                return new TheoryData<ModelStateDictionary>
+                {
+                    emptyModelState,
+                    modelState,
+                };
+            }
+        }
+
+        [Theory]
+        [MemberData(nameof(ProcessAsync_GeneratesExpectedOutput_WithNoErrorsData))]
+        public async Task ProcessAsync_GeneratesExpectedOutput_WithNoErrors(
+            ModelStateDictionary modelState)
         {
             // Arrange
             var expectedTagName = "not-div";
             var metadataProvider = new TestModelMetadataProvider();
             var htmlGenerator = new TestableHtmlGenerator(metadataProvider);
 
+            var expectedPreContent = "original pre-content";
+            var expectedContent = "original content";
+            var tagHelperContext = new TagHelperContext(
+                allAttributes: new TagHelperAttributeList(
+                    Enumerable.Empty<TagHelperAttribute>()),
+                items: new Dictionary<object, object>(),
+                uniqueId: "test");
+            var output = new TagHelperOutput(
+                expectedTagName,
+                attributes: new TagHelperAttributeList
+                {
+                    { "class", "form-control" }
+                },
+                getChildContentAsync: (useCachedResult, encoder) =>
+                {
+                    var tagHelperContent = new DefaultTagHelperContent();
+                    tagHelperContent.SetContent("Something");
+                    return Task.FromResult<TagHelperContent>(tagHelperContent);
+                });
+            output.PreContent.SetContent(expectedPreContent);
+            output.Content.SetContent(expectedContent);
+            output.PostContent.SetContent("Custom Content");
+
+            var model = new Model();
+            var viewContext = TestableHtmlGenerator.GetViewContext(model, htmlGenerator, metadataProvider, modelState);
+            var validationSummaryTagHelper = new ValidationSummaryTagHelper(htmlGenerator)
+            {
+                ValidationSummary = ValidationSummary.All,
+                ViewContext = viewContext,
+            };
+
+            // Act
+            await validationSummaryTagHelper.ProcessAsync(tagHelperContext, output);
+
+            // Assert
+            Assert.Equal(2, output.Attributes.Count);
+            var attribute = Assert.Single(output.Attributes, attr => attr.Name.Equals("class"));
+            Assert.Equal("form-control validation-summary-valid", attribute.Value);
+            attribute = Assert.Single(output.Attributes, attr => attr.Name.Equals("data-valmsg-summary"));
+            Assert.Equal("true", attribute.Value);
+            Assert.Equal(expectedPreContent, output.PreContent.GetContent());
+            Assert.Equal(expectedContent, output.Content.GetContent());
+            Assert.Equal(
+                $"Custom Content<ul><li style=\"display:none\"></li>{Environment.NewLine}</ul>",
+                output.PostContent.GetContent());
+            Assert.Equal(expectedTagName, output.TagName);
+        }
+
+        [Theory]
+        [InlineData(ValidationSummary.All)]
+        [InlineData(ValidationSummary.ModelOnly)]
+        public async Task ProcessAsync_GeneratesExpectedOutput_WithModelError(ValidationSummary validationSummary)
+        {
+            // Arrange
+            var expectedError = "I am an error.";
+            var expectedTagName = "not-div";
+            var metadataProvider = new TestModelMetadataProvider();
+            var htmlGenerator = new TestableHtmlGenerator(metadataProvider);
+
+            var validationSummaryTagHelper = new ValidationSummaryTagHelper(htmlGenerator)
+            {
+                ValidationSummary = validationSummary,
+            };
+
+            var expectedPreContent = "original pre-content";
+            var expectedContent = "original content";
+            var tagHelperContext = new TagHelperContext(
+                allAttributes: new TagHelperAttributeList(
+                    Enumerable.Empty<TagHelperAttribute>()),
+                items: new Dictionary<object, object>(),
+                uniqueId: "test");
+            var output = new TagHelperOutput(
+                expectedTagName,
+                attributes: new TagHelperAttributeList
+                {
+                    { "class", "form-control" }
+                },
+                getChildContentAsync: (useCachedResult, encoder) =>
+                {
+                    var tagHelperContent = new DefaultTagHelperContent();
+                    tagHelperContent.SetContent("Something");
+                    return Task.FromResult<TagHelperContent>(tagHelperContent);
+                });
+            output.PreContent.SetContent(expectedPreContent);
+            output.Content.SetContent(expectedContent);
+            output.PostContent.SetContent("Custom Content");
+
+            var model = new Model();
+            var viewContext = TestableHtmlGenerator.GetViewContext(model, htmlGenerator, metadataProvider);
+            validationSummaryTagHelper.ViewContext = viewContext;
+
+            var modelState = viewContext.ModelState;
+            SetValidModelState(modelState);
+            modelState.AddModelError(string.Empty, expectedError);
+
+            // Act
+            await validationSummaryTagHelper.ProcessAsync(tagHelperContext, output);
+
+            // Assert
+            Assert.InRange(output.Attributes.Count, low: 1, high: 2);
+            var attribute = Assert.Single(output.Attributes, attr => attr.Name.Equals("class"));
+            Assert.Equal("form-control validation-summary-errors", attribute.Value);
+            Assert.Equal(expectedPreContent, output.PreContent.GetContent());
+            Assert.Equal(expectedContent, output.Content.GetContent());
+            Assert.Equal(
+                $"Custom Content<ul><li>{expectedError}</li>{Environment.NewLine}</ul>",
+                output.PostContent.GetContent());
+            Assert.Equal(expectedTagName, output.TagName);
+        }
+
+        [Fact]
+        public async Task ProcessAsync_GeneratesExpectedOutput_WithPropertyErrors()
+        {
+            // Arrange
+            var expectedError0 = "I am an error.";
+            var expectedError2 = "I am also an error.";
+            var expectedTagName = "not-div";
+            var metadataProvider = new TestModelMetadataProvider();
+            var htmlGenerator = new TestableHtmlGenerator(metadataProvider);
+
             var validationSummaryTagHelper = new ValidationSummaryTagHelper(htmlGenerator)
             {
                 ValidationSummary = ValidationSummary.All,
@@ -59,23 +198,30 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers
             output.Content.SetContent(expectedContent);
             output.PostContent.SetContent("Custom Content");
 
-            Model model = null;
+            var model = new Model();
             var viewContext = TestableHtmlGenerator.GetViewContext(model, htmlGenerator, metadataProvider);
             validationSummaryTagHelper.ViewContext = viewContext;
 
+            var modelState = viewContext.ModelState;
+            SetValidModelState(modelState);
+            modelState.AddModelError(key: $"{nameof(Model.Strings)}[0]", errorMessage: expectedError0);
+            modelState.AddModelError(key: $"{nameof(Model.Strings)}[2]", errorMessage: expectedError2);
+
             // Act
             await validationSummaryTagHelper.ProcessAsync(tagHelperContext, output);
 
             // Assert
             Assert.Equal(2, output.Attributes.Count);
             var attribute = Assert.Single(output.Attributes, attr => attr.Name.Equals("class"));
-            Assert.Equal("form-control validation-summary-valid", attribute.Value);
+            Assert.Equal("form-control validation-summary-errors", attribute.Value);
             attribute = Assert.Single(output.Attributes, attr => attr.Name.Equals("data-valmsg-summary"));
             Assert.Equal("true", attribute.Value);
             Assert.Equal(expectedPreContent, output.PreContent.GetContent());
             Assert.Equal(expectedContent, output.Content.GetContent());
-            Assert.Equal("Custom Content<ul><li style=\"display:none\"></li>" + Environment.NewLine + "</ul>",
+            Assert.Equal(
-                         output.PostContent.GetContent());
+                $"Custom Content<ul><li>{expectedError0}</li>{Environment.NewLine}" +
+                $"<li>{expectedError2}</li>{Environment.NewLine}</ul>",
+                output.PostContent.GetContent());
             Assert.Equal(expectedTagName, output.TagName);
         }
 
@@ -337,9 +483,29 @@ namespace Microsoft.AspNetCore.Mvc.TagHelpers
                 new HtmlHelperOptions());
         }
 
+        private static void SetValidModelState(ModelStateDictionary modelState)
+        {
+            modelState.SetModelValue(key: nameof(Model.Empty), rawValue: null, attemptedValue: null);
+            modelState.SetModelValue(key: $"{nameof(Model.Strings)}[0]", rawValue: null, attemptedValue: null);
+            modelState.SetModelValue(key: $"{nameof(Model.Strings)}[1]", rawValue: null, attemptedValue: null);
+            modelState.SetModelValue(key: $"{nameof(Model.Strings)}[2]", rawValue: null, attemptedValue: null);
+            modelState.SetModelValue(key: nameof(Model.Text), rawValue: null, attemptedValue: null);
+
+            foreach (var key in modelState.Keys)
+            {
+                modelState.MarkFieldValid(key);
+            }
+        }
+
         private class Model
         {
             public string Text { get; set; }
+
+            public string[] Strings { get; set; }
+
+            // Exists to ensure #4989 does not regress. Issue specific to case where collection has a ModelStateEntry
+            // but no element does.
+            public byte[] Empty { get; set; }
         }
     }
 }

test/Microsoft.AspNetCore.Mvc.ViewFeatures.Test/Rendering/HtmlHelperValidationSummaryTest.cs

@@ -672,6 +672,8 @@ namespace Microsoft.AspNetCore.Mvc.Rendering
             modelState.AddModelError("Property3.Property2", "This is an error for Property3.Property2.");
             modelState.AddModelError("Property3.OrderedProperty3", "This is an error for Property3.OrderedProperty3.");
             modelState.AddModelError("Property3.OrderedProperty2", "This is an error for Property3.OrderedProperty2.");
+            modelState.SetModelValue("Property3.Empty", rawValue: null, attemptedValue: null);
+            modelState.MarkFieldValid("Property3.Empty");
 
             var provider = new EmptyModelMetadataProvider();
             var metadata = provider.GetMetadataForProperty(typeof(ValidationModel), nameof(ValidationModel.Property3));
@@ -712,6 +714,9 @@ namespace Microsoft.AspNetCore.Mvc.Rendering
 
             modelState.AddModelError("OrderedProperty1", "This is an error for OrderedProperty1.");
             modelState.AddModelError("OrderedProperty2", "This is yet-another error for OrderedProperty2.");
+
+            modelState.SetModelValue("Empty", rawValue: null, attemptedValue: null);
+            modelState.MarkFieldValid("Empty");
         }
 
         private class ValidationModel
@@ -738,6 +743,10 @@ namespace Microsoft.AspNetCore.Mvc.Rendering
             public string OrderedProperty2 { get; set; }
             [Display(Order = 23)]
             public string OrderedProperty1 { get; set; }
+
+            // Exists to ensure #4989 does not regress. Issue specific to case where collection has a ModelStateEntry
+            // but no element does.
+            public byte[] Empty { get; set; }
         }
 
         private class ModelWithCollection