aaubry / YamlDotNet

YamlDotNet is a .NET library for YAML
MIT License
2.56k stars 482 forks source link

Exception during deserialization of nullable enum >= YamlDotNet v9.1.0 #544

Closed BernieWhite closed 3 years ago

BernieWhite commented 3 years ago

Problem deserializing nullable enum with YamlDotNet v9.1.0, no issue on v8.1.2.

Is this intended?


public enum LanguageMode
{
    FullLanguage = 0,

    ConstrainedLanguage = 1
}

public sealed class ExecutionOption : IEquatable<ExecutionOption>
{
    [DefaultValue(null)]
    public LanguageMode? LanguageMode { get; set; }
}
aaubry commented 3 years ago

That's not intended. Thanks for reporting this issue. I haven't looked into it yet, but there was a change recently to handle billable types - #539. Maybe the bug was introduced there.

naefp commented 3 years ago

Does brake our projects as well:

System.MissingMethodException: Method not found: 'System.Collections.Generic.IDictionary`2<YamlDotNet.RepresentationModel.YamlNode,YamlDotNet.RepresentationModel.YamlNode> YamlDotNet.RepresentationModel.YamlMappingNode.get_Children()'.
   at NetEscapades.Configuration.Yaml.YamlConfigurationFileParser.VisitYamlMappingNode(YamlMappingNode node)
   at NetEscapades.Configuration.Yaml.YamlConfigurationFileParser.Parse(Stream input)
   at NetEscapades.Configuration.Yaml.YamlConfigurationProvider.Load(Stream stream)
   at Microsoft.Extensions.Configuration.FileConfigurationProvider.Load(Boolean reload)
maxgira commented 3 years ago

I am also experiencing this on version 9.1.0 with .NET 5.0.0. It works fine with version 8.1.2 for now. Here's the file I am trying to deserialize for reference.


# µblog API settings

# Public URL
# Publicly exposed URL for your µblog site
publicUrl: "http://localhost:5000"

# Allowed Hosts
# List of hosts allowed to connect to API server directly
allowedHosts: "*"

# Serilog Configuration
# Configuration for logging with Serilog
# https://github.com/serilog/serilog-settings-configuration
serilog:
  using:
    - "Serilog.Sinks.Console"
  #    - "Serilog.Sinks.File"
  writeTo:
    - name: "Console"
  #    - Name: "File"
  #      Args: 
  #        path: "Logs/log.txt"
  enrich:
    - "FromLogContext"
  properties:
    application: "µblog-server"
brian-reichle commented 3 years ago

The issue seems to be that it checks if the type is an enum before unwrapping a nullable type.

When the enum type check fails, it uses GetTypeCode on the underlying type which apparently returns Int32, and so it attempts to parse the enum code as an integer.

Perhaps it should unwrap the nullable type first.

            var underlyingType = Nullable.GetUnderlyingType(expectedType) ?? expectedType;

            if (underlyingType.IsEnum())
            {
                value = Enum.Parse(underlyingType, scalar.Value, true);
            }
            else
            {
                var typeCode = underlyingType.GetTypeCode();
BlowaXD commented 3 years ago

Hello,

I noticed that we are also experiencing that issue when I tried to upgrade from 8.1.2 to 9.1.0

Hope it gets fixed soon

Trojaner commented 3 years ago

Here is a simple workaround I wrote:

public class YamlNullableEnumTypeConverter : IYamlTypeConverter
{
      public bool Accepts(Type type)
      {
          return Nullable.GetUnderlyingType(type)?.IsEnum ?? false;
      }

      public object ReadYaml(IParser parser, Type type)
      {
          type = Nullable.GetUnderlyingType(type) ?? throw new ArgumentException("Expected nullable enum type for ReadYaml");
          var scalar = parser.Consume<Scalar>();

          if (string.IsNullOrWhiteSpace(scalar.Value))
          {
              return null;
          }

          try
          {
              return Enum.Parse(type, scalar.Value);
          }
          catch(Exception ex)
          {
              throw new Exception($"Invalid value: \"{scalar.Value}\" for {type.Name}", ex);
          }
      }

      public void WriteYaml(IEmitter emitter, object value, Type type)
      {
          type = Nullable.GetUnderlyingType(type) ?? throw new ArgumentException("Expected nullable enum type for WriteYaml");

          if (value != null)
          {
              var toWrite = Enum.GetName(type, value) ?? throw new InvalidOperationException($"Invalid value {value} for enum: {type}");
              emitter.Emit(new Scalar(null, null, toWrite, ScalarStyle.Any, true, false));
          }
     }
}

To use it: image

atruskie commented 3 years ago

@Trojaner - I improved your workaround slightly so it handles more null literal cases, and does a case-insensitive enum parse (as is the default in YamlDotNet).

private class YamlNullableEnumTypeConverter : IYamlTypeConverter
{
    public bool Accepts(Type type)
    {
        return Nullable.GetUnderlyingType(type)?.IsEnum ?? false;
    }

    public object ReadYaml(IParser parser, Type type)
    {
        type = Nullable.GetUnderlyingType(type) ?? throw new ArgumentException("Expected nullable enum type for ReadYaml");

        if (parser.Accept<NodeEvent>(out var @event))
        {
            if (NodeIsNull(@event))
            {
                parser.SkipThisAndNestedEvents();
                return null;
            }
        }

        var scalar = parser.Consume<Scalar>();
        try
        {
            return Enum.Parse(type, scalar.Value, ignoreCase: true);
        }
        catch (Exception ex)
        {
            throw new Exception($"Invalid value: \"{scalar.Value}\" for {type.Name}", ex);
        }
    }

    public void WriteYaml(IEmitter emitter, object value, Type type)
    {
        type = Nullable.GetUnderlyingType(type) ?? throw new ArgumentException("Expected nullable enum type for WriteYaml");

        if (value != null)
        {
            var toWrite = Enum.GetName(type, value) ?? throw new InvalidOperationException($"Invalid value {value} for enum: {type}");
            emitter.Emit(new Scalar(null, null, toWrite, ScalarStyle.Any, true, false));
        }
    }

    private static bool NodeIsNull(NodeEvent nodeEvent)
    {
        // http://yaml.org/type/null.html

        if (nodeEvent.Tag == "tag:yaml.org,2002:null")
        {
            return true;
        }

        if (nodeEvent is Scalar scalar && scalar.Style == ScalarStyle.Plain)
        {
            var value = scalar.Value;
            return value is "" or "~" or "null" or "Null" or "NULL";
        }

        return false;
    }
}
aaubry commented 3 years ago

There is a PR that should fix this issue. I have published a pre-release version containing this fix, named 11.1.3-nullable-enums-0003. Can someone test it and confirm that it fixes this issue? Thanks.

arturcic commented 3 years ago

I can confirm it works in GitVersion

aaubry commented 3 years ago

Apparently this issue got closed automatically, so I'm reopening it.

sgrassie commented 3 years ago

Confirming it works for me as well.

BernieWhite commented 3 years ago

@aaubry The fix works for PSRule as well.

aaubry commented 3 years ago

Thanks to everyone who took the time to test. A release that includes this fix is currently building.

aaubry commented 3 years ago

A fix for this issue has been released in version 11.1.1.

BernieWhite commented 3 years ago

Thanks @aaubry @pensono and community. Version 11.1.1 addresses the issue.