Skip to content
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

[System.Text.Json] Regression: [JsonIgnore] Attribute not Applied to Ref Returns #59936

Closed
Sewer56 opened this issue Oct 4, 2021 · 5 comments · Fixed by #60024
Closed

[System.Text.Json] Regression: [JsonIgnore] Attribute not Applied to Ref Returns #59936

Sewer56 opened this issue Oct 4, 2021 · 5 comments · Fixed by #60024

Comments

@Sewer56
Copy link
Contributor

Sewer56 commented Oct 4, 2021

Description

Found a regression in System.Text.Json where using properties with ref returns cannot be ignored for serialization, leading to an unavoidable exception.

Regression was introduced in System.Text.Json 6.0.0-rc.1.21406.5, going by the available packages in the dotnet6 NuGet repo. This package was pushed on Friday, August 6th, 2021.

As I'm not familiar with how dotnet is exactly built, I do not know how to pinpoint the sdk build number to the exact commit causing the issue however; but the build version should hopefully be useful.

Minimal Reproduction:

using System.Diagnostics;
using System.Text.Json;
using System.Text.Json.Serialization;

var sampleText = new TypeWithRefString();
var json = JsonSerializer.Serialize(sampleText); // Exception Here
var deserialized = JsonSerializer.Deserialize<TypeWithRefString>(json); // And Here

internal class TypeWithRefString
{
    [JsonIgnore]
    public ref string NameRef => ref Name;
    public string Name = "How many Richard Landers does it take to write a blog entry? Just one, you rock!";
}

Debug in your own desired way.
This snippet can be pasted and ran over a blank .NET 6 console app.

Configuration

N/A

Regression?

Yes, this use case worked with any version of System.Text.Json <= 6.0.0-rc.1.21406.4;

Other information

Exception:

