This repository was archived by the owner on Jun 13, 2025. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 29
fix: Add graphql max depth and aliases limits #955
Merged
Merged
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
32fa397
fix: Add graphql max depth and aliases limits
suejung-sentry 93053ff
Merge remote-tracking branch 'origin/main' into sshin/fix/max-depth
suejung-sentry 5f6cb7e
add types
suejung-sentry fff11f0
cleanup
suejung-sentry dcefcab
Merge remote-tracking branch 'origin/main' into sshin/fix/max-depth
suejung-sentry c1250c6
fix dict access pattern
suejung-sentry 1d5d33c
Merge remote-tracking branch 'origin/main' into sshin/fix/max-depth
suejung-sentry 2450670
incorporate ajay tips
suejung-sentry 9442105
Merge remote-tracking branch 'origin/main' into sshin/fix/max-depth
suejung-sentry 903ed78
Merge remote-tracking branch 'origin/main' into sshin/fix/max-depth
suejung-sentry File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,100 @@ | ||
| from graphql import ( | ||
| GraphQLField, | ||
| GraphQLObjectType, | ||
| GraphQLSchema, | ||
| GraphQLString, | ||
| parse, | ||
| validate, | ||
| ) | ||
|
|
||
| from ..validation import ( | ||
| create_max_aliases_rule, | ||
| create_max_depth_rule, | ||
| ) | ||
|
|
||
|
|
||
| def resolve_field(*args): | ||
| return "test" | ||
|
|
||
|
|
||
| QueryType = GraphQLObjectType( | ||
| "Query", {"field": GraphQLField(GraphQLString, resolve=resolve_field)} | ||
| ) | ||
| schema = GraphQLSchema(query=QueryType) | ||
|
|
||
|
|
||
| def validate_query(query, *rules): | ||
| ast = parse(query) | ||
| return validate(schema, ast, rules=rules) | ||
|
|
||
|
|
||
| def test_max_depth_rule_allows_within_depth(): | ||
| query = """ | ||
| query { | ||
| field | ||
| } | ||
| """ | ||
| errors = validate_query(query, create_max_depth_rule(2)) | ||
| assert not errors, "Expected no errors for depth within the limit" | ||
|
|
||
|
|
||
| def test_max_depth_rule_rejects_exceeding_depth(): | ||
| query = """ | ||
| query { | ||
| field { | ||
| field { | ||
| field | ||
| } | ||
| } | ||
| } | ||
| """ | ||
| errors = validate_query(query, create_max_depth_rule(2)) | ||
| assert errors, "Expected errors for exceeding depth limit" | ||
| assert any( | ||
| "Query depth exceeds the maximum allowed depth" in str(e) for e in errors | ||
| ) | ||
|
|
||
|
|
||
| def test_max_depth_rule_exact_depth(): | ||
| query = """ | ||
| query { | ||
| field | ||
| } | ||
| """ | ||
| errors = validate_query(query, create_max_depth_rule(2)) | ||
| assert not errors, "Expected no errors when query depth matches the limit" | ||
|
|
||
|
|
||
| def test_max_aliases_rule_allows_within_alias_limit(): | ||
| query = """ | ||
| query { | ||
| alias1: field | ||
| alias2: field | ||
| } | ||
| """ | ||
| errors = validate_query(query, create_max_aliases_rule(2)) | ||
| assert not errors, "Expected no errors for alias count within the limit" | ||
|
|
||
|
|
||
| def test_max_aliases_rule_rejects_exceeding_alias_limit(): | ||
| query = """ | ||
| query { | ||
| alias1: field | ||
| alias2: field | ||
| alias3: field | ||
| } | ||
| """ | ||
| errors = validate_query(query, create_max_aliases_rule(2)) | ||
| assert errors, "Expected errors for exceeding alias limit" | ||
| assert any("Query uses too many aliases" in str(e) for e in errors) | ||
|
|
||
|
|
||
| def test_max_aliases_rule_exact_alias_limit(): | ||
| query = """ | ||
| query { | ||
| alias1: field | ||
| alias2: field | ||
| } | ||
| """ | ||
| errors = validate_query(query, create_max_aliases_rule(2)) | ||
| assert not errors, "Expected no errors when alias count matches the limit" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| from typing import Any, Type | ||
|
|
||
| from graphql import GraphQLError, ValidationRule | ||
| from graphql.language.ast import DocumentNode, FieldNode, OperationDefinitionNode | ||
| from graphql.validation import ValidationContext | ||
|
|
||
|
|
||
| def create_max_depth_rule(max_depth: int) -> Type[ValidationRule]: | ||
| class MaxDepthRule(ValidationRule): | ||
| def __init__(self, context: ValidationContext) -> None: | ||
| super().__init__(context) | ||
| self.operation_depth: int = 1 | ||
| self.max_depth_reached: bool = False | ||
| self.max_depth: int = max_depth | ||
|
|
||
| def enter_operation_definition( | ||
| self, node: OperationDefinitionNode, *_args: Any | ||
| ) -> None: | ||
| self.operation_depth = 1 | ||
| self.max_depth_reached = False | ||
|
|
||
| def enter_field(self, node: FieldNode, *_args: Any) -> None: | ||
| self.operation_depth += 1 | ||
|
|
||
| if self.operation_depth > self.max_depth and not self.max_depth_reached: | ||
| self.max_depth_reached = True | ||
| self.report_error( | ||
| GraphQLError( | ||
| "Query depth exceeds the maximum allowed depth", | ||
| node, | ||
| ) | ||
| ) | ||
|
|
||
| def leave_field(self, node: FieldNode, *_args: Any) -> None: | ||
| self.operation_depth -= 1 | ||
|
|
||
| return MaxDepthRule | ||
|
|
||
|
|
||
| def create_max_aliases_rule(max_aliases: int) -> Type[ValidationRule]: | ||
| class MaxAliasesRule(ValidationRule): | ||
| def __init__(self, context: ValidationContext) -> None: | ||
| super().__init__(context) | ||
| self.alias_count: int = 0 | ||
| self.has_reported_error: bool = False | ||
| self.max_aliases: int = max_aliases | ||
|
|
||
| def enter_document(self, node: DocumentNode, *_args: Any) -> None: | ||
| self.alias_count = 0 | ||
| self.has_reported_error = False | ||
|
|
||
| def enter_field(self, node: FieldNode, *_args: Any) -> None: | ||
| if node.alias: | ||
| self.alias_count += 1 | ||
|
|
||
| if self.alias_count > self.max_aliases and not self.has_reported_error: | ||
| self.has_reported_error = True | ||
| self.report_error( | ||
| GraphQLError( | ||
| "Query uses too many aliases", | ||
| node, | ||
| ) | ||
| ) | ||
|
|
||
| return MaxAliasesRule | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just curious, are these functions base functions you had to override default behavior of?
Similar story with enter_field, leave_field, and enter_document
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup! Though I don't think about it so much as "override" as "implement the interface" that is defined here. The function names
enter_Xandleave_Xare dynamic per here. And for any method that's not implemented explicitly, it behaves as a no-op (here).this is the stuff I looked at in Ariadne doc & this example implementation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very cool! Thanks for linking all those docs, that was a fun set of reads