From e6ba87c7b823b8bd581e98f79331c0882392ee82 Mon Sep 17 00:00:00 2001 From: Michael Staib Date: Tue, 16 Jun 2026 09:23:20 +0200 Subject: [PATCH] Add Filter Operation Limit --- .../Core/src/Abstractions/ErrorCodes.cs | 1 + src/HotChocolate/Data/src/Data/ErrorHelper.cs | 16 ++ .../FilterConventionDescriptorExtensions.cs | 1 + .../Filters/Convention/FilterConvention.cs | 7 +- .../Convention/FilterConventionDefinition.cs | 3 + .../Convention/FilterConventionDescriptor.cs | 16 ++ .../Convention/FilterConventionExtension.cs | 6 + .../Filters/Convention/IFilterConvention.cs | 5 + .../Convention/IFilterConventionDescriptor.cs | 8 + .../Expressions/QueryableCombinator.cs | 44 ++++- .../Expressions/QueryableFilterProvider.cs | 89 +++++++++ .../Data/Filters/Visitor/FilterProvider.cs | 3 + .../QueryableFilterVisitorVariablesTests.cs | 185 ++++++++++++++++++ .../SchemaCache.cs | 12 ++ .../QueryableFilterVisitorStringTests.cs | 33 ++++ 15 files changed, 419 insertions(+), 10 deletions(-) diff --git a/src/HotChocolate/Core/src/Abstractions/ErrorCodes.cs b/src/HotChocolate/Core/src/Abstractions/ErrorCodes.cs index d18b87831d1..117b434e014 100644 --- a/src/HotChocolate/Core/src/Abstractions/ErrorCodes.cs +++ b/src/HotChocolate/Core/src/Abstractions/ErrorCodes.cs @@ -295,6 +295,7 @@ public static class Data public const string FilteringProjectionFailed = "HC0023"; public const string SortingProjectionFailed = "HC0024"; public const string NoPaginationProviderFound = "HC0025"; + public const string MaxFilterOperationsExceeded = "HC0117"; /// /// Type does not contain a valid node field. Only `items` and `nodes` are supported diff --git a/src/HotChocolate/Data/src/Data/ErrorHelper.cs b/src/HotChocolate/Data/src/Data/ErrorHelper.cs index 6762ec90aa0..94ce103d888 100644 --- a/src/HotChocolate/Data/src/Data/ErrorHelper.cs +++ b/src/HotChocolate/Data/src/Data/ErrorHelper.cs @@ -46,6 +46,22 @@ public static IError SortingVisitor_ListValues(ISortField field, ListValueNode n .SetExtension(nameof(field), field) .Build(); + public static IError MaxAllowedFilterOperationsExceeded( + IValueNode node, + int filterOperations, + int maxAllowedFilterOperations) => + ErrorBuilder.New() + .SetMessage( + "The filter argument contains {0} operations, which exceeds the maximum allowed " + + "number of {1}.", + filterOperations, + maxAllowedFilterOperations) + .AddLocation(node) + .SetCode(ErrorCodes.Data.MaxFilterOperationsExceeded) + .SetExtension(nameof(filterOperations), filterOperations) + .SetExtension(nameof(maxAllowedFilterOperations), maxAllowedFilterOperations) + .Build(); + public static IError CreateNonNullError( ISortField field, IValueNode value, diff --git a/src/HotChocolate/Data/src/Data/Filters/Convention/Extensions/FilterConventionDescriptorExtensions.cs b/src/HotChocolate/Data/src/Data/Filters/Convention/Extensions/FilterConventionDescriptorExtensions.cs index 11e2d40c763..0c94cf9e6d5 100644 --- a/src/HotChocolate/Data/src/Data/Filters/Convention/Extensions/FilterConventionDescriptorExtensions.cs +++ b/src/HotChocolate/Data/src/Data/Filters/Convention/Extensions/FilterConventionDescriptorExtensions.cs @@ -27,6 +27,7 @@ public static IFilterConventionDescriptor AddDefaults( descriptor .AddDefaultOperations() .BindDefaultTypes(compatibilityMode) + .MaxAllowedFilterOperations(FilterConventionDefinition.DefaultMaxAllowedFilterOperations) .UseQueryableProvider(); /// diff --git a/src/HotChocolate/Data/src/Data/Filters/Convention/FilterConvention.cs b/src/HotChocolate/Data/src/Data/Filters/Convention/FilterConvention.cs index 6faa70f82b8..ea1bfa1087b 100644 --- a/src/HotChocolate/Data/src/Data/Filters/Convention/FilterConvention.cs +++ b/src/HotChocolate/Data/src/Data/Filters/Convention/FilterConvention.cs @@ -14,7 +14,7 @@ namespace HotChocolate.Data.Filters; /// public class FilterConvention : Convention - , IFilterConvention + , IFilterConvention { private const string _inputPostFix = "FilterInput"; private const string _inputTypePostFix = "FilterInputType"; @@ -28,6 +28,7 @@ public class FilterConvention private string _argumentName = default!; private IFilterProvider _provider = default!; private ITypeInspector _typeInspector = default!; + private int? _maxAllowedFilterOperations; private bool _useAnd; private bool _useOr; @@ -99,6 +100,7 @@ protected internal override void Complete(IConventionContext context) _bindings = Definition.Bindings; _configs = Definition.Configurations; _argumentName = Definition.ArgumentName; + _maxAllowedFilterOperations = Definition.MaxAllowedFilterOperations; _useAnd = Definition.UseAnd; _useOr = Definition.UseOr; @@ -234,6 +236,9 @@ public string GetOperationName(int operation) /// public string GetArgumentName() => _argumentName; + /// + public int? GetMaxAllowedFilterOperations() => _maxAllowedFilterOperations; + /// public void ApplyConfigurations( TypeReference typeReference, diff --git a/src/HotChocolate/Data/src/Data/Filters/Convention/FilterConventionDefinition.cs b/src/HotChocolate/Data/src/Data/Filters/Convention/FilterConventionDefinition.cs index 1326275644f..45d20d066a7 100644 --- a/src/HotChocolate/Data/src/Data/Filters/Convention/FilterConventionDefinition.cs +++ b/src/HotChocolate/Data/src/Data/Filters/Convention/FilterConventionDefinition.cs @@ -7,6 +7,7 @@ namespace HotChocolate.Data.Filters; public class FilterConventionDefinition : IHasScope { public static readonly string DefaultArgumentName = "where"; + public const int DefaultMaxAllowedFilterOperations = 64; private string _argumentName = DefaultArgumentName; public string? Scope { get; set; } @@ -33,6 +34,8 @@ public string ArgumentName public List ProviderExtensionsTypes { get; } = []; + public int? MaxAllowedFilterOperations { get; set; } + public bool UseOr { get; set; } = true; public bool UseAnd { get; set; } = true; diff --git a/src/HotChocolate/Data/src/Data/Filters/Convention/FilterConventionDescriptor.cs b/src/HotChocolate/Data/src/Data/Filters/Convention/FilterConventionDescriptor.cs index 3dc9a553aea..c98899f0bbc 100644 --- a/src/HotChocolate/Data/src/Data/Filters/Convention/FilterConventionDescriptor.cs +++ b/src/HotChocolate/Data/src/Data/Filters/Convention/FilterConventionDescriptor.cs @@ -162,6 +162,22 @@ public IFilterConventionDescriptor ArgumentName(string argumentName) return this; } + /// + public IFilterConventionDescriptor MaxAllowedFilterOperations(int? maxAllowedFilterOperations) + { + if (maxAllowedFilterOperations is < 1) + { + throw new ArgumentOutOfRangeException( + nameof(maxAllowedFilterOperations), + maxAllowedFilterOperations, + "The maximum number of filter operations must be greater than zero."); + } + + Definition.MaxAllowedFilterOperations = maxAllowedFilterOperations; + + return this; + } + public IFilterConventionDescriptor AddProviderExtension() where TExtension : class, IFilterProviderExtension { diff --git a/src/HotChocolate/Data/src/Data/Filters/Convention/FilterConventionExtension.cs b/src/HotChocolate/Data/src/Data/Filters/Convention/FilterConventionExtension.cs index 1db6c3c91dd..05cfe2fc749 100644 --- a/src/HotChocolate/Data/src/Data/Filters/Convention/FilterConventionExtension.cs +++ b/src/HotChocolate/Data/src/Data/Filters/Convention/FilterConventionExtension.cs @@ -77,6 +77,12 @@ Definition is not null && filterConvention.Definition.ArgumentName = Definition.ArgumentName; } + if (Definition.MaxAllowedFilterOperations.HasValue) + { + filterConvention.Definition.MaxAllowedFilterOperations = + Definition.MaxAllowedFilterOperations; + } + if (Definition.Provider is not null) { filterConvention.Definition.Provider = Definition.Provider; diff --git a/src/HotChocolate/Data/src/Data/Filters/Convention/IFilterConvention.cs b/src/HotChocolate/Data/src/Data/Filters/Convention/IFilterConvention.cs index db0292bb7d8..dc96735b85a 100644 --- a/src/HotChocolate/Data/src/Data/Filters/Convention/IFilterConvention.cs +++ b/src/HotChocolate/Data/src/Data/Filters/Convention/IFilterConvention.cs @@ -98,6 +98,11 @@ public interface IFilterConvention : IConvention /// string GetArgumentName(); + /// + /// Gets the maximum number of filter operations allowed in a single filter argument. + /// + int? GetMaxAllowedFilterOperations(); + /// /// Applies configurations to a filter type. /// diff --git a/src/HotChocolate/Data/src/Data/Filters/Convention/IFilterConventionDescriptor.cs b/src/HotChocolate/Data/src/Data/Filters/Convention/IFilterConventionDescriptor.cs index c70e860374c..edfc724e4dc 100644 --- a/src/HotChocolate/Data/src/Data/Filters/Convention/IFilterConventionDescriptor.cs +++ b/src/HotChocolate/Data/src/Data/Filters/Convention/IFilterConventionDescriptor.cs @@ -94,6 +94,14 @@ IFilterConventionDescriptor Provider(TProvider provider) /// IFilterConventionDescriptor ArgumentName(string argumentName); + /// + /// Defines the maximum number of filter operations allowed in a single filter argument. + /// + /// + /// The maximum number of filter operations. If null, no limit is applied. + /// + IFilterConventionDescriptor MaxAllowedFilterOperations(int? maxAllowedFilterOperations); + /// /// Add a extensions that is applied to /// diff --git a/src/HotChocolate/Data/src/Data/Filters/Expressions/QueryableCombinator.cs b/src/HotChocolate/Data/src/Data/Filters/Expressions/QueryableCombinator.cs index a349592a76c..6e5b7aa2f66 100644 --- a/src/HotChocolate/Data/src/Data/Filters/Expressions/QueryableCombinator.cs +++ b/src/HotChocolate/Data/src/Data/Filters/Expressions/QueryableCombinator.cs @@ -18,19 +18,45 @@ public override bool TryCombineOperations( return false; } - combined = operations.Dequeue(); + combined = CombineOperations(operations, combinator); - while (operations.Count > 0) + return true; + } + + private Expression CombineOperations( + Queue operations, + FilterCombinator combinator) + { + while (operations.Count > 1) { - combined = combinator switch + var operationCount = operations.Count; + var pairCount = operationCount / 2; + + for (var i = 0; i < pairCount; i++) { - FilterCombinator.And => Expression.AndAlso(combined, operations.Dequeue()), - FilterCombinator.Or => Expression.OrElse(combined, operations.Dequeue()), - _ => throw ThrowHelper - .Filtering_QueryableCombinator_InvalidCombinator(this, combinator), - }; + var left = operations.Dequeue(); + var right = operations.Dequeue(); + + operations.Enqueue(Combine(left, right, combinator)); + } + + if ((operationCount & 1) == 1) + { + operations.Enqueue(operations.Dequeue()); + } } - return true; + return operations.Dequeue(); } + + private BinaryExpression Combine( + Expression left, + Expression right, + FilterCombinator combinator) + => combinator switch + { + FilterCombinator.And => Expression.AndAlso(left, right), + FilterCombinator.Or => Expression.OrElse(left, right), + _ => throw ThrowHelper.Filtering_QueryableCombinator_InvalidCombinator(this, combinator), + }; } diff --git a/src/HotChocolate/Data/src/Data/Filters/Expressions/QueryableFilterProvider.cs b/src/HotChocolate/Data/src/Data/Filters/Expressions/QueryableFilterProvider.cs index 38c00d9adbe..05bcc7e14a6 100644 --- a/src/HotChocolate/Data/src/Data/Filters/Expressions/QueryableFilterProvider.cs +++ b/src/HotChocolate/Data/src/Data/Filters/Expressions/QueryableFilterProvider.cs @@ -100,6 +100,7 @@ public override void ConfigureField( string argumentName, IObjectFieldDescriptor descriptor) { + var maxAllowedFilterOperations = FilterConvention.GetMaxAllowedFilterOperations(); var contextData = descriptor.Extend().Definition.ContextData; var argumentKey = (VisitFilterArgument)VisitFilterArgumentExecutor; contextData[ContextVisitFilterArgumentKey] = argumentKey; @@ -111,6 +112,11 @@ QueryableFilterContext VisitFilterArgumentExecutor( IFilterInputType filterInput, bool inMemory) { + if (maxAllowedFilterOperations is { } maxAllowed) + { + ValidateFilterOperations(valueNode, filterInput, maxAllowed); + } + var visitorContext = new QueryableFilterContext(filterInput, inMemory); // rewrite GraphQL input object into expression tree. @@ -120,6 +126,89 @@ QueryableFilterContext VisitFilterArgumentExecutor( } } + private static void ValidateFilterOperations( + IValueNode valueNode, + IFilterInputType filterInput, + int maxAllowedFilterOperations) + { + var filterOperations = 0; + var stack = new Stack<(IValueNode Value, IInputType Type)>(); + stack.Push((valueNode, filterInput)); + + while (stack.Count > 0) + { + var (value, type) = stack.Pop(); + + switch (value) + { + case ObjectValueNode objectValue + when type.NamedType() is InputObjectType inputObject: + for (var i = objectValue.Fields.Count - 1; i >= 0; i--) + { + var fieldValue = objectValue.Fields[i]; + + if (!inputObject.Fields.TryGetField(fieldValue.Name.Value, out var field)) + { + continue; + } + + if (field is IFilterOperationField and not IAndField and not IOrField) + { + filterOperations++; + + if (filterOperations > maxAllowedFilterOperations) + { + throw new GraphQLException( + ErrorHelper.MaxAllowedFilterOperationsExceeded( + fieldValue.Value, + filterOperations, + maxAllowedFilterOperations)); + } + } + + if (CanContainFilterOperations(field.Type, fieldValue.Value)) + { + stack.Push((fieldValue.Value, field.Type)); + } + } + + break; + + case ListValueNode listValue + when TryGetListElementType(type, out var elementType): + for (var i = listValue.Items.Count - 1; i >= 0; i--) + { + stack.Push((listValue.Items[i], elementType)); + } + + break; + } + } + } + + private static bool CanContainFilterOperations(IInputType type, IValueNode value) + => value switch + { + ObjectValueNode => type.NamedType() is InputObjectType, + ListValueNode => TryGetListElementType(type, out var elementType) + && elementType.NamedType() is InputObjectType, + _ => false, + }; + + private static bool TryGetListElementType( + IInputType type, + [NotNullWhen(true)] out IInputType? elementType) + { + if (type.IsListType() && type.ElementType() is IInputType inputType) + { + elementType = inputType; + return true; + } + + elementType = null; + return false; + } + /// public override IFilterMetadata? CreateMetaData( ITypeCompletionContext context, diff --git a/src/HotChocolate/Data/src/Data/Filters/Visitor/FilterProvider.cs b/src/HotChocolate/Data/src/Data/Filters/Visitor/FilterProvider.cs index 5c712a8858e..b55378f90f3 100644 --- a/src/HotChocolate/Data/src/Data/Filters/Visitor/FilterProvider.cs +++ b/src/HotChocolate/Data/src/Data/Filters/Visitor/FilterProvider.cs @@ -38,6 +38,9 @@ public FilterProvider(Action> configure) /// public IReadOnlyCollection FieldHandlers => _fieldHandlers; + protected IFilterConvention FilterConvention + => _filterConvention ?? throw FilterConvention_ProviderHasToBeInitializedByConvention(GetType(), Scope); + /// protected override FilterProviderDefinition CreateDefinition(IConventionContext context) { diff --git a/src/HotChocolate/Data/test/Data.Filters.InMemory.Tests/QueryableFilterVisitorVariablesTests.cs b/src/HotChocolate/Data/test/Data.Filters.InMemory.Tests/QueryableFilterVisitorVariablesTests.cs index acdd63a30c9..f8973c3b11b 100644 --- a/src/HotChocolate/Data/test/Data.Filters.InMemory.Tests/QueryableFilterVisitorVariablesTests.cs +++ b/src/HotChocolate/Data/test/Data.Filters.InMemory.Tests/QueryableFilterVisitorVariablesTests.cs @@ -68,6 +68,191 @@ await Snapshot .MatchAsync(); } + [Fact] + public async Task ExecuteAsync_Should_Error_When_FilterVariableExceedsMaxAllowedOperations() + { + // arrange + var convention = new FilterConvention( + x => x + .AddDefaults() + .BindRuntimeType() + .MaxAllowedFilterOperations(2)); + var tester = cache.CreateSchemaWithConvention(_fooEntities, convention); + const string query = + """ + query Test($where: FooFilterInput) { + root(where: $where) { + bar + } + } + """; + + // act + var result = await tester.ExecuteAsync( + OperationRequestBuilder.New() + .SetDocument(query) + .SetVariableValues(new Dictionary + { + { "where", CreateOrFilter(3) }, + }) + .Build()); + + // assert + result.MatchInlineSnapshot( + """ + { + "errors": [ + { + "message": "The filter argument contains 3 operations, which exceeds the maximum allowed number of 2.", + "locations": [ + { + "line": 2, + "column": 3 + } + ], + "path": [ + "root" + ], + "extensions": { + "code": "HC0117", + "filterOperations": 3, + "maxAllowedFilterOperations": 2 + } + } + ], + "data": { + "root": null + } + } + """); + } + + [Fact] + public async Task ExecuteAsync_Should_Error_When_FilterVariableExceedsDefaultMaxAllowedOperations() + { + // arrange + var tester = cache.CreateSchema(_fooEntities); + const string query = + """ + query Test($where: FooFilterInput) { + root(where: $where) { + bar + } + } + """; + + // act + var result = await tester.ExecuteAsync( + OperationRequestBuilder.New() + .SetDocument(query) + .SetVariableValues(new Dictionary + { + { + "where", + CreateOrFilter( + FilterConventionDefinition.DefaultMaxAllowedFilterOperations + 1) + }, + }) + .Build()); + + // assert + result.MatchInlineSnapshot( + """ + { + "errors": [ + { + "message": "The filter argument contains 65 operations, which exceeds the maximum allowed number of 64.", + "locations": [ + { + "line": 2, + "column": 3 + } + ], + "path": [ + "root" + ], + "extensions": { + "code": "HC0117", + "filterOperations": 65, + "maxAllowedFilterOperations": 64 + } + } + ], + "data": { + "root": null + } + } + """); + } + + [Fact] + public async Task ExecuteAsync_Should_NotOverflow_When_FilterVariableContainsLargeOrArray() + { + // arrange + var convention = new FilterConvention( + x => x + .AddDefaults() + .BindRuntimeType() + .MaxAllowedFilterOperations(null)); + var tester = cache.CreateSchemaWithConvention(_fooEntities, convention); + const string query = + """ + query Test($where: FooFilterInput) { + root(where: $where) { + bar + } + } + """; + + // act + var result = await tester.ExecuteAsync( + OperationRequestBuilder.New() + .SetDocument(query) + .SetVariableValues(new Dictionary + { + { "where", CreateOrFilter(5_000) }, + }) + .Build()); + + // assert + result.MatchInlineSnapshot( + """ + { + "data": { + "root": [ + { + "bar": true + } + ] + } + } + """); + } + + private static Dictionary CreateOrFilter(int itemCount) + { + var items = new object?[itemCount]; + + for (var i = 0; i < items.Length; i++) + { + items[i] = new Dictionary + { + { + "bar", + new Dictionary + { + { "eq", true }, + } + }, + }; + } + + return new Dictionary + { + { "or", items }, + }; + } + public class Foo { public int Id { get; set; } diff --git a/src/HotChocolate/Data/test/Data.Filters.InMemory.Tests/SchemaCache.cs b/src/HotChocolate/Data/test/Data.Filters.InMemory.Tests/SchemaCache.cs index d1d9f363280..d15a9c65800 100644 --- a/src/HotChocolate/Data/test/Data.Filters.InMemory.Tests/SchemaCache.cs +++ b/src/HotChocolate/Data/test/Data.Filters.InMemory.Tests/SchemaCache.cs @@ -24,6 +24,18 @@ public IRequestExecutor CreateSchema( configure: configure)); } + public IRequestExecutor CreateSchemaWithConvention( + T[] entities, + FilterConvention convention, + bool withPaging = false, + Action? configure = null) + where TType : FilterInputType + => base.CreateSchema( + entities, + convention, + withPaging, + configure); + public void Dispose() { } diff --git a/src/HotChocolate/Data/test/Data.Filters.Tests/Expression/QueryableFilterVisitorStringTests.cs b/src/HotChocolate/Data/test/Data.Filters.Tests/Expression/QueryableFilterVisitorStringTests.cs index b1d90d50b26..2e15c35deb7 100644 --- a/src/HotChocolate/Data/test/Data.Filters.Tests/Expression/QueryableFilterVisitorStringTests.cs +++ b/src/HotChocolate/Data/test/Data.Filters.Tests/Expression/QueryableFilterVisitorStringTests.cs @@ -210,6 +210,39 @@ public void Create_StringNotEndsWith_Expression() Assert.False(func(b)); } + [Fact] + public void Build_Should_NotOverflow_When_OrContainsManyItems() + { + // arrange + var value = CreateOrFilter(10_000); + var tester = CreateProviderTester(new FooFilterInput()); + + // act + var func = tester.Build(value); + + // assert + Assert.True(func(new Foo { Bar = "a", })); + Assert.False(func(new Foo { Bar = "b", })); + } + + private static ObjectValueNode CreateOrFilter(int itemCount) + { + var items = new IValueNode[itemCount]; + + for (var i = 0; i < items.Length; i++) + { + items[i] = + new ObjectValueNode( + new ObjectFieldNode( + "bar", + new ObjectValueNode( + new ObjectFieldNode("eq", "a")))); + } + + return new ObjectValueNode( + new ObjectFieldNode("or", new ListValueNode(items))); + } + public class Foo { public string? Bar { get; set; }