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

[ghost]

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
}

[Sergio0694]

This looks awesome! 😄

A couple questions:

• I assume the switch would also work downlevel? Eg. can we enable this in the Microsoft Store as well (UWP, so using the .NET Standard 2.0 target for System.Text.Json), so we can enforce that reflection paths are never taken accidentally?
• Just wondering - looking at the generated s_defaultOptions field in a custom JsonSerializerContext type, it seems that JsonSerializerOptions object is being created using the default constructor. Is this fine anyway due to how that's then later wired up through the generated context, or would the new release also update the generator to instantiate those options in a different way? I know the generated path is guaranteed to be linker safe, this is more just me being curious as to how this specific bit will work and/or be updated to account for the new annotations 🙂

Thank you!

[eiriktsarpalis]

• I assume the switch would also work downlevel? Eg. can we enable this in the Microsoft Store as well (UWP, so using the .NET Standard 2.0 target for System.Text.Json), so we can enforce that reflection paths are never taken accidentally?

Yes, the feature flag should work although I'm not sure if the trimmer would be aware of it in that case. @eerhardt might be able to illuminate.

or would the new release also update the generator to instantiate those options in a different way?

That's right, the source generator will be updated to use the new constructor that was just approved.

[Sergio0694]

"I'm not sure if the trimmer would be aware of it in that case"

Not entirely sure whether the compiler/linker on .NET Native is able to recognize those flags at compile time and account for them while trimming (@MichalStrehovsky might know), but even if not, the main advantage for us would be that such a flag would enforce that we'd never accidentally use a reflection fallback path. That's pretty handy because on UWP there's no trimming annotations, so you don't have an easy way to validate that trimming doesn't break anything at compile time. Your only option is to run the app with .NET Native (Debug builds by default just use the UWP .NET Core 2.1 franken-fork) and then get that code path to execute in your app to double check it works fine. In practice, you'd just run the app with the Debug CoreCLR runtime most of the time (especially given that building the Microsoft Store in Release takes like 15 minutes just for a single arch 🥲), which means it's relatively easy to miss accidental regressions due to trimming. So being able to just completely disable those fallback paths with a build setting would be pretty nice to have on its own 🙂

[eerhardt]

I don't believe the UWP / .NET Native toolset supports the feature switches at trim time. The design was first implemented in the ILLinker and was adopted by the NativeAOT compiler in net7.0. I would be super surprised if UWP supported the ILLink.Substitutions.xml files.

but even if not, the main advantage for us would be that such a flag would enforce that we'd never accidentally use a reflection fallback path

This part does have a chance of working in UWP though. Since there is an AppContext switch that is respected at runtime (regardless if the code has been trimmed/AOT'd/etc), the same behavior will occur in Debug and Release builds of UWP. I don't know how to set AppContext switches in UWP (is there a runtimeconfig.json file?), but if you can get the switch set in your UWP app, it will be respected by System.Text.Json, and it will enforce that the Reflection path isn't taken.

[MichalStrehovsky]

I would be super surprised if UWP supported the ILLink.Substitutions.xml files.

It doesn't - it supports a simpler predecessor (dotnet/designs#42). The simplification is that it solely replaces method bodies and doesn't require dead branch elimination. It's less powerful, but easier to reason about (the rules for when dead branch elimination happens with the shipped feature switches are arbitrary).

I don't know how to set AppContext switches in UWP

If this is about setting an AppContext switch to true, adding the name of the switch to the semicolon-delimited AppContextSwitches MSBuild property should do it for .NET Native builds (but not for F5 debug). Alternatively, just do it first thing in Main or a module initializer (IIRC module initializers do work with .NET Native so this can be another unsupported thing the Store app can stress test with .NET Native ).

[Sergio0694]

"Alternatively, just do it first thing in Main"

Yup that's how we're currently setting them, so they're always enabled. Eg. we had to do this to set "System.Resources.UseSystemResourceKeys", which was otherwise causing System.Text.Json to throw in Release builds, because it was trying to load resource that .NET Native had deleted (see #78099). Works just fine with this set 🙂

Given that, then I'd expect this new flag to also work the same, so yeah this would indeed be valuable for us too even leaving the annotation/trimming considerations aside, just for the "enforcement" aspect we discussed, so that's very nice.

"this can be another unsupported thing the Store app can stress test with .NET Native "

Hey if we don't run this beauty on C# 11, who will? Someone has to! 😄 /s

[eiriktsarpalis]

Following feedback from #83844 (comment), we've decided to rename the feature switch to System.Text.Json.Serialization.UseReflectionDefault which defaults to true. cc @terrajobst

[terrajobst]

Video

• The AppContext switch will match the fully qualified property name System.Text.Json.JsonSerializer.IsReflectionEnabledByDefault
• The switch can be included in ILLink.Substitutions.xml to make it a link time constant
• We're probably automatically setting this to false when AOT is enabled, at least for console apps and ASP.NET Core
  • However, certain app models like WinForms, WPF, and MAUI will probably still want reflection by default

namespace System.Text.Json;

public partial static class JsonSerializer
{
    public static bool IsReflectionEnabledByDefault { get; }
}