From 86fcacea92839a2d260e447f4427bf3048e06be9 Mon Sep 17 00:00:00 2001 From: Harsh Gupta Date: Tue, 21 Apr 2015 14:59:16 -0700 Subject: [PATCH] Fix for #2357 : We prevent assigining null values to non nullable controller properties. --- .../DefaultControllerActionArgumentBinder.cs | 6 +- .../ControllerActionArgumentBinderTests.cs | 62 ++++++++++++++++--- 2 files changed, 59 insertions(+), 9 deletions(-) diff --git a/src/Microsoft.AspNet.Mvc.Core/DefaultControllerActionArgumentBinder.cs b/src/Microsoft.AspNet.Mvc.Core/DefaultControllerActionArgumentBinder.cs index 2957f1aa20..4f829bf454 100644 --- a/src/Microsoft.AspNet.Mvc.Core/DefaultControllerActionArgumentBinder.cs +++ b/src/Microsoft.AspNet.Mvc.Core/DefaultControllerActionArgumentBinder.cs @@ -111,7 +111,11 @@ namespace Microsoft.AspNet.Mvc return; } - propertyHelper.SetValue(controller, property.Value); + // Do not set the property if the type is a non nullable type. + if (property.Value != null || propertyHelper.Property.PropertyType.AllowsNullValue()) + { + propertyHelper.SetValue(controller, property.Value); + } } } diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ParameterBinding/ControllerActionArgumentBinderTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ParameterBinding/ControllerActionArgumentBinderTests.cs index c53a3d16b2..d1df43d745 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/ParameterBinding/ControllerActionArgumentBinderTests.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ParameterBinding/ControllerActionArgumentBinderTests.cs @@ -52,7 +52,7 @@ namespace Microsoft.AspNet.Mvc.Core.Test } [Fact] - public async Task GetActionArgumentsAsync_DoesNotAddActionArguments_IfBinderReturnsFalse() + public async Task BindActionArgumentsAsync_DoesNotAddActionArguments_IfBinderReturnsFalse() { // Arrange var actionDescriptor = GetActionDescriptor(); @@ -88,7 +88,7 @@ namespace Microsoft.AspNet.Mvc.Core.Test } [Fact] - public async Task GetActionArgumentsAsync_DoesNotAddActionArguments_IfBinderDoesNotSetModel() + public async Task BindActionArgumentsAsync_DoesNotAddActionArguments_IfBinderDoesNotSetModel() { // Arrange var actionDescriptor = GetActionDescriptor(); @@ -127,7 +127,7 @@ namespace Microsoft.AspNet.Mvc.Core.Test } [Fact] - public async Task GetActionArgumentsAsync_AddsActionArguments_IfBinderReturnsTrue() + public async Task BindActionArgumentsAsync_AddsActionArguments_IfBinderReturnsTrue() { // Arrange Func method = foo => 1; @@ -174,7 +174,7 @@ namespace Microsoft.AspNet.Mvc.Core.Test } [Fact] - public async Task GetActionArgumentsAsync_CallsValidator_IfModelBinderSucceeds() + public async Task BindActionArgumentsAsync_CallsValidator_IfModelBinderSucceeds() { // Arrange var actionDescriptor = GetActionDescriptor(); @@ -204,7 +204,7 @@ namespace Microsoft.AspNet.Mvc.Core.Test } [Fact] - public async Task GetActionArgumentsAsync_DoesNotCallValidator_IfModelBinderFails() + public async Task BindActionArgumentsAsync_DoesNotCallValidator_IfModelBinderFails() { // Arrange Func method = foo => 1; @@ -246,7 +246,7 @@ namespace Microsoft.AspNet.Mvc.Core.Test } [Fact] - public async Task GetActionArgumentsAsync_CallsValidator_ForControllerProperties_IfModelBinderSucceeds() + public async Task BindActionArgumentsAsync_CallsValidator_ForControllerProperties_IfModelBinderSucceeds() { // Arrange var actionDescriptor = GetActionDescriptor(); @@ -276,7 +276,7 @@ namespace Microsoft.AspNet.Mvc.Core.Test } [Fact] - public async Task GetActionArgumentsAsync_DoesNotCallValidator_ForControllerProperties_IfModelBinderFails() + public async Task BindActionArgumentsAsync_DoesNotCallValidator_ForControllerProperties_IfModelBinderFails() { // Arrange Func method = foo => 1; @@ -318,7 +318,7 @@ namespace Microsoft.AspNet.Mvc.Core.Test [Fact] - public async Task GetActionArgumentsAsync_SetsControllerProperties() + public async Task BindActionArgumentsAsync_SetsControllerProperties() { // Arrange var actionDescriptor = GetActionDescriptor(); @@ -343,6 +343,44 @@ namespace Microsoft.AspNet.Mvc.Core.Test Assert.Null(controller.UnmarkedProperty); } + [Fact] + public async Task BindActionArgumentsAsync_DoesNotSetNullValues_ForNonNullableProperties() + { + // Arrange + var actionDescriptor = GetActionDescriptor(); + actionDescriptor.BoundProperties.Add( + new ParameterDescriptor + { + Name = "ValueBinderMarkedProperty", + BindingInfo = new BindingInfo(), + ParameterType = typeof(int) + }); + + var actionContext = GetActionContext(actionDescriptor); + + var binder = new Mock(); + binder + .Setup(b => b.BindModelAsync(It.IsAny())) + .Returns(Task.FromResult( + result: new ModelBindingResult(model: null, key: string.Empty, isModelSet: true))); + var actionBindingContext = new ActionBindingContext() + { + ModelBinder = binder.Object, + }; + + var argumentBinder = GetArgumentBinder(); + var controller = new TestController(); + + // Some non default value. + controller.NotNullableProperty = -1; + + // Act + var result = await argumentBinder.BindActionArgumentsAsync(actionContext, actionBindingContext, controller); + + // Assert + Assert.Equal(-1, controller.NotNullableProperty); + } + private static ActionContext GetActionContext(ActionDescriptor descriptor = null) { return new ActionContext( @@ -400,6 +438,9 @@ namespace Microsoft.AspNet.Mvc.Core.Test [ValueProviderMetadata] public string ValueBinderMarkedProperty { get; set; } + [CustomBindingSource] + public int NotNullableProperty { get; set; } + public Person ActionWithBodyParam([FromBody] Person bodyParam) { return bodyParam; @@ -421,6 +462,11 @@ namespace Microsoft.AspNet.Mvc.Core.Test public BindingSource BindingSource { get { return BindingSource.Body; } } } + private class CustomBindingSourceAttribute : Attribute, IBindingSourceMetadata + { + public BindingSource BindingSource { get { return BindingSource.Custom; } } + } + private class ValueProviderMetadataAttribute : Attribute, IBindingSourceMetadata { public BindingSource BindingSource { get { return BindingSource.Query; } }