Json MVC model binding should have an option to respect non-nullable reference types when they are used

[rathga]

Summary

When deserializing Json to DTOs in MVC model binding, for non-nullable value type members null Json values are rejected, a nice error message is added to ModelState and a 400 error returned. However, for non-nullable reference types null values are let through which can then trip ArgumentNullExceptions in object constructors or property setters leading to a more cryptic 500 error rather than producing a 400 and a friendlier ModelState error.

An option could be added to (probably) JsonSerializerOptions so that it respects non-nullable reference types and throws a JsonException when encountering a null value, rather than binding the null value.

Motivation and goals

• Current work around is to always use nullable reference types for all reference type members in DTOs, and defer null checks until after the constructor in validation or business logic. The DTO is unable to defend its not-null invariants itself, leading to pollution of the codebase with null checks elsewhere. Effectively this makes using nullable reference types not usable/useful for controller action DTOs.

• An option could be added to (probably) JsonSerializerOptions so that it respects non-nullable reference types and throws a JsonException when encountering a null value, rather than binding the null value.

e.g. in Startup.cs

services.AddControllersWithViews()
    .AddJsonOptions(cfg => 
        cfg.JsonSerializerOptions.RespectNonNullableReferenceTypes = true
        );

• This may be more aptly posted in the runtime repo as presumably the changes need to be in System.Text.Json, but as it is impacting me in my controllers I thought it would make sense to get feedback here first.

Examples

This small console app demonstrates the issue:

namespace JsonNullable
{
    static class Program
    {
        public record NullableInt(int? someNumber);
        public record NonNullableInt(int someNumber);

        public record NullableString(string? SomeString);
        public record NonNullableString
        {
            public string SomeString { get; }
            public NonNullableString(string someString) =>
                SomeString = someString ?? throw new ArgumentNullException(nameof(someString));
        }

        static void Main(string[] args)
        {
            var nullableInt = new NullableInt(null);
            var jsonInt = JsonSerializer.Serialize(nullableInt);
            var nonNullableInt = JsonSerializer.Deserialize<NonNullableInt>(jsonInt); 
            // ^^ throws Cannot get the value of a token type 'Null' as a number.
            // Gets handled gracefully by MVC model binding

            var nullableString = new NullableString(null);
            var jsonString = JsonSerializer.Serialize(nullableString); 
            var nonNullableString = JsonSerializer.Deserialize<NonNullableString>(jsonString);
            // ^^ throws ArgumentNullException from constructor
            // Causes 500 exception in MVC, forcing the null check to be removed from the DTO and performed after binding
        }
    }
}

[mkArtakMSFT]

@ericstj FYI

[layomia]

Duplicate of #1256.