diff --git a/src/Microsoft.AspNetCore.Mvc.Core/BindPropertiesAttribute.cs b/src/Microsoft.AspNetCore.Mvc.Core/BindPropertiesAttribute.cs new file mode 100644 index 0000000000..28a1558290 --- /dev/null +++ b/src/Microsoft.AspNetCore.Mvc.Core/BindPropertiesAttribute.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. + +using System; + +namespace Microsoft.AspNetCore.Mvc +{ + /// + /// An attribute that enables binding for all properties the decorated controller or Razor Page model defines. + /// + [AttributeUsage(AttributeTargets.Class, AllowMultiple = false, Inherited = true)] + public class BindPropertiesAttribute : Attribute + { + /// + /// When true, allows properties to be bound on GET requests. When false, properties + /// do not get model bound or validated on GET requests. + /// + /// Defaults to false. + /// + /// + public bool SupportsGet { get; set; } + } +} diff --git a/src/Microsoft.AspNetCore.Mvc.Core/BindPropertyAttribute.cs b/src/Microsoft.AspNetCore.Mvc.Core/BindPropertyAttribute.cs index b36b79444f..2058d7d907 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/BindPropertyAttribute.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/BindPropertyAttribute.cs @@ -6,7 +6,7 @@ using Microsoft.AspNetCore.Mvc.ModelBinding; namespace Microsoft.AspNetCore.Mvc { - [AttributeUsage(AttributeTargets.Class | AttributeTargets.Property, AllowMultiple = false, Inherited = true)] + [AttributeUsage(AttributeTargets.Property, AllowMultiple = false, Inherited = true)] public class BindPropertyAttribute : Attribute, IModelNameProvider, IBinderTypeProviderMetadata, IRequestPredicateProvider { private static readonly Func _supportsAllRequests = (c) => true; diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/DefaultApplicationModelProvider.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/DefaultApplicationModelProvider.cs index 943b92b495..fc380933c2 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/DefaultApplicationModelProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/DefaultApplicationModelProvider.cs @@ -20,6 +20,8 @@ namespace Microsoft.AspNetCore.Mvc.Internal { private readonly MvcOptions _mvcOptions; private readonly IModelMetadataProvider _modelMetadataProvider; + private readonly Func _supportsAllRequests; + private readonly Func _supportsNonGetRequests; public DefaultApplicationModelProvider( IOptions mvcOptionsAccessor, @@ -27,6 +29,9 @@ namespace Microsoft.AspNetCore.Mvc.Internal { _mvcOptions = mvcOptionsAccessor.Value; _modelMetadataProvider = modelMetadataProvider; + + _supportsAllRequests = _ => true; + _supportsNonGetRequests = context => !string.Equals(context.HttpContext.Request.Method, "GET", StringComparison.OrdinalIgnoreCase); } /// @@ -218,16 +223,24 @@ namespace Microsoft.AspNetCore.Mvc.Internal var attributes = propertyInfo.GetCustomAttributes(inherit: true); - + // BindingInfo for properties can be either specified by decorating the property with binding specific attributes. + // ModelMetadata also adds information from the property's type and any configured IBindingMetadataProvider. var modelMetadata = _modelMetadataProvider.GetMetadataForProperty(propertyInfo.DeclaringType, propertyInfo.Name); var bindingInfo = BindingInfo.GetBindingInfo(attributes, modelMetadata); + if (bindingInfo == null) { + // Look for BindPropertiesAttribute on the handler type if no BindingInfo was inferred for the property. + // This allows a user to enable model binding on properties by decorating the controller type with BindPropertiesAttribute. var declaringType = propertyInfo.DeclaringType; - if (declaringType.IsDefined(typeof(BindPropertyAttribute), inherit: true)) + var bindPropertiesAttribute = declaringType.GetCustomAttribute(inherit: true); + if (bindPropertiesAttribute != null) { - // Specify a BindingInfo so that the property is now considered for model binding. - bindingInfo = new BindingInfo(); + var requestPredicate = bindPropertiesAttribute.SupportsGet ? _supportsAllRequests : _supportsNonGetRequests; + bindingInfo = new BindingInfo + { + RequestPredicate = requestPredicate, + }; } } diff --git a/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/DefaultPageApplicationModelProvider.cs b/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/DefaultPageApplicationModelProvider.cs index 138c14cbe8..cc6a02af02 100644 --- a/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/DefaultPageApplicationModelProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.RazorPages/Internal/DefaultPageApplicationModelProvider.cs @@ -20,6 +20,9 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal private readonly PageHandlerResultFilter _pageHandlerResultFilter = new PageHandlerResultFilter(); private readonly IModelMetadataProvider _modelMetadataProvider; private readonly MvcOptions _options; + private readonly Func _supportsAllRequests; + private readonly Func _supportsNonGetRequests; + public DefaultPageApplicationModelProvider( IModelMetadataProvider modelMetadataProvider, @@ -27,6 +30,9 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal { _modelMetadataProvider = modelMetadataProvider; _options = options.Value; + + _supportsAllRequests = _ => true; + _supportsNonGetRequests = context => !string.Equals(context.HttpContext.Request.Method, "GET", StringComparison.OrdinalIgnoreCase); } /// @@ -262,17 +268,25 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal var propertyAttributes = property.GetCustomAttributes(inherit: true); + // BindingInfo for properties can be either specified by decorating the property with binding-specific attributes. + // ModelMetadata also adds information from the property's type and any configured IBindingMetadataProvider. var propertyMetadata = _modelMetadataProvider.GetMetadataForProperty(property.DeclaringType, property.Name); var bindingInfo = BindingInfo.GetBindingInfo(propertyAttributes, propertyMetadata); if (bindingInfo == null) { - // Look for binding info on the handler if nothing is specified on the property. - // This allows BindProperty attributes on handlers to apply to properties. - var handlerType = property.DeclaringType; - var handlerAttributes = handlerType.GetCustomAttributes(inherit: true); - var handlerMetadata = _modelMetadataProvider.GetMetadataForType(property.DeclaringType); - bindingInfo = BindingInfo.GetBindingInfo(handlerAttributes, handlerMetadata); + // Look for BindPropertiesAttribute on the handler type if no BindingInfo was inferred for the property. + // This allows a user to enable model binding on properties by decorating the controller type with BindPropertiesAttribute. + var declaringType = property.DeclaringType; + var bindPropertiesAttribute = declaringType.GetCustomAttribute(inherit: true); + if (bindPropertiesAttribute != null) + { + var requestPredicate = bindPropertiesAttribute.SupportsGet ? _supportsAllRequests : _supportsNonGetRequests; + bindingInfo = new BindingInfo + { + RequestPredicate = requestPredicate, + }; + } } var model = new PagePropertyModel(property, propertyAttributes) diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/DefaultApplicationModelProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/DefaultApplicationModelProviderTest.cs index 3488448ecd..c7f548efa8 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/DefaultApplicationModelProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/Internal/DefaultApplicationModelProviderTest.cs @@ -1127,7 +1127,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal Assert.Equal(typeInfo, action.ActionMethod.DeclaringType.GetTypeInfo()); } - [BindProperty] + [BindProperties] public class BindPropertyController { public string Property { get; set; } @@ -1140,7 +1140,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal } [Fact] - public void CreatePropertyModel_AddsBindingInfoToProperty_IfDeclaringTypeHasBindPropertyAttribute() + public void CreatePropertyModel_AddsBindingInfoToProperty_IfDeclaringTypeHasBindPropertiesAttribute() { // Arrange var propertyInfo = typeof(BindPropertyController).GetProperty(nameof(BindPropertyController.Property)); @@ -1155,7 +1155,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal Assert.Null(bindingInfo.BinderType); Assert.Null(bindingInfo.BindingSource); Assert.Null(bindingInfo.PropertyFilterProvider); - Assert.Null(bindingInfo.RequestPredicate); + Assert.NotNull(bindingInfo.RequestPredicate); } [Fact] @@ -1206,7 +1206,7 @@ namespace Microsoft.AspNetCore.Mvc.Internal Assert.NotNull(property.BindingInfo); } - [BindProperty] + [BindProperties] public class UserController : ControllerBase { public string DerivedProperty { get; set; } diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/BasicTests.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/BasicTests.cs index dd44efae04..c93092b339 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/BasicTests.cs +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/BasicTests.cs @@ -406,9 +406,9 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests public async Task RedirectToAction_WithEmptyActionName_UsesAmbientValue() { // Arrange - var product = new List> + var product = new Dictionary { - new KeyValuePair("SampleInt", "20") + { "SampleInt", "20" } }; // Act @@ -522,10 +522,17 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests } [Fact] - public async Task BindPropertyCanBeAppliedToControllers() + public async Task BindPropertiesAttribute_CanBeAppliedToControllers() { + // Arrange + var formContent = new Dictionary + { + { "Name", "TestName" }, + { "Id", "10" }, + }; + // Act - var response = await Client.GetAsync("BindProperty/Action?Name=TestName&Id=10"); + var response = await Client.PostAsync("BindProperties/Action", new FormUrlEncodedContent(formContent)); // Assert await response.AssertStatusCodeAsync(HttpStatusCode.OK); @@ -537,10 +544,18 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests } [Fact] - public async Task BindProperty_DoesNotApplyToPropertiesWithBindingInfo() + public async Task BindPropertiesAttribute_DoesNotApplyToPropertiesWithBindingInfo() { + // Arrange + var formContent = new Dictionary + { + { "Id", "10" }, + { "FromRoute", "12" }, + { "CustomBound", "Test" }, + }; + // Act - var response = await Client.GetAsync("BindProperty/Action?Id=10&IdFromRoute=12&CustomBound=Test"); + var response = await Client.PostAsync("BindProperties/Action", new FormUrlEncodedContent(formContent)); // Assert await response.AssertStatusCodeAsync(HttpStatusCode.OK); @@ -552,6 +567,56 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests Assert.Equal("CustomBoundValue", data.CustomBound); } + [Fact] + public async Task BindPropertiesAttribute_DoesNotCausePropertiesWithBindNeverAttributeToBeModelBound() + { + // Arrange + var formContent = new Dictionary + { + { "BindNeverProperty", "Hello world" }, + }; + + // Act + var response = await Client.PostAsync("BindProperties/Action", new FormUrlEncodedContent(formContent)); + + // Assert + await response.AssertStatusCodeAsync(HttpStatusCode.OK); + var content = await response.Content.ReadAsStringAsync(); + var data = JsonConvert.DeserializeObject(content); + + Assert.Null(data.BindNeverProperty); + } + + [Fact] + public async Task BindPropertiesAttributeWithSupportsGet_BindsOnNonGet() + { + // Arrange + var formContent = new Dictionary + { + { "Name", "TestName" }, + }; + + // Act + var response = await Client.PostAsync("BindPropertiesSupportsGet/Action", new FormUrlEncodedContent(formContent)); + + // Assert + await response.AssertStatusCodeAsync(HttpStatusCode.OK); + var content = await response.Content.ReadAsStringAsync(); + Assert.Equal("TestName", content); + } + + [Fact] + public async Task BindPropertiesAttributeWithSupportsGet_BindsOnGet() + { + // Act + var response = await Client.GetAsync("BindPropertiesSupportsGet/Action?Name=OnGetTestName"); + + // Assert + await response.AssertStatusCodeAsync(HttpStatusCode.OK); + var content = await response.Content.ReadAsStringAsync(); + Assert.Equal("OnGetTestName", content); + } + public class BindPropertyControllerData { public string Name { get; set; } @@ -561,6 +626,8 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests public int? IdFromRoute { get; set; } public string CustomBound { get; set; } + + public string BindNeverProperty { get; set; } } } } diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RazorPagesTest.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RazorPagesTest.cs index 9076b9d344..d50c1ef6cc 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RazorPagesTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RazorPagesTest.cs @@ -1153,15 +1153,15 @@ Microsoft.AspNetCore.Mvc.ViewFeatures.ViewDataDictionary`1[AspNetCore.InjectedPa } [Fact] - public async Task BindPropertyAttribute_CanBeAppliedToModelType() + public async Task BindPropertiesAttribute_CanBeAppliedToModelType() { // Arrange var expected = "Property1 = 123, Property2 = 25,"; - var request = new HttpRequestMessage(HttpMethod.Post, "/Pages/PropertyBinding/BindPropertyOnModel?Property1=123") + var request = new HttpRequestMessage(HttpMethod.Post, "/Pages/PropertyBinding/BindPropertiesOnModel?Property1=123") { - Content = new FormUrlEncodedContent(new[] + Content = new FormUrlEncodedContent(new Dictionary { - new KeyValuePair("Property2", "25"), + { "Property2", "25" }, }), }; await AddAntiforgeryHeaders(request); @@ -1175,12 +1175,27 @@ Microsoft.AspNetCore.Mvc.ViewFeatures.ViewDataDictionary`1[AspNetCore.InjectedPa Assert.StartsWith(expected, responseContent.Trim()); } + [Fact] + public async Task BindPropertiesAttribute_CanBeAppliedToModelType_AllowsBindingOnGet() + { + // Arrange + var url = "/Pages/PropertyBinding/BindPropertiesWithSupportsGetOnModel?Property=Property-Value"; + + // Act + var response = await Client.GetAsync(url); + + // Assert + await response.AssertStatusCodeAsync(HttpStatusCode.OK); + var content = await response.Content.ReadAsStringAsync(); + Assert.Equal("Property-Value", content.Trim()); + } + [Fact] public async Task BindingInfoOnPropertiesIsPreferredToBindingInfoOnType() { // Arrange var expected = "Property1 = 123, Property2 = 25,"; - var request = new HttpRequestMessage(HttpMethod.Post, "/Pages/PropertyBinding/BindPropertyOnModel?Property1=123") + var request = new HttpRequestMessage(HttpMethod.Post, "/Pages/PropertyBinding/BindPropertiesOnModel?Property1=123") { Content = new FormUrlEncodedContent(new[] { diff --git a/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/DefaultPageApplicationModelProviderTest.cs b/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/DefaultPageApplicationModelProviderTest.cs index 99aeded947..1a82c834ec 100644 --- a/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/DefaultPageApplicationModelProviderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.RazorPages.Test/Internal/DefaultPageApplicationModelProviderTest.cs @@ -273,7 +273,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal public string Property1 { get; set; } } - [BindProperty] + [BindProperties] private class ModelLevel2 : ModelLevel1 { public string Property2 { get; set; } @@ -397,7 +397,7 @@ namespace Microsoft.AspNetCore.Mvc.RazorPages.Internal public override Task ExecuteAsync() => null; } - [BindProperty] + [BindProperties] [PageModel] private class ModelWithBindProperty { diff --git a/test/WebSites/BasicWebSite/Controllers/BindPropertyController.cs b/test/WebSites/BasicWebSite/Controllers/BindPropertiesController.cs similarity index 83% rename from test/WebSites/BasicWebSite/Controllers/BindPropertyController.cs rename to test/WebSites/BasicWebSite/Controllers/BindPropertiesController.cs index da016af93e..0ed0ebf5d4 100644 --- a/test/WebSites/BasicWebSite/Controllers/BindPropertyController.cs +++ b/test/WebSites/BasicWebSite/Controllers/BindPropertiesController.cs @@ -8,8 +8,8 @@ using Microsoft.AspNetCore.Mvc.ModelBinding; namespace BasicWebSite { - [BindProperty] - public class BindPropertyController : Controller + [BindProperties] + public class BindPropertiesController : Controller { public string Name { get; set; } @@ -21,7 +21,10 @@ namespace BasicWebSite [ModelBinder(typeof(CustomBoundModelBinder))] public string CustomBound { get; set; } - public object Action() => new { Name, Id, IdFromRoute, CustomBound }; + [BindNever] + public string BindNeverProperty { get; set; } + + public object Action() => new { Name, Id, IdFromRoute, CustomBound, BindNeverProperty }; private class CustomBoundModelBinder : IModelBinder { diff --git a/test/WebSites/BasicWebSite/Controllers/BindPropertiesSupportsGetController.cs b/test/WebSites/BasicWebSite/Controllers/BindPropertiesSupportsGetController.cs new file mode 100644 index 0000000000..b4220801ed --- /dev/null +++ b/test/WebSites/BasicWebSite/Controllers/BindPropertiesSupportsGetController.cs @@ -0,0 +1,18 @@ +// 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.Threading.Tasks; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.ModelBinding; + +namespace BasicWebSite +{ + [BindProperties(SupportsGet = true)] + public class BindPropertiesSupportsGetController : Controller + { + public string Name { get; set; } + + public IActionResult Action() => Content(Name); + } +} diff --git a/test/WebSites/RazorPagesWebSite/Pages/PropertyBinding/BindPropertyOnModel.cs b/test/WebSites/RazorPagesWebSite/Pages/PropertyBinding/BindPropertiesOnModel.cs similarity index 86% rename from test/WebSites/RazorPagesWebSite/Pages/PropertyBinding/BindPropertyOnModel.cs rename to test/WebSites/RazorPagesWebSite/Pages/PropertyBinding/BindPropertiesOnModel.cs index 4972e2a653..b06c380178 100644 --- a/test/WebSites/RazorPagesWebSite/Pages/PropertyBinding/BindPropertyOnModel.cs +++ b/test/WebSites/RazorPagesWebSite/Pages/PropertyBinding/BindPropertiesOnModel.cs @@ -7,8 +7,8 @@ using Microsoft.AspNetCore.Mvc.RazorPages; namespace RazorPagesWebSite { - [BindProperty] - public class BindPropertyOnModel : PageModel + [BindProperties] + public class BindPropertiesOnModel : PageModel { [FromQuery] public string Property1 { get; set; } diff --git a/test/WebSites/RazorPagesWebSite/Pages/PropertyBinding/BindPropertyOnModel.cshtml b/test/WebSites/RazorPagesWebSite/Pages/PropertyBinding/BindPropertiesOnModel.cshtml similarity index 80% rename from test/WebSites/RazorPagesWebSite/Pages/PropertyBinding/BindPropertyOnModel.cshtml rename to test/WebSites/RazorPagesWebSite/Pages/PropertyBinding/BindPropertiesOnModel.cshtml index 0c0fc39cd6..14b6136e32 100644 --- a/test/WebSites/RazorPagesWebSite/Pages/PropertyBinding/BindPropertyOnModel.cshtml +++ b/test/WebSites/RazorPagesWebSite/Pages/PropertyBinding/BindPropertiesOnModel.cshtml @@ -1,5 +1,5 @@ @page -@model BindPropertyOnModel +@model BindPropertiesOnModel Property1 = @Model.Property1, Property2 = @Model.Property2, diff --git a/test/WebSites/RazorPagesWebSite/Pages/PropertyBinding/BindPropertiesWithSupportsGetOnModel.cs b/test/WebSites/RazorPagesWebSite/Pages/PropertyBinding/BindPropertiesWithSupportsGetOnModel.cs new file mode 100644 index 0000000000..d4b1f83155 --- /dev/null +++ b/test/WebSites/RazorPagesWebSite/Pages/PropertyBinding/BindPropertiesWithSupportsGetOnModel.cs @@ -0,0 +1,15 @@ +// 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 Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.ModelBinding; +using Microsoft.AspNetCore.Mvc.RazorPages; + +namespace RazorPagesWebSite +{ + [BindProperties(SupportsGet = true)] + public class BindPropertiesWithSupportsGetOnModel : PageModel + { + public string Property { get; set; } + } +} diff --git a/test/WebSites/RazorPagesWebSite/Pages/PropertyBinding/BindPropertiesWithSupportsGetOnModel.cshtml b/test/WebSites/RazorPagesWebSite/Pages/PropertyBinding/BindPropertiesWithSupportsGetOnModel.cshtml new file mode 100644 index 0000000000..eb13d32677 --- /dev/null +++ b/test/WebSites/RazorPagesWebSite/Pages/PropertyBinding/BindPropertiesWithSupportsGetOnModel.cshtml @@ -0,0 +1,3 @@ +@page +@model BindPropertiesWithSupportsGetOnModel +@Model.Property