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: How to serialize only certain properties without using JsonIgnoreAttribute #593

Closed
epignosisx opened this issue Dec 5, 2019 · 16 comments
Labels
api-needs-work API needs work before it is approved, it is NOT ready for implementation area-System.Text.Json
Milestone

Comments

@epignosisx
Copy link

epignosisx commented Dec 5, 2019

I'm trying to migrate some of our Newtonsoft.Json functionality over to System.Text.Json and we used to have code that would opt-in to serialization instead of the opt-out model of the JsonIgnoreAttribute.

The scenario is in a web app that we like to log the request object when validation fails, but just certain properties to avoid any PII/PCI issues.

We can't use the JsonIgnoreAttribute for two reasons:

1- Adding the attribute will stop the serializer used by ASP.NET Core from deserializing the HTTP request into an object. We still want this. We want that when we serialize it with our own JsonSerializer we add our customization to only allow certain properties.

2- For security reasons we want to have an opt-in model instead of opt-out in case a new sensitive property is added but the attribute is forgotten. We want to avoid getting this data into our logs.

This is our code simplified:

[AttributeUsage(AttributeTargets.Property | AttributeTargets.Field)]
public sealed class LogAttribute : Attribute
{
}

public class PersonApiRequest
{
    [Log]  // <-- serialization opt-in attribute
    public int Id { get; set; }

    [RegularExpression("SomeRegex")]
    public string Name { get; set; }
    
    [RegularExpression("SomeRegex")]
    public string Ssn { get; set; }
}

public class LogContractResolver : DefaultContractResolver
{
    protected override JsonProperty CreateProperty(MemberInfo member, MemberSerialization memberSerialization)
    {
        JsonProperty property = base.CreateProperty(member, memberSerialization);
        var attributes = property.AttributeProvider.GetAttributes(typeof(LogAttribute), true);
        property.Ignored = attributes.Count == 0;
        return property;
    }
}

public class PersonController : Controller
{
    private static readonly JsonSerializerSettings SerializerSettings = new JsonSerializerSettings { ContractResolver = new LogContractResolver() };
    
    [HttpPost]
    public IActionResult Index([FromBody]PersonApiRequest apiRequest)
    {
        if (!ModelState.IsValid)
        {
            var modelSerialized = JsonConvert.SerializeObject(apiRequest, SerializerSettings);
            _logger.LogWarning("Invalid {Request}", modelSerialized);
            return BadRequest(ModelState);
        }
        ...
    }
}
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-System.Text.Json untriaged New issue has not been triaged by the area owner labels Dec 5, 2019
@Symbai
Copy link

Symbai commented Dec 5, 2019

Sounds like a dupe of https://github.com/dotnet/corefx/issues/39277 my bad, I'm sorry.

@epignosisx
Copy link
Author

@Symbai it’s related, but not exactly the same.

In this case I need an extensibility point to implement the custom behavior myself. If the library were to provide this functionality out of the box, it would not work for this use case because it would affect the deserialization done by ASP.NET Core during model binding.

@layomia
Copy link
Contributor

layomia commented Feb 20, 2020

This issue is related to work in #1562, but requires a lot of feature work. Moving to future.

@layomia layomia removed the untriaged New issue has not been triaged by the area owner label Feb 20, 2020
@layomia layomia added this to the Future milestone Feb 20, 2020
@daniel-p-tech
Copy link

I'm looking to accomplish something similar. I have a custom attribute [SensitiveData] decorating certain properties (SSN, Password, DateOfBirth) that need be serialized in a redacted format (e.g. ****) for logging purposes.

I've spent several hours trying to figure how to implement this using System.Text.Json but eventually gave up and resorted back to my old Newtonsoft.Json solution.

Do you have any update on this? Ideally, I should be able to implement my own converter that is triggered if a certain attribute is applied to a property. Or be able to run a converter on any property and check at run-time using reflection if a particular object property is decorated with my custom attribute.

Thanks!

@daniel-p-tech
Copy link

Hi @layomia, have you had a chance to review my previous post? Thanks.

@aktxyz
Copy link

aktxyz commented May 25, 2020

+10

@nwsm
Copy link

nwsm commented Jan 25, 2021

+1

@marc-wilson
Copy link

marc-wilson commented Feb 4, 2021

You can almost accomplish this in System.Text.Json with a custom JsonConverter. The only issue is that you have to write everything to the writer manually. All of the methods to write the property to output are by type (ie: WriteString, WriteNumber, WriteBoolean, etc...).

All I'm trying to do is "null out" property based on an attribute. If there was some way to write a generic value without having to specify its type, it would work.

public override void Write(Utf8JsonWriter writer, MyClass value, JsonSerializerOptions options)
{
    var props = value.GetType().GetProperties().ToList();
    writer.WriteStartObject();
    foreach (var prop in props)
    {
        var sensitiveDataAttributes = prop.GetCustomAttributes(false).FirstOrDefault(c => c is SensitiveDataAttribute);
        if (sensitiveDataAttributes != null)
        {
            writer.WriteNull(prop.Name); // null out the value because it's sensitive data
        }
        else
        {
            // Not possible: Just write the property/value to the json
        }
    }
    writer.WriteEndObject();
}

I'd rather not have a big switch statement checking for each T and then using the associated writer.Write*() method. Seems error-prone/unecessary.

@q512
Copy link

q512 commented Feb 15, 2021

@mswilson4040

If there was some way to write a generic value without having to specify its type, it would work.

Could not we do something like this:

else
{
    writer.WritePropertyName(prop.Name);
    JsonSerializer.Serialize(writer, prop.GetValue(value), prop.PropertyType,
        options);
}

@eiriktsarpalis eiriktsarpalis modified the milestones: Future, 6.0.0 Feb 25, 2021
MichalStrehovsky added a commit to MichalStrehovsky/runtime that referenced this issue Mar 25, 2021
Force the reflection-only code paths that don't Reflection.Emit.
@urielzen
Copy link

+1

@layomia
Copy link
Contributor

layomia commented Jul 23, 2021

Triage - we didn't get to this in .NET 6 but can consider for 7. We're working toward providing APIs that allow users to configure type-serialization metadata - #34456.

@layomia layomia modified the milestones: 6.0.0, 7.0.0 Jul 23, 2021
@eiriktsarpalis eiriktsarpalis added api-needs-work API needs work before it is approved, it is NOT ready for implementation api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Oct 15, 2021
@eiriktsarpalis
Copy link
Member

We might want to consider #40395 as a more flexible alternative to this proposal.

@robertmclaws
Copy link

Ok folks, I wrote a converter to solve this problem. It's probably not as fast as it could be, and potentially allocates more than it needs to. But it gets the job done.

it uses the Factory pattern, because you need to pass in the generic type in order to get full object serialization. Our base type is DbObservableObject, but just swap yours out with something else.

    /// <summary>
    /// 
    /// </summary>
    /// <summary>
    /// <para>
    /// The converter we create needs to know the exact type we're converting, otherwise you would only get base object properties every
    /// time. Therefore it has to be generic. So the Factory creates the right Converter instance type for the object and sends it on its' way.
    /// </para>
    /// <para>
    /// For more details, 
    /// <see href="https://docs.microsoft.com/en-us/dotnet/standard/serialization/system-text-json-converters-how-to?pivots=dotnet-6-0">see Microsoft's converter documentation.</see>
    /// </para>
    /// </summary>
    public class IgnoreAuditFieldsJsonConverterFactory : JsonConverterFactory
    {

        /// <summary>
        /// 
        /// </summary>
        /// <param name="typeToConvert"></param>
        /// <returns></returns>
        public override bool CanConvert(Type typeToConvert) =>
#if NET5_0_OR_GREATER
            typeToConvert.IsAssignableTo(typeof(DbObservableObject));
#else
            typeof(DbObservableObject).IsAssignableFrom(typeToConvert);
#endif

        /// <summary>
        /// 
        /// </summary>
        /// <param name="typeToConvert"></param>
        /// <param name="options"></param>
        /// <returns></returns>
        public override JsonConverter CreateConverter(Type typeToConvert, JsonSerializerOptions options)
        {
            JsonConverter converter = (JsonConverter)Activator.CreateInstance(
                typeof(IgnoreAuditFieldsJsonConverter<>)
                    .MakeGenericType(new Type[] { typeToConvert }),
                BindingFlags.Instance | BindingFlags.Public,
                binder: null,
                args: null,
                culture: null)!;

            return converter;
        }
    }

    /// <summary>
    /// 
    /// </summary>
    public class IgnoreAuditFieldsJsonConverter<T> : JsonConverter<T> where T : DbObservableObject
    {

        //RWM: 
        private static readonly List<string> _propertiesToIgnore = new()
        {
            nameof(IChangeTracking.IsChanged),
            nameof(DbObservableObject.IsGraphChanged),
            nameof(DbObservableObject.ShouldTrackChanges)
        };

        /// <summary>
        /// 
        /// </summary>
        public override bool HandleNull => false;

        /// <summary>
        /// 
        /// </summary>
        /// <param name="reader"></param>
        /// <param name="typeToConvert"></param>
        /// <param name="options"></param>
        /// <returns></returns>
        public override T Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
        {
            return JsonSerializer.Deserialize<T>(ref reader, options);
        }

        /// <summary>
        /// 
        /// </summary>
        /// <param name="writer"></param>
        /// <param name="value"></param>
        /// <param name="options"></param>
        public override void Write(Utf8JsonWriter writer, T value, JsonSerializerOptions options)
        {
            if (value is not null)
            {
                writer.WriteStartObject();

                foreach (var property in value.GetType().GetProperties().Where(c => !_propertiesToIgnore.Contains(c.Name)))
                {
                    var propValue = property.GetValue(value);
                    switch (true)
                    {
                        case true when propValue is not null:
                        case true when propValue is null && options.DefaultIgnoreCondition == JsonIgnoreCondition.Never:
                            writer.WritePropertyName(property.Name);
                            JsonSerializer.Serialize(writer, propValue, options);
                            break;
                    }
                }

                writer.WriteEndObject();
            }
        }
    }

Hope this helps!

@eiriktsarpalis
Copy link
Member

eiriktsarpalis commented Jan 13, 2022

Based on the original code sample I would assume #63686 will provide the appropriate extensibility for authoring such custom attributes (without needing to write custom converters). Closing in favor of that issue.

@robertmclaws
Copy link

robertmclaws commented Jan 14, 2022

Attributes are not the solution when you have situations where you WANT the information serialized (like coming back from a server) and then DON'T want them serialized (like creating a diff to PATCH back to the server).

@eiriktsarpalis
Copy link
Member

A custom contract resolver doesn't require attribute annotations to function, my point was that it should be possible to introduce new attributes (like the one requested in the OP) using a custom contract resolver.

@ghost ghost locked as resolved and limited conversation to collaborators Feb 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-needs-work API needs work before it is approved, it is NOT ready for implementation area-System.Text.Json
Projects
None yet
Development

No branches or pull requests