Fix CollectionModelBinder BindSimpleCollection null value handling

src/Mvc/Mvc.Core/src/ModelBinding/Binders/CollectionModelBinder.cs

@@ -273,23 +273,24 @@ internal async Task<CollectionResult> BindSimpleCollection(
         var boundCollection = new List<TElement?>();
 
         var elementMetadata = bindingContext.ModelMetadata.ElementMetadata!;
+        var valueProvider = bindingContext.ValueProvider;
 
         foreach (var value in values)
         {
-            bindingContext.ValueProvider = new CompositeValueProvider
-                {
-                    // our temporary provider goes at the front of the list
-                    new ElementalValueProvider(bindingContext.ModelName, value, values.Culture),
-                    bindingContext.ValueProvider
-                };
-
             // Enter new scope to change ModelMetadata and isolate element binding operations.
             using (bindingContext.EnterNestedScope(
                 elementMetadata,
                 fieldName: bindingContext.FieldName,
                 modelName: bindingContext.ModelName,
                 model: null))
             {
+                bindingContext.ValueProvider = new CompositeValueProvider
+                {
+                    // our temporary provider goes at the front of the list
+                    new ElementalValueProvider(bindingContext.ModelName, value, values.Culture),
+                    valueProvider
+                };
+
                 await ElementBinder.BindModelAsync(bindingContext);
 
                 if (bindingContext.Result.IsModelSet)

src/Mvc/Mvc.Core/test/ModelBinding/Binders/CollectionModelBinderTest.cs

@@ -205,6 +205,26 @@ public async Task BindSimpleCollection_RawValueIsEmptyCollection_ReturnsEmptyLis
         Assert.Empty(boundCollection.Model);
     }
 
+    [Fact]
+    public async Task BindSimpleCollection_RawValueWithNull_ReturnsListWithoutNull()
+    {
+        // Arrange
+        var binder = new CollectionModelBinder<int>(CreateIntBinder(), NullLoggerFactory.Instance);
+        var valueProvider = new SimpleValueProvider
+            {
+                { "someName", "420" },
+            };
+        var context = GetModelBindingContext(valueProvider);
+        var valueProviderResult = new ValueProviderResult(new[] { null, "42", "", "100", null, "200" });
+
+        // Act
+        var boundCollection = await binder.BindSimpleCollection(context, valueProviderResult);
+
+        // Assert
+        Assert.NotNull(boundCollection.Model);
+        Assert.Equal(new[] { 420, 42, 100, 420, 200 }, boundCollection.Model);
+    }
+
     private IActionResult ActionWithListParameter(List<string> parameter) => null;
 
     [Theory]

src/Mvc/test/Mvc.FunctionalTests/CustomValueProviderTest.cs

@@ -0,0 +1,93 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.Net;
+using System.Net.Http;
+using Microsoft.AspNetCore.Hosting;
+using Microsoft.AspNetCore.TestHost;
+using Microsoft.Extensions.DependencyInjection;
+
+namespace Microsoft.AspNetCore.Mvc.FunctionalTests;
+
+public class CustomValueProviderTest : IClassFixture<MvcTestFixture<BasicWebSite.StartupWithCustomValueProvider>>
+{
+    private IServiceCollection _serviceCollection;
+
+    public CustomValueProviderTest(MvcTestFixture<BasicWebSite.StartupWithCustomValueProvider> fixture)
+    {
+        var factory = fixture.Factories.FirstOrDefault() ?? fixture.WithWebHostBuilder(b => b.UseStartup<BasicWebSite.StartupWithCustomValueProvider>());
+        factory = factory.WithWebHostBuilder(b => b.ConfigureTestServices(serviceCollection => _serviceCollection = serviceCollection));
+
+        Client = factory.CreateDefaultClient();
+    }
+
+    public HttpClient Client { get; }
+
+    [Fact]
+    public async Task CustomValueProvider_DisplayName()
+    {
+        // Arrange
+        var url = "http://localhost/CustomValueProvider/CustomValueProviderDisplayName";
+        var request = new HttpRequestMessage(HttpMethod.Get, url);
+
+        // Act
+        var response = await Client.SendAsync(request);
+        var content = await response.Content.ReadAsStringAsync();
+
+        // Assert
+        Assert.Equal(HttpStatusCode.OK, response.StatusCode);
+        Assert.Equal("text/plain", response.Content.Headers.ContentType.MediaType);
+        Assert.Equal("BasicWebSite.Controllers.CustomValueProviderController.CustomValueProviderDisplayName (BasicWebSite)", content);
+    }
+
+    [Fact]
+    public async Task CustomValueProvider_IntValues()
+    {
+        // Arrange
+        var url = "http://localhost/CustomValueProvider/CustomValueProviderIntValues";
+        var request = new HttpRequestMessage(HttpMethod.Get, url);
+
+        // Act
+        var response = await Client.SendAsync(request);
+        var content = await response.Content.ReadAsStringAsync();
+
+        // Assert
+        Assert.Equal(HttpStatusCode.OK, response.StatusCode);
+        Assert.Equal("application/json", response.Content.Headers.ContentType.MediaType);
+        Assert.Equal("[42,100,200]", content);
+    }
+
+    [Fact]
+    public async Task CustomValueProvider_NullableIntValues()
+    {
+        // Arrange
+        var url = "http://localhost/CustomValueProvider/CustomValueProviderNullableIntValues";
+        var request = new HttpRequestMessage(HttpMethod.Get, url);
+
+        // Act
+        var response = await Client.SendAsync(request);
+        var content = await response.Content.ReadAsStringAsync();
+
+        // Assert
+        Assert.Equal(HttpStatusCode.OK, response.StatusCode);
+        Assert.Equal("application/json", response.Content.Headers.ContentType.MediaType);
+        Assert.Equal("[null,42,null,100,null,200]", content);
+    }
+
+    [Fact]
+    public async Task CustomValueProvider_StringValues()
+    {
+        // Arrange
+        var url = "http://localhost/CustomValueProvider/CustomValueProviderStringValues";
+        var request = new HttpRequestMessage(HttpMethod.Get, url);
+
+        // Act
+        var response = await Client.SendAsync(request);
+        var content = await response.Content.ReadAsStringAsync();
+
+        // Assert
+        Assert.Equal(HttpStatusCode.OK, response.StatusCode);
+        Assert.Equal("application/json", response.Content.Headers.ContentType.MediaType);
+        Assert.Equal(@"[null,""foo"",null,""bar"",null,""baz""]", content);
+    }
+}

src/Mvc/test/WebSites/BasicWebSite/Controllers/CustomValueProviderController.cs

@@ -0,0 +1,26 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using System.ComponentModel;
+using Microsoft.AspNetCore.Mvc;
+
+namespace BasicWebSite.Controllers;
+
+public class CustomValueProviderController : Controller
+{
+    [HttpGet]
+    public string CustomValueProviderDisplayName(string customValueProviderDisplayName)
+        => customValueProviderDisplayName;
+
+    [HttpGet]
+    public int[] CustomValueProviderIntValues(int[] customValueProviderIntValues)
+        => customValueProviderIntValues;
+
+    [HttpGet]
+    public int?[] CustomValueProviderNullableIntValues(int?[] customValueProviderNullableIntValues)
+        => customValueProviderNullableIntValues;
+
+    [HttpGet]
+    public string[] CustomValueProviderStringValues(string[] customValueProviderStringValues)
+        => customValueProviderStringValues;
+}

src/Mvc/test/WebSites/BasicWebSite/StartupWithCustomValueProvider.cs

@@ -0,0 +1,27 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using BasicWebSite.ValueProviders;
+
+namespace BasicWebSite;
+
+public class StartupWithCustomValueProvider
+{
+    public void ConfigureServices(IServiceCollection services)
+    {
+        services
+            .AddMvc(o =>
+            {
+                o.ValueProviderFactories.Add(new CustomValueProviderFactory());
+            });
+    }
+
+    public void Configure(IApplicationBuilder app)
+    {
+        app.UseDeveloperExceptionPage();
+
+        app.UseRouting();
+
+        app.UseEndpoints((endpoints) => endpoints.MapDefaultControllerRoute());
+    }
+}

src/Mvc/test/WebSites/BasicWebSite/ValueProviders/CustomValueProviderFactory.cs

@@ -0,0 +1,45 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+
+using Microsoft.AspNetCore.Mvc.ModelBinding;
+using Microsoft.Extensions.Primitives;
+
+namespace BasicWebSite.ValueProviders;
+
+public class CustomValueProviderFactory : IValueProviderFactory
+{
+    public Task CreateValueProviderAsync(ValueProviderFactoryContext context)
+    {
+        context.ValueProviders.Add(new CustomValueProvider(context));
+        return Task.CompletedTask;
+    }
+
+    private class CustomValueProvider : IValueProvider
+    {
+        private static readonly Dictionary<string, Func<ValueProviderFactoryContext, StringValues>> Values = new()
+        {
+            { "customValueProviderDisplayName", context => context.ActionContext.ActionDescriptor.DisplayName },
+            { "customValueProviderIntValues", _ => new []{ null, "42", "100", null, "200" } },
+            { "customValueProviderNullableIntValues", _ => new []{ null, "42", "", "100", null, "200" } },
+            { "customValueProviderStringValues", _ => new []{ null, "foo", "", "bar", null, "baz" } },
+        };
+
+        private readonly ValueProviderFactoryContext _context;
+
+        public CustomValueProvider(ValueProviderFactoryContext context)
+        {
+            _context = context;
+        }
+
+        public bool ContainsPrefix(string prefix) => Values.ContainsKey(prefix);
+
+        public ValueProviderResult GetValue(string key)
+        {
+            if (Values.TryGetValue(key, out var fn))
+            {
+                return new ValueProviderResult(fn(_context));
+            }
+            return ValueProviderResult.None;
+        }
+    }
+}