From d163d561887d03252bc7b4dd6e1df13ffaaf6f93 Mon Sep 17 00:00:00 2001 From: Lee Byron Date: Fri, 27 Jan 2017 13:24:05 -0800 Subject: [PATCH] Fix: Type safety for TypeInfo. Previously we just coerce these values to the expected types, record them and carry on. However that is not safe and results in TypeInfo potentially returning invalid types when traversing an invalid query. This instead checks the type before inserting it, upholding type safety. There was one validator which was taking advantage of the unsafe behavior, so I re-wrote it slightly to be type safe. --- src/utilities/TypeInfo.js | 25 +++++++++++++------ .../rules/FragmentsOnCompositeTypes.js | 17 +++++++------ 2 files changed, 27 insertions(+), 15 deletions(-) diff --git a/src/utilities/TypeInfo.js b/src/utilities/TypeInfo.js index 27951c6f76..0965ccb44e 100644 --- a/src/utilities/TypeInfo.js +++ b/src/utilities/TypeInfo.js @@ -11,6 +11,8 @@ import * as Kind from '../language/kinds'; import { isCompositeType, + isInputType, + isOutputType, getNullableType, getNamedType, GraphQLObjectType, @@ -117,12 +119,11 @@ export class TypeInfo { switch (node.kind) { case Kind.SELECTION_SET: const namedType = getNamedType(this.getType()); - let compositeType: ?GraphQLCompositeType; - if (isCompositeType(namedType)) { - // isCompositeType is a type refining predicate, so this is safe. - compositeType = ((namedType: any): GraphQLCompositeType); - } - this._parentTypeStack.push(compositeType); + this._parentTypeStack.push( + isCompositeType(namedType) ? + ((namedType: any): GraphQLCompositeType) : + undefined + ); break; case Kind.FIELD: const parentType = this.getParentType(); @@ -153,11 +154,19 @@ export class TypeInfo { const outputType = typeConditionAST ? typeFromAST(schema, typeConditionAST) : this.getType(); - this._typeStack.push(((outputType: any): GraphQLOutputType)); + this._typeStack.push( + isOutputType(outputType) ? + ((outputType: any): GraphQLOutputType) : + undefined + ); break; case Kind.VARIABLE_DEFINITION: const inputType = typeFromAST(schema, node.type); - this._inputTypeStack.push(((inputType: any): GraphQLInputType)); + this._inputTypeStack.push( + isInputType(inputType) ? + ((inputType: any): GraphQLInputType) : + undefined + ); break; case Kind.ARGUMENT: let argDef; diff --git a/src/validation/rules/FragmentsOnCompositeTypes.js b/src/validation/rules/FragmentsOnCompositeTypes.js index 892410ea23..77909073d9 100644 --- a/src/validation/rules/FragmentsOnCompositeTypes.js +++ b/src/validation/rules/FragmentsOnCompositeTypes.js @@ -13,6 +13,7 @@ import { GraphQLError } from '../../error'; import { print } from '../../language/printer'; import { isCompositeType } from '../../type/definition'; import type { GraphQLType } from '../../type/definition'; +import { typeFromAST } from '../../utilities/typeFromAST'; export function inlineFragmentOnNonCompositeErrorMessage( @@ -39,16 +40,18 @@ export function fragmentOnNonCompositeErrorMessage( export function FragmentsOnCompositeTypes(context: ValidationContext): any { return { InlineFragment(node) { - const type = context.getType(); - if (node.typeCondition && type && !isCompositeType(type)) { - context.reportError(new GraphQLError( - inlineFragmentOnNonCompositeErrorMessage(print(node.typeCondition)), - [ node.typeCondition ] - )); + if (node.typeCondition) { + const type = typeFromAST(context.getSchema(), node.typeCondition); + if (type && !isCompositeType(type)) { + context.reportError(new GraphQLError( + inlineFragmentOnNonCompositeErrorMessage(print(node.typeCondition)), + [ node.typeCondition ] + )); + } } }, FragmentDefinition(node) { - const type = context.getType(); + const type = typeFromAST(context.getSchema(), node.typeCondition); if (type && !isCompositeType(type)) { context.reportError(new GraphQLError( fragmentOnNonCompositeErrorMessage(