diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ac09c47b..8fc837bf2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,8 @@ # Changelog + +## Unreleased +- Add schema validation: Input Objects must not contain non-nullable circular references (#492) + ## v0.13.0 This release brings several breaking changes. Please refer to [UPGRADE](UPGRADE.md) document for details. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 399716c8d..d51a0ac9a 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -10,8 +10,9 @@ For smaller contributions just use this workflow: * Fork the project. * Add your features and or bug fixes. * Add tests. Tests are important for us. -* Check your changes using `composer check-all` -* Send a pull request +* Check your changes using `composer check-all`. +* Add an entry to the [Changelog's Unreleases section](CHANGELOG.md#unreleased). +* Send a pull request. ## Setup the Development Environment First, copy the URL of your fork and `git clone` it to your local machine. diff --git a/src/Type/SchemaValidationContext.php b/src/Type/SchemaValidationContext.php index 63f54dd46..f00d3e588 100644 --- a/src/Type/SchemaValidationContext.php +++ b/src/Type/SchemaValidationContext.php @@ -29,6 +29,7 @@ use GraphQL\Type\Definition\ObjectType; use GraphQL\Type\Definition\Type; use GraphQL\Type\Definition\UnionType; +use GraphQL\Type\Validation\InputObjectCircularRefs; use GraphQL\Utils\TypeComparators; use GraphQL\Utils\Utils; use function array_filter; @@ -48,9 +49,13 @@ class SchemaValidationContext /** @var Schema */ private $schema; + /** @var InputObjectCircularRefs */ + private $inputObjectCircularRefs; + public function __construct(Schema $schema) { - $this->schema = $schema; + $this->schema = $schema; + $this->inputObjectCircularRefs = new InputObjectCircularRefs($this); } /** @@ -99,7 +104,7 @@ public function validateRootTypes() * @param string $message * @param Node[]|Node|TypeNode|TypeDefinitionNode|null $nodes */ - private function reportError($message, $nodes = null) + public function reportError($message, $nodes = null) { $nodes = array_filter($nodes && is_array($nodes) ? $nodes : [$nodes]); $this->addError(new Error($message, $nodes)); @@ -275,6 +280,9 @@ public function validateTypes() } elseif ($type instanceof InputObjectType) { // Ensure Input Object fields are valid. $this->validateInputFields($type); + + // Ensure Input Objects do not contain non-nullable circular references + $this->inputObjectCircularRefs->validate($type); } } } diff --git a/src/Type/Validation/InputObjectCircularRefs.php b/src/Type/Validation/InputObjectCircularRefs.php new file mode 100644 index 000000000..ace97dc14 --- /dev/null +++ b/src/Type/Validation/InputObjectCircularRefs.php @@ -0,0 +1,105 @@ + int $index] + * + * @var int[] + */ + private $fieldPathIndexByTypeName = []; + + public function __construct(SchemaValidationContext $schemaValidationContext) + { + $this->schemaValidationContext = $schemaValidationContext; + } + + /** + * This does a straight-forward DFS to find cycles. + * It does not terminate when a cycle was found but continues to explore + * the graph to find all possible cycles. + */ + public function validate(InputObjectType $inputObj) : void + { + if (isset($this->visitedTypes[$inputObj->name])) { + return; + } + + $this->visitedTypes[$inputObj->name] = true; + $this->fieldPathIndexByTypeName[$inputObj->name] = count($this->fieldPath); + + $fieldMap = $inputObj->getFields(); + foreach ($fieldMap as $fieldName => $field) { + $type = $field->getType(); + + if ($type instanceof NonNull) { + $fieldType = $type->getWrappedType(); + + // If the type of the field is anything else then a non-nullable input object, + // there is no chance of an unbreakable cycle + if ($fieldType instanceof InputObjectType) { + $this->fieldPath[] = $field; + + if (! isset($this->fieldPathIndexByTypeName[$fieldType->name])) { + $this->validate($fieldType); + } else { + $cycleIndex = $this->fieldPathIndexByTypeName[$fieldType->name]; + $cyclePath = array_slice($this->fieldPath, $cycleIndex); + $fieldNames = array_map( + static function (InputObjectField $field) : string { + return $field->name; + }, + $cyclePath + ); + + $this->schemaValidationContext->reportError( + 'Cannot reference Input Object "' . $fieldType->name . '" within itself ' + . 'through a series of non-null fields: "' . implode('.', $fieldNames) . '".', + array_map( + static function (InputObjectField $field) : ?InputValueDefinitionNode { + return $field->astNode; + }, + $cyclePath + ) + ); + } + } + } + + array_pop($this->fieldPath); + } + + unset($this->fieldPathIndexByTypeName[$inputObj->name]); + } +} diff --git a/tests/Type/ValidationTest.php b/tests/Type/ValidationTest.php index 5bafd30ec..c83bd806f 100644 --- a/tests/Type/ValidationTest.php +++ b/tests/Type/ValidationTest.php @@ -879,6 +879,144 @@ public function testRejectsAnInputObjectTypeWithMissingFields() : void ); } + /** + * @see it('accepts an Input Object with breakable circular reference') + */ + public function testAcceptsAnInputObjectWithBreakableCircularReference() : void + { + $schema = BuildSchema::build(' + input AnotherInputObject { + parent: SomeInputObject + } + + type Query { + field(arg: SomeInputObject): String + } + + input SomeInputObject { + self: SomeInputObject + arrayOfSelf: [SomeInputObject] + nonNullArrayOfSelf: [SomeInputObject]! + nonNullArrayOfNonNullSelf: [SomeInputObject!]! + intermediateSelf: AnotherInputObject + } + '); + self::assertEquals([], $schema->validate()); + } + + /** + * @see it('rejects an Input Object with non-breakable circular reference') + */ + public function testRejectsAnInputObjectWithNonBreakableCircularReference() : void + { + $schema = BuildSchema::build(' + type Query { + field(arg: SomeInputObject): String + } + + input SomeInputObject { + nonNullSelf: SomeInputObject! + } + '); + $this->assertMatchesValidationMessage( + $schema->validate(), + [ + [ + 'message' => 'Cannot reference Input Object "SomeInputObject" within itself through a series of non-null fields: "nonNullSelf".', + 'locations' => [['line' => 7, 'column' => 9]], + ], + ] + ); + } + + /** + * @see it('rejects Input Objects with non-breakable circular reference spread across them') + */ + public function testRejectsInputObjectsWithNonBreakableCircularReferenceSpreadAcrossThem() : void + { + $schema = BuildSchema::build(' + type Query { + field(arg: SomeInputObject): String + } + + input SomeInputObject { + startLoop: AnotherInputObject! + } + + input AnotherInputObject { + nextInLoop: YetAnotherInputObject! + } + + input YetAnotherInputObject { + closeLoop: SomeInputObject! + } + '); + $this->assertMatchesValidationMessage( + $schema->validate(), + [ + [ + 'message' => 'Cannot reference Input Object "SomeInputObject" within itself through a series of non-null fields: "startLoop.nextInLoop.closeLoop".', + 'locations' => [ + ['line' => 7, 'column' => 9], + ['line' => 11, 'column' => 9], + ['line' => 15, 'column' => 9], + ], + ], + ] + ); + } + + /** + * @see it('rejects Input Objects with multiple non-breakable circular reference') + */ + public function testRejectsInputObjectsWithMultipleNonBreakableCircularReferences() : void + { + $schema = BuildSchema::build(' + type Query { + field(arg: SomeInputObject): String + } + + input SomeInputObject { + startLoop: AnotherInputObject! + } + + input AnotherInputObject { + closeLoop: SomeInputObject! + startSecondLoop: YetAnotherInputObject! + } + + input YetAnotherInputObject { + closeSecondLoop: AnotherInputObject! + nonNullSelf: YetAnotherInputObject! + } + '); + $this->assertMatchesValidationMessage( + $schema->validate(), + [ + [ + 'message' => 'Cannot reference Input Object "SomeInputObject" within itself through a series of non-null fields: "startLoop.closeLoop".', + 'locations' => [ + ['line' => 7, 'column' => 9], + ['line' => 11, 'column' => 9], + ], + ], + [ + 'message' => 'Cannot reference Input Object "AnotherInputObject" within itself through a series of non-null fields: "startSecondLoop.closeSecondLoop".', + 'locations' => [ + ['line' => 12, 'column' => 9], + ['line' => 16, 'column' => 9], + ], + ], + [ + 'message' => 'Cannot reference Input Object "YetAnotherInputObject" within itself through a series of non-null fields: "nonNullSelf".', + 'locations' => [ + ['line' => 17, 'column' => 9], + ], + ], + ] + ); + } + /** * @see it('rejects an Input Object type with incorrectly typed fields') */