diff --git a/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelBindingContext.cs b/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelBindingContext.cs index e133b6816e..bc362837ca 100644 --- a/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelBindingContext.cs +++ b/src/Microsoft.AspNetCore.Mvc.Abstractions/ModelBinding/ModelBindingContext.cs @@ -67,6 +67,12 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding /// public abstract string ModelName { get; set; } + /// + /// Gets or sets the name of the top-level model. This is not reset to when value + /// providers have no match for that model. + /// + public string OriginalModelName { get; protected set; } + /// /// Gets or sets the used to capture values /// for properties in the object graph of the model when binding. diff --git a/src/Microsoft.AspNetCore.Mvc.Core/Internal/DefaultModelBindingContext.cs b/src/Microsoft.AspNetCore.Mvc.Core/Internal/DefaultModelBindingContext.cs index 0ff9292a02..c57992ceed 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/Internal/DefaultModelBindingContext.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/Internal/DefaultModelBindingContext.cs @@ -237,6 +237,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding // Because this is the top-level context, FieldName and ModelName should be the same. FieldName = binderModelName ?? modelName, ModelName = binderModelName ?? modelName, + OriginalModelName = binderModelName ?? modelName, IsTopLevelObject = true, ModelMetadata = metadata, diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/ComplexTypeModelBinder.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/ComplexTypeModelBinder.cs index 3f15ffdf20..2ab1cdb70c 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/ComplexTypeModelBinder.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/ComplexTypeModelBinder.cs @@ -216,8 +216,8 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders return true; } - // 2. Any of the model properties can be bound using a value provider. - if (CanValueBindAnyModelProperties(bindingContext)) + // 2. Any of the model properties can be bound. + if (CanBindAnyModelProperties(bindingContext)) { return true; } @@ -225,7 +225,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders return false; } - private bool CanValueBindAnyModelProperties(ModelBindingContext bindingContext) + private bool CanBindAnyModelProperties(ModelBindingContext bindingContext) { // If there are no properties on the model, there is nothing to bind. We are here means this is not a top // level object. So we return false. @@ -235,20 +235,19 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders return false; } - // We want to check to see if any of the properties of the model can be bound using the value providers, - // because that's all that MutableObjectModelBinder can handle. + // We want to check to see if any of the properties of the model can be bound using the value providers or + // a greedy binder. // - // However, because a property might specify a custom binding source ([FromForm]), it's not correct - // for us to just try bindingContext.ValueProvider.ContainsPrefixAsync(bindingContext.ModelName), - // because that may include other value providers - that would lead us to mistakenly create the model + // Because a property might specify a custom binding source ([FromForm]), it's not correct + // for us to just try bindingContext.ValueProvider.ContainsPrefixAsync(bindingContext.ModelName); + // that may include other value providers - that would lead us to mistakenly create the model // when the data is coming from a source we should use (ex: value found in query string, but the // model has [FromForm]). // // To do this we need to enumerate the properties, and see which of them provide a binding source // through metadata, then we decide what to do. // - // If a property has a binding source, and it's a greedy source, then it's not - // allowed to come from a value provider, so we skip it. + // If a property has a binding source, and it's a greedy source, then it's always bound. // // If a property has a binding source, and it's a non-greedy source, then we'll filter the // the value providers to just that source, and see if we can find a matching prefix @@ -256,12 +255,10 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders // // If a property does not have a binding source, then it's fair game for any value provider. // - // If any property meets the above conditions and has a value from ValueProviders, then we'll - // create the model and try to bind it. OR if ALL properties of the model have a greedy source, + // Bottom line, if any property meets the above conditions and has a value from ValueProviders, then we'll + // create the model and try to bind it. Of, if ANY properties of the model have a greedy source, // then we go ahead and create it. // - var hasBindableProperty = false; - var isAnyPropertyEnabledForValueProviderBasedBinding = false; for (var i = 0; i < bindingContext.ModelMetadata.Properties.Count; i++) { var propertyMetadata = bindingContext.ModelMetadata.Properties[i]; @@ -270,41 +267,30 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders continue; } - hasBindableProperty = true; - - // This check will skip properties which are marked explicitly using a non value binder. + // If any property can be bound from a greedy binding source, then success. var bindingSource = propertyMetadata.BindingSource; - if (bindingSource == null || !bindingSource.IsGreedy) + if (bindingSource != null && bindingSource.IsGreedy) { - isAnyPropertyEnabledForValueProviderBasedBinding = true; + return true; + } - var fieldName = propertyMetadata.BinderModelName ?? propertyMetadata.PropertyName; - var modelName = ModelNames.CreatePropertyModelName( - bindingContext.ModelName, - fieldName); - - using (bindingContext.EnterNestedScope( - modelMetadata: propertyMetadata, - fieldName: fieldName, - modelName: modelName, - model: null)) + // Otherwise, check whether the (perhaps filtered) value providers have a match. + var fieldName = propertyMetadata.BinderModelName ?? propertyMetadata.PropertyName; + var modelName = ModelNames.CreatePropertyModelName(bindingContext.ModelName, fieldName); + using (bindingContext.EnterNestedScope( + modelMetadata: propertyMetadata, + fieldName: fieldName, + modelName: modelName, + model: null)) + { + // If any property can be bound from a value provider, then success. + if (bindingContext.ValueProvider.ContainsPrefix(bindingContext.ModelName)) { - // If any property can be bound from a value provider then continue. - if (bindingContext.ValueProvider.ContainsPrefix(bindingContext.ModelName)) - { - return true; - } + return true; } } } - if (hasBindableProperty && !isAnyPropertyEnabledForValueProviderBasedBinding) - { - // All the properties are marked with a non value provider based marker like [FromHeader] or - // [FromBody]. - return true; - } - _logger.CannotBindToComplexType(bindingContext); return false; diff --git a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/FormFileModelBinder.cs b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/FormFileModelBinder.cs index 5a7f1157ac..76b4bf54ca 100644 --- a/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/FormFileModelBinder.cs +++ b/src/Microsoft.AspNetCore.Mvc.Core/ModelBinding/Binders/FormFileModelBinder.cs @@ -48,7 +48,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders _logger = loggerFactory.CreateLogger(); } - + /// public async Task BindModelAsync(ModelBindingContext bindingContext) { @@ -85,6 +85,19 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders await GetFormFilesAsync(modelName, bindingContext, postedFiles); + // If ParameterBinder incorrectly overrode ModelName, fall back to OriginalModelName prefix. Comparisons + // are tedious because e.g. top-level parameter or property is named Blah and it contains a BlahBlah + // property. OriginalModelName may be null in tests. + if (postedFiles.Count == 0 && + bindingContext.OriginalModelName != null && + !string.Equals(modelName, bindingContext.OriginalModelName, StringComparison.Ordinal) && + !modelName.StartsWith(bindingContext.OriginalModelName + "[", StringComparison.Ordinal) && + !modelName.StartsWith(bindingContext.OriginalModelName + ".", StringComparison.Ordinal)) + { + modelName = ModelNames.CreatePropertyModelName(bindingContext.OriginalModelName, modelName); + await GetFormFilesAsync(modelName, bindingContext, postedFiles); + } + object value; if (bindingContext.ModelType == typeof(IFormFile)) { diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Binders/ComplexTypeModelBinderTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Binders/ComplexTypeModelBinderTest.cs index c76eb27c0a..7eaedfa106 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Binders/ComplexTypeModelBinderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Binders/ComplexTypeModelBinderTest.cs @@ -155,10 +155,6 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders [Theory] [InlineData(typeof(TypeWithNoBinderMetadata), false)] [InlineData(typeof(TypeWithNoBinderMetadata), true)] - [InlineData(typeof(TypeWithAtLeastOnePropertyMarkedUsingValueBinderMetadata), false)] - [InlineData(typeof(TypeWithAtLeastOnePropertyMarkedUsingValueBinderMetadata), true)] - [InlineData(typeof(TypeWithUnmarkedAndBinderMetadataMarkedProperties), false)] - [InlineData(typeof(TypeWithUnmarkedAndBinderMetadataMarkedProperties), true)] public void CanCreateModel_CreatesModelForValueProviderBasedBinderMetadatas_IfAValueProviderProvidesValue( Type modelType, bool valueProviderProvidesValue) @@ -182,6 +178,34 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders Assert.Equal(valueProviderProvidesValue, canCreate); } + [Theory] + [InlineData(typeof(TypeWithAtLeastOnePropertyMarkedUsingValueBinderMetadata), false)] + [InlineData(typeof(TypeWithAtLeastOnePropertyMarkedUsingValueBinderMetadata), true)] + [InlineData(typeof(TypeWithUnmarkedAndBinderMetadataMarkedProperties), false)] + [InlineData(typeof(TypeWithUnmarkedAndBinderMetadataMarkedProperties), true)] + public void CanCreateModel_CreatesModelForValueProviderBasedBinderMetadatas_IfPropertyHasGreedyBindingSource( + Type modelType, + bool valueProviderProvidesValue) + { + var valueProvider = new Mock(); + valueProvider + .Setup(o => o.ContainsPrefix(It.IsAny())) + .Returns(valueProviderProvidesValue); + + var bindingContext = CreateContext(GetMetadataForType(modelType)); + bindingContext.IsTopLevelObject = false; + bindingContext.ValueProvider = valueProvider.Object; + bindingContext.OriginalValueProvider = valueProvider.Object; + + var binder = CreateBinder(bindingContext.ModelMetadata); + + // Act + var canCreate = binder.CanCreateModel(bindingContext); + + // Assert + Assert.True(canCreate); + } + [Theory] [InlineData(typeof(TypeWithAtLeastOnePropertyMarkedUsingValueBinderMetadata), false)] [InlineData(typeof(TypeWithAtLeastOnePropertyMarkedUsingValueBinderMetadata), true)] @@ -214,17 +238,18 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders var canCreate = binder.CanCreateModel(bindingContext); // Assert - Assert.Equal(originalValueProviderProvidesValue, canCreate); + Assert.True(canCreate); } [Theory] - [InlineData(typeof(TypeWithUnmarkedAndBinderMetadataMarkedProperties), false)] - [InlineData(typeof(TypeWithUnmarkedAndBinderMetadataMarkedProperties), true)] - [InlineData(typeof(TypeWithNoBinderMetadata), false)] - [InlineData(typeof(TypeWithNoBinderMetadata), true)] + [InlineData(typeof(TypeWithUnmarkedAndBinderMetadataMarkedProperties), false, true)] + [InlineData(typeof(TypeWithUnmarkedAndBinderMetadataMarkedProperties), true, true)] + [InlineData(typeof(TypeWithNoBinderMetadata), false, false)] + [InlineData(typeof(TypeWithNoBinderMetadata), true, true)] public void CanCreateModel_UnmarkedProperties_UsesCurrentValueProvider( Type modelType, - bool valueProviderProvidesValue) + bool valueProviderProvidesValue, + bool expectedCanCreate) { var valueProvider = new Mock(); valueProvider @@ -247,7 +272,7 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders var canCreate = binder.CanCreateModel(bindingContext); // Assert - Assert.Equal(valueProviderProvidesValue, canCreate); + Assert.Equal(expectedCanCreate, canCreate); } [Fact] diff --git a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Binders/FormFileModelBinderTest.cs b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Binders/FormFileModelBinderTest.cs index d738cdc6ff..40f025b53d 100644 --- a/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Binders/FormFileModelBinderTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Core.Test/ModelBinding/Binders/FormFileModelBinderTest.cs @@ -20,8 +20,10 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders public async Task FormFileModelBinder_SingleFile_BindSuccessful() { // Arrange - var formFiles = new FormFileCollection(); - formFiles.Add(GetMockFormFile("file", "file1.txt")); + var formFiles = new FormFileCollection + { + GetMockFormFile("file", "file1.txt") + }; var httpContext = GetMockHttpContext(GetMockFormCollection(formFiles)); var bindingContext = GetBindingContext(typeof(IEnumerable), httpContext); var binder = new FormFileModelBinder(NullLoggerFactory.Instance); @@ -38,6 +40,192 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders Assert.Null(entry.Metadata); } + [Fact] + public async Task FormFileModelBinder_SingleFileAtTopLevel_BindSuccessfully_WithEmptyModelName() + { + // Arrange + var formFiles = new FormFileCollection + { + GetMockFormFile("file", "file1.txt") + }; + + var httpContext = GetMockHttpContext(GetMockFormCollection(formFiles)); + var binder = new FormFileModelBinder(NullLoggerFactory.Instance); + + // Mimic ParameterBinder overwriting ModelName on top level model. In this top-level binding case, + // FormFileModelBinder uses FieldName from the get-go. (OriginalModelName will be checked but ignored.) + var bindingContext = DefaultModelBindingContext.CreateBindingContext( + new ActionContext { HttpContext = httpContext }, + Mock.Of(), + new EmptyModelMetadataProvider().GetMetadataForType(typeof(IFormFile)), + bindingInfo: null, + modelName: "file"); + bindingContext.ModelName = string.Empty; + + // Act + await binder.BindModelAsync(bindingContext); + + // Assert + Assert.True(bindingContext.Result.IsModelSet); + + var entry = bindingContext.ValidationState[bindingContext.Result.Model]; + Assert.False(entry.SuppressValidation); + Assert.Equal("file", entry.Key); + Assert.Null(entry.Metadata); + } + + [Fact] + public async Task FormFileModelBinder_SingleFileWithinTopLevelPoco_BindSuccessfully() + { + // Arrange + const string propertyName = nameof(NestedFormFiles.Files); + var formFiles = new FormFileCollection + { + GetMockFormFile($"{propertyName}", "file1.txt") + }; + + var httpContext = GetMockHttpContext(GetMockFormCollection(formFiles)); + var binder = new FormFileModelBinder(NullLoggerFactory.Instance); + + // In this non-top-level binding case, FormFileModelBinder tries ModelName and succeeds. + var propertyInfo = typeof(NestedFormFiles).GetProperty(propertyName); + var metadata = new EmptyModelMetadataProvider().GetMetadataForProperty( + propertyInfo, + propertyInfo.PropertyType); + var bindingContext = DefaultModelBindingContext.CreateBindingContext( + new ActionContext { HttpContext = httpContext }, + Mock.Of(), + metadata, + bindingInfo: null, + modelName: "FileList"); + bindingContext.IsTopLevelObject = false; + bindingContext.Model = new FileList(); + bindingContext.ModelName = propertyName; + + // Act + await binder.BindModelAsync(bindingContext); + + // Assert + Assert.True(bindingContext.Result.IsModelSet); + + var entry = bindingContext.ValidationState[bindingContext.Result.Model]; + Assert.False(entry.SuppressValidation); + Assert.Equal($"{propertyName}", entry.Key); + Assert.Null(entry.Metadata); + } + + [Fact] + public async Task FormFileModelBinder_SingleFileWithinTopLevelPoco_BindSuccessfully_WithShortenedModelName() + { + // Arrange + const string propertyName = nameof(NestedFormFiles.Files); + var formFiles = new FormFileCollection + { + GetMockFormFile($"FileList.{propertyName}", "file1.txt") + }; + + var httpContext = GetMockHttpContext(GetMockFormCollection(formFiles)); + var binder = new FormFileModelBinder(NullLoggerFactory.Instance); + + // Mimic ParameterBinder overwriting ModelName on top level model then ComplexTypeModelBinder entering a + // nested context for the NestedFormFiles property. In this non-top-level binding case, FormFileModelBinder + // tries ModelName then falls back to add an (OriginalModelName + ".") prefix. + var propertyInfo = typeof(NestedFormFiles).GetProperty(propertyName); + var metadata = new EmptyModelMetadataProvider().GetMetadataForProperty( + propertyInfo, + propertyInfo.PropertyType); + var bindingContext = DefaultModelBindingContext.CreateBindingContext( + new ActionContext { HttpContext = httpContext }, + Mock.Of(), + metadata, + bindingInfo: null, + modelName: "FileList"); + bindingContext.IsTopLevelObject = false; + bindingContext.Model = new FileList(); + bindingContext.ModelName = propertyName; + + // Act + await binder.BindModelAsync(bindingContext); + + // Assert + Assert.True(bindingContext.Result.IsModelSet); + + var entry = bindingContext.ValidationState[bindingContext.Result.Model]; + Assert.False(entry.SuppressValidation); + Assert.Equal($"FileList.{propertyName}", entry.Key); + Assert.Null(entry.Metadata); + } + + [Fact] + public async Task FormFileModelBinder_SingleFileWithinTopLevelDictionary_BindSuccessfully() + { + // Arrange + var formFiles = new FormFileCollection + { + GetMockFormFile("[myFile]", "file1.txt") + }; + + var httpContext = GetMockHttpContext(GetMockFormCollection(formFiles)); + var binder = new FormFileModelBinder(NullLoggerFactory.Instance); + + // In this non-top-level binding case, FormFileModelBinder tries ModelName and succeeds. + var bindingContext = DefaultModelBindingContext.CreateBindingContext( + new ActionContext { HttpContext = httpContext }, + Mock.Of(), + new EmptyModelMetadataProvider().GetMetadataForType(typeof(IFormFile)), + bindingInfo: null, + modelName: "FileDictionary"); + bindingContext.IsTopLevelObject = false; + bindingContext.ModelName = "[myFile]"; + + // Act + await binder.BindModelAsync(bindingContext); + + // Assert + Assert.True(bindingContext.Result.IsModelSet); + + var entry = bindingContext.ValidationState[bindingContext.Result.Model]; + Assert.False(entry.SuppressValidation); + Assert.Equal("[myFile]", entry.Key); + Assert.Null(entry.Metadata); + } + + [Fact] + public async Task FormFileModelBinder_SingleFileWithinTopLevelDictionary_BindSuccessfully_WithShortenedModelName() + { + // Arrange + var formFiles = new FormFileCollection + { + GetMockFormFile("FileDictionary[myFile]", "file1.txt") + }; + + var httpContext = GetMockHttpContext(GetMockFormCollection(formFiles)); + var binder = new FormFileModelBinder(NullLoggerFactory.Instance); + + // Mimic ParameterBinder overwriting ModelName on top level model then DictionaryModelBinder entering a + // nested context for the KeyValuePair.Value property. In this non-top-level binding case, + // FormFileModelBinder tries ModelName then falls back to add an OriginalModelName prefix. + var bindingContext = DefaultModelBindingContext.CreateBindingContext( + new ActionContext { HttpContext = httpContext }, + Mock.Of(), + new EmptyModelMetadataProvider().GetMetadataForType(typeof(IFormFile)), + bindingInfo: null, + modelName: "FileDictionary"); + bindingContext.IsTopLevelObject = false; + bindingContext.ModelName = "[myFile]"; + + // Act + await binder.BindModelAsync(bindingContext); + + // Assert + Assert.True(bindingContext.Result.IsModelSet); + + var entry = bindingContext.ValidationState[bindingContext.Result.Model]; + Assert.False(entry.SuppressValidation); + Assert.Equal("FileDictionary[myFile]", entry.Key); + Assert.Null(entry.Metadata); + } + [Fact] public async Task FormFileModelBinder_ExpectMultipleFiles_BindSuccessful() { @@ -127,8 +315,10 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders public async Task FormFileModelBinder_ReturnsFailedResult_WhenNamesDoNotMatch() { // Arrange - var formFiles = new FormFileCollection(); - formFiles.Add(GetMockFormFile("different name", "file1.txt")); + var formFiles = new FormFileCollection + { + GetMockFormFile("different name", "file1.txt") + }; var httpContext = GetMockHttpContext(GetMockFormCollection(formFiles)); var bindingContext = GetBindingContext(typeof(IFormFile), httpContext); var binder = new FormFileModelBinder(NullLoggerFactory.Instance); @@ -147,9 +337,11 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders public async Task FormFileModelBinder_UsesFieldNameForTopLevelObject(bool isTopLevel, string expected) { // Arrange - var formFiles = new FormFileCollection(); - formFiles.Add(GetMockFormFile("FieldName", "file1.txt")); - formFiles.Add(GetMockFormFile("ModelName", "file1.txt")); + var formFiles = new FormFileCollection + { + GetMockFormFile("FieldName", "file1.txt"), + GetMockFormFile("ModelName", "file1.txt") + }; var httpContext = GetMockHttpContext(GetMockFormCollection(formFiles)); var bindingContext = GetBindingContext(typeof(IFormFile), httpContext); @@ -173,8 +365,10 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders public async Task FormFileModelBinder_ReturnsFailedResult_WithEmptyContentDisposition() { // Arrange - var formFiles = new FormFileCollection(); - formFiles.Add(new Mock().Object); + var formFiles = new FormFileCollection + { + new Mock().Object + }; var httpContext = GetMockHttpContext(GetMockFormCollection(formFiles)); var bindingContext = GetBindingContext(typeof(IFormFile), httpContext); var binder = new FormFileModelBinder(NullLoggerFactory.Instance); @@ -191,8 +385,10 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders public async Task FormFileModelBinder_ReturnsFailedResult_WithNoFileNameAndZeroLength() { // Arrange - var formFiles = new FormFileCollection(); - formFiles.Add(GetMockFormFile("file", "")); + var formFiles = new FormFileCollection + { + GetMockFormFile("file", "") + }; var httpContext = GetMockHttpContext(GetMockFormCollection(formFiles)); var bindingContext = GetBindingContext(typeof(IFormFile), httpContext); var binder = new FormFileModelBinder(NullLoggerFactory.Instance); @@ -323,5 +519,10 @@ namespace Microsoft.AspNetCore.Mvc.ModelBinding.Binders private class FileList : List { } + + private class NestedFormFiles + { + public FileList Files { get; } + } } } diff --git a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RazorPagesWithBasePathTest.cs b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RazorPagesWithBasePathTest.cs index 4518e7b442..3de7a91ebe 100644 --- a/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RazorPagesWithBasePathTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.FunctionalTests/RazorPagesWithBasePathTest.cs @@ -4,6 +4,7 @@ using System.Collections.Generic; using System.Net; using System.Net.Http; +using System.Net.Http.Headers; using System.Threading.Tasks; using Xunit; @@ -623,6 +624,41 @@ Hello from /Pages/Shared/"; Assert.Equal("Value from Page", valueSetInPage); } + [Fact] + public async Task RoundTrippingFormFileInputWorks() + { + // Arrange + var url = "/PropertyBinding/BindFormFile"; + var response = await Client.GetAsync(url); + await response.AssertStatusCodeAsync(HttpStatusCode.OK); + + var document = await response.GetHtmlDocumentAsync(); + + var property1 = document.RequiredQuerySelector("#property1").GetAttribute("name"); + var file1 = document.RequiredQuerySelector("#file1").GetAttribute("name"); + var file2 = document.RequiredQuerySelector("#file2").GetAttribute("name"); + var file3 = document.RequiredQuerySelector("#file3").GetAttribute("name"); + var antiforgeryToken = document.RetrieveAntiforgeryToken(); + + var cookie = AntiforgeryTestHelper.RetrieveAntiforgeryCookie(response); + + var content = new MultipartFormDataContent(); + content.Add(new StringContent("property1-value"), property1); + content.Add(new StringContent("test-value1"), file1, "test1.txt"); + content.Add(new StringContent("test-value2"), file3, "test2.txt"); + + var request = new HttpRequestMessage(HttpMethod.Post, url) + { + Content = content, + }; + request.Headers.Add("Cookie", cookie.Key + "=" + cookie.Value); + request.Headers.Add("RequestVerificationToken", antiforgeryToken); + + response = await Client.SendAsync(request); + + await response.AssertStatusCodeAsync(HttpStatusCode.OK); + } + private async Task AddAntiforgeryHeadersAsync(HttpRequestMessage request) { var response = await Client.GetAsync(request.RequestUri); diff --git a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ComplexTypeModelBinderIntegrationTest.cs b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ComplexTypeModelBinderIntegrationTest.cs index 9eb3af091e..8f4fc6470f 100644 --- a/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ComplexTypeModelBinderIntegrationTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.IntegrationTests/ComplexTypeModelBinderIntegrationTest.cs @@ -16,7 +16,7 @@ using Xunit; namespace Microsoft.AspNetCore.Mvc.IntegrationTests { - // Integration tests targeting the behavior of the MutableObjectModelBinder and related classes + // Integration tests targeting the behavior of the ComplexTypeModelBinder and related classes // with other model binders. public class ComplexTypeModelBinderIntegrationTest { @@ -197,10 +197,8 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests Assert.Equal("bill", entry.RawValue); } - // We don't provide enough data in this test for the 'Person' model to be created. So even though there is - // body data in the request, it won't be used. [Fact] - public async Task MutableObjectModelBinder_BindsNestedPOCO_WithBodyModelBinder_WithPrefix_PartialData() + public async Task ComplexTypeModelBinder_BindsNestedPOCO_WithBodyModelBinder_WithPrefix_PartialData() { // Arrange var parameter = new ParameterDescriptor() @@ -235,22 +233,21 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests Assert.True(modelBindingResult.IsModelSet); var model = Assert.IsType(modelBindingResult.Model); - Assert.Null(model.Customer); + Assert.NotNull(model.Customer); + Assert.Equal("1 Microsoft Way", model.Customer.Address.Street); + Assert.Equal(10, model.ProductId); - Assert.Single(modelState); Assert.Equal(0, modelState.ErrorCount); Assert.True(modelState.IsValid); - var entry = Assert.Single(modelState, e => e.Key == "parameter.ProductId").Value; + var entry = Assert.Single(modelState).Value; Assert.Equal("10", entry.AttemptedValue); Assert.Equal("10", entry.RawValue); } - // We don't provide enough data in this test for the 'Person' model to be created. So even though there is - // body data in the request, it won't be used. [Fact] - public async Task MutableObjectModelBinder_BindsNestedPOCO_WithBodyModelBinder_WithPrefix_NoData() + public async Task ComplexTypeModelBinder_BindsNestedPOCO_WithBodyModelBinder_WithPrefix_NoData() { // Arrange var parameter = new ParameterDescriptor() @@ -285,7 +282,8 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests Assert.True(modelBindingResult.IsModelSet); var model = Assert.IsType(modelBindingResult.Model); - Assert.Null(model.Customer); + Assert.NotNull(model.Customer); + Assert.Equal("1 Microsoft Way", model.Customer.Address.Street); Assert.Empty(modelState); Assert.Equal(0, modelState.ErrorCount); @@ -630,10 +628,8 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests Assert.Equal("bill", entry.RawValue); } - // We don't provide enough data in this test for the 'Person' model to be created. So even though there are - // form files in the request, it won't be used. [Fact] - public async Task MutableObjectModelBinder_BindsNestedPOCO_WithFormFileModelBinder_WithPrefix_PartialData() + public async Task ComplexTypeModelBinder_BindsNestedPOCO_WithFormFileModelBinder_WithPrefix_PartialData() { // Arrange var parameter = new ParameterDescriptor() @@ -668,22 +664,29 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests Assert.True(modelBindingResult.IsModelSet); var model = Assert.IsType(modelBindingResult.Model); - Assert.Null(model.Customer); + Assert.NotNull(model.Customer); + + var document = Assert.Single(model.Customer.Documents); + Assert.Equal("text.txt", document.FileName); + using (var reader = new StreamReader(document.OpenReadStream())) + { + Assert.Equal("Hello, World!", await reader.ReadToEndAsync()); + } + Assert.Equal(10, model.ProductId); - Assert.Single(modelState); + Assert.Equal(2, modelState.Count); Assert.Equal(0, modelState.ErrorCount); Assert.True(modelState.IsValid); + Assert.Single(modelState, e => e.Key == "parameter.Customer.Documents"); var entry = Assert.Single(modelState, e => e.Key == "parameter.ProductId").Value; Assert.Equal("10", entry.AttemptedValue); Assert.Equal("10", entry.RawValue); } - // We don't provide enough data in this test for the 'Person' model to be created. So even though there is - // body data in the request, it won't be used. [Fact] - public async Task MutableObjectModelBinder_BindsNestedPOCO_WithFormFileModelBinder_WithPrefix_NoData() + public async Task ComplexTypeModelBinder_BindsNestedPOCO_WithFormFileModelBinder_WithPrefix_NoData() { // Arrange var parameter = new ParameterDescriptor() @@ -696,7 +699,7 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests var testContext = ModelBindingTestHelper.GetTestContext(request => { request.QueryString = new QueryString("?"); - SetFormFileBodyContent(request, "Hello, World!", "parameter.Customer.Documents"); + SetFormFileBodyContent(request, "Hello, World!", "Customer.Documents"); }); var modelState = testContext.ModelState; @@ -718,11 +721,20 @@ namespace Microsoft.AspNetCore.Mvc.IntegrationTests Assert.True(modelBindingResult.IsModelSet); var model = Assert.IsType(modelBindingResult.Model); - Assert.Null(model.Customer); + Assert.NotNull(model.Customer); + + var document = Assert.Single(model.Customer.Documents); + Assert.Equal("text.txt", document.FileName); + using (var reader = new StreamReader(document.OpenReadStream())) + { + Assert.Equal("Hello, World!", await reader.ReadToEndAsync()); + } - Assert.Empty(modelState); Assert.Equal(0, modelState.ErrorCount); Assert.True(modelState.IsValid); + + var entry = Assert.Single(modelState); + Assert.Equal("Customer.Documents", entry.Key); } private class Order5 diff --git a/test/WebSites/RazorPagesWebSite/Pages/PropertyBinding/BindFormFile.cshtml b/test/WebSites/RazorPagesWebSite/Pages/PropertyBinding/BindFormFile.cshtml new file mode 100644 index 0000000000..f388f549db --- /dev/null +++ b/test/WebSites/RazorPagesWebSite/Pages/PropertyBinding/BindFormFile.cshtml @@ -0,0 +1,10 @@ +@page +@model BindFormFile +@addTagHelper "*, Microsoft.AspNetCore.Mvc.TagHelpers" + +
+ + + + +
diff --git a/test/WebSites/RazorPagesWebSite/Pages/PropertyBinding/BindFormFile.cshtml.cs b/test/WebSites/RazorPagesWebSite/Pages/PropertyBinding/BindFormFile.cshtml.cs new file mode 100644 index 0000000000..ccfcc87440 --- /dev/null +++ b/test/WebSites/RazorPagesWebSite/Pages/PropertyBinding/BindFormFile.cshtml.cs @@ -0,0 +1,52 @@ +// 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 Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.RazorPages; + +namespace RazorPagesWebSite +{ + [BindProperties] + public class BindFormFile : PageModel + { + public string Property1 { get; set; } + + public IFormFile Form3 { get; set; } + + public FormFiles Forms { get; set; } + + public IActionResult OnPost() + { + if (string.IsNullOrEmpty(Property1)) + { + throw new Exception($"{nameof(Property1)} is not bound."); + } + + if (string.IsNullOrEmpty(Form3.Name) || Form3.Length == 0) + { + throw new Exception($"{nameof(Form3)} is not bound."); + } + + if (string.IsNullOrEmpty(Forms.Form1.Name) || Forms.Form1.Length == 0) + { + throw new Exception($"{nameof(Forms.Form1)} is not bound."); + } + + if (Forms.Form2 != null) + { + throw new Exception($"{nameof(Forms.Form2)} is bound."); + } + + return new OkResult(); + } + } + + public class FormFiles + { + public IFormFile Form1 { get; set; } + + public IFormFile Form2 { get; set; } + } +}