Unexplained custom model binding / validation behaviour

[stevendarby]

Hello, we've seen an unusual thing happen in production which we've never seen before, and I would appreciate any help trying to work it out.

We have some code which inserts a model binder to essentially allow async validation, the results of which are checked later, at the 'true' validation stage. I have tried to create a minimal reproduction of all the key bits - see below.

Given this code, I would not expect ValidationInfo.Results to ever throw the InvalidOperationException when calling the Test controller/action, when it reaches the attribute IsValid method. We have seen this happen once with the production code, among hundreds of thousands of other successful calls at the same endpoint.

I'm wondering if anyone can spot a bug in the code below - perhaps there is something around model binding / validation that is not as deterministic as believed - model binders in a different order, things processed in a different order etc.? Could an error during model binding be swallowed and it continue to the validation stage without setting ValidationInfo.Results?

Sorry if this lacking more detail, we don't have a lot to go on ourselves. We're using .NET 6.

Code

using Microsoft.AspNetCore.Mvc;
using Microsoft.AspNetCore.Mvc.ModelBinding;
using Microsoft.AspNetCore.Mvc.ModelBinding.Binders;
using System.ComponentModel.DataAnnotations;
using System.Reflection;

var builder = WebApplication.CreateBuilder(args);

builder.Services
    .AddScoped<ValidationInfo>()
    .AddControllers(o =>
    {
        var defaultModelBinder = o.ModelBinderProviders.OfType<BodyModelBinderProvider>().FirstOrDefault();
        if (defaultModelBinder == null)
        {
            return;
        }

        o.ModelBinderProviders.Insert(
            o.ModelBinderProviders.IndexOf(defaultModelBinder),
            new CustomModelBinderProvider(defaultModelBinder));
    });


var app = builder.Build();

app.UseHttpsRedirection();
app.UseAuthorization();
app.MapControllers();

app.Run();

public class CustomModelBinderProvider : IModelBinderProvider
{
    private readonly IModelBinderProvider _underlyingModelBinderProvider;

    public CustomModelBinderProvider(IModelBinderProvider underlyingModelBinderProvider)
        => _underlyingModelBinderProvider = underlyingModelBinderProvider;

    public IModelBinder? GetBinder(ModelBinderProviderContext context)
    {
        var isDtoBase = typeof(DtoBase).IsAssignableFrom(context.Metadata.ModelType);

        return isDtoBase
            ? new CustomModelBinder(_underlyingModelBinderProvider.GetBinder(context)!)
            : null;
    }
}

public class CustomModelBinder : IModelBinder
{
    private readonly IModelBinder _underlyingModelBinder;

    public CustomModelBinder(IModelBinder underlyingModelBinder)
        => _underlyingModelBinder = underlyingModelBinder;

    public async Task BindModelAsync(ModelBindingContext bindingContext)
    {
        await _underlyingModelBinder.BindModelAsync(bindingContext);

        if (bindingContext.Result.Model is DtoBase model)
        {
            var validationInfo = bindingContext.HttpContext.RequestServices.GetRequiredService<ValidationInfo>();

            validationInfo.Results = await Task.WhenAll(GetValidValues(model));
        }
    }

    private static IEnumerable<Task<(string Type, ValidationInfoResult Result)>> GetValidValues(DtoBase model)
        => model.GetType()
            .GetProperties()
            .Select(p => new { Property = p, p.GetCustomAttribute<LookupAttribute>()?.Type })
            .Where(x => x.Type != null)
            .Select(x => new { x.Type, Value = x.Property.GetValue(model)?.ToString() })
            .Where(x => x.Value != null)
            .Select(async x => (x.Type!, Result: await ValidateAsync(x.Type!, x.Value!)));

    private static async Task<ValidationInfoResult> ValidateAsync(string type, string value)
    {
        await Task.Delay(Random.Shared.Next(10, 100)); // request to another service
        return new ValidationInfoResult { Value = value, IsValid = Random.Shared.Next(0, 2) == 1 };
    }
}

public class LookupAttribute : ValidationAttribute
{
    public string Type { get; }
    public override bool RequiresValidationContext => true;

    public LookupAttribute(string type)
        => Type = type;

    protected override ValidationResult? IsValid(object? value, ValidationContext validationContext)
    {
        var validationInfo = validationContext.GetRequiredService<ValidationInfo>();

        return value is null || validationInfo.Results.Any(x => x.Type == Type && x.Result.Value == value.ToString() && x.Result.IsValid)
            ? ValidationResult.Success
            : new ValidationResult("Invalid value");
    }
}

public class ValidationInfoResult
{
    public string Value { get; init; }
    public bool IsValid { get; init; }
}

public class ValidationInfo
{
    private (string Type, ValidationInfoResult Result)[]? _results;

    public (string Type, ValidationInfoResult Result)[] Results
    {
        get => _results ?? throw new InvalidOperationException("Results has not been set!");
        set => _results = value;
    }
}

public class DtoBase
{
}

public class TestDto : DtoBase
{
    [Lookup("Title")] public string? Title { get; set; }
    [Lookup("Country")] public string? Country { get; set; }
    [Lookup("Status")] public string? Status { get; set; }
}

[ApiController]
[Route("[controller]")]
public class TestController : ControllerBase
{
    [HttpPut]
    public TestDto Test(TestDto dto)
        => dto;
}

Send in anything with values like below. You should get a mix of 200 and 400 based on the random validation but never a 500.

{
    "title": "Ms",
    "country": "UK",
    "status": "OK"
}

[captainsafia]

Send in anything with values like below. You should get a mix of 200 and 400 based on the random validation but never a 500.

It seems that you're saying that you are getting 500s with the deployed application in production but not in the repro?

Hi @stevendarby. We have added the "Needs: Author Feedback" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

[stevendarby]

@captainsafia We have once seen a 500 as a result of the invalid operation exception thrown when Results is null. An integration test failed and we found this in application insights logs. We've never seen it before, nor since, and these tests run regularly.

I can't reproduce it and I don't know how to, because my reading of our code and my understanding of how ASP.NET Core works would lead me to believe it's not possible. I can't see how it gets to the attribute validation stage without having already set the Results during model binding.

However, it must be possible, as it happened! So I've tried to reproduce the key parts of the code, to share here, in the hope that someone with more expertise might be able to point out some possible scenarios it could occur.

[captainsafia]

@stevendarby Thanks for clarifying! Unfortunately, I'll have to stick this in the backlog since we don't have much capacity to investigate issues like this at the moment. You may also want to consider posting a question on StackOverflow to see if you can crowdsource some ideas from the community.

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.