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

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

Closed
BernieWhite opened this issue Nov 22, 2020 · 15 comments · Fixed by #600
Closed

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

BernieWhite opened this issue Nov 22, 2020 · 15 comments · Fixed by #600
Labels

Comments

@BernieWhite
Copy link

BernieWhite commented Nov 22, 2020

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
Copy link
Owner

aaubry commented Nov 22, 2020

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
Copy link

naefp commented Nov 28, 2020

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
Copy link

maxgira commented Dec 1, 2020

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
Copy link

brian-reichle commented Dec 7, 2020

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
Copy link

BlowaXD commented Dec 19, 2020

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
Copy link

Trojaner commented Jan 17, 2021

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

Trojaner added a commit to openmod/openmod that referenced this issue Jan 17, 2021
@atruskie
Copy link
Contributor

@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
Copy link
Owner

aaubry commented Apr 9, 2021

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
Copy link

arturcic commented Apr 9, 2021

I can confirm it works in GitVersion

@aaubry
Copy link
Owner

aaubry commented Apr 9, 2021

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

@aaubry aaubry reopened this Apr 9, 2021
@sgrassie
Copy link

sgrassie commented Apr 9, 2021

Confirming it works for me as well.

@BernieWhite
Copy link
Author

@aaubry The fix works for PSRule as well.

@aaubry
Copy link
Owner

aaubry commented Apr 9, 2021

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

@aaubry
Copy link
Owner

aaubry commented Apr 9, 2021

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

@BernieWhite
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants