diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcCoreMvcOptionsSetup.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcCoreMvcOptionsSetup.cs index 537a82a16c..713a1278c8 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcCoreMvcOptionsSetup.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/MvcCoreMvcOptionsSetup.cs @@ -8,6 +8,7 @@ using Microsoft.AspNetCore.Mvc.Core; using Microsoft.AspNetCore.Mvc.Formatters; using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.AspNetCore.Mvc.ModelBinding.Binders; +using Microsoft.AspNetCore.Mvc.ModelBinding.Metadata; using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; using Microsoft.Extensions.Options; @@ -73,6 +74,11 @@ namespace Microsoft.AspNetCore.Mvc.Internal options.ValueProviderFactories.Add(new JQueryFormValueProviderFactory()); // Set up metadata providers + + // Don't bind the Type class by default as it's expensive. A user can override this behavior + // by altering the collection of providers. + options.ModelMetadataDetailsProviders.Add(new ExcludeBindingMetadataProvider(typeof(Type))); + options.ModelMetadataDetailsProviders.Add(new DefaultBindingMetadataProvider(messageProvider)); options.ModelMetadataDetailsProviders.Add(new DefaultValidationMetadataProvider()); diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/NoOpBinder.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/NoOpBinder.cs new file mode 100644 index 0000000000..74e66212a7 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/NoOpBinder.cs @@ -0,0 +1,19 @@ +// 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. + +using System.Threading.Tasks; +using Microsoft.AspNetCore.Mvc.ModelBinding; + +namespace Microsoft.AspNetCore.Mvc.Internal +{ + public class NoOpBinder : IModelBinder + { + public static readonly IModelBinder Instance = new NoOpBinder(); + + public Task BindModelAsync(ModelBindingContext bindingContext) + { + bindingContext.Result = ModelBindingResult.Failed(bindingContext.ModelName); + return TaskCache.CompletedTask; + } + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/BodyModelBinder.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/BodyModelBinder.cs index 5e1a4c636f..b5cd8a422f 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/BodyModelBinder.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/BodyModelBinder.cs @@ -14,7 +14,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders { /// /// An which binds models from the request body using an - /// when a model has the binding source / + /// when a model has the binding source . /// public class BodyModelBinder : IModelBinder { diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/ExcludeBindingMetadataProvider.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/ExcludeBindingMetadataProvider.cs new file mode 100644 index 0000000000..f0c236cc21 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Metadata/ExcludeBindingMetadataProvider.cs @@ -0,0 +1,51 @@ +// 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. + +using System; +using System.Reflection; + +namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata +{ + /// + /// An which configures to + /// false for matching types. + /// + public class ExcludeBindingMetadataProvider : IBindingMetadataProvider + { + private readonly Type _type; + + /// + /// Creates a new for the given . + /// + /// + /// The . All properties of this will have + /// set to false. + /// + public ExcludeBindingMetadataProvider(Type type) + { + if (type == null) + { + throw new ArgumentNullException(nameof(type)); + } + + _type = type; + } + + /// + public void CreateBindingMetadata(BindingMetadataProviderContext context) + { + if (context == null) + { + throw new ArgumentNullException(nameof(context)); + } + + // No-op if the metadata is not for the target type + if (!_type.IsAssignableFrom(context.Key.ModelType)) + { + return; + } + + context.BindingMetadata.IsBindingAllowed = false; + } + } +} \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/ModelBinderFactory.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/ModelBinderFactory.cs index a551d20057..e17538b719 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/ModelBinderFactory.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/ModelBinderFactory.cs @@ -73,6 +73,11 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding private IModelBinder CreateBinderCore(DefaultModelBinderProviderContext providerContext, object token) { + if (!providerContext.Metadata.IsBindingAllowed) + { + return NoOpBinder.Instance; + } + // A non-null token will usually be passed in at the the top level (ParameterDescriptor likely). // This prevents us from treating a parameter the same as a collection-element - which could // happen looking at just model metadata. @@ -188,17 +193,6 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding } } - private class NoOpBinder : IModelBinder - { - public static readonly IModelBinder Instance = new NoOpBinder(); - - public Task BindModelAsync(ModelBindingContext bindingContext) - { - bindingContext.Result = ModelBindingResult.Failed(bindingContext.ModelName); - return TaskCache.CompletedTask; - } - } - private struct Key : IEquatable { private readonly ModelMetadata _metadata; diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Metadata/ExcludeBindingMetadataProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Metadata/ExcludeBindingMetadataProviderTest.cs new file mode 100644 index 0000000000..a380c17399 --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Metadata/ExcludeBindingMetadataProviderTest.cs @@ -0,0 +1,63 @@ +// 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. + +using Xunit; + +namespace Microsoft.AspNetCore.Mvc.ModelBinding.Metadata +{ + public class ExcludeBindingMetadataProviderTest + { + [Theory] + [InlineData(true)] + [InlineData(false)] + public void IsBindingAllowed_LeftAlone_WhenTypeDoesntMatch(bool initialValue) + { + // Arrange + var provider = new ExcludeBindingMetadataProvider(typeof(string)); + + var key = ModelMetadataIdentity.ForProperty( + typeof(int), + nameof(Person.Age), + typeof(Person)); + + var context = new BindingMetadataProviderContext(key, new ModelAttributes(new object[0], new object[0])); + + context.BindingMetadata.IsBindingAllowed = initialValue; + + // Act + provider.CreateBindingMetadata(context); + + // Assert + Assert.Equal(initialValue, context.BindingMetadata.IsBindingAllowed); + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public void IsBindingAllowed_IsFalse_WhenTypeMatches(bool initialValue) + { + // Arrange + var provider = new ExcludeBindingMetadataProvider(typeof(int)); + + var key = ModelMetadataIdentity.ForProperty( + typeof(int), + nameof(Person.Age), + typeof(Person)); + + var context = new BindingMetadataProviderContext(key, new ModelAttributes(new object[0], new object[0])); + + context.BindingMetadata.IsBindingAllowed = initialValue; + + // Act + provider.CreateBindingMetadata(context); + + // Assert + Assert.False(context.BindingMetadata.IsBindingAllowed); + } + + private class Person + { + public int Age { get; set; } + } + } +} diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/ModelBinderFactoryTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/ModelBinderFactoryTest.cs index d26440a439..5a951f1ac2 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/ModelBinderFactoryTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/ModelBinderFactoryTest.cs @@ -2,6 +2,8 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using Microsoft.AspNetCore.Mvc.Internal; +using Microsoft.AspNetCore.Mvc.ModelBinding.Binders; using Microsoft.AspNetCore.Mvc.ModelBinding.Internal; using Moq; using Xunit; @@ -14,7 +16,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding [Fact] public void CreateBinder_Throws_WhenBinderNotCreated() { - // Arrange + // Arrange var metadataProvider = new TestModelMetadataProvider(); var options = new TestOptionsManager(); var factory = new ModelBinderFactory(metadataProvider, options); @@ -36,7 +38,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding [Fact] public void CreateBinder_CreatesNoOpBinder_WhenPropertyDoesntHaveABinder() { - // Arrange + // Arrange var metadataProvider = new TestModelMetadataProvider(); // There isn't a provider that can handle WidgetId. @@ -66,10 +68,47 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding Assert.NotNull(result); } + [Fact] + public void CreateBinder_CreatesNoOpBinder_WhenPropertyBindingIsNotAllowed() + { + // Arrange + var metadataProvider = new TestModelMetadataProvider(); + metadataProvider + .ForProperty(nameof(Widget.Id)) + .BindingDetails(m => m.IsBindingAllowed = false); + + var modelBinder = new ByteArrayModelBinder(); + + var options = new TestOptionsManager(); + options.Value.ModelBinderProviders.Add(new TestModelBinderProvider(c => + { + if (c.Metadata.ModelType == typeof(WidgetId)) + { + return modelBinder; + } + + return null; + })); + + var factory = new ModelBinderFactory(metadataProvider, options); + + var context = new ModelBinderFactoryContext() + { + Metadata = metadataProvider.GetMetadataForProperty(typeof(Widget), nameof(Widget.Id)), + }; + + // Act + var result = factory.CreateBinder(context); + + // Assert + Assert.NotNull(result); + Assert.IsType(result); + } + [Fact] public void CreateBinder_NestedProperties() { - // Arrange + // Arrange var metadataProvider = new TestModelMetadataProvider(); var options = new TestOptionsManager(); @@ -105,7 +144,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding [Fact] public void CreateBinder_BreaksCycles() { - // Arrange + // Arrange var metadataProvider = new TestModelMetadataProvider(); var callCount = 0; @@ -142,7 +181,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding [Fact] public void CreateBinder_DoesNotCache_WhenTokenIsNull() { - // Arrange + // Arrange var metadataProvider = new TestModelMetadataProvider(); var options = new TestOptionsManager(); @@ -170,7 +209,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding [Fact] public void CreateBinder_Caches_WhenTokenIsNotNull() { - // Arrange + // Arrange var metadataProvider = new TestModelMetadataProvider(); var options = new TestOptionsManager(); diff --git a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ExcludeBindingMetadataProviderIntegrationTest.cs b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ExcludeBindingMetadataProviderIntegrationTest.cs new file mode 100644 index 0000000000..1ba6b2a82e --- /dev/null +++ b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ExcludeBindingMetadataProviderIntegrationTest.cs @@ -0,0 +1,111 @@ +// 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. + +using System; +using System.Collections.Generic; +using System.Reflection; +using System.Threading.Tasks; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc.Abstractions; +using Microsoft.AspNetCore.Mvc.Internal; +using Microsoft.AspNetCore.Mvc.ModelBinding; +using Microsoft.Extensions.Primitives; +using Xunit; + +namespace Microsoft.AspNetCore.Mvc.IntegrationTests +{ + public class ExcludeBindingMetadataProviderIntegrationTest + { + [Fact] + public async Task BindParameter_WithTypeProperty_IsNotBound() + { + // Arrange + var options = new MvcOptions(); + var setup = new MvcCoreMvcOptionsSetup(new TestHttpRequestStreamReaderFactory()); + + // Adding a custom model binder for Type to ensure it doesn't get called + options.ModelBinderProviders.Insert(0, new TypeModelBinderProvider()); + + setup.Configure(options); + + var argumentBinder = ModelBindingTestHelper.GetArgumentBinder(options); + var parameter = new ParameterDescriptor() + { + Name = "Parameter1", + BindingInfo = new BindingInfo(), + ParameterType = typeof(TypesBundle), + }; + + var operationContext = ModelBindingTestHelper.GetOperationBindingContext(request => + { + request.Form = new FormCollection(new Dictionary + { + { "name", new[] { "Fred" } }, + { "type", new[] { "SomeType" } }, + { "typeArray", new[] { "SomeType1", "SomeType2" } }, + { "typeList", new[] { "SomeType1", "SomeType2" } }, + { "typeDictionary", new[] { "parameter[0].Key=key", "parameter[0].Value=value" } }, + { "methodInfo", new[] { "value" } }, + { "func", new[] { "value" } }, + } + ); + }); + + var modelState = operationContext.ActionContext.ModelState; + + // Act + var modelBindingResult = await argumentBinder.BindModelAsync(parameter, operationContext) ?? + default(ModelBindingResult); + + // Assert + // ModelBindingResult + Assert.True(modelBindingResult.IsModelSet); + + // Model + var boundPerson = Assert.IsType(modelBindingResult.Model); + Assert.NotNull(boundPerson); + Assert.Equal("Fred", boundPerson.Name); + + // ModelState + + // The TypeModelBinder should not be called + Assert.True(modelState.IsValid); + } + + private class TypesBundle + { + public string Name { get; set; } + + public Type Type { get; set; } + + public Type[] TypeArray { get; set; } + + public List TypeList { get; set; } + + public Dictionary TypeDictionary { get; set; } + + public MethodInfo MethodInfo { get; set; } + + public Func Func { get; set; } + } + + public class TypeModelBinderProvider : IModelBinderProvider + { + /// + public IModelBinder GetBinder(ModelBinderProviderContext context) + { + if (context == null) + { + throw new ArgumentNullException(nameof(context)); + } + + if (context.Metadata.ModelType == typeof(Type)) + { + throw new Exception(); + } + + return null; + } + } + } +} \ No newline at end of file diff --git a/test/Microsoft.AspNetCore.Mvc.Test/MvcOptionsSetupTest.cs b/test/Microsoft.AspNetCore.Mvc.Test/MvcOptionsSetupTest.cs index de12b9bed4..16d31155a7 100644 --- a/test/Microsoft.AspNetCore.Mvc.Test/MvcOptionsSetupTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Test/MvcOptionsSetupTest.cs @@ -157,6 +157,7 @@ namespace Microsoft.AspNetCore.Mvc // Assert var providers = options.ModelMetadataDetailsProviders; Assert.Collection(providers, + provider => Assert.IsType(provider), provider => Assert.IsType(provider), provider => Assert.IsType(provider), provider =>