HotChocolate.Types.Filters Null Reference issue

[olu-lebara]

Hello,
I use GraphQL HotChocolate nuget (HotChocolate.AspNetCore and HotChocolate.Types.Filters) of version 11.0.9 (need to use quite old, but reproduced even in 13.9.14) with enabled filtering capabilities for collections.
In general, filtering works well, but for some queries it causes NullReferenceException during filtering (middleware stage).

Bit details (I've created a small project to reproduce this issue):

public class Query
{
    [UseFiltering]
    public IEnumerable<Item> GetItems()
    {
        return [
            new Item
            {
                Id = 1,
                ItemSetting = new ItemSetting
                {
                    Data = 123
                },
                Categories = [ new Category
                {
                    Name = "Cat1"
                }]
            },
            new Item
            {
                Id = 2,
                ItemSetting = null,
                Categories = [ new Category
                {
                    Name = "Cat2"
                }]
            }
        ];
    }
}

public class Item
{
    public int Id { get; set; }

    public ItemSetting ItemSetting { get; set; }

    public IEnumerable<Category> Categories { get; set; }
}

public class ItemSetting
{
    public int Data { get; set; }
}

public class Category
{
    public string Name { get; set; }
}

And query, which causes the error:
query { items(where: {categories_some: {name: "Cat1"}, itemSetting: {data: 123}}) { id itemSetting { data } categories { name } } }

Error itself:
{ "errors": [ { "message": "Unexpected Execution Error", "locations": [ { "line": 2, "column": 3 } ], "path": [ "items" ], "extensions": { "message": "Object reference not set to an instance of an object.", "stackTrace": " at lambda_method83(Closure, Item)\r\n at System.Linq.Enumerable.WhereEnumerableIterator 1.MoveNext()\r\n at HotChocolate.Execution.Processing.ResolverTask.<>c__DisplayClass13_0.<ExecuteResolverPipelineAsync>b__0()\r\n at System.Threading.Tasks.Task 1.InnerInvoke()\r\n at System.Threading.ExecutionContext.RunFromThreadPoolDispatchLoop(Thread threadPoolThread, ExecutionContext executionContext, ContextCallback callback, Object state)\r\n--- End of stack trace from previous location ---\r\n at System.Threading.ExecutionContext.RunFromThreadPoolDispatchLoop(Thread threadPoolThread, ExecutionContext executionContext, ContextCallback callback, Object state)\r\n at System.Threading.Tasks.Task.ExecuteWithThreadLocal(Task& currentTaskSlot, Thread threadPoolThread)\r\n--- End of stack trace from previous location ---\r\n at HotChocolate.Execution.Processing.ResolverTask.ExecuteResolverPipelineAsync(CancellationToken cancellationToken)\r\n at HotChocolate.Execution.Processing.ResolverTask.TryExecuteAsync(CancellationToken cancellationToken)" } } ], "data": { "items": null } }

The strange thing is, if I change the order of filter condition, no issue:
items(where: { itemSetting: {data: 123}, categories_some: {name: "Cat1"}})

I've bit investigated, and looks like QueryableFilterVisitor creates Expression for IQueriable with wrong order (first it checks value of Data, and then it checks ItemSetting object is not null).

I have a small project to reproduce this issue if needed (https://github.com/olu-lebara/FilterIssueTest).
Please help to find out any fix. Because workaround to change order is not applicable in my case (clients do not know about this issue).

Thanks.

[olu-lebara]

I've loaded code from GitHub and found kind of easy fix (not sure it will not cause new issues).
If in class QueryableFilterVisitor change private method TryCombineOperations from

                while (operations.Count != 0)
                {
                    combined = combine(combined, operations.Dequeue());
                }

to

                while (operations.Count != 0)
                {
                    combined = combine(operations.Dequeue(), combined);
                }

It works for both filters.
Looks like A && B && C it represents reverted like C && B && A.
That's why condition first check ItemSetting.Data value and only then ItemSetting != null

[olu-lebara]

Expression for IQueryable without fix:

.Lambda #Lambda1<System.Func`2[FilterIssueTest.GraphQl.Item,System.Boolean]>(FilterIssueTest.GraphQl.Item $r) {
    $r != null && ($r.ItemSetting).Data == 123 && $r.ItemSetting != null && $r.Categories != null && .Call System.Linq.Enumerable.Any(
        $r.Categories,
        .Lambda #Lambda2<System.Func`2[FilterIssueTest.GraphQl.Category,System.Boolean]>)
}

.Lambda #Lambda2<System.Func`2[FilterIssueTest.GraphQl.Category,System.Boolean]>(FilterIssueTest.GraphQl.Category $_s1) {
    $_s1 != null && $_s1.Name == "Cat1"
}

And with fix:

.Lambda #Lambda1<System.Func`2[FilterIssueTest.GraphQl.Item,System.Boolean]>(FilterIssueTest.GraphQl.Item $r) {
    $r != null && $r.ItemSetting != null && $r.Categories != null && .Call System.Linq.Enumerable.Any(
        $r.Categories,
        .Lambda #Lambda2<System.Func`2[FilterIssueTest.GraphQl.Category,System.Boolean]>) && ($r.ItemSetting).Data == 123
}

.Lambda #Lambda2<System.Func`2[FilterIssueTest.GraphQl.Category,System.Boolean]>(FilterIssueTest.GraphQl.Category $_s1) {
    $_s1 != null && $_s1.Name == "Cat1"
}

In first case ($r.ItemSetting).Data == 123 is before $r.ItemSetting != null. In second case order is correct.

[olu-lebara]

PR with possible fix (for test repo): olu-lebara/FilterIssueTest#1
Basically, only 1 line changed in inherited class (change is in private method, that's why need to override 2 protected methods with no changes) and forced to use custom visitor.