Skip to content

Fix recursive type model binding issue in Blazor SSR forms #63163

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ public void Map(FormValueMappingContext context)
if (!CanMap(context.ValueType, context.AcceptMappingScopeName, context.AcceptFormName))
{
context.SetResult(null);
return;
}

var deserializer = _cache.GetOrAdd(context.ValueType, CreateDeserializer);
Expand Down
45 changes: 45 additions & 0 deletions src/Components/Endpoints/test/Binding/FormDataMapperTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
using Microsoft.AspNetCore.Components.Forms;
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.Primitives;
using Microsoft.Extensions.Logging.Abstractions;

namespace Microsoft.AspNetCore.Components.Endpoints.FormMapping;

Expand Down Expand Up @@ -1464,6 +1465,44 @@ public void CanDeserialize_ComplexRecursiveTypes_ThrowsWhenMaxRecursionDepthExce
});
}

[Fact]
public void CanDeserialize_SimpleRecursiveModel_WithOnlyNameProperty()
{
// This reproduces the issue from GitHub #61341
// A model with a recursive property that has no form data provided
var data = new Dictionary<string, StringValues>()
{
["Name"] = "Test Name"
// Note: No data for Parent property
};

var reader = CreateFormDataReader(data, CultureInfo.InvariantCulture);
var options = new FormDataMapperOptions();

// Act
var result = FormDataMapper.Map<MyModel>(reader, options);

// Assert
Assert.NotNull(result);
Assert.Equal("Test Name", result.Name);
Assert.Null(result.Parent);
}

[Fact]
public void ComplexTypeConverterFactory_CanConvert_RecursiveType()
{
// Test if ComplexTypeConverterFactory can convert recursive types
// This tests the exact scenario from GitHub issue #61341
var options = new FormDataMapperOptions();
var factory = new ComplexTypeConverterFactory(options, new NullLoggerFactory());

// Act
var canConvert = factory.CanConvert(typeof(MyModel), options);

// Assert
Assert.True(canConvert, "ComplexTypeConverterFactory should be able to convert recursive types");
}

[Fact]
public void CanDeserialize_ComplexRecursiveCollectionTypes_RecursiveTree()
{
Expand Down Expand Up @@ -2449,6 +2488,12 @@ internal class RecursiveDictionaryTree

internal record ClassRecordType(string Key, int Value);

internal class MyModel
{
public string Name { get; set; }
public MyModel Parent { get; set; }
}

internal record struct StructRecordType(string Key, int Value);

internal class DataMemberAttributesType
Expand Down
115 changes: 115 additions & 0 deletions src/Components/Endpoints/test/HttpContextFormValueMapperTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Options;
using Microsoft.Extensions.Primitives;
using Microsoft.AspNetCore.Components.Forms.Mapping;

namespace Microsoft.AspNetCore.Components.Endpoints;

Expand Down Expand Up @@ -42,4 +43,118 @@ public void CanMap_MatchesOnScopeAndFormName(bool expectedResult, string incomin
var canMap = mapper.CanMap(typeof(string), scopeName, formNameOrNull);
Assert.Equal(expectedResult, canMap);
}

[Fact]
public void CanMap_SimpleRecursiveModel_ReturnsTrue()
{
// Test that CanMap works correctly for recursive types (GitHub issue #61341)
var formData = new HttpContextFormDataProvider();
formData.SetFormData("test-form", new Dictionary<string, StringValues>
{
["Name"] = "Test Name"
}, new FormFileCollection());

var mapper = new HttpContextFormValueMapper(formData, Options.Create<RazorComponentsServiceOptions>(new()));

var canMap = mapper.CanMap(typeof(MyModel), "", null);
Assert.True(canMap);
}

[Fact]
public void Map_SetsNullResult_WhenCanMapReturnsFalse()
{
// This test verifies the fix for GitHub issue #61341
// The Map method should return early when CanMap returns false
var formData = new HttpContextFormDataProvider();
// Don't set any form data so CanMap will return false due to no incoming handler name

var mapper = new HttpContextFormValueMapper(formData, Options.Create<RazorComponentsServiceOptions>(new()));
var context = new FormValueMappingContext("", null, typeof(MyModel), "Model");

// Act
mapper.Map(context);

// Assert
Assert.Null(context.Result);
}

[Fact]
public void Map_DoesNotDeserializeWhenCanMapReturnsFalse()
{
// This test demonstrates the original bug and verifies the fix
// Before the fix, Map would call deserializer even when CanMap returned false
var formData = new HttpContextFormDataProvider();
// Set form data but no incoming handler name, so CanMap returns false
formData.SetFormData("", new Dictionary<string, StringValues>
{
["Name"] = "Test"
}, new FormFileCollection());

var mapper = new HttpContextFormValueMapper(formData, Options.Create<RazorComponentsServiceOptions>(new()));
var context = new FormValueMappingContext("mismatched-scope", null, typeof(MyModel), "Model");

// Act
mapper.Map(context);

// Assert - The result should be null because CanMap returns false
// Before the fix, this might have been a MyModel instance due to deserializer running
Assert.Null(context.Result);
}

[Fact]
public void SupplyParameterFromForm_WithRecursiveModel_ExactGitHubIssueScenario()
{
// Arrange - Reproduce the EXACT scenario from GitHub issue #61341
var formData = new Dictionary<string, StringValues>()
{
["Name"] = "Test Name"
// Note: No data for Parent property, but it should still bind the Name
};

var httpContext = new DefaultHttpContext();
httpContext.Request.Method = "POST";
httpContext.Request.ContentType = "application/x-www-form-urlencoded";

// Set up form data provider to simulate the exact scenario
var formDataProvider = new HttpContextFormDataProvider();
formDataProvider.SetFormData("RequestForm", formData, new FormFileCollection());

// Create the mapper (this will use our fix)
var options = Options.Create(new RazorComponentsServiceOptions());
var mapper = new HttpContextFormValueMapper(formDataProvider, options);

// Act - Try to map the recursive model type (the exact case that was failing)
var context = new FormValueMappingContext("", "RequestForm", typeof(MyModel), "Model");
mapper.Map(context);

// Assert - The model should be created successfully, not null
// Before the fix, this would be null due to the missing return statement
Assert.NotNull(context.Result);
var result = Assert.IsType<MyModel>(context.Result);
Assert.Equal("Test Name", result.Name);
Assert.Null(result.Parent); // Parent should be null since no data was provided
}

[Fact]
public void CanMap_WithRecursiveModel_ShouldReturnTrue()
{
// Arrange - Test the CanMap method specifically for recursive types
var formDataProvider = new HttpContextFormDataProvider();
formDataProvider.SetFormData("RequestForm", new Dictionary<string, StringValues>(), new FormFileCollection());

var options = Options.Create(new RazorComponentsServiceOptions());
var mapper = new HttpContextFormValueMapper(formDataProvider, options);

// Act
var canMap = mapper.CanMap(typeof(MyModel), "", "RequestForm");

// Assert - Should be able to map recursive model types
Assert.True(canMap, "Should be able to map recursive model types");
}
}

internal class MyModel
{
public string Name { get; set; }
public MyModel Parent { get; set; }
}
124 changes: 124 additions & 0 deletions src/Components/Web/test/Forms/Mapping/SupplyParameterFromFormTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using Microsoft.AspNetCore.Components.Forms.Mapping;
using Microsoft.AspNetCore.Components.Test.Helpers;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.AspNetCore.Components.Endpoints.FormMapping;

namespace Microsoft.AspNetCore.Components.Forms.PostHandling;

Expand Down Expand Up @@ -58,6 +59,87 @@ public async Task FindCascadingParameters_HandlesSupplyParameterFromFormValues_W
Assert.Equal(formMappingScope, supplier.ValueSupplier);
}

[Fact]
public async Task FindCascadingParameters_HandlesRecursiveModelTypes()
{
// This test reproduces GitHub issue #61341
// Arrange
var renderer = CreateRendererWithFormValueModelBinder();
var formComponent = new RecursiveFormParametersComponent();

// Act
var componentId = renderer.AssignRootComponentId(formComponent);
await renderer.RenderRootComponentAsync(componentId);
var formComponentState = renderer.GetComponentState(formComponent);

var result = CascadingParameterState.FindCascadingParameters(formComponentState, out _);

// Assert
var supplier = Assert.Single(result);
Assert.IsType<SupplyParameterFromFormValueProvider>(supplier.ValueSupplier);
}

[Fact]
public async Task SupplyParameterFromForm_WithRecursiveType_ShouldBindCorrectly()
{
// Arrange - Test if recursive types can be bound correctly
// This reproduces the scenario from GitHub #61341
var renderer = CreateRendererWithRealFormBinding();
var formComponent = new RecursiveFormParametersComponent();

// Act
var componentId = renderer.AssignRootComponentId(formComponent);
await renderer.RenderRootComponentAsync(componentId);

// Assert
var parameters = CascadingParameterState.FindCascadingParameters(
renderer.GetComponentState(formComponent), out _);

var supplier = Assert.Single(parameters);
Assert.IsType<SupplyParameterFromFormValueProvider>(supplier.ValueSupplier);

// The key test: verify that the recursive type can be resolved for binding
var valueMapper = new TestFormValueMapperWithRealBinding();
var canMap = valueMapper.CanMap(typeof(MyModel), "", null);

Assert.True(canMap, "Should be able to map recursive types");
}

[Fact]
public async Task SupplyParameterFromForm_WithNestedRecursiveProperties_ShouldBindCorrectly()
{
// Test more complex scenarios with actual nested data
var renderer = CreateRendererWithRealFormBinding();
var formComponent = new RecursiveFormParametersComponent();

// Act
var componentId = renderer.AssignRootComponentId(formComponent);
await renderer.RenderRootComponentAsync(componentId);

// Assert
var parameters = CascadingParameterState.FindCascadingParameters(
renderer.GetComponentState(formComponent), out _);

var supplier = Assert.Single(parameters);
Assert.IsType<SupplyParameterFromFormValueProvider>(supplier.ValueSupplier);

// Test that nested recursive properties can be handled
var valueMapper = new TestFormValueMapperWithRealBinding();
var canMapRoot = valueMapper.CanMap(typeof(MyModel), "", null);

Assert.True(canMapRoot, "Should be able to map recursive types with nested properties");
}

static TestRenderer CreateRendererWithRealFormBinding()
{
var services = new ServiceCollection();
var valueBinder = new TestFormValueMapperWithRealBinding();
services.AddSingleton<IFormValueMapper>(valueBinder);
services.AddSingleton<ICascadingValueSupplier>(_ => new SupplyParameterFromFormValueProvider(
valueBinder, mappingScopeName: ""));
return new TestRenderer(services.BuildServiceProvider());
}

static TestRenderer CreateRendererWithFormValueModelBinder()
{
var services = new ServiceCollection();
Expand All @@ -78,6 +160,17 @@ class FormParametersComponentWithName : TestComponentBase
[SupplyParameterFromForm(FormName = "handler-name")] public string FormParameter { get; set; }
}

class RecursiveFormParametersComponent : TestComponentBase
{
[SupplyParameterFromForm] public MyModel FormParameter { get; set; }
}

class MyModel
{
public string Name { get; set; }
public MyModel Parent { get; set; }
}

class TestFormModelValueBinder(string IncomingScopeQualifiedFormName = "") : IFormValueMapper
{
public void Map(FormValueMappingContext context) { }
Expand All @@ -95,6 +188,37 @@ public bool CanMap(Type valueType, string mappingScopeName, string formName)
}
}

class TestFormValueMapperWithRealBinding : IFormValueMapper
{
public void Map(FormValueMappingContext context)
{
// Simple test implementation - just create an instance if possible
try
{
if (context.ValueType == typeof(MyModel))
{
context.SetResult(new MyModel());
}
else if (context.ValueType.IsClass && context.ValueType.GetConstructor(Type.EmptyTypes) != null)
{
context.SetResult(Activator.CreateInstance(context.ValueType));
}
}
catch
{
// If there's an exception creating the instance, don't set result
}
}

public bool CanMap(Type valueType, string mappingScopeName, string formName)
{
// Test if we can handle recursive types
// For this test, we accept MyModel and other simple types
return valueType == typeof(MyModel) ||
(valueType.IsClass && valueType.GetConstructor(Type.EmptyTypes) != null);
}
}

class TestComponentBase : IComponent
{
public void Attach(RenderHandle renderHandle)
Expand Down
Loading