From c9faa3489bfd93d9e0772b85e2c716413142b422 Mon Sep 17 00:00:00 2001 From: spawnia Date: Mon, 10 Jun 2019 22:15:23 +0200 Subject: [PATCH 1/4] Add schema validation: Input Objects must not contain non-nullable circular references Spec change: https://github.com/graphql/graphql-spec/pull/445 Reference implementation: https://github.com/graphql/graphql-js/pull/1359 --- src/Type/SchemaValidationContext.php | 12 +- .../Validation/InputObjectCircularRefs.php | 105 +++++++++++++ tests/Type/ValidationTest.php | 138 ++++++++++++++++++ 3 files changed, 253 insertions(+), 2 deletions(-) create mode 100644 src/Type/Validation/InputObjectCircularRefs.php diff --git a/src/Type/SchemaValidationContext.php b/src/Type/SchemaValidationContext.php index 63f54dd..f00d3e5 100644 --- a/src/Type/SchemaValidationContext.php +++ b/src/Type/SchemaValidationContext.php @@ -29,6 +29,7 @@ use GraphQL\Type\Definition\NonNull; 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 @@ class SchemaValidationContext * @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 @@ class SchemaValidationContext } 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 0000000..ace97dc --- /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 5bafd30..c83bd80 100644 --- a/tests/Type/ValidationTest.php +++ b/tests/Type/ValidationTest.php @@ -879,6 +879,144 @@ class ValidationTest extends TestCase ); } + /** + * @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') */ From e5528d14abed17776eba220311dc5096401181b1 Mon Sep 17 00:00:00 2001 From: spawnia Date: Mon, 10 Jun 2019 22:23:06 +0200 Subject: [PATCH 2/4] Add changelog entry --- CHANGELOG.md | 19 +++++++++++++++++++ CONTRIBUTING.md | 5 +++-- 2 files changed, 22 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ac09c4..38b52f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,23 @@ # Changelog + +## Unreleased +- Add schema validation: Input Objects must not contain non-nullable circular references (https://github.com/webonyx/graphql-php/pull/492) + +#### v0.13.4 +- Force int when setting max query depth (https://github.com/webonyx/graphql-php/pull/477) + +#### v0.13.3 +- Reverted minor possible breaking change (https://github.com/webonyx/graphql-php/pull/476) + +#### v0.13.2 +- Added QueryPlan support (https://github.com/webonyx/graphql-php/pull/436) +- Fixed an issue with NodeList iteration over missing keys (https://github.com/webonyx/graphql-php/pull/475) + +#### v0.13.1 +- Better validation of field/directive arguments +- Support for Apollo-style client/server persisted queries +- Minor tweaks and fixes + ## 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 399716c..636ef53 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](CHANGELOG.md). +* Send a pull request. ## Setup the Development Environment First, copy the URL of your fork and `git clone` it to your local machine. From 6c82b85e79c4552e5e268bd9ec8997997968552e Mon Sep 17 00:00:00 2001 From: spawnia Date: Mon, 10 Jun 2019 22:24:54 +0200 Subject: [PATCH 3/4] Revert "Add changelog entry" This reverts commit e5528d14 --- CHANGELOG.md | 19 ------------------- CONTRIBUTING.md | 5 ++--- 2 files changed, 2 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 38b52f7..0ac09c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,23 +1,4 @@ # Changelog - -## Unreleased -- Add schema validation: Input Objects must not contain non-nullable circular references (https://github.com/webonyx/graphql-php/pull/492) - -#### v0.13.4 -- Force int when setting max query depth (https://github.com/webonyx/graphql-php/pull/477) - -#### v0.13.3 -- Reverted minor possible breaking change (https://github.com/webonyx/graphql-php/pull/476) - -#### v0.13.2 -- Added QueryPlan support (https://github.com/webonyx/graphql-php/pull/436) -- Fixed an issue with NodeList iteration over missing keys (https://github.com/webonyx/graphql-php/pull/475) - -#### v0.13.1 -- Better validation of field/directive arguments -- Support for Apollo-style client/server persisted queries -- Minor tweaks and fixes - ## 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 636ef53..399716c 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -10,9 +10,8 @@ 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`. -* Add an entry to the [Changelog](CHANGELOG.md). -* Send a pull request. +* Check your changes using `composer check-all` +* Send a pull request ## Setup the Development Environment First, copy the URL of your fork and `git clone` it to your local machine. From 692d10c127d13e0018c907288d85fe3fefffa0bb Mon Sep 17 00:00:00 2001 From: spawnia Date: Mon, 10 Jun 2019 22:30:25 +0200 Subject: [PATCH 4/4] Add Unreleases section to the Changelog --- CHANGELOG.md | 4 ++++ CONTRIBUTING.md | 5 +++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ac09c4..8fc837b 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 399716c..d51a0ac 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.