System.ArgumentException: 'The type 'System.String&' may not be used as a type argument.'
   at System.RuntimeType.ThrowIfTypeNeverValidGenericArgument(RuntimeType type)
   at System.RuntimeType.SanityCheckGenericArguments(RuntimeType[] genericArguments, RuntimeType[] genericParamters)
   at System.RuntimeType.MakeGenericType(Type[] instantiation)
   at System.Text.Json.Serialization.Converters.ObjectConverterFactory.CreateConverter(Type typeToConvert, JsonSerializerOptions options)
   at System.Text.Json.Serialization.JsonConverterFactory.GetConverterInternal(Type typeToConvert, JsonSerializerOptions options)
   at System.Text.Json.JsonSerializerOptions.GetConverterInternal(Type typeToConvert)
   at System.Text.Json.JsonSerializerOptions.DetermineConverter(Type parentClassType, Type runtimePropertyType, MemberInfo memberInfo)
   at System.Text.Json.Serialization.Metadata.JsonTypeInfo.GetConverter(Type type, Type parentClassType, MemberInfo memberInfo, Type& runtimeType, JsonSerializerOptions options)
   at System.Text.Json.Serialization.Metadata.JsonTypeInfo.AddProperty(MemberInfo memberInfo, Type memberType, Type parentClassType, Boolean isVirtual, Nullable`1 parentTypeNumberHandling, JsonSerializerOptions options)
   at System.Text.Json.Serialization.Metadata.JsonTypeInfo.CacheMember(Type declaringType, Type memberType, MemberInfo memberInfo, Boolean isVirtual, Nullable`1 typeNumberHandling, Boolean& propertyOrderSpecified, Dictionary`2& ignoredMembers)
   at System.Text.Json.Serialization.Metadata.JsonTypeInfo..ctor(Type type, JsonConverter converter, Type runtimeType, JsonSerializerOptions options)
   at System.Text.Json.Serialization.Metadata.JsonTypeInfo..ctor(Type type, JsonSerializerOptions options)
   at System.Text.Json.JsonSerializerOptions.<RootBuiltInConvertersAndTypeInfoCreator>g__CreateJsonTypeInfo|107_0(Type type, JsonSerializerOptions options)
   at System.Text.Json.JsonSerializerOptions.GetClassFromContextOrCreate(Type type)
   at System.Text.Json.JsonSerializerOptions.GetOrAddClass(Type type)
   at System.Text.Json.JsonSerializerOptions.GetOrAddClassForRootType(Type type)
   at System.Text.Json.JsonSerializer.GetTypeInfo(Type runtimeType, JsonSerializerOptions options)
   at System.Text.Json.JsonSerializer.Write[TValue](TValue& value, Type runtimeType, JsonSerializerOptions options)
   at System.Text.Json.JsonSerializer.Serialize[TValue](TValue value, JsonSerializerOptions options)
   at Program.<Main>$(String[] args) in C:\Users\sewer\Desktop\Projects\SystemTextJsonBug\SystemTextJsonBug\SystemTextJsonBug\Program.cs:line 12
@dotnet-issue-labeler dotnet-issue-labeler bot added area-System.Text.Json untriaged New issue has not been triaged by the area owner labels Oct 4, 2021
@ghost
Copy link

ghost commented Oct 4, 2021

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

Issue Details

Description

Found a regression in System.Text.Json where using properties with ref returns cannot be ignored for serialization, leading to an unavoidable exception.

Regression was introduced in System.Text.Json 6.0.0-rc.1.21406.5, going by the available packages in the dotnet6 NuGet repo. This package was pushed on Friday, August 6th, 2021.

As I'm not familiar with how dotnet is exactly built, I do not know how to pinpoint the sdk build number to the exact commit causing the issue however; but the build version should hopefully be useful.

Minimal Reproduction:

using System.Diagnostics;
using System.Text.Json;
using System.Text.Json.Serialization;

var sampleText = new TypeWithRefString();
var json = JsonSerializer.Serialize(sampleText); // Exception Here
var deserialized = JsonSerializer.Deserialize<TypeWithRefString>(json); // And Here

internal class TypeWithRefString
{
    [JsonIgnore]
    public ref string NameRef => ref Name;
    public string Name = "How many Richard Landers does it take to write a blog entry? Just one, you rock!";
}

Debug in your own desired way.
This snipped can be pasted and ran over a blank .NET 6 console app.

Configuration

N/A

Regression?

Yes, this use case worked with any version of System.Text.Json <= 6.0.0-rc.1.21406.4;

Other information

Exception:

System.ArgumentException: 'The type 'System.String&' may not be used as a type argument.'
   at System.RuntimeType.ThrowIfTypeNeverValidGenericArgument(RuntimeType type)
   at System.RuntimeType.SanityCheckGenericArguments(RuntimeType[] genericArguments, RuntimeType[] genericParamters)
   at System.RuntimeType.MakeGenericType(Type[] instantiation)
   at System.Text.Json.Serialization.Converters.ObjectConverterFactory.CreateConverter(Type typeToConvert, JsonSerializerOptions options)
   at System.Text.Json.Serialization.JsonConverterFactory.GetConverterInternal(Type typeToConvert, JsonSerializerOptions options)
   at System.Text.Json.JsonSerializerOptions.GetConverterInternal(Type typeToConvert)
   at System.Text.Json.JsonSerializerOptions.DetermineConverter(Type parentClassType, Type runtimePropertyType, MemberInfo memberInfo)
   at System.Text.Json.Serialization.Metadata.JsonTypeInfo.GetConverter(Type type, Type parentClassType, MemberInfo memberInfo, Type& runtimeType, JsonSerializerOptions options)
   at System.Text.Json.Serialization.Metadata.JsonTypeInfo.AddProperty(MemberInfo memberInfo, Type memberType, Type parentClassType, Boolean isVirtual, Nullable`1 parentTypeNumberHandling, JsonSerializerOptions options)
   at System.Text.Json.Serialization.Metadata.JsonTypeInfo.CacheMember(Type declaringType, Type memberType, MemberInfo memberInfo, Boolean isVirtual, Nullable`1 typeNumberHandling, Boolean& propertyOrderSpecified, Dictionary`2& ignoredMembers)
   at System.Text.Json.Serialization.Metadata.JsonTypeInfo..ctor(Type type, JsonConverter converter, Type runtimeType, JsonSerializerOptions options)
   at System.Text.Json.Serialization.Metadata.JsonTypeInfo..ctor(Type type, JsonSerializerOptions options)
   at System.Text.Json.JsonSerializerOptions.<RootBuiltInConvertersAndTypeInfoCreator>g__CreateJsonTypeInfo|107_0(Type type, JsonSerializerOptions options)
   at System.Text.Json.JsonSerializerOptions.GetClassFromContextOrCreate(Type type)
   at System.Text.Json.JsonSerializerOptions.GetOrAddClass(Type type)
   at System.Text.Json.JsonSerializerOptions.GetOrAddClassForRootType(Type type)
   at System.Text.Json.JsonSerializer.GetTypeInfo(Type runtimeType, JsonSerializerOptions options)
   at System.Text.Json.JsonSerializer.Write[TValue](TValue& value, Type runtimeType, JsonSerializerOptions options)
   at System.Text.Json.JsonSerializer.Serialize[TValue](TValue value, JsonSerializerOptions options)
   at Program.<Main>$(String[] args) in C:\Users\sewer\Desktop\Projects\SystemTextJsonBug\SystemTextJsonBug\SystemTextJsonBug\Program.cs:line 12
Author: Sewer56
Assignees: -
Labels:

area-System.Text.Json, untriaged

Milestone: -

@eiriktsarpalis
Copy link
Member

Can repro -- I believe this is the same regression as the one reported in #58469 (comment).

We made the call earlier not to revert the change, but it seems to me that this particular issue makes a more compelling case /cc @layomia

Related to #59364.

@eiriktsarpalis eiriktsarpalis added this to the 6.0.0 milestone Oct 4, 2021
@eiriktsarpalis eiriktsarpalis added bug blocking-release and removed untriaged New issue has not been triaged by the area owner labels Oct 4, 2021
@richlander
Copy link
Member

Ha! I'll be writing the RC2 post this week.

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Oct 5, 2021
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Oct 12, 2021
@layomia
Copy link
Contributor

layomia commented Oct 12, 2021

Re-opening so we can consider porting the fix to .NET 6.0.

@eiriktsarpalis
Copy link
Member

Fixed by #60299

@ghost ghost locked as resolved and limited conversation to collaborators Nov 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.