SAHB.GraphQLClient icon indicating copy to clipboard operation
SAHB.GraphQLClient copied to clipboard

Null dereference bug during deserialization

Open WesToleman opened this issue 4 years ago • 0 comments

I have found a scenario that causes a null dereference in GraphQLDeserilization.cs:L92.

This response causes the error when a GraphQLUnionOrInterface is involved yet works fine when there is no inheritance.

{
  "data": {
    "foo": null
  }
}

E.g. FooQuery works fine and Foo is null. FooBarQuery causes an issue in the deserializer.

public class FooQuery
{
    [GraphQLArguments("id", "String!", "fooId", isRequired: true, inlineArgument: false)]
    public Foo Foo { get; set; }
}

public class FooBarQuery
{
    [GraphQLArguments("id", "String!", "fooId", isRequired: true, inlineArgument: false)]
    [GraphQLUnionOrInterface("FooBar", typeof(FooBar))]
    public Foo Foo { get; set; }
}

public class Foo
{
    public string Id { get; set; }
}

public class FooBar : Foo
{
    public string Bar { get; set; }
}

Client fix

It's possible to fix the client by annotating the potentially null value with [JsonProperty(NullValueHandling = NullValueHandling.Ignore)].

public class FooBarQuery
{
    [JsonProperty(NullValueHandling = NullValueHandling.Ignore)]
    [GraphQLArguments("id", "String!", "fooId", isRequired: true, inlineArgument: false)]
    [GraphQLUnionOrInterface("FooBar", typeof(FooBar))]
    public Foo Foo { get; set; }
}

Possible library fix

It's possible to globally specify how nulls and missing values should be handled by the deserializer. I'm not sure of the full implications of this but I believe ignoring nulls may be safe considering default nullability of GraphQL types. Missing members may mask errors though.

--- GraphQLDeserilization.cs
+++ GraphQLDeserilization.cs
@@ -22,7 +22,11 @@
             var converters = GetFieldConverters(fields);

             // Get JsonSerilizerSettings
-            var settings = new JsonSerializerSettings();
+            var settings = new JsonSerializerSettings
+            {
+                NullValueHandling = NullValueHandling.Ignore,
+                MissingMemberHandling = MissingMemberHandling.Ignore,
+            };

             foreach (var converter in converters)
             {

WesToleman avatar Jul 16 '20 03:07 WesToleman