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

JsonLogic v3.1.0 breaks null checks #313

Closed
ghost opened this issue Aug 4, 2022 · 5 comments · Fixed by #314
Closed

JsonLogic v3.1.0 breaks null checks #313

ghost opened this issue Aug 4, 2022 · 5 comments · Fixed by #314
Labels
bug Something isn't working

Comments

@ghost
Copy link

ghost commented Aug 4, 2022

Environment

  • Nuget Package: JsonLogic
  • Nuget Version: 3.1.0
  • OS: Windows 11 64 Bit
  • .Net Target: .net core 3.1

Describe the bug
Operations like "!=", "==" and possible more against or with null are broken after upgrading to v3.1.0. They were working in v3.0.0
See the code snipped in the "To Reproduce" section for an example. The code there throws an exception like this:

Object reference not set to an instance of an object.
   at Json.Logic.Rules.LooseEqualsRule.Apply(JsonNode data, JsonNode contextData)
   at async Program.<Initialize>(?) in :line 17
   at Program.<Main>()

To Reproduce
Code to reproduce in RoslynPad or the likes:

#r "nuget:System.Text.Json/6.0.5"
#r "nuget:JsonLogic/3.1.0"

using System.Text.Json;
using System.Text.Json.Nodes;
using Json.Logic;

var node = JsonNode.Parse(@"
{
        ""=="": [
            {""var"": ""value""},
            null
        ]
}");

var rule = System.Text.Json.JsonSerializer.Deserialize<Rule>(node.ToJsonString());
rule.Apply(JsonNode.Parse("{\"value\": null}"));

Expected behavior
The .Apply() call should return true for the test case

Additional context
The issue doesn't exist in v3.0.0 but in 3.0.0 other things are broken therefore we've upgraded to 3.1.0

@ghost ghost added the bug Something isn't working label Aug 4, 2022
@gregsdennis
Copy link
Owner

Thanks for the report. This seems to be related to say least another issue.

I'll dig in tonight.

@ghost
Copy link
Author

ghost commented Aug 5, 2022

Thanks for the report. This seems to be related to say least another issue.

I'll dig in tonight.

Thanks, if you need anything let me know. I'm located in Europe and my day just started :)

@gregsdennis
Copy link
Owner

Minor suggestion: the .ToJsonString() isn't necessary. JsonNode has a deserialization extension method on it that lets you change

var rule = System.Text.Json.JsonSerializer.Deserialize<Rule>(node.ToJsonString());

to

var rule = node.Deserialize<Rule>();

I've replicated the issue, though. Continuing to dig.

@gregsdennis
Copy link
Owner

Okay. Props to the .Net team for anticipating this one.

By default, the serializer skips the JSON converter for reference types when the value is null and just "deserializes" a null. This was causing the arguments for the == rule to deserialize (expecting a Rule[]) to [ <VariableRule>, null ], and I couldn't handle it in the Rule converter (because it was being skipped).

But they put in a virtual property on the converter base called HandleNull. I've set this to true, and now the Rule converter can handle the null and coerce it to another VariableRule with a null value.

This fixes the issue of throwing the exception, and the rule now returns true.

I'll have an update out shortly.

@ghost
Copy link
Author

ghost commented Aug 5, 2022

Thanks a lot! We really appreciate your efforts and especially the very quick turn-around time. It has been a pleasure using your libraries
MotiHeartsGIF

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant