From 70745350acd0a321cc176faeaf07adac74f8a459 Mon Sep 17 00:00:00 2001 From: Nikola Mladenovic Date: Thu, 27 Feb 2020 20:27:56 +0100 Subject: [PATCH 1/3] Add InputObjectTypeRecursionValidator --- .../sangria/schema/SchemaValidationRule.scala | 30 +++++++++++++++++-- .../scala/sangria/validation/Violation.scala | 4 +++ 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/modules/core/src/main/scala/sangria/schema/SchemaValidationRule.scala b/modules/core/src/main/scala/sangria/schema/SchemaValidationRule.scala index 345efc63..2f1290e8 100644 --- a/modules/core/src/main/scala/sangria/schema/SchemaValidationRule.scala +++ b/modules/core/src/main/scala/sangria/schema/SchemaValidationRule.scala @@ -1,6 +1,6 @@ package sangria.schema -import sangria.ast.{AstLocation, Document, ObjectTypeDefinition, ObjectTypeExtensionDefinition, UnionTypeDefinition, UnionTypeExtensionDefinition} +import sangria.ast.{AstLocation, Document, NamedType, NotNullType, ObjectTypeDefinition, ObjectTypeExtensionDefinition, UnionTypeDefinition, UnionTypeExtensionDefinition} import language.higherKinds import sangria.execution._ @@ -24,7 +24,8 @@ object SchemaValidationRule { EnumValueReservedNameValidator, ContainerMembersValidator, ValidNamesValidator, - IntrospectionNamesValidator) + IntrospectionNamesValidator, + InputObjectTypeRecursionValidator) val default: List[SchemaValidationRule] = List( DefaultValuesValidationRule, @@ -410,6 +411,31 @@ object EnumValueReservedNameValidator extends SchemaElementValidator { else Vector.empty } +object InputObjectTypeRecursionValidator extends SchemaElementValidator { + override def validateInputObjectType(schema: Schema[_, _], tpe: InputObjectType[_]): Vector[Violation] = { + containsRecursiveInputObject(tpe.namedType.name, List(), schema, tpe) + } + + private def containsRecursiveInputObject(rootTypeName: String, path: List[String], schema: Schema[_, _], tpe: InputObjectType[_]): Vector[Violation] = { + val recursiveFields = tpe.fields.filter(childField => childField.fieldType.namedType.name == rootTypeName && !childField.fieldType.isOptional && !childField.fieldType.isList) + if (recursiveFields.nonEmpty) { + recursiveFields.flatMap(field => Vector(InputObjectTypeRecursion(tpe.name, field.name, path, None, Nil))).toVector + } else { + var violations = Vector[Violation]() + val childTypesToCheck = tpe.fields.filter(field => !field.fieldType.isOptional && !field.fieldType.isList && field.fieldType.isInstanceOf[InputObjectType[_]]) + childTypesToCheck.foreach { field => + schema.getInputType(NotNullType(NamedType(field.fieldType.namedType.name))).asInstanceOf[Option[InputObjectType[_]]] match { + case Some(objectType) if objectType != tpe => + val updatedPath = path :+ field.name + violations = violations ++ containsRecursiveInputObject(rootTypeName, updatedPath, schema, objectType) + case _ => + } + } + violations + } + } +} + trait SchemaElementValidator { def validateUnionType(schema: Schema[_, _], tpe: UnionType[_]): Vector[Violation] = Vector.empty diff --git a/modules/core/src/main/scala/sangria/validation/Violation.scala b/modules/core/src/main/scala/sangria/validation/Violation.scala index 571780fe..2ffb7d02 100644 --- a/modules/core/src/main/scala/sangria/validation/Violation.scala +++ b/modules/core/src/main/scala/sangria/validation/Violation.scala @@ -608,3 +608,7 @@ case class ExistingTypeViolation(typeName: String, sourceMapper: Option[SourceMa case class InvalidTypeUsageViolation(expectedTypeKind: String, tpe: String, sourceMapper: Option[SourceMapper], locations: List[AstLocation]) extends AstNodeViolation { lazy val simpleErrorMessage = s"Type '$tpe' is not an $expectedTypeKind type." } + +case class InputObjectTypeRecursion(name: String, fieldName: String, path: List[String], sourceMapper: Option[SourceMapper], locations: List[AstLocation]) extends AstNodeViolation { + lazy val simpleErrorMessage: String = s"Cannot reference InputObjectType '$name' within itself through a series of non-null fields: '$fieldName${if (path.isEmpty) "" else "."}${path.mkString(".")}'." +} \ No newline at end of file From b5d966938220ebfd4d676e9ea3e3d0134523a674 Mon Sep 17 00:00:00 2001 From: Nikola Mladenovic Date: Fri, 28 Feb 2020 02:06:05 +0100 Subject: [PATCH 2/3] Test InputObjectTypeRecursionValidator --- .../schema/AstSchemaMaterializerSpec.scala | 110 ++++++++++++++++++ 1 file changed, 110 insertions(+) diff --git a/modules/core/src/test/scala/sangria/schema/AstSchemaMaterializerSpec.scala b/modules/core/src/test/scala/sangria/schema/AstSchemaMaterializerSpec.scala index ab9e29d4..21ecab9b 100644 --- a/modules/core/src/test/scala/sangria/schema/AstSchemaMaterializerSpec.scala +++ b/modules/core/src/test/scala/sangria/schema/AstSchemaMaterializerSpec.scala @@ -882,6 +882,116 @@ class AstSchemaMaterializerSpec extends WordSpec with Matchers with FutureResult error.getMessage should include ("Object type 'Query' can include field 'field1' only once.") } + "accepts an Input Object with breakable circular reference" in { + val ast = + graphql""" + schema { + query: Query + } + + type Query { + field(arg: SomeInputObject): String + } + + input SomeInputObject { + self: SomeInputObject + arrayOfSelf: [SomeInputObject] + nonNullArrayOfSelf: [SomeInputObject]! + nonNullArrayOfNonNullSelf: [SomeInputObject!]! + intermediateSelf: AnotherInputObject + } + + input AnotherInputObject { + parent: SomeInputObject + } + """ + + noException should be thrownBy (Schema.buildFromAst(ast)) + } + + "rejects an Input Object with non-breakable circular reference" in { + val ast = + graphql""" + schema { + query: Query + } + + type Query { + field(arg: SomeInputObject): String + } + + input SomeInputObject { + nonNullSelf: SomeInputObject! + } + """ + + val error = intercept [SchemaValidationException] (Schema.buildFromAst(ast)) + + error.getMessage should include ("Cannot reference InputObjectType 'SomeInputObject' within itself through a series of non-null fields: 'nonNullSelf'.") + } + + "rejects Input Objects with non-breakable circular reference spread across them" in { + val ast = + graphql""" + schema { + query: Query + } + + type Query { + field(arg: SomeInputObject): String + } + + input SomeInputObject { + startLoop: AnotherInputObject! + } + + input AnotherInputObject { + nextInLoop: YetAnotherInputObject! + } + + input YetAnotherInputObject { + closeLoop: SomeInputObject! + } + """ + + val error = intercept [SchemaValidationException] (Schema.buildFromAst(ast)) + + error.getMessage should include ("Cannot reference InputObjectType 'SomeInputObject' within itself through a series of non-null fields: 'startLoop.nextInLoop.closeLoop'.") + } + + "rejects Input Objects with multiple non-breakable circular reference" in { + val ast = + graphql""" + schema { + query: Query + } + + type Query { + field(arg: SomeInputObject): String + } + + input SomeInputObject { + startLoop: AnotherInputObject! + } + + input AnotherInputObject { + closeLoop: SomeInputObject! + startSecondLoop: YetAnotherInputObject! + } + + input YetAnotherInputObject { + closeSecondLoop: AnotherInputObject! + nonNullSelf: YetAnotherInputObject! + } + """ + + val error = intercept [SchemaValidationException] (Schema.buildFromAst(ast)) + + error.getMessage should include ("Cannot reference InputObjectType 'SomeInputObject' within itself through a series of non-null fields: 'startLoop.closeLoop'.") + error.getMessage should include ("Cannot reference InputObjectType 'AnotherInputObject' within itself through a series of non-null fields: 'closeLoop.startLoop'.") + error.getMessage should include ("Cannot reference InputObjectType 'YetAnotherInputObject' within itself through a series of non-null fields: 'nonNullSelf'.") + } + "don't allow to have extensions on non-existing types" in { val ast = graphql""" From 9995042f3ef83deeaf6bbe718bff089713a158a2 Mon Sep 17 00:00:00 2001 From: Nikola Mladenovic Date: Mon, 2 Mar 2020 12:53:06 +0100 Subject: [PATCH 3/3] Update modules/core/src/main/scala/sangria/schema/SchemaValidationRule.scala Co-Authored-By: Yann Simon --- .../scala/sangria/schema/SchemaValidationRule.scala | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/modules/core/src/main/scala/sangria/schema/SchemaValidationRule.scala b/modules/core/src/main/scala/sangria/schema/SchemaValidationRule.scala index 2f1290e8..236e6608 100644 --- a/modules/core/src/main/scala/sangria/schema/SchemaValidationRule.scala +++ b/modules/core/src/main/scala/sangria/schema/SchemaValidationRule.scala @@ -421,17 +421,15 @@ object InputObjectTypeRecursionValidator extends SchemaElementValidator { if (recursiveFields.nonEmpty) { recursiveFields.flatMap(field => Vector(InputObjectTypeRecursion(tpe.name, field.name, path, None, Nil))).toVector } else { - var violations = Vector[Violation]() val childTypesToCheck = tpe.fields.filter(field => !field.fieldType.isOptional && !field.fieldType.isList && field.fieldType.isInstanceOf[InputObjectType[_]]) - childTypesToCheck.foreach { field => - schema.getInputType(NotNullType(NamedType(field.fieldType.namedType.name))).asInstanceOf[Option[InputObjectType[_]]] match { - case Some(objectType) if objectType != tpe => + childTypesToCheck.foldLeft(Vector.empty[Violation]) { case (acc, field) => + schema.getInputType(NotNullType(NamedType(field.fieldType.namedType.name))) match { + case Some(objectType: InputObjectType[_]) if objectType != tpe => val updatedPath = path :+ field.name - violations = violations ++ containsRecursiveInputObject(rootTypeName, updatedPath, schema, objectType) - case _ => + acc ++ containsRecursiveInputObject(rootTypeName, updatedPath, schema, objectType) + case _ => acc } } - violations } } }