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

Serialization of ReadOnlyMemory<byte> with [JsonIgnore] attribute in .NET 6 and .NET 7 RC1 #76807

Closed
MartyIX opened this issue Oct 10, 2022 · 12 comments · Fixed by #76828
Closed
Assignees
Milestone

Comments

@MartyIX
Copy link
Contributor

MartyIX commented Oct 10, 2022

Description

JsonSerializer.Serialize(..) method works differently in .NET 6 and in .NET 7 for ReadOnlyMemory<byte> property with [JsonIgnore] attribute.

Reproduction Steps

The issue can be reproduced using the following two files

JsonSerializerRepro.csproj:

<Project Sdk="Microsoft.NET.Sdk">
    <PropertyGroup>
        <OutputType>Exe</OutputType>
		
	<!-- Works with .NET 6 -->
        <TargetFramework>net6.0</TargetFramework>
		
	<!-- Fails with .NET 7 -->
        <!--<TargetFramework>net7.0</TargetFramework>-->
        <ImplicitUsings>disable</ImplicitUsings>
        <Nullable>enable</Nullable>
    </PropertyGroup>
</Project>

Program.cs:

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

namespace JsonSerializeRepro;

public static class Program
{
    public static void Main(string[] args)
    {
        ReadOnlyMemory<byte> binaryData = Array.Empty<byte>();
        InstallScriptRequest installScriptRequest = new(binaryData, upgrade: true);

        string json = JsonSerializer.Serialize(installScriptRequest);
        Console.WriteLine(json);
    }
}

public class InstallScriptRequest
{
    [JsonIgnore]
    public int BinaryDataMaxSize => 1024 * 1024;

    [JsonIgnore]
    public ReadOnlyMemory<byte> BinaryData { get; set; }

    public bool Upgrade { get; }

    public InstallScriptRequest(ReadOnlyMemory<byte> binaryData, bool upgrade)
    {
        this.BinaryData = binaryData;
        this.Upgrade = upgrade;
    }
}

Expected behavior

When run as .NET 6 project, I get: {"Upgrade":true}

Actual behavior

When run as .NET 7 project (RC1), I get:

Unhandled exception. System.InvalidOperationException: The type 'System.ReadOnlySpan`1[System.Byte]' of property 'Span' on type 'System.ReadOnlyMemory`1[System.Byte]' is invalid for serialization or deserialization because it is a pointer type, is a ref struct, or contains generic parameters that have not been replaced by specific types.
   at System.Text.Json.ThrowHelper.ThrowInvalidOperationException_CannotSerializeInvalidType(Type typeToConvert, Type declaringType, MemberInfo memberInfo)
   at System.Text.Json.Serialization.Metadata.ReflectionJsonTypeInfo`1.CreateProperty(Type typeToConvert, MemberInfo memberInfo, JsonSerializerOptions options, Boolean shouldCheckForRequiredKeyword)
   at System.Text.Json.Serialization.Metadata.ReflectionJsonTypeInfo`1.CacheMember(Type typeToConvert, MemberInfo memberInfo, Boolean& propertyOrderSpecified, Dictionary`2& ignoredMembers, Boolean shouldCheckForRequiredKeyword)
   at System.Text.Json.Serialization.Metadata.ReflectionJsonTypeInfo`1.AddPropertiesAndParametersUsingReflection()
   at System.Text.Json.Serialization.Metadata.ReflectionJsonTypeInfo`1..ctor(JsonConverter converter, JsonSerializerOptions options)
   at System.Text.Json.Serialization.JsonConverter`1.CreateReflectionJsonTypeInfo(JsonSerializerOptions options)
   at System.Text.Json.Serialization.Metadata.DefaultJsonTypeInfoResolver.CreateJsonTypeInfo(Type type, JsonSerializerOptions options)
   at System.Text.Json.Serialization.Metadata.DefaultJsonTypeInfoResolver.GetTypeInfo(Type type, JsonSerializerOptions options)
   at System.Text.Json.JsonSerializerOptions.GetTypeInfoNoCaching(Type type)
   at System.Collections.Concurrent.ConcurrentDictionary`2.GetOrAdd(TKey key, Func`2 valueFactory)
   at System.Text.Json.JsonSerializerOptions.GetTypeInfoInternal(Type type, Boolean ensureConfigured, Boolean resolveIfMutable)
   at System.Text.Json.Serialization.Metadata.JsonPropertyInfo.Configure()
   at System.Text.Json.Serialization.Metadata.JsonTypeInfo.InitializePropertyCache()
   at System.Text.Json.Serialization.Metadata.JsonTypeInfo.Configure()
   at System.Text.Json.Serialization.Metadata.JsonTypeInfo.<EnsureConfigured>g__ConfigureLocked|138_0()
   at System.Text.Json.JsonSerializerOptions.GetTypeInfoInternal(Type type, Boolean ensureConfigured, Boolean resolveIfMutable)
   at System.Text.Json.JsonSerializer.GetTypeInfo(JsonSerializerOptions options, Type inputType)
   at System.Text.Json.JsonSerializer.GetTypeInfo[T](JsonSerializerOptions options)
   at System.Text.Json.JsonSerializer.Serialize[TValue](TValue value, JsonSerializerOptions options)
   at JsonSerializeRepro.Program.Main(String[] args) in C:\Users\USER\JsonSerializeRepro\JsonSerializeRepro\Program.cs:line 14

Regression?

Maybe it's a regression. I'm not sure.

Known Workarounds

Implement custom JSON converter

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

namespace JsonSerializeRepro;

public static class Program
{
    public static void Main(string[] args)
    {
        ReadOnlyMemory<byte> binaryData = Array.Empty<byte>();
        InstallScriptRequest installScriptRequest = new(binaryData, upgrade: true);

        JsonSerializerOptions serializationOptions = new();
        serializationOptions.Converters.Add(new InstallScriptRequestConverter());

        string json = JsonSerializer.Serialize(installScriptRequest, serializationOptions);
        Console.WriteLine(json);
    }
}

public class InstallScriptRequestConverter : JsonConverter<InstallScriptRequest>
{
    /// <inheritdoc/>
    public override InstallScriptRequest Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
    {
        throw new NotImplementedException();
    }

    /// <inheritdoc/>
    public override void Write(Utf8JsonWriter writer, InstallScriptRequest value, JsonSerializerOptions options)
    {
        writer.WriteStartObject();
        writer.WritePropertyName("upgrade");
        writer.WriteBooleanValue(value.Upgrade);
        writer.WriteEndObject();
    }
}

public class InstallScriptRequest
{
    /// <inheritdoc/>
    [JsonIgnore]
    public int BinaryDataMaxSize => 1024 * 1024;

    /// <summary>ZIP archive that contains the script package to install.</summary>
    [JsonIgnore]
    public ReadOnlyMemory<byte> BinaryData { get; set; }

    public bool Upgrade { get; }

    public InstallScriptRequest(ReadOnlyMemory<byte> binaryData, bool upgrade)
    {
        this.BinaryData = binaryData;
        this.Upgrade = upgrade;
    }
}

Configuration

Tested on Windows 11. However, I don't believe it's a platform-specific issue.

Other information

No response

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Oct 10, 2022
@ghost
Copy link

ghost commented Oct 10, 2022

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

Description

JsonSerializer.Serialize(..) method works differently in .NET 6 and in .NET 7 for ReadOnlyMemory<byte> property with [JsonIgnore] attribute.

Reproduction Steps

The issue can be reproduced using the following two files

JsonSerializerRepro.csproj:

<Project Sdk="Microsoft.NET.Sdk">
    <PropertyGroup>
        <OutputType>Exe</OutputType>
		
	<!-- Works with .NET 6 -->
        <TargetFramework>net6.0</TargetFramework>
		
	<!-- Fails with .NET 7 -->
        <!--<TargetFramework>net7.0</TargetFramework>-->
        <ImplicitUsings>disable</ImplicitUsings>
        <Nullable>enable</Nullable>
    </PropertyGroup>
</Project>

Program.cs:

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

namespace JsonSerializeRepro;

public static class Program
{
    public static void Main(string[] args)
    {
        ReadOnlyMemory<byte> binaryData = Array.Empty<byte>();
        InstallScriptRequest installScriptRequest = new(binaryData, upgrade: true);

        string json = JsonSerializer.Serialize(installScriptRequest);
        Console.WriteLine(json);
    }
}

public class InstallScriptRequest
{
    /// <inheritdoc/>
    [JsonIgnore]
    public int BinaryDataMaxSize => 1024 * 1024;

    /// <summary>ZIP archive that contains the script package to install.</summary>
    [JsonIgnore]
    public ReadOnlyMemory<byte> BinaryData { get; set; }

    public bool Upgrade { get; }

    public InstallScriptRequest(ReadOnlyMemory<byte> binaryData, bool upgrade)
    {
        this.BinaryData = binaryData;
        this.Upgrade = upgrade;
    }
}

Expected behavior

When run as .NET 6 project, I get: {"Upgrade":true}

Actual behavior

When run as .NET 7 project (RC1), I get:

Unhandled exception. System.InvalidOperationException: The type 'System.ReadOnlySpan`1[System.Byte]' of property 'Span' on type 'System.ReadOnlyMemory`1[System.Byte]' is invalid for serialization or deserialization because it is a pointer type, is a ref struct, or contains generic parameters that have not been replaced by specific types.
   at System.Text.Json.ThrowHelper.ThrowInvalidOperationException_CannotSerializeInvalidType(Type typeToConvert, Type declaringType, MemberInfo memberInfo)
   at System.Text.Json.Serialization.Metadata.ReflectionJsonTypeInfo`1.CreateProperty(Type typeToConvert, MemberInfo memberInfo, JsonSerializerOptions options, Boolean shouldCheckForRequiredKeyword)
   at System.Text.Json.Serialization.Metadata.ReflectionJsonTypeInfo`1.CacheMember(Type typeToConvert, MemberInfo memberInfo, Boolean& propertyOrderSpecified, Dictionary`2& ignoredMembers, Boolean shouldCheckForRequiredKeyword)
   at System.Text.Json.Serialization.Metadata.ReflectionJsonTypeInfo`1.AddPropertiesAndParametersUsingReflection()
   at System.Text.Json.Serialization.Metadata.ReflectionJsonTypeInfo`1..ctor(JsonConverter converter, JsonSerializerOptions options)
   at System.Text.Json.Serialization.JsonConverter`1.CreateReflectionJsonTypeInfo(JsonSerializerOptions options)
   at System.Text.Json.Serialization.Metadata.DefaultJsonTypeInfoResolver.CreateJsonTypeInfo(Type type, JsonSerializerOptions options)
   at System.Text.Json.Serialization.Metadata.DefaultJsonTypeInfoResolver.GetTypeInfo(Type type, JsonSerializerOptions options)
   at System.Text.Json.JsonSerializerOptions.GetTypeInfoNoCaching(Type type)
   at System.Collections.Concurrent.ConcurrentDictionary`2.GetOrAdd(TKey key, Func`2 valueFactory)
   at System.Text.Json.JsonSerializerOptions.GetTypeInfoInternal(Type type, Boolean ensureConfigured, Boolean resolveIfMutable)
   at System.Text.Json.Serialization.Metadata.JsonPropertyInfo.Configure()
   at System.Text.Json.Serialization.Metadata.JsonTypeInfo.InitializePropertyCache()
   at System.Text.Json.Serialization.Metadata.JsonTypeInfo.Configure()
   at System.Text.Json.Serialization.Metadata.JsonTypeInfo.<EnsureConfigured>g__ConfigureLocked|138_0()
   at System.Text.Json.JsonSerializerOptions.GetTypeInfoInternal(Type type, Boolean ensureConfigured, Boolean resolveIfMutable)
   at System.Text.Json.JsonSerializer.GetTypeInfo(JsonSerializerOptions options, Type inputType)
   at System.Text.Json.JsonSerializer.GetTypeInfo[T](JsonSerializerOptions options)
   at System.Text.Json.JsonSerializer.Serialize[TValue](TValue value, JsonSerializerOptions options)
   at JsonSerializeRepro.Program.Main(String[] args) in C:\Users\USER\JsonSerializeRepro\JsonSerializeRepro\Program.cs:line 14

Regression?

Maybe it's a regression. I'm not sure.

Known Workarounds

No response

Configuration

Tested on Windows 11. However, I don't believe it's a platform-specific issue.

Other information

No response

Author: MartyIX
Assignees: -
Labels:

area-System.Text.Json, untriaged

Milestone: -

@MartyIX MartyIX changed the title Serialization of ReadOnlyMemory<byte> with [JsonIgnore] attribute in .NET 6 and .NET 7 Serialization of ReadOnlyMemory<byte> with [JsonIgnore] attribute in .NET 6 and .NET 7 RC1 Oct 10, 2022
@eiriktsarpalis
Copy link
Member

I can reproduce -- root cause is infrastructural changes related to #63686, namely we're now eagerly computing type metadata even for properties that have been marked JsonIgnoreCondition.Always:

_jsonTypeInfo ??= Options.GetTypeInfoInternal(PropertyType, ensureConfigured: false);

We should try to fix this in .NET 7 since it clearly breaks a common user workaround when dealing with unsupported property types.

In the meantime, a simpler workaround for the issue would be to register a dummy converter for ReadOnlyMemory:

var options = new JsonSerializerOptions { Converters = { new ReadOnlyMemoryConverter() } };
JsonSerializer.Serialize(installScriptRequest, options); // serializes as expected

public class ReadOnlyMemoryConverter : JsonConverter<ReadOnlyMemory<byte>>
{
    public override ReadOnlyMemory<byte> Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
        => throw new NotImplementedException();

    public override void Write(Utf8JsonWriter writer, ReadOnlyMemory<byte> value, JsonSerializerOptions options)
        => throw new NotImplementedException();
}

@eiriktsarpalis eiriktsarpalis added this to the 7.0.0 milestone Oct 10, 2022
@eiriktsarpalis eiriktsarpalis removed the untriaged New issue has not been triaged by the area owner label Oct 10, 2022
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Oct 10, 2022
@eiriktsarpalis
Copy link
Member

eiriktsarpalis commented Oct 10, 2022

FWIW this is simply surfacing a deeper issue with how the metadata model is interpreting JsonIgnoreCondition.Always. For example, the same example transcribed to source gen fails both in .NET 6 and .NET 7 with a compile-time error:

JsonSerializer.Serialize(new MyPoco(), MyContext.Default.MyPoco);

[JsonSerializable(typeof(MyPoco))]
public partial class MyContext : JsonSerializerContext { }

public class MyPoco
{
    [JsonIgnore]
    public ReadOnlyMemory<byte> Memory { get; set; }
}
Severity	Code	Description	Project	File	Line	Suppression State
Error	CS0306	The type 'ReadOnlySpan<byte>' may not be used as a type argument	ConsoleApp1	C:\Users\eitsarpa\source\repos\ConsoleApp1\ConsoleApp1\System.Text.Json.SourceGeneration\System.Text.Json.SourceGeneration.JsonSourceGenerator\MyContext.ReadOnlySpanByte.g.cs	8	Active

Root cause is essentially the same -- the source generator is eagerly attempting to generate metadata for the type even though it is ignored by the user.

@eiriktsarpalis
Copy link
Member

These checks specifically validate whether the property is a valid type argument (i.e. not a pointer, ref struct, etc) and suppressing custom converter factory exceptions, but nothing more. This wasn't changed in .NET 7, what did change is eager resolution of JsonTypeInfo for all configured JsonPropertyInfo, including those that are marked as ignored.

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

layomia commented Oct 10, 2022

A fix for this is important - we serviced for this same scenario in .NET 6.0: #60299.

@eiriktsarpalis
Copy link
Member

A fix for this is important - we serviced for this same scenario in .NET 6.0: #60299.

Curious that regression testing didn't catch it.

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

layomia commented Oct 11, 2022

Re-opening till it's fixed in 7.0: #76869.

@layomia layomia reopened this Oct 11, 2022
@dotMorten
Copy link

dotMorten commented Oct 12, 2022

I wonder if this is equivalent to this issue I'm hitting with this code:

    [JsonSerializable(typeof(MyObject))]
    [JsonSourceGenerationOptions(GenerationMode = JsonSourceGenerationMode.Metadata, IgnoreReadOnlyFields = true)]
    internal partial class MyObjectSerializationContext : JsonSerializerContext
    {
    }
    public class MyObject
    {
        [JsonPropertyName("test")]
        public string? Test { get; set; }
        [JsonIgnore]
        private readonly ComplexObject? _data; // Metadata generated for this type. Why?!?
    }

    public class ComplexObject
    {
        private protected object _thisLock = new object(); // This causes code gen problem
    }

This will give me errors like:

1>E:\sources.tmp\TrimTest\ClassLibrary1\System.Text.Json.SourceGeneration\System.Text.Json.SourceGeneration.JsonSourceGenerator\MyObjectSerializationContext.ComplexObject.g.cs(58,85,58,94): error CS0122: 'ComplexObject._thisLock' is inaccessible due to its protection level
1>E:\sources.tmp\TrimTest\ClassLibrary1\System.Text.Json.SourceGeneration\System.Text.Json.SourceGeneration.JsonSourceGenerator\MyObjectSerializationContext.ComplexObject.g.cs(59,92,59,101): error CS0122: 'ComplexObject._thisLock' is inaccessible due to its protection level

I don't get why it is even trying to generate anything for ComplexObject - I explicitly told it to ignore that field. This happens with .net6 also though (also tested 6.0.6 nuget package).

In my specific case, ComplexObject is potentially a huge object graph and it is generating metadata for hundreds of classes.

@eiriktsarpalis
Copy link
Member

It appears to be the same problem as #76807 (comment). Could you create a new issue with the problem? This one is specifically tracking the .NET 7 regression of the reflection serializer.

@dotMorten
Copy link

@eiriktsarpalis Thanks - logged #76937

@layomia
Copy link
Contributor

layomia commented Oct 27, 2022

A fix for this is important - we serviced for this same scenario in .NET 6.0: #60299.

Curious that regression testing didn't catch it.

The failure and fix then were for the source generation implementation; tests should have been added for reflection as well.

@ghost ghost locked as resolved and limited conversation to collaborators Nov 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants