Ignore empty keys in QueryStringValueProvider

Fixes 8484

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

@@ -13,7 +13,6 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
     /// </summary>
     public class FormValueProvider : BindingSourceValueProvider, IEnumerableValueProvider
     {
-        private readonly CultureInfo _culture;
         private readonly IFormCollection _values;
         private PrefixContainer _prefixContainer;
 
@@ -40,10 +39,10 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
             }
 
             _values = values;
-            _culture = culture;
+            Culture = culture;
         }
 
-        public CultureInfo Culture => _culture;
+        public CultureInfo Culture { get; }
 
         protected PrefixContainer PrefixContainer
         {
@@ -83,6 +82,15 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
                 throw new ArgumentNullException(nameof(key));
             }
 
+            if (key.Length == 0)
+            {
+                // Top level parameters will fall back to an empty prefix when the parameter name does not
+                // appear in any value provider. This would result in the parameter binding to a form parameter
+                // with a empty key (e.g. Request body looks like "=test") which isn't a scenario we want to support.
+                // Return a "None" result in this event.
+                return ValueProviderResult.None;
+            }
+
             var values = _values[key];
             if (values.Count == 0)
             {
@@ -90,7 +98,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
             }
             else
             {
-                return new ValueProviderResult(values, _culture);
+                return new ValueProviderResult(values, Culture);
             }
         }
     }

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

@@ -79,6 +79,11 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
         /// <inheritdoc />
         public override ValueProviderResult GetValue(string key)
         {
+            if (key == null)
+            {
+                throw new ArgumentNullException(nameof(key));
+            }
+
             if (_values.TryGetValue(key, out var values) && values.Count > 0)
             {
                 return new ValueProviderResult(values, Culture);

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

@@ -13,7 +13,6 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
     /// </summary>
     public class QueryStringValueProvider : BindingSourceValueProvider, IEnumerableValueProvider
     {
-        private readonly CultureInfo _culture;
         private readonly IQueryCollection _values;
         private PrefixContainer _prefixContainer;
 
@@ -40,10 +39,10 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
             }
 
             _values = values;
-            _culture = culture;
+            Culture = culture;
         }
 
-        public CultureInfo Culture => _culture;
+        public CultureInfo Culture { get; }
 
         protected PrefixContainer PrefixContainer
         {
@@ -83,6 +82,15 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
                 throw new ArgumentNullException(nameof(key));
             }
 
+            if (key.Length == 0)
+            {
+                // Top level parameters will fall back to an empty prefix when the parameter name does not
+                // appear in any value provider. This would result in the parameter binding to a query string
+                // parameter with a empty key (e.g. /User?=test) which isn't a scenario we want to support.
+                // Return a "None" result in this event.
+                return ValueProviderResult.None;
+            }
+
             var values = _values[key];
             if (values.Count == 0)
             {
@@ -90,7 +98,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
             }
             else
             {
-                return new ValueProviderResult(values, _culture);
+                return new ValueProviderResult(values, Culture);
             }
         }
     }

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

@@ -85,6 +85,15 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
                 throw new ArgumentNullException(nameof(key));
             }
 
+            if (key.Length == 0)
+            {
+                // Top level parameters will fall back to an empty prefix when the parameter name does not
+                // appear in any value provider. This would result in the parameter binding to a route value
+                // an empty key which isn't a scenario we want to support.
+                // Return a "None" result in this event.
+                return ValueProviderResult.None;
+            }
+
             if (_values.TryGetValue(key, out var value))
             {
                 var stringValue = value as string ?? Convert.ToString(value, Culture) ?? string.Empty;

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

@@ -5,6 +5,8 @@ using System;
 using System.Collections.Generic;
 using System.Globalization;
 using System.Linq;
+using Microsoft.AspNetCore.Http;
+using Microsoft.AspNetCore.Http.Internal;
 using Microsoft.Extensions.Primitives;
 using Moq;
 using Xunit;
@@ -38,9 +40,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
             Dictionary<string, StringValues> values,
             CultureInfo culture)
         {
-            var emptyValueProvider =
+            var emptyValueProvider = new QueryStringValueProvider(bindingSource, new QueryCollection(), culture);
-                new JQueryFormValueProvider(bindingSource, new Dictionary<string, StringValues>(), culture);
+            var valueProvider = new FormValueProvider(bindingSource, new FormCollection(values), culture);
-            var valueProvider = new JQueryFormValueProvider(bindingSource, values, culture);
 
             return new CompositeValueProvider() { emptyValueProvider, valueProvider };
         }

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

@@ -235,6 +235,23 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
             Assert.Equal(ValueProviderResult.None, result);
         }
 
+        [Fact]
+        public virtual void GetValue_EmptyKey()
+        {
+            // Arrange
+            var store = new Dictionary<string, StringValues>(BackingStore)
+            {
+                { string.Empty, "some-value" },
+            };
+            var valueProvider = GetEnumerableValueProvider(BindingSource.Query, store, culture: null);
+
+            // Act
+            var result = valueProvider.GetValue(string.Empty);
+
+            // Assert
+            Assert.Equal(ValueProviderResult.None, result);
+        }
+
         [Fact]
         public virtual void FilterInclude()
         {

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

@@ -31,5 +31,22 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
             // Assert
             Assert.Null(result);
         }
+
+        [Fact]
+        public override void GetValue_EmptyKey()
+        {
+            // Arrange
+            var store = new Dictionary<string, StringValues>(BackingStore)
+            {
+                { string.Empty, "some-value" },
+            };
+            var valueProvider = GetEnumerableValueProvider(BindingSource.Query, store, culture: null);
+
+            // Act
+            var result = valueProvider.GetValue(string.Empty);
+
+            // Assert
+            Assert.Equal("some-value", (string)result);
+        }
     }
 }

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

@@ -34,5 +34,22 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding
             // Assert
             Assert.Null(result);
         }
+
+        [Fact]
+        public override void GetValue_EmptyKey()
+        {
+            // Arrange
+            var store = new Dictionary<string, StringValues>(BackingStore)
+            {
+                { string.Empty, "some-value" },
+            };
+            var valueProvider = GetEnumerableValueProvider(BindingSource.Query, store, culture: null);
+
+            // Act
+            var result = valueProvider.GetValue(string.Empty);
+
+            // Assert
+            Assert.Equal("some-value", (string)result);
+        }
     }
 }

test/Microsoft.AspNetCore.Mvc.IntegrationTests/SimpleTypeModelBinderIntegrationTest.cs

@@ -152,6 +152,39 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests
             Assert.Equal(ModelValidationState.Valid, modelState[key].ValidationState);
         }
 
+        [Fact]
+        public async Task BindParameter_WithEmptyQueryStringKey_DoesNotGetBound()
+        {
+            // Arrange
+            var parameterBinder = ModelBindingTestHelper.GetParameterBinder();
+            var parameter = new ParameterDescriptor
+            {
+                Name = "Parameter1",
+                BindingInfo = new BindingInfo(),
+
+                ParameterType = typeof(string)
+            };
+
+            var testContext = ModelBindingTestHelper.GetTestContext(request =>
+            {
+                request.QueryString = new QueryString("?=someValue");
+            });
+
+            var modelState = testContext.ModelState;
+
+            // Act
+            var modelBindingResult = await parameterBinder.BindModelAsync(parameter, testContext);
+
+            // Assert
+
+            // ModelBindingResult
+            Assert.False(modelBindingResult.IsModelSet);
+
+            // ModelState
+            Assert.True(modelState.IsValid);
+            Assert.Empty(modelState.Keys);
+        }
+
         [Fact]
         [ReplaceCulture("en-GB", "en-GB")]
         public async Task BindDecimalParameter_WithData_GetsBound()