`ByteArrayModelBinder` should return `null` only when type is not matched

- #2456
- visible behaviours now match MVC 5
- prevent `CollectionModelBinder` from converting no value to `byte[0]`
- prevent `TypeConverterModelBinder` from converting empty value to `byte[] { '\0' }`

nit:
- remove additional delegates `ModelBindingTestHelper.GetOperationBindingContext()` calls
 - a few left despite #2625 cleanup

src/Microsoft.AspNet.Mvc.Core/ModelBinding/ByteArrayModelBinder.cs

@@ -15,6 +15,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
         /// <inheritdoc />
         public async Task<ModelBindingResult> BindModelAsync([NotNull] ModelBindingContext bindingContext)
         {
+            // Check if this binder applies.
             if (bindingContext.ModelType != typeof(byte[]))
             {
                 return null;
@@ -22,18 +23,18 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
 
             var valueProviderResult = await bindingContext.ValueProvider.GetValueAsync(bindingContext.ModelName);
 
-            // case 1: there was no <input ... /> element containing this data
+            // Check for missing data case 1: There was no <input ... /> element containing this data.
             if (valueProviderResult == null)
             {
-                return null;
+                return new ModelBindingResult(model: null, key: bindingContext.ModelName, isModelSet: false);
             }
 
             var value = valueProviderResult.AttemptedValue;
 
-            // case 2: there was an <input ... /> element but it was left blank
+            // Check for missing data case 2: There was an <input ... /> element but it was left blank.
             if (string.IsNullOrEmpty(value))
             {
-                return null;
+                return new ModelBindingResult(model: null, key: bindingContext.ModelName, isModelSet: false);
             }
 
             try
@@ -55,8 +56,8 @@ namespace Microsoft.AspNet.Mvc.ModelBinding
                 bindingContext.ModelState.TryAddModelError(bindingContext.ModelName, ex);
             }
 
-            // Matched the type (byte[]) only this binder supports.
+            // Matched the type (byte[]) only this binder supports. As in missing data cases, always tell the model
-            // Always tell the model binding system to skip other model binders i.e. return non-null.
+            // binding system to skip other model binders i.e. return non-null.
             return new ModelBindingResult(model: null, key: bindingContext.ModelName, isModelSet: false);
         }
     }

test/Microsoft.AspNet.Mvc.Core.Test/ModelBinding/ByteArrayModelBinderTests.cs

@@ -29,7 +29,12 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test
             var binderResult = await binder.BindModelAsync(bindingContext);
 
             // Assert
-            Assert.Null(binderResult);
+            Assert.NotNull(binderResult);
+            Assert.False(binderResult.IsModelSet);
+            Assert.Equal("foo", binderResult.Key);
+            Assert.Null(binderResult.Model);
+
+            Assert.Empty(bindingContext.ModelState); // No submitted value for "foo".
         }
 
         [Fact]
@@ -80,7 +85,7 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test
         }
 
         [Fact]
-        public async Task BindModel_ReturnsNull_WhenValueNotFound()
+        public async Task BindModel_ReturnsWithIsModelSetFalse_WhenValueNotFound()
         {
             // Arrange
             var valueProvider = new SimpleHttpValueProvider()
@@ -95,7 +100,12 @@ namespace Microsoft.AspNet.Mvc.ModelBinding.Test
             var binderResult = await binder.BindModelAsync(bindingContext);
 
             // Assert
-            Assert.Null(binderResult);
+            Assert.NotNull(binderResult);
+            Assert.False(binderResult.IsModelSet);
+            Assert.Equal("foo", binderResult.Key);
+            Assert.Null(binderResult.Model);
+
+            Assert.Empty(bindingContext.ModelState); // No submitted data for "foo".
         }
 
         [Fact]

test/Microsoft.AspNet.Mvc.FunctionalTests/ModelBindingTest.cs

@@ -778,7 +778,7 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests
 
             //Assert
             Assert.Equal(HttpStatusCode.OK, response.StatusCode);
-            Assert.Equal("\0", await response.Content.ReadAsStringAsync());
+            Assert.Equal(string.Empty, await response.Content.ReadAsStringAsync());
         }
 
         [Fact]

test/Microsoft.AspNet.Mvc.IntegrationTests/ByteArrayModelBinderIntegrationTest.cs

@@ -76,7 +76,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests
             Assert.Equal(ModelValidationState.Valid, modelState[key].ValidationState); // Should be skipped. bug#2447
         }
 
-        [Fact(Skip = "ByteArrayModelBinder should return a non-null result #2456")]
+        [Fact]
         public async Task BindParameter_NoData_DoesNotGetBound()
         {
             // Arrange
@@ -93,7 +93,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests
             };
 
             // No data is passed.
-            var operationContext = ModelBindingTestHelper.GetOperationBindingContext(httpContext => { });
+            var operationContext = ModelBindingTestHelper.GetOperationBindingContext();
             var modelState = new ModelStateDictionary();
 
             // Act
@@ -102,15 +102,11 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests
             // Assert
 
             // ModelBindingResult
-            Assert.NotNull(modelBindingResult);
+            Assert.Null(modelBindingResult);
 
             // ModelState
             Assert.True(modelState.IsValid);
             Assert.Empty(modelState.Keys);
-
-            Assert.Equal("CustomParameter", modelBindingResult.Key);
-            Assert.True(modelBindingResult.IsModelSet);
-            Assert.Equal(new byte[0], modelBindingResult.Model);
         }
 
         [Fact(Skip = "ModelState.Value not set due to #2445")]

test/Microsoft.AspNet.Mvc.IntegrationTests/CancellationTokenModelBinderIntegrationTest.cs

@@ -35,7 +35,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests
                 ParameterType = typeof(Person)
             };
 
-            var operationContext = ModelBindingTestHelper.GetOperationBindingContext(httpContext => { });
+            var operationContext = ModelBindingTestHelper.GetOperationBindingContext();
             var modelState = new ModelStateDictionary();
 
             // Act
@@ -78,7 +78,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests
                 ParameterType = typeof(Person)
             };
 
-            var operationContext = ModelBindingTestHelper.GetOperationBindingContext(httpContext => { });
+            var operationContext = ModelBindingTestHelper.GetOperationBindingContext();
             var modelState = new ModelStateDictionary();
 
             // Act
@@ -122,7 +122,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests
                 ParameterType = typeof(CancellationToken)
             };
 
-            var operationContext = ModelBindingTestHelper.GetOperationBindingContext(httpContext => { });
+            var operationContext = ModelBindingTestHelper.GetOperationBindingContext();
             var modelState = new ModelStateDictionary();
 
             // Act

test/Microsoft.AspNet.Mvc.IntegrationTests/ServicesModelBinderIntegrationTest.cs

@@ -1,11 +1,7 @@
 // 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.ComponentModel.DataAnnotations;
-using System.Linq;
 using System.Threading.Tasks;
-using Microsoft.AspNet.Http;
 using Microsoft.AspNet.Mvc.ModelBinding;
 using Xunit;
 
@@ -41,7 +37,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests
                 ParameterType = typeof(Person)
             };
 
-            var operationContext = ModelBindingTestHelper.GetOperationBindingContext(httpContext => { });
+            var operationContext = ModelBindingTestHelper.GetOperationBindingContext();
             var modelState = new ModelStateDictionary();
 
             // Act
@@ -75,7 +71,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests
                 ParameterType = typeof(Person)
             };
 
-            var operationContext = ModelBindingTestHelper.GetOperationBindingContext(httpContext => { });
+            var operationContext = ModelBindingTestHelper.GetOperationBindingContext();
             var modelState = new ModelStateDictionary();
 
             // Act
@@ -117,7 +113,7 @@ namespace Microsoft.AspNet.Mvc.IntegrationTests
                 ParameterType = typeof(JsonOutputFormatter)
             };
 
-            var operationContext = ModelBindingTestHelper.GetOperationBindingContext(httpContext => { });
+            var operationContext = ModelBindingTestHelper.GetOperationBindingContext();
             var modelState = new ModelStateDictionary();
 
             // Act

test/WebSites/ModelBindingWebSite/Controllers/HomeController.cs

@@ -15,6 +15,11 @@ namespace ModelBindingWebSite.Controllers
         [HttpGet]
         public IActionResult Index(byte[] byteValues)
         {
+            if (byteValues == null)
+            {
+                return Content(content: null);
+            }
+
             return Content(System.Text.Encoding.UTF8.GetString(byteValues));
         }