diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ActionResultOfT.cs b/src/Microsoft.AspNetCore.Mvc.Core/ActionResultOfT.cs index 0077a236d7..f443d78805 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ActionResultOfT.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ActionResultOfT.cs @@ -10,7 +10,7 @@ namespace Microsoft.AspNetCore.Mvc /// A type that wraps either an instance or an . /// /// The type of the result. - public class ActionResult : IConvertToActionResult + public sealed class ActionResult : IConvertToActionResult { /// /// Initializes a new instance of using the specified . diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/DefaultModelBindingContext.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/DefaultModelBindingContext.cs index 5648576fd6..0ff9292a02 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/DefaultModelBindingContext.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/DefaultModelBindingContext.cs @@ -12,6 +12,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding /// public class DefaultModelBindingContext : ModelBindingContext { + private static readonly IValueProvider EmptyValueProvider = new CompositeValueProvider(); + private IValueProvider _originalValueProvider; private ActionContext _actionContext; private ModelStateDictionary _modelState; @@ -314,13 +316,12 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding return valueProvider; } - var bindingSourceValueProvider = valueProvider as IBindingSourceValueProvider; - if (bindingSourceValueProvider == null) + if (!(valueProvider is IBindingSourceValueProvider bindingSourceValueProvider)) { return valueProvider; } - return bindingSourceValueProvider.Filter(bindingSource) ?? new CompositeValueProvider(); + return bindingSourceValueProvider.Filter(bindingSource) ?? EmptyValueProvider; } private struct State diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/CollectionModelBinder.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/CollectionModelBinder.cs index d219519e4f..16834ebadc 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/CollectionModelBinder.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/CollectionModelBinder.cs @@ -23,6 +23,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders /// Type of elements in the collection. public class CollectionModelBinder : ICollectionModelBinder { + private static readonly IValueProvider EmptyValueProvider = new CompositeValueProvider(); private Func _modelCreator; /// @@ -244,7 +245,15 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders Logger.AttemptingToBindCollectionUsingIndices(bindingContext); var indexPropertyName = ModelNames.CreatePropertyModelName(bindingContext.ModelName, "index"); - var valueProviderResultIndex = bindingContext.ValueProvider.GetValue(indexPropertyName); + + // Remove any value provider that may not use indexPropertyName as-is. Don't match e.g. Model[index]. + var valueProvider = bindingContext.ValueProvider; + if (valueProvider is IKeyRewriterValueProvider keyRewriterValueProvider) + { + valueProvider = keyRewriterValueProvider.Filter() ?? EmptyValueProvider; + } + + var valueProviderResultIndex = valueProvider.GetValue(indexPropertyName); var indexNames = GetIndexNamesFromValueProviderResult(valueProviderResultIndex); return BindComplexCollectionFromIndexes(bindingContext, indexNames); diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/CompositeValueProvider.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/CompositeValueProvider.cs index 9faf5c38c3..eaf15df1be 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/CompositeValueProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/CompositeValueProvider.cs @@ -4,7 +4,6 @@ using System; using System.Collections.Generic; using System.Collections.ObjectModel; -using System.Linq; using System.Threading.Tasks; namespace Microsoft.AspNetCore.Mvc.ModelBinding @@ -15,7 +14,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding public class CompositeValueProvider : Collection, IEnumerableValueProvider, - IBindingSourceValueProvider + IBindingSourceValueProvider, + IKeyRewriterValueProvider { /// /// Initializes a new instance of . @@ -117,8 +117,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding { foreach (var valueProvider in this) { - var enumeratedProvider = valueProvider as IEnumerableValueProvider; - if (enumeratedProvider != null) + if (valueProvider is IEnumerableValueProvider enumeratedProvider) { var result = enumeratedProvider.GetKeysFromPrefix(prefix); if (result != null && result.Count > 0) @@ -160,13 +159,34 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding throw new ArgumentNullException(nameof(bindingSource)); } - var filteredValueProviders = new List(); - foreach (var valueProvider in this.OfType()) + var shouldFilter = false; + for (var i = 0; i < Count; i++) { - var result = valueProvider.Filter(bindingSource); - if (result != null) + var valueProvider = Items[i]; + if (valueProvider is IBindingSourceValueProvider) { - filteredValueProviders.Add(result); + shouldFilter = true; + break; + } + } + + if (!shouldFilter) + { + // No inner IBindingSourceValueProvider implementations. Result will be empty. + return null; + } + + var filteredValueProviders = new List(); + for (var i = 0; i < Count; i++) + { + var valueProvider = Items[i]; + if (valueProvider is IBindingSourceValueProvider bindingSourceValueProvider) + { + var result = bindingSourceValueProvider.Filter(bindingSource); + if (result != null) + { + filteredValueProviders.Add(result); + } } } @@ -176,12 +196,58 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding return null; } - if (filteredValueProviders.Count == Count) + return new CompositeValueProvider(filteredValueProviders); + } + + /// + /// + /// Value providers are included by default. If a contained does not implement + /// , will not remove it. + /// + public IValueProvider Filter() + { + var shouldFilter = false; + for (var i = 0; i < Count; i++) { - // No need for a new CompositeValueProvider. + var valueProvider = Items[i]; + if (valueProvider is IKeyRewriterValueProvider) + { + shouldFilter = true; + break; + } + } + + if (!shouldFilter) + { + // No inner IKeyRewriterValueProvider implementations. Nothing to exclude. return this; } + var filteredValueProviders = new List(); + for (var i = 0; i < Count; i++) + { + var valueProvider = Items[i]; + if (valueProvider is IKeyRewriterValueProvider keyRewriterValueProvider) + { + var result = keyRewriterValueProvider.Filter(); + if (result != null) + { + filteredValueProviders.Add(result); + } + } + else + { + // Assume value providers that aren't rewriter-aware do not rewrite their keys. + filteredValueProviders.Add(valueProvider); + } + } + + if (filteredValueProviders.Count == 0) + { + // Do not create an empty CompositeValueProvider. + return null; + } + return new CompositeValueProvider(filteredValueProviders); } } diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/IKeyRewriterValueProvider.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/IKeyRewriterValueProvider.cs new file mode 100644 index 0000000000..f2bb03cdb7 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/IKeyRewriterValueProvider.cs @@ -0,0 +1,23 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +namespace Microsoft.AspNetCore.Mvc.ModelBinding +{ + /// + /// A value provider which can filter its contents to remove keys rewritten compared to the request data. + /// + public interface IKeyRewriterValueProvider : IValueProvider + { + /// + /// Filters the value provider to remove keys rewritten compared to the request data. + /// + /// + /// If the request contains values with keys Model.Property and Collection[index], the returned + /// will not match Model[Property] or Collection.index. + /// + /// + /// The filtered value provider or if the value provider only contains rewritten keys. + /// + IValueProvider Filter(); + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/JQueryValueProvider.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/JQueryValueProvider.cs index a2ffca0f2a..659a943934 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/JQueryValueProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/JQueryValueProvider.cs @@ -10,9 +10,12 @@ using Microsoft.Extensions.Primitives; namespace Microsoft.AspNetCore.Mvc.ModelBinding { /// - /// An for jQuery formatted form data. + /// An for jQuery formatted data. /// - public abstract class JQueryValueProvider : BindingSourceValueProvider, IEnumerableValueProvider + public abstract class JQueryValueProvider : + BindingSourceValueProvider, + IEnumerableValueProvider, + IKeyRewriterValueProvider { private readonly IDictionary _values; private PrefixContainer _prefixContainer; @@ -85,5 +88,16 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding return ValueProviderResult.None; } + + /// + /// + /// Always returns because creates this + /// with rewritten keys (if original contains brackets) or duplicate keys + /// (that will match). + /// + public IValueProvider Filter() + { + return null; + } } } diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/CompositeValueProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/CompositeValueProviderTest.cs index 86e6be22d9..0da5681ef6 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/CompositeValueProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/CompositeValueProviderTest.cs @@ -4,6 +4,8 @@ using System; using System.Collections.Generic; using System.Globalization; +using System.Linq; +using Microsoft.AspNetCore.Mvc.Internal; using Microsoft.Extensions.Primitives; using Moq; using Xunit; @@ -12,6 +14,26 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding { public class CompositeValueProviderTest : EnumerableValueProviderTest { + [Fact] + public override void FilterInclude() + { + // Arrange + var provider = GetBindingSourceValueProvider(BindingSource.Query, BackingStore, culture: null); + var originalProviders = ((CompositeValueProvider)provider).ToArray(); + var bindingSource = new BindingSource( + BindingSource.Query.Id, + displayName: null, + isGreedy: true, + isFromRequest: true); + + // Act + var result = provider.Filter(bindingSource); + + // Assert (does not change inner providers) + var newProvider = Assert.IsType(result); + Assert.Equal(originalProviders, newProvider, ReferenceEqualityComparer.Instance); + } + protected override IEnumerableValueProvider GetEnumerableValueProvider( BindingSource bindingSource, Dictionary values, @@ -96,7 +118,113 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding Assert.Same(valueProvider1.Object, filteredProvider); } - private Mock GetMockValueProvider(string bindingSourceId) + public static TheoryData Filter_ReturnsProviderData + { + get + { + // None filter themselves out. + var noneRewrite = new[] + { + GetValueProvider(rewritesKeys: false), + GetValueProvider(rewritesKeys: false), + }; + // None implement IKeyRewriterValueProvider. + var noneImplement = new[] { GetMockValueProvider("One").Object, GetMockValueProvider("Two").Object }; + + return new TheoryData + { + // Starts empty + new CompositeValueProvider(), + + new CompositeValueProvider(noneRewrite), + new CompositeValueProvider(noneImplement), + }; + } + } + + [Theory] + [MemberData(nameof(Filter_ReturnsProviderData))] + public void Filter_ReturnsProvider(CompositeValueProvider provider) + { + // Arrange + var originalProviders = provider.ToArray(); + + // Act + var result = provider.Filter(); + + // Assert (does not change inner providers) + var newProvider = Assert.IsType(result); + Assert.Equal(originalProviders, newProvider, ReferenceEqualityComparer.Instance); + } + + [Fact] + public void Filter_ReturnsNull() + { + // Arrange + var allRewrite = new[] { GetValueProvider(rewritesKeys: true), GetValueProvider(rewritesKeys: true) }; + var provider = new CompositeValueProvider(allRewrite); + + // Act + var result = provider.Filter(); + + // Assert + Assert.Null(result); + } + + [Fact] + public void Filter_RemovesThoseThatRewrite() + { + // Arrange + var doesNotRewrite1 = GetValueProvider(rewritesKeys: false); + var doesNotRewrite2 = GetValueProvider(rewritesKeys: false); + var doesNotImplement1 = GetMockValueProvider("One").Object; + var doesNotImplement2 = GetMockValueProvider("Two").Object; + var rewrites1 = GetValueProvider(rewritesKeys: true); + var rewrites2 = GetValueProvider(rewritesKeys: true); + var providers = new IValueProvider[] + { + doesNotRewrite1, + doesNotImplement1, + rewrites1, + doesNotRewrite2, + doesNotImplement2, + rewrites2, + }; + var expectedProviders = new IValueProvider[] + { + doesNotRewrite1, + doesNotImplement1, + doesNotRewrite2, + doesNotImplement2, + }; + + var provider = new CompositeValueProvider(providers); + + // Act + var result = provider.Filter(); + + // Assert + Assert.NotSame(provider, result); + var newProvider = Assert.IsType(result); + Assert.Equal(expectedProviders, newProvider, ReferenceEqualityComparer.Instance); + } + + private static IKeyRewriterValueProvider GetValueProvider(bool rewritesKeys) + { + var valueProvider = new Mock(MockBehavior.Strict); + if (rewritesKeys) + { + valueProvider.Setup(vp => vp.Filter()).Returns(null); + } + else + { + valueProvider.Setup(vp => vp.Filter()).Returns(valueProvider.Object); + } + + return valueProvider.Object; + } + + private static Mock GetMockValueProvider(string bindingSourceId) { var valueProvider = new Mock(MockBehavior.Strict); diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/EnumerableValueProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/EnumerableValueProviderTest.cs index 8b01f266b6..19c3d37407 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/EnumerableValueProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/EnumerableValueProviderTest.cs @@ -11,7 +11,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding { public abstract class EnumerableValueProviderTest { - private static readonly Dictionary _backingStore = new Dictionary + protected static Dictionary BackingStore { get; } = new Dictionary { { "some", new[] { "someValue1", "someValue2" } }, { "null_value", StringValues.Empty }, @@ -46,7 +46,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding public void ContainsPrefix_WithNonEmptyCollection_ReturnsTrueForEmptyPrefix() { // Arrange - var valueProvider = GetEnumerableValueProvider(BindingSource.Query, _backingStore, culture: null); + var valueProvider = GetEnumerableValueProvider(BindingSource.Query, BackingStore, culture: null); // Act var result = valueProvider.ContainsPrefix(string.Empty); @@ -64,7 +64,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding public void ContainsPrefix_WithNonEmptyCollection_ReturnsTrueForKnownPrefixes(string prefix) { // Arrange - var valueProvider = GetEnumerableValueProvider(BindingSource.Query, _backingStore, culture: null); + var valueProvider = GetEnumerableValueProvider(BindingSource.Query, BackingStore, culture: null); // Act & Assert Assert.True(valueProvider.ContainsPrefix(prefix)); @@ -74,7 +74,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding public void ContainsPrefix_WithNonEmptyCollection_ReturnsFalseForUnknownPrefix() { // Arrange - var valueProvider = GetEnumerableValueProvider(BindingSource.Query, _backingStore, culture: null); + var valueProvider = GetEnumerableValueProvider(BindingSource.Query, BackingStore, culture: null); // Act var result = valueProvider.ContainsPrefix("biff"); @@ -94,7 +94,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding { "prefix", "prefix" }, { "some", "some" }, }; - var valueProvider = GetEnumerableValueProvider(BindingSource.Query, _backingStore, culture: null); + var valueProvider = GetEnumerableValueProvider(BindingSource.Query, BackingStore, culture: null); // Act var result = valueProvider.GetKeysFromPrefix(string.Empty); @@ -107,7 +107,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding public void GetKeysFromPrefix_UnknownPrefix_ReturnsEmptyDictionary() { // Arrange - var valueProvider = GetEnumerableValueProvider(BindingSource.Query, _backingStore, culture: null); + var valueProvider = GetEnumerableValueProvider(BindingSource.Query, BackingStore, culture: null); // Act var result = valueProvider.GetKeysFromPrefix("abc"); @@ -129,7 +129,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding { "index1", "prefix[index1]" }, { "index2", "prefix[index2]" }, }; - var valueProvider = GetEnumerableValueProvider(BindingSource.Query, _backingStore, culture: null); + var valueProvider = GetEnumerableValueProvider(BindingSource.Query, BackingStore, culture: null); // Act var result = valueProvider.GetKeysFromPrefix("prefix"); @@ -147,7 +147,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding { "property", "[index].property" }, { "anotherIndex", "[index][anotherIndex]" } }; - var valueProvider = GetEnumerableValueProvider(BindingSource.Query, _backingStore, culture: null); + var valueProvider = GetEnumerableValueProvider(BindingSource.Query, BackingStore, culture: null); // Act var result = valueProvider.GetKeysFromPrefix("[index]"); @@ -161,7 +161,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding { // Arrange var culture = new CultureInfo("fr-FR"); - var valueProvider = GetEnumerableValueProvider(BindingSource.Query, _backingStore, culture); + var valueProvider = GetEnumerableValueProvider(BindingSource.Query, BackingStore, culture); // Act var result = valueProvider.GetValue("prefix.name"); @@ -176,7 +176,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding { // Arrange var culture = new CultureInfo("fr-FR"); - var valueProvider = GetEnumerableValueProvider(BindingSource.Query, _backingStore, culture); + var valueProvider = GetEnumerableValueProvider(BindingSource.Query, BackingStore, culture); // Act var result = valueProvider.GetValue("some"); @@ -194,7 +194,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding { // Arrange var culture = new CultureInfo("fr-FR"); - var valueProvider = GetEnumerableValueProvider(BindingSource.Query, _backingStore, culture); + var valueProvider = GetEnumerableValueProvider(BindingSource.Query, BackingStore, culture); // Act var result = valueProvider.GetValue(key); @@ -226,7 +226,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding public void GetValue_ReturnsNullIfKeyNotFound() { // Arrange - var valueProvider = GetEnumerableValueProvider(BindingSource.Query, _backingStore, culture: null); + var valueProvider = GetEnumerableValueProvider(BindingSource.Query, BackingStore, culture: null); // Act var result = valueProvider.GetValue("prefix"); @@ -236,10 +236,10 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding } [Fact] - public void FilterInclude() + public virtual void FilterInclude() { // Arrange - var provider = GetBindingSourceValueProvider(BindingSource.Query, _backingStore, culture: null); + var provider = GetBindingSourceValueProvider(BindingSource.Query, BackingStore, culture: null); var bindingSource = new BindingSource( BindingSource.Query.Id, @@ -259,7 +259,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding public void FilterExclude() { // Arrange - var provider = GetBindingSourceValueProvider(BindingSource.Query, _backingStore, culture: null); + var provider = GetBindingSourceValueProvider(BindingSource.Query, BackingStore, culture: null); var bindingSource = new BindingSource( "Test", @@ -274,7 +274,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding Assert.Null(result); } - private IBindingSourceValueProvider GetBindingSourceValueProvider( + protected IBindingSourceValueProvider GetBindingSourceValueProvider( BindingSource bindingSource, Dictionary values, CultureInfo culture) diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/JQueryFormValueProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/JQueryFormValueProviderTest.cs index 3dc15a4570..608180ac3b 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/JQueryFormValueProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/JQueryFormValueProviderTest.cs @@ -4,6 +4,7 @@ using System.Collections.Generic; using System.Globalization; using Microsoft.Extensions.Primitives; +using Xunit; namespace Microsoft.AspNetCore.Mvc.ModelBinding { @@ -16,5 +17,19 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding { return new JQueryFormValueProvider(bindingSource, values, culture); } + + [Fact] + public void Filter_ExcludesItself() + { + // Arrange + var dictionary = new Dictionary(); + var provider = new JQueryFormValueProvider(BindingSource.Form, dictionary, CultureInfo.CurrentCulture); + + // Act + var result = provider.Filter(); + + // Assert + Assert.Null(result); + } } } diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/JQueryQueryStringValueProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/JQueryQueryStringValueProviderTest.cs index cb8453fd6b..830ceac1ab 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/JQueryQueryStringValueProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/JQueryQueryStringValueProviderTest.cs @@ -4,6 +4,7 @@ using System.Collections.Generic; using System.Globalization; using Microsoft.Extensions.Primitives; +using Xunit; namespace Microsoft.AspNetCore.Mvc.ModelBinding { @@ -16,5 +17,22 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding { return new JQueryQueryStringValueProvider(bindingSource, values, culture); } + + [Fact] + public void Filter_ExcludesItself() + { + // Arrange + var dictionary = new Dictionary(); + var provider = new JQueryQueryStringValueProvider( + BindingSource.Form, + dictionary, + CultureInfo.CurrentCulture); + + // Act + var result = provider.Filter(); + + // Assert + Assert.Null(result); + } } } diff --git a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/CollectionModelBinderIntegrationTest.cs b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/CollectionModelBinderIntegrationTest.cs index 9cb5b988eb..7cb0c782d2 100644 --- a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/CollectionModelBinderIntegrationTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/CollectionModelBinderIntegrationTest.cs @@ -63,6 +63,8 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests [Theory] [InlineData("?prefix[0]=10&prefix[1]=11")] [InlineData("?prefix.index=low&prefix.index=high&prefix[low]=10&prefix[high]=11")] + [InlineData("?prefix.index=index&prefix.index=indexer&prefix[index]=10&prefix[indexer]=11")] + [InlineData("?prefix.index=index&prefix.index=indexer&prefix[index]=10&prefix[indexer]=11&prefix[extra]=12")] public async Task CollectionModelBinder_BindsListOfSimpleType_WithExplicitPrefix_Success(string queryString) { // Arrange @@ -101,6 +103,8 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests [Theory] [InlineData("?[0]=10&[1]=11")] [InlineData("?index=low&index=high&[high]=11&[low]=10")] + [InlineData("?index=index&index=indexer&[indexer]=11&[index]=10")] + [InlineData("?index=index&index=indexer&[indexer]=11&[index]=10&[extra]=12")] public async Task CollectionModelBinder_BindsCollectionOfSimpleType_EmptyPrefix_Success(string queryString) { // Arrange @@ -172,6 +176,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests [InlineData("?index=low&index=high&[low].Id=10&[high].Id=11")] [InlineData("?parameter[0].Id=10¶meter[1].Id=11")] [InlineData("?parameter.index=low¶meter.index=high¶meter[low].Id=10¶meter[high].Id=11")] + [InlineData("?parameter.index=index¶meter.index=indexer¶meter[index].Id=10¶meter[indexer].Id=11")] public async Task CollectionModelBinder_BindsListOfComplexType_ImpliedPrefix_Success(string queryString) { // Arrange @@ -207,6 +212,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests [Theory] [InlineData("?prefix[0].Id=10&prefix[1].Id=11")] [InlineData("?prefix.index=low&prefix.index=high&prefix[high].Id=11&prefix[low].Id=10")] + [InlineData("?prefix.index=index&prefix.index=indexer&prefix[indexer].Id=11&prefix[index].Id=10")] public async Task CollectionModelBinder_BindsListOfComplexType_ExplicitPrefix_Success(string queryString) { // Arrange @@ -683,6 +689,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests [InlineData("?index=low&[low].Street=LongStreet")] [InlineData("?parameter[0].Street=LongStreet")] [InlineData("?parameter.index=low¶meter[low].Street=LongStreet")] + [InlineData("?parameter.index=index¶meter[index].Street=LongStreet")] public async Task CollectionModelBinder_BindsCollectionOfComplexType_ImpliedPrefix_FindsValidationErrors( string queryString) { @@ -1249,4 +1256,4 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests } } } -} \ No newline at end of file +} diff --git a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/DictionaryModelBinderIntegrationTest.cs b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/DictionaryModelBinderIntegrationTest.cs index fdc41aacaa..79a77e57f3 100644 --- a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/DictionaryModelBinderIntegrationTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/DictionaryModelBinderIntegrationTest.cs @@ -9,6 +9,7 @@ using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.ModelBinding; +using Microsoft.Extensions.Primitives; using Xunit; namespace Microsoft.AspNetCore.Mvc.IntegrationTests @@ -141,6 +142,8 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests [InlineData("?prefix[key0]=10")] [InlineData("?prefix[0].Key=key0&prefix[0].Value=10")] [InlineData("?prefix.index=low&prefix[low].Key=key0&prefix[low].Value=10")] + [InlineData("?prefix.index=index&prefix[index].Key=key0&prefix[index].Value=10")] + [InlineData("?prefix.index=index&prefix[index].Key=key0&prefix[index].Value=10&prefix[extra].Key=key4&prefix[extra].Value=5")] public async Task DictionaryModelBinder_BindsDictionaryOfSimpleType_WithExplicitPrefix_Success( string queryString) { @@ -181,6 +184,8 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests [InlineData("?[key0]=10")] [InlineData("?[0].Key=key0&[0].Value=10")] [InlineData("?index=low&[low].Key=key0&[low].Value=10")] + [InlineData("?index=index&[index].Key=key0&[index].Value=10")] + [InlineData("?index=index&[index].Key=key0&[index].Value=10&[extra].Key=key4&[extra].Value=5")] public async Task DictionaryModelBinder_BindsDictionaryOfSimpleType_EmptyPrefix_Success(string queryString) { // Arrange @@ -212,6 +217,186 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests Assert.True(modelState.IsValid); } + public static TheoryData> ThreeEntryTestData + { + get + { + var impliedPrefixQueryString = "?parameter[archive]=1¶meter[correlation]=2¶meter[index]=3"; + var noPrefixQueryString = "?[archive]=1&[correlation]=2&[index]=3"; + var reversedNoPrefixQueryString = "?[index]=3&[correlation]=2&[archive]=1"; + var impliedPrefixDictionary = new Dictionary + { + { "parameter[archive]", "1" }, + { "parameter[correlation]", "2" }, + { "parameter[index]", "3" }, + }; + var reversedImpliedPrefixDictionary = new Dictionary + { + { "parameter[index]", "3" }, + { "parameter[correlation]", "2" }, + { "parameter[archive]", "1" }, + }; + var longFormDictionary = new Dictionary + { + { "parameter[0].Key", "archive" }, + { "parameter[0].Value", "1" }, + { "parameter[1].Key", "correlation" }, + { "parameter[1].Value", "2" }, + { "parameter[2].Key", "index" }, + { "parameter[2].Value", "3" }, + }; + var longerFormDictionary = new Dictionary + { + { "parameter[indexer].Key", "archive" }, + { "parameter[indexer].Value", "1" }, + { "parameter[index].Key", "correlation" }, + { "parameter.index", new[] { "indexer", "index", "indexes" } }, + { "parameter[index].Value", "2" }, + { "parameter[indexes].Key", "index" }, + { "parameter[indexes].Value", "3" }, + }; + var longestFormDictionary = new Dictionary + { + { "parameter[indexer].Key", "archive" }, + { "parameter[indexer].Value", "1" }, + { "parameter[index].Key", "correlation" }, + { "parameter[extra].Key", "index" }, + { "parameter[extra].Value", "4" }, + { "parameter.index", new[] { "indexer", "index", "indexes" } }, + { "parameter[index].Value", "2" }, + { "parameter[indexes].Key", "index" }, + { "parameter[indexes].Value", "3" }, + { "parameter[another].Key", "index" }, + { "parameter[another].Value", "5" }, + }; + var noPrefixDictionary = new Dictionary + { + { "[archive]", "1" }, + { "[correlation]", "2" }, + { "[index]", "3" }, + }; + var reversedNoPrefixDictionary = new Dictionary + { + { "[index]", "3" }, + { "[correlation]", "2" }, + { "[archive]", "1" }, + }; + + return new TheoryData> + { + request => request.QueryString = new QueryString(impliedPrefixQueryString), + request => request.QueryString = new QueryString(noPrefixQueryString), + request => request.QueryString = new QueryString(reversedNoPrefixQueryString), + request => + { + request.ContentType = "application/x-www-form-urlencoded"; + request.Form = new FormCollection(impliedPrefixDictionary); + }, + request => + { + request.ContentType = "application/x-www-form-urlencoded"; + request.Form = new FormCollection(reversedImpliedPrefixDictionary); + }, + request => + { + request.ContentType = "application/x-www-form-urlencoded"; + request.Form = new FormCollection(longFormDictionary); + }, + request => + { + request.ContentType = "application/x-www-form-urlencoded"; + request.Form = new FormCollection(longerFormDictionary); + }, + request => + { + request.ContentType = "application/x-www-form-urlencoded"; + request.Form = new FormCollection(longestFormDictionary); + }, + request => + { + request.ContentType = "application/x-www-form-urlencoded"; + request.Form = new FormCollection(noPrefixDictionary); + }, + request => + { + request.ContentType = "application/x-www-form-urlencoded"; + request.Form = new FormCollection(reversedNoPrefixDictionary); + }, + }; + } + } + + [Theory] + [MemberData(nameof(ThreeEntryTestData))] + public async Task DictionaryModelBinder_Binds3EntriesOfSimpleType(Action updateRequest) + { + // Arrange + var expectedDictionary = new Dictionary + { + { "archive", 1 }, + { "correlation", 2 }, + { "index", 3 }, + }; + + var parameterBinder = ModelBindingTestHelper.GetParameterBinder(); + var testContext = ModelBindingTestHelper.GetTestContext(updateRequest); + var modelState = testContext.ModelState; + var parameter = new ParameterDescriptor + { + Name = "parameter", + ParameterType = typeof(Dictionary), + }; + + // Act + var result = await parameterBinder.BindModelAsync(parameter, testContext); + + // Assert + Assert.True(result.IsModelSet); + + var model = Assert.IsType>(result.Model); + Assert.Equal(expectedDictionary, model); + + Assert.NotEmpty(modelState); + Assert.True(modelState.IsValid); + } + + [Theory] + [MemberData(nameof(ThreeEntryTestData))] + public async Task DictionaryModelBinder_Binds3EntriesOfSimpleType_WithJQueryQueryString( + Action updateRequest) + { + // Arrange + var expectedDictionary = new Dictionary + { + { "archive", 1 }, + { "correlation", 2 }, + { "index", 3 }, + }; + + var parameterBinder = ModelBindingTestHelper.GetParameterBinder(); + var testContext = ModelBindingTestHelper.GetTestContext( + updateRequest, + options => options.ValueProviderFactories.Add(new JQueryQueryStringValueProviderFactory())); + var modelState = testContext.ModelState; + var parameter = new ParameterDescriptor + { + Name = "parameter", + ParameterType = typeof(Dictionary), + }; + + // Act + var result = await parameterBinder.BindModelAsync(parameter, testContext); + + // Assert + Assert.True(result.IsModelSet); + + var model = Assert.IsType>(result.Model); + Assert.Equal(expectedDictionary, model); + + Assert.NotEmpty(modelState); + Assert.True(modelState.IsValid); + } + [Fact] public async Task DictionaryModelBinder_BindsDictionaryOfSimpleType_NoData() { @@ -251,9 +436,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests public override bool Equals(object obj) { - var other = obj as Person; - - return other != null && Id == other.Id; + return obj is Person other && Id == other.Id; } public override int GetHashCode() @@ -274,6 +457,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests [InlineData("?parameter[key0].Id=10")] [InlineData("?parameter[0].Key=key0¶meter[0].Value.Id=10")] [InlineData("?parameter.index=low¶meter[low].Key=key0¶meter[low].Value.Id=10")] + [InlineData("?parameter.index=index¶meter[index].Key=key0¶meter[index].Value.Id=10")] public async Task DictionaryModelBinder_BindsDictionaryOfComplexType_ImpliedPrefix_Success(string queryString) { // Arrange @@ -309,6 +493,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests [InlineData("?prefix[key0].Id=10")] [InlineData("?prefix[0].Key=key0&prefix[0].Value.Id=10")] [InlineData("?prefix.index=low&prefix[low].Key=key0&prefix[low].Value.Id=10")] + [InlineData("?prefix.index=index&prefix[index].Key=key0&prefix[index].Value.Id=10")] public async Task DictionaryModelBinder_BindsDictionaryOfComplexType_ExplicitPrefix_Success( string queryString) { @@ -352,6 +537,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests [InlineData("?parameter[key0].Id=100")] [InlineData("?parameter[0].Key=key0¶meter[0].Value.Id=100")] [InlineData("?parameter.index=low¶meter[low].Key=key0¶meter[low].Value.Id=100")] + [InlineData("?parameter.index=index¶meter[index].Key=key0¶meter[index].Value.Id=100")] public async Task DictionaryModelBinder_BindsDictionaryOfComplexType_ImpliedPrefix_FindsValidationErrors( string queryString) { @@ -757,4 +943,4 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests } } } -} \ No newline at end of file +}