From 8f8c6d1f90c54ed127c04bce40090092ddf9cdb9 Mon Sep 17 00:00:00 2001 From: dondonz <13839920+dondonz@users.noreply.github.com> Date: Tue, 24 Oct 2023 15:52:03 +1100 Subject: [PATCH 1/5] Revert "Revert "A test to show that validation rules do not catch a mutation query to schema mismatch"" --- .../graphql/ParseAndValidateTest.groovy | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/src/test/groovy/graphql/ParseAndValidateTest.groovy b/src/test/groovy/graphql/ParseAndValidateTest.groovy index 949b4aeb5..a5ffd6071 100644 --- a/src/test/groovy/graphql/ParseAndValidateTest.groovy +++ b/src/test/groovy/graphql/ParseAndValidateTest.groovy @@ -1,6 +1,12 @@ package graphql +import graphql.language.Document import graphql.parser.InvalidSyntaxException +import graphql.parser.Parser +import graphql.schema.GraphQLSchema +import graphql.schema.idl.SchemaParser +import graphql.schema.idl.TypeDefinitionRegistry +import graphql.schema.idl.UnExecutableSchemaGenerator import graphql.validation.ValidationError import graphql.validation.ValidationErrorType import graphql.validation.rules.NoUnusedFragments @@ -155,4 +161,30 @@ class ParseAndValidateTest extends Specification { then: !rs.errors.isEmpty() // all rules apply - we have errors } + + def "issue 2740 - evidence of not working"() { + def sdl = ''' + type Query { + myquery : String! + } + ''' + + def registry = new SchemaParser().parse(sdl) + def schema = UnExecutableSchemaGenerator.makeUnExecutableSchema(registry) + def graphQL = GraphQL.newGraphQL(schema).build() + + String request = "mutation MyMutation { mymutation }" + + when: + def er = graphQL.execute(request) + then: + er.errors.size() == 1 + + when: + Document inputDocument = new Parser().parseDocument(request) + List errors = ParseAndValidate.validate(schema, inputDocument) + + then: + errors.size() == 1 + } } From 9a69970ca86e03e380c80cc1edd9aa4d5aa00a0e Mon Sep 17 00:00:00 2001 From: dondonz <13839920+dondonz@users.noreply.github.com> Date: Sun, 22 Dec 2024 17:14:08 +1100 Subject: [PATCH 2/5] Add validation at document parse time --- .../validation/ValidationErrorType.java | 3 +- .../java/graphql/validation/Validator.java | 8 +++- .../validation/rules/KnownOperationTypes.java | 43 +++++++++++++++++++ src/main/resources/i18n/Validation.properties | 2 + .../graphql/ParseAndValidateTest.groovy | 38 ++++++++++++---- 5 files changed, 82 insertions(+), 12 deletions(-) create mode 100644 src/main/java/graphql/validation/rules/KnownOperationTypes.java diff --git a/src/main/java/graphql/validation/ValidationErrorType.java b/src/main/java/graphql/validation/ValidationErrorType.java index 5710a1b0b..e701a5d77 100644 --- a/src/main/java/graphql/validation/ValidationErrorType.java +++ b/src/main/java/graphql/validation/ValidationErrorType.java @@ -43,5 +43,6 @@ public enum ValidationErrorType implements ValidationErrorClassification { NullValueForNonNullArgument, SubscriptionMultipleRootFields, SubscriptionIntrospectionRootField, - UniqueObjectFieldName + UniqueObjectFieldName, + UnknownOperation } diff --git a/src/main/java/graphql/validation/Validator.java b/src/main/java/graphql/validation/Validator.java index d7c3db2fd..52709109d 100644 --- a/src/main/java/graphql/validation/Validator.java +++ b/src/main/java/graphql/validation/Validator.java @@ -1,7 +1,6 @@ package graphql.validation; -import graphql.ExperimentalApi; import graphql.Internal; import graphql.i18n.I18n; import graphql.language.Document; @@ -10,6 +9,7 @@ import graphql.validation.rules.DeferDirectiveLabel; import graphql.validation.rules.DeferDirectiveOnRootLevel; import graphql.validation.rules.DeferDirectiveOnValidOperation; +import graphql.validation.rules.KnownOperationTypes; import graphql.validation.rules.UniqueObjectFieldName; import graphql.validation.rules.ExecutableDefinitions; import graphql.validation.rules.FieldsOnCorrectType; @@ -52,7 +52,7 @@ public class Validator { * `graphql-java` will stop validation after a maximum number of validation messages has been reached. Attackers * can send pathologically invalid queries to induce a Denial of Service attack and fill memory with 10000s of errors * and burn CPU in process. - * + *

* By default, this is set to 100 errors. You can set a new JVM wide value as the maximum allowed validation errors. * * @param maxValidationErrors the maximum validation errors allow JVM wide @@ -169,6 +169,10 @@ public List createRules(ValidationContext validationContext, Valid DeferDirectiveLabel deferDirectiveLabel = new DeferDirectiveLabel(validationContext, validationErrorCollector); rules.add(deferDirectiveLabel); + + KnownOperationTypes knownOperationTypes = new KnownOperationTypes(validationContext, validationErrorCollector); + rules.add(knownOperationTypes); + return rules; } } diff --git a/src/main/java/graphql/validation/rules/KnownOperationTypes.java b/src/main/java/graphql/validation/rules/KnownOperationTypes.java new file mode 100644 index 000000000..2e705276a --- /dev/null +++ b/src/main/java/graphql/validation/rules/KnownOperationTypes.java @@ -0,0 +1,43 @@ +package graphql.validation.rules; + +import graphql.Internal; +import graphql.language.OperationDefinition; +import graphql.schema.GraphQLSchema; +import graphql.validation.AbstractRule; +import graphql.validation.ValidationContext; +import graphql.validation.ValidationErrorCollector; + +import static graphql.validation.ValidationErrorType.UnknownOperation; + +/** + * Unique variable names + *

+ * A GraphQL operation is only valid if all its variables are uniquely named. + */ +@Internal +public class KnownOperationTypes extends AbstractRule { + + public KnownOperationTypes(ValidationContext validationContext, ValidationErrorCollector validationErrorCollector) { + super(validationContext, validationErrorCollector); + } + + @Override + public void checkOperationDefinition(OperationDefinition operationDefinition) { + OperationDefinition.Operation documentOperation = operationDefinition.getOperation(); + GraphQLSchema graphQLSchema = getValidationContext().getSchema(); + if (documentOperation == OperationDefinition.Operation.MUTATION + && graphQLSchema.getMutationType() == null) { + String message = i18n(UnknownOperation, "KnownOperationTypes.noOperation", documentOperation.name()); + addError(UnknownOperation, operationDefinition.getSourceLocation(), message); + } else if (documentOperation == OperationDefinition.Operation.SUBSCRIPTION + && graphQLSchema.getSubscriptionType() == null) { + String message = i18n(UnknownOperation, "KnownOperationTypes.noOperation", documentOperation.name()); + addError(UnknownOperation, operationDefinition.getSourceLocation(), message); + } else if (documentOperation == OperationDefinition.Operation.QUERY + && graphQLSchema.getQueryType() == null) { + // This is unlikely to happen, as a validated GraphQLSchema must have a Query type by definition + String message = i18n(UnknownOperation, "KnownOperationTypes.noOperation", documentOperation.name()); + addError(UnknownOperation, operationDefinition.getSourceLocation(), message); + } + } +} diff --git a/src/main/resources/i18n/Validation.properties b/src/main/resources/i18n/Validation.properties index 4f5c42ab2..a9403bea5 100644 --- a/src/main/resources/i18n/Validation.properties +++ b/src/main/resources/i18n/Validation.properties @@ -38,6 +38,8 @@ KnownFragmentNames.undefinedFragment=Validation error ({0}) : Undefined fragment # KnownTypeNames.unknownType=Validation error ({0}) : Unknown type ''{1}'' # +KnownOperationTypes.noOperation=Validation error ({0}): The ''{1}'' operation is not supported by the schema +# LoneAnonymousOperation.withOthers=Validation error ({0}) : Anonymous operation with other operations LoneAnonymousOperation.namedOperation=Validation error ({0}) : Operation ''{1}'' is following anonymous operation # diff --git a/src/test/groovy/graphql/ParseAndValidateTest.groovy b/src/test/groovy/graphql/ParseAndValidateTest.groovy index a5ffd6071..b9a190e64 100644 --- a/src/test/groovy/graphql/ParseAndValidateTest.groovy +++ b/src/test/groovy/graphql/ParseAndValidateTest.groovy @@ -1,11 +1,10 @@ package graphql import graphql.language.Document +import graphql.language.SourceLocation import graphql.parser.InvalidSyntaxException import graphql.parser.Parser -import graphql.schema.GraphQLSchema import graphql.schema.idl.SchemaParser -import graphql.schema.idl.TypeDefinitionRegistry import graphql.schema.idl.UnExecutableSchemaGenerator import graphql.validation.ValidationError import graphql.validation.ValidationErrorType @@ -162,23 +161,40 @@ class ParseAndValidateTest extends Specification { !rs.errors.isEmpty() // all rules apply - we have errors } - def "issue 2740 - evidence of not working"() { + def "validation error raised if mutation operation does not exist in schema"() { def sdl = ''' type Query { - myquery : String! + myQuery : String! } ''' def registry = new SchemaParser().parse(sdl) def schema = UnExecutableSchemaGenerator.makeUnExecutableSchema(registry) - def graphQL = GraphQL.newGraphQL(schema).build() - - String request = "mutation MyMutation { mymutation }" + String request = "mutation MyMutation { myMutation }" when: - def er = graphQL.execute(request) + Document inputDocument = new Parser().parseDocument(request) + List errors = ParseAndValidate.validate(schema, inputDocument) + then: - er.errors.size() == 1 + errors.size() == 1 + def error = errors.first() + error.validationErrorType == ValidationErrorType.UnknownOperation + error.message == "Validation error (UnknownOperation): The 'MUTATION' operation is not supported by the schema" + error.locations == [new SourceLocation(1, 1)] + } + + def "validation error raised if subscription operation does not exist in schema"() { + def sdl = ''' + type Query { + myQuery : String! + } + ''' + + def registry = new SchemaParser().parse(sdl) + def schema = UnExecutableSchemaGenerator.makeUnExecutableSchema(registry) + + String request = "subscription MySubscription { mySubscription }" when: Document inputDocument = new Parser().parseDocument(request) @@ -186,5 +202,9 @@ class ParseAndValidateTest extends Specification { then: errors.size() == 1 + def error = errors.first() + error.validationErrorType == ValidationErrorType.UnknownOperation + error.message == "Validation error (UnknownOperation): The 'SUBSCRIPTION' operation is not supported by the schema" + error.locations == [new SourceLocation(1, 1)] } } From d6be93af13f9d3d0dbc3070c9a34b8cd4fe58963 Mon Sep 17 00:00:00 2001 From: dondonz <13839920+dondonz@users.noreply.github.com> Date: Sun, 22 Dec 2024 17:31:21 +1100 Subject: [PATCH 3/5] Make operation capitalised rather than all caps --- .../graphql/validation/rules/KnownOperationTypes.java | 11 ++++++++--- src/main/resources/i18n/Validation_de.properties | 2 ++ src/test/groovy/graphql/ParseAndValidateTest.groovy | 4 ++-- .../validation/rules/KnownDirectivesTest.groovy | 4 ++++ 4 files changed, 16 insertions(+), 5 deletions(-) diff --git a/src/main/java/graphql/validation/rules/KnownOperationTypes.java b/src/main/java/graphql/validation/rules/KnownOperationTypes.java index 2e705276a..2ef9af48f 100644 --- a/src/main/java/graphql/validation/rules/KnownOperationTypes.java +++ b/src/main/java/graphql/validation/rules/KnownOperationTypes.java @@ -3,6 +3,7 @@ import graphql.Internal; import graphql.language.OperationDefinition; import graphql.schema.GraphQLSchema; +import graphql.util.StringKit; import graphql.validation.AbstractRule; import graphql.validation.ValidationContext; import graphql.validation.ValidationErrorCollector; @@ -27,17 +28,21 @@ public void checkOperationDefinition(OperationDefinition operationDefinition) { GraphQLSchema graphQLSchema = getValidationContext().getSchema(); if (documentOperation == OperationDefinition.Operation.MUTATION && graphQLSchema.getMutationType() == null) { - String message = i18n(UnknownOperation, "KnownOperationTypes.noOperation", documentOperation.name()); + String message = i18n(UnknownOperation, "KnownOperationTypes.noOperation", formatOperation(documentOperation)); addError(UnknownOperation, operationDefinition.getSourceLocation(), message); } else if (documentOperation == OperationDefinition.Operation.SUBSCRIPTION && graphQLSchema.getSubscriptionType() == null) { - String message = i18n(UnknownOperation, "KnownOperationTypes.noOperation", documentOperation.name()); + String message = i18n(UnknownOperation, "KnownOperationTypes.noOperation", formatOperation(documentOperation)); addError(UnknownOperation, operationDefinition.getSourceLocation(), message); } else if (documentOperation == OperationDefinition.Operation.QUERY && graphQLSchema.getQueryType() == null) { // This is unlikely to happen, as a validated GraphQLSchema must have a Query type by definition - String message = i18n(UnknownOperation, "KnownOperationTypes.noOperation", documentOperation.name()); + String message = i18n(UnknownOperation, "KnownOperationTypes.noOperation", formatOperation(documentOperation)); addError(UnknownOperation, operationDefinition.getSourceLocation(), message); } } + + private String formatOperation(OperationDefinition.Operation operation) { + return StringKit.capitalize(operation.name().toLowerCase()); + } } diff --git a/src/main/resources/i18n/Validation_de.properties b/src/main/resources/i18n/Validation_de.properties index fec637643..7823c9d51 100644 --- a/src/main/resources/i18n/Validation_de.properties +++ b/src/main/resources/i18n/Validation_de.properties @@ -30,6 +30,8 @@ KnownFragmentNames.undefinedFragment=Validierungsfehler ({0}) : Undefiniertes Fr # KnownTypeNames.unknownType=Validierungsfehler ({0}) : Unbekannter Typ ''{1}'' # +KnownOperationTypes.noOperation=Validierungsfehler ({0}): ''{1}'' Operation wird vom Schema nicht unterstützt +# LoneAnonymousOperation.withOthers=Validierungsfehler ({0}) : Anonyme Operation mit anderen Operationen LoneAnonymousOperation.namedOperation=Validierungsfehler ({0}) : Operation ''{1}'' folgt der anonymen Operation # diff --git a/src/test/groovy/graphql/ParseAndValidateTest.groovy b/src/test/groovy/graphql/ParseAndValidateTest.groovy index b9a190e64..c5c20a71f 100644 --- a/src/test/groovy/graphql/ParseAndValidateTest.groovy +++ b/src/test/groovy/graphql/ParseAndValidateTest.groovy @@ -180,7 +180,7 @@ class ParseAndValidateTest extends Specification { errors.size() == 1 def error = errors.first() error.validationErrorType == ValidationErrorType.UnknownOperation - error.message == "Validation error (UnknownOperation): The 'MUTATION' operation is not supported by the schema" + error.message == "Validation error (UnknownOperation): The 'Mutation' operation is not supported by the schema" error.locations == [new SourceLocation(1, 1)] } @@ -204,7 +204,7 @@ class ParseAndValidateTest extends Specification { errors.size() == 1 def error = errors.first() error.validationErrorType == ValidationErrorType.UnknownOperation - error.message == "Validation error (UnknownOperation): The 'SUBSCRIPTION' operation is not supported by the schema" + error.message == "Validation error (UnknownOperation): The 'Subscription' operation is not supported by the schema" error.locations == [new SourceLocation(1, 1)] } } diff --git a/src/test/groovy/graphql/validation/rules/KnownDirectivesTest.groovy b/src/test/groovy/graphql/validation/rules/KnownDirectivesTest.groovy index 2149a0a17..8ac2b0d03 100644 --- a/src/test/groovy/graphql/validation/rules/KnownDirectivesTest.groovy +++ b/src/test/groovy/graphql/validation/rules/KnownDirectivesTest.groovy @@ -246,6 +246,10 @@ class KnownDirectivesTest extends Specification { field: String } + type Subscription { + field: String + } + ''' def schema = TestUtil.schema(sdl) From 36e56e0981c8ce5088ad4db7c454a0e737c85baa Mon Sep 17 00:00:00 2001 From: dondonz <13839920+dondonz@users.noreply.github.com> Date: Sun, 22 Dec 2024 17:35:48 +1100 Subject: [PATCH 4/5] Adjust tests now that validation runs earlier --- src/test/groovy/graphql/GraphQLTest.groovy | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/groovy/graphql/GraphQLTest.groovy b/src/test/groovy/graphql/GraphQLTest.groovy index 8235ac523..51b0fe9ce 100644 --- a/src/test/groovy/graphql/GraphQLTest.groovy +++ b/src/test/groovy/graphql/GraphQLTest.groovy @@ -350,7 +350,7 @@ class GraphQLTest extends Specification { thrown(GraphQLException) } - def "null mutation type does not throw an npe re: #345 but returns and error"() { + def "null mutation type does not throw an npe but returns and error"() { given: GraphQLSchema schema = newSchema().query( @@ -370,7 +370,7 @@ class GraphQLTest extends Specification { then: result.errors.size() == 1 - result.errors[0].class == MissingRootTypeException + ((ValidationError) result.errors[0]).validationErrorType == ValidationErrorType.UnknownOperation } def "#875 a subscription query against a schema that doesn't support subscriptions should result in a GraphQL error"() { @@ -393,7 +393,7 @@ class GraphQLTest extends Specification { then: result.errors.size() == 1 - result.errors[0].class == MissingRootTypeException + ((ValidationError) result.errors[0]).validationErrorType == ValidationErrorType.UnknownOperation } def "query with int literal too large"() { From c467a0220d979ef7200e57acd409fc14577b0080 Mon Sep 17 00:00:00 2001 From: dondonz <13839920+dondonz@users.noreply.github.com> Date: Wed, 5 Feb 2025 16:02:30 +1100 Subject: [PATCH 5/5] Add extra test to demonstrate all operations are checked in a document --- .../graphql/ParseAndValidateTest.groovy | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/test/groovy/graphql/ParseAndValidateTest.groovy b/src/test/groovy/graphql/ParseAndValidateTest.groovy index c5c20a71f..fa66c3cbe 100644 --- a/src/test/groovy/graphql/ParseAndValidateTest.groovy +++ b/src/test/groovy/graphql/ParseAndValidateTest.groovy @@ -207,4 +207,32 @@ class ParseAndValidateTest extends Specification { error.message == "Validation error (UnknownOperation): The 'Subscription' operation is not supported by the schema" error.locations == [new SourceLocation(1, 1)] } + + def "known operation validation rule checks all operations in document"() { + def sdl = ''' + type Query { + myQuery : String! + } + ''' + + def registry = new SchemaParser().parse(sdl) + def schema = UnExecutableSchemaGenerator.makeUnExecutableSchema(registry) + String request = "mutation MyMutation { myMutation } subscription MySubscription { mySubscription }" + + when: + Document inputDocument = new Parser().parseDocument(request) + List errors = ParseAndValidate.validate(schema, inputDocument) + + then: + errors.size() == 2 + def error1 = errors.get(0) + error1.validationErrorType == ValidationErrorType.UnknownOperation + error1.message == "Validation error (UnknownOperation): The 'Mutation' operation is not supported by the schema" + error1.locations == [new SourceLocation(1, 1)] + + def error2 = errors.get(1) + error2.validationErrorType == ValidationErrorType.UnknownOperation + error2.message == "Validation error (UnknownOperation): The 'Subscription' operation is not supported by the schema" + error2.locations == [new SourceLocation(1, 36)] + } }