Add an AppContext switch that disables using reflection by default in JsonSerializerOptions

[eiriktsarpalis]

Background and motivation

Since the release of the JsonSerializerOptions type in .NET Core 3, the default behavior of unconfigured options instances created with the default constructor has been to use reflection-based contract resolution. With the advent of new features such as the source generator and contract customization that go beyond reflection, this default behavior has forced a number of interesting design decisions on the type itself and other STJ components: for example, it has made the semantics of configuring the TypeInfoResolver property particularly difficult to work with.

We've received a lot of feedback about the issue from the aspnetcore team, who have been trying to make minimal APIs work in NativeAOT in a way that is consistent with CoreCLR. They ultimately addressed the issue on the aspnet layer using a special EnsureJsonTrimmability feature flag that was shipped in Preview 1. When turned on, this flag disables reflection-by-default and ensures that only user-provided configuration is being used. When left turned off, it offers the predictable and backward-compatible experience that works well in CoreCLR apps. What's more, the feature flag is recognized by the linker, ensuring that reflection components don't get rooted when turned on.

There's consensus between the JSON crew and the aspnet team that this flag should be moved to the System.Text.Json layer. This issue proposes the creation of a System.Text.Json.JsonSerializer.UseReflectionDefault feature flag that offers similar semantics to the current aspnetcore implementation.

API Proposal

As mentioned above, we want to introduce a System.Text.Json.JsonSerializer.IsReflectionEnabledByDefault feature flag. Note that this diverges from the namespace/naming convention used in the System.Text.Json.Serialization.EnableSourceGenReflectionFallback compatibility switch that was introduced in .NET 7.

Moreover, we would require the following changes to API:

namespace System.Text.Json;

public partial static class JsonSerializer
{
    // maps to the feature switch, and is registered in ILLink.Substitutions.xml
    public static bool IsReflectionEnabledByDefault { get; } // true if no feature switch is set.
}

Related to #74492.

API Usage

if (JsonSerializer.IsReflectionEnabledByDefault)
{
     // branch trimmed by the linker if the 
     // feature switch is set to false at link time.
     JsonSerializer.Serialize(value);
}

cc @eerhardt @davidfowl @halter73

Tagging subscribers to this area: @dotnet/area-system-text-json, @gregsdennis
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Background and motivation

Since the release of the JsonSerializerOptions type in .NET Core 3, the default behavior of unconfigured options instances created with the default constructor has been to use reflection-based contract resolution. With the advent of new features such as the source generator and contract customization that go beyond reflection, this default behavior has forced a number of interesting design decisions on the type itself and other STJ components: for example, it has made the semantics of configuring the TypeInfoResolver property particularly difficult to work with.

We've received a lot of feedback about the issue from the aspnetcore team, who have been trying to make minimal APIs work in NativeAOT in a way that is consistent with CoreCLR. They ultimately addressed the issue on the aspnet layer using a special EnsureJsonTrimmability feature flag that was shipped in Preview 1. When turned on, this flag disables reflection-by-default and ensures that only user-provided configuration is being used. When left turned off, it offers the predictable and backward-compatible experience that works well in CoreCLR apps. What's more, the feature flag is recognized by the linker, ensuring that reflection components don't get rooted when turned on.

There's consensus between the JSON crew and the aspnet team that this flag should be moved to the System.Text.Json layer. This issue proposes the creation of a System.Text.Json.Serialization.DisableDefaultReflection feature flag that offers similar semantics to the current aspnetcore implementation.

API Proposal

As mentioned above, we want to introduce a System.Text.Json.Serialization.DisableDefaultReflection feature flag. This follows the naming convention of the System.Text.Json.Serialization.EnableSourceGenReflectionFallback compatibility switch that was introduced in .NET 7.

Moreover, we would require the following changes to API:

namespace System.Text.Json;

public partial class JsonSerializerOptions
{
+   [RequiresUnreferencedCode, RequiresDynamicCode]
    public JsonSerializerOptions(); // default constructor now populates the resolver eagerly by default

+   public JsonSerializerOptions(IJsonTypeInfoResolver typeInfoResolver); // new constructor offering linker-safe workaround

    public JsonSerializerOptions(JsonSerializerOptions options); // copy constructor remains linker safe

-   [RequiresUnreferencedCode, RequiresDynamicCode]
    public JsonConverter GetConverter(Type type); // Annotation no longer needed since resolver is not populated lazily
}

API Usage

Current behavior (all configurations)

JsonSerializerOptions options = new();

Assert.Null(options.TypeInfoResolver);
JsonSerializer.Serializer(42, options); // success
Assert.IsType<DefaultJsonTypeInfoResolver>(options.TypeInfoResolver); // default resolver populated lazily

New behavior (DisableDefaultReflection feature flag not enabled)

JsonSerializerOptions options = new();

Assert.IsType<DefaultJsonTypeInfoResolver>(options.TypeInfoResolver); // default resolver populated eagerly
JsonSerializer.Serializer(42, options); // success

New behavior (DisableDefaultReflection feature flag enabled)

JsonSerializerOptions options = new();

Assert.Null(options.TypeInfoResolver);
JsonSerializer.Serializer(42, options); // NotSupportedException: no metadata for type int

Alternative Designs

Should IJsonTypeInfoResolver argument in the new constructor be nullable? If so, new JsonSerializerOptions() would have different semantics compared to new JsonSerializerOptions((JsonTypeInfoResolver)null) depending on the feature flag state.

Risks

The obvious downside to the proposal is that we're adding RUC/RDC annotations to the default constructor in JsonSerializerOptions. However, on closer inspection that should not impact users substantially since

1. Source generator users consume JsonSerializerContext instances and don't need to explicitly create blank JsonSerializerOptions instances.
2. Aspnetcore users are configuring materialized JsonSerializerOptions instances that are being handed to them in configuration callbacks.
3. A JsonSerializerOptions instance can only be consumed meaningfully if passed to one of the JsonSerializer methods that accept JsonSerializerOptions. All of these methods are already marked RUC/RDC.
4. The new proposed constructor overload should provide an easy workaround for users looking to avoid linker warnings.

Author:	eiriktsarpalis
Assignees:	eiriktsarpalis
Labels:	api-suggestion, area-System.Text.Json
Milestone:	-

[terrajobst]

Video

• AppCompat switch System.Text.Json.Serialization.DisableDefaultReflection
  • We might want to expose an API that we can enlighten the linker about that its return value is tight to this setting.

namespace System.Text.Json;

public partial class JsonSerializerOptions
{
+   [RequiresUnreferencedCode, RequiresDynamicCode]
    public JsonSerializerOptions(); // default constructor now populates the resolver eagerly by default

+   // new constructor offering linker-safe JsonSerializerOptions construction
+   public JsonSerializerOptions(IJsonTypeInfoResolver? typeInfoResolver); 

    public JsonSerializerOptions(JsonSerializerOptions options); // copy constructor remains linker safe

-   [RequiresUnreferencedCode, RequiresDynamicCode]
    public JsonConverter GetConverter(Type type); // Annotation no longer needed since resolver is not populated lazily
}