From b90c7986e854aefd5efa534656e148cc35e0c372 Mon Sep 17 00:00:00 2001 From: Ivan Goncharov Date: Fri, 25 May 2018 22:03:59 +0300 Subject: [PATCH] validateSchema: validate Input Objects self-references --- src/type/__tests__/validation-test.js | 74 ++++++++++++++++++++++++ src/type/validate.js | 61 +++++++++++++++++++ src/validation/rules/NoFragmentCycles.js | 25 ++++---- 3 files changed, 148 insertions(+), 12 deletions(-) diff --git a/src/type/__tests__/validation-test.js b/src/type/__tests__/validation-test.js index c04905b3c91..8bcea297809 100644 --- a/src/type/__tests__/validation-test.js +++ b/src/type/__tests__/validation-test.js @@ -687,6 +687,80 @@ describe('Type System: Input Objects must have fields', () => { ]); }); + it('accepts an Input Object with breakable circular reference', () => { + const schema = buildSchema(` + type Query { + field(arg: SomeInputObject): String + } + + input SomeInputObject { + self: SomeInputObject + arrayOfSelf: [SomeInputObject] + nonNullArrayOfSelf: [SomeInputObject]! + nonNullArrayOfNonNullSelf: [SomeInputObject!]! + intermediateSelf: AnotherInputObject + } + + input AnotherInputObject { + parent: SomeInputObject + } + `); + + expect(validateSchema(schema)).to.deep.equal([]); + }); + + it('rejects an Input Object with non-breakable circular reference', () => { + const schema = buildSchema(` + type Query { + field(arg: SomeInputObject): String + } + + input SomeInputObject { + nonNullSelf: SomeInputObject! + } + `); + + expect(validateSchema(schema)).to.deep.equal([ + { + message: + 'Cannot reference Input Object "SomeInputObject" within itself through a series of non-null field: "nonNullSelf".', + locations: [{ line: 7, column: 9 }], + }, + ]); + }); + + it('rejects Input Objects with non-breakable circular reference spread across them', () => { + const schema = buildSchema(` + type Query { + field(arg: SomeInputObject): String + } + + input SomeInputObject { + startLoop: AnotherInputObject! + } + + input AnotherInputObject { + nextInLoop: YetAnotherInputObject! + } + + input YetAnotherInputObject { + closeLoop: SomeInputObject! + } + `); + + expect(validateSchema(schema)).to.deep.equal([ + { + message: + 'Cannot reference Input Object "SomeInputObject" within itself through a series of non-null field: "startLoop.nextInLoop.closeLoop".', + locations: [ + { line: 7, column: 9 }, + { line: 11, column: 9 }, + { line: 15, column: 9 }, + ], + }, + ]); + }); + it('rejects an Input Object type with incorrectly typed fields', () => { const schema = buildSchema(` type Query { diff --git a/src/type/validate.js b/src/type/validate.js index 830418d122f..bd9db16828c 100644 --- a/src/type/validate.js +++ b/src/type/validate.js @@ -271,6 +271,9 @@ function validateTypes(context: SchemaValidationContext): void { validateInputFields(context, type); } }); + + // Ensure Input Objects do not contain non-nullable circular references + validateInputObjectCircularReferences(context); } function validateFields( @@ -578,6 +581,64 @@ function validateInputFields( }); } +function validateInputObjectCircularReferences( + context: SchemaValidationContext, +): void { + // Modified copy of algorithm from 'src/validation/rules/NoFragmentCycles.js'. + // Tracks already visited types to maintain O(N) and to ensure that cycles + // are not redundantly reported. + const visitedTypes = Object.create(null); + + // Array of types nodes used to produce meaningful errors + const fieldPath = []; + + // Position in the type path + const fieldPathIndexByTypeName = Object.create(null); + + const typeMap = context.schema.getTypeMap(); + for (const type of objectValues(typeMap)) { + if (isInputObjectType(type)) { + detectCycleRecursive(type); + } + } + + // 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. + function detectCycleRecursive(inputObj: GraphQLInputObjectType) { + if (visitedTypes[inputObj.name]) { + return; + } + + visitedTypes[inputObj.name] = true; + fieldPathIndexByTypeName[inputObj.name] = fieldPath.length; + + const fields = objectValues(inputObj.getFields()); + for (const field of fields) { + if (isNonNullType(field.type) && isInputObjectType(field.type.ofType)) { + const fieldType = field.type.ofType; + const cycleIndex = fieldPathIndexByTypeName[fieldType.name]; + + fieldPath.push(field); + if (cycleIndex === undefined) { + detectCycleRecursive(fieldType); + } else { + const cyclePath = fieldPath.slice(cycleIndex); + const fieldNames = cyclePath.map(fieldObj => fieldObj.name); + context.reportError( + `Cannot reference Input Object "${fieldType.name}" within itself ` + + `through a series of non-null fields: "${fieldNames.join('.')}".`, + cyclePath.map(fieldObj => fieldObj.astNode), + ); + } + fieldPath.pop(); + } + } + + fieldPathIndexByTypeName[inputObj.name] = undefined; + } +} + function getAllNodes(object: { +astNode: ?T, +extensionASTNodes?: ?$ReadOnlyArray, diff --git a/src/validation/rules/NoFragmentCycles.js b/src/validation/rules/NoFragmentCycles.js index 0a511227592..1066e2a83b2 100644 --- a/src/validation/rules/NoFragmentCycles.js +++ b/src/validation/rules/NoFragmentCycles.js @@ -34,9 +34,7 @@ export function NoFragmentCycles(context: ValidationContext): ASTVisitor { return { OperationDefinition: () => false, FragmentDefinition(node) { - if (!visitedFrags[node.name.value]) { - detectCycleRecursive(node); - } + detectCycleRecursive(node); return false; }, }; @@ -45,6 +43,10 @@ export function NoFragmentCycles(context: ValidationContext): ASTVisitor { // It does not terminate when a cycle was found but continues to explore // the graph to find all possible cycles. function detectCycleRecursive(fragment: FragmentDefinitionNode) { + if (visitedFrags[fragment.name.value]) { + return; + } + const fragmentName = fragment.name.value; visitedFrags[fragmentName] = true; @@ -60,24 +62,23 @@ export function NoFragmentCycles(context: ValidationContext): ASTVisitor { const spreadName = spreadNode.name.value; const cycleIndex = spreadPathIndexByName[spreadName]; + spreadPath.push(spreadNode); if (cycleIndex === undefined) { - spreadPath.push(spreadNode); - if (!visitedFrags[spreadName]) { - const spreadFragment = context.getFragment(spreadName); - if (spreadFragment) { - detectCycleRecursive(spreadFragment); - } + const spreadFragment = context.getFragment(spreadName); + if (spreadFragment) { + detectCycleRecursive(spreadFragment); } - spreadPath.pop(); } else { const cyclePath = spreadPath.slice(cycleIndex); + const fragmentNames = cyclePath.slice(0, -1).map(s => s.name.value); context.reportError( new GraphQLError( - cycleErrorMessage(spreadName, cyclePath.map(s => s.name.value)), - cyclePath.concat(spreadNode), + cycleErrorMessage(spreadName, fragmentNames), + cyclePath, ), ); } + spreadPath.pop(); } spreadPathIndexByName[fragmentName] = undefined;