From 379a3084392179d2cae92c211a4559f9836299c6 Mon Sep 17 00:00:00 2001 From: Lee Byron Date: Tue, 22 Nov 2016 15:38:19 -0800 Subject: [PATCH] FIX: Uphold spec for non-validation names not beginning with __ The spec describing introspection (http://facebook.github.io/graphql/#sec-Naming-conventions) restricts naming non-introspection related artifacts starting with __. This enforces that specification. --- src/type/__tests__/validation-test.js | 11 +++++++++ src/type/definition.js | 13 +++++------ src/type/introspection.js | 8 +++++++ src/utilities/assertValidName.js | 32 +++++++++++++++++++-------- 4 files changed, 47 insertions(+), 17 deletions(-) diff --git a/src/type/__tests__/validation-test.js b/src/type/__tests__/validation-test.js index f7f10e308a..e541ca7d2f 100644 --- a/src/type/__tests__/validation-test.js +++ b/src/type/__tests__/validation-test.js @@ -341,6 +341,17 @@ describe('Type System: Objects must have fields', () => { ); }); + it('rejects an Object type with reserved named fields', () => { + expect( + () => schemaWithFieldType(new GraphQLObjectType({ + name: 'SomeObject', + fields: { __notPartOfIntrospection: { type: GraphQLString } } + })) + ).to.throw( + 'Name "__notPartOfIntrospection" must not begin with "__", which is reserved by GraphQL introspection.' + ); + }); + it('rejects an Object type with incorrectly typed fields', () => { expect( () => schemaWithFieldType(new GraphQLObjectType({ diff --git a/src/type/definition.js b/src/type/definition.js index 25909da450..2cf5a61d86 100644 --- a/src/type/definition.js +++ b/src/type/definition.js @@ -291,7 +291,6 @@ export class GraphQLScalarType { _scalarConfig: GraphQLScalarTypeConfig<*, *>; constructor(config: GraphQLScalarTypeConfig<*, *>) { - invariant(config.name, 'Type must be named.'); assertValidName(config.name); this.name = config.name; this.description = config.description; @@ -400,8 +399,7 @@ export class GraphQLObjectType { _interfaces: Array; constructor(config: GraphQLObjectTypeConfig<*, *>) { - invariant(config.name, 'Type must be named.'); - assertValidName(config.name); + assertValidName(config.name, config.isIntrospection); this.name = config.name; this.description = config.description; if (config.isTypeOf) { @@ -547,7 +545,8 @@ export type GraphQLObjectTypeConfig = { interfaces?: Thunk>; fields: Thunk>; isTypeOf?: ?GraphQLIsTypeOfFn; - description?: ?string + description?: ?string; + isIntrospection?: boolean; }; export type GraphQLTypeResolver = ( @@ -656,7 +655,6 @@ export class GraphQLInterfaceType { _fields: GraphQLFieldMap<*, *>; constructor(config: GraphQLInterfaceTypeConfig<*, *>) { - invariant(config.name, 'Type must be named.'); assertValidName(config.name); this.name = config.name; this.description = config.description; @@ -735,7 +733,6 @@ export class GraphQLUnionType { _possibleTypeNames: {[typeName: string]: boolean}; constructor(config: GraphQLUnionTypeConfig<*, *>) { - invariant(config.name, 'Type must be named.'); assertValidName(config.name); this.name = config.name; this.description = config.description; @@ -845,7 +842,7 @@ export class GraphQLEnumType/* */ { constructor(config: GraphQLEnumTypeConfig/* */) { this.name = config.name; - assertValidName(config.name); + assertValidName(config.name, config.isIntrospection); this.description = config.description; this._values = defineEnumValues(this, config.values); this._enumConfig = config; @@ -953,6 +950,7 @@ export type GraphQLEnumTypeConfig/* */ = { name: string; values: GraphQLEnumValueConfigMap/* */; description?: ?string; + isIntrospection?: boolean; }; export type GraphQLEnumValueConfigMap/* */ = { @@ -1003,7 +1001,6 @@ export class GraphQLInputObjectType { _fields: GraphQLInputFieldMap; constructor(config: GraphQLInputObjectTypeConfig) { - invariant(config.name, 'Type must be named.'); assertValidName(config.name); this.name = config.name; this.description = config.description; diff --git a/src/type/introspection.js b/src/type/introspection.js index fe67e97498..09e94c5fe4 100644 --- a/src/type/introspection.js +++ b/src/type/introspection.js @@ -28,6 +28,7 @@ import type { GraphQLField } from './definition'; export const __Schema = new GraphQLObjectType({ name: '__Schema', + isIntrospection: true, description: 'A GraphQL Schema defines the capabilities of a GraphQL server. It ' + 'exposes all available types and directives on the server, as well as ' + @@ -69,6 +70,7 @@ export const __Schema = new GraphQLObjectType({ export const __Directive = new GraphQLObjectType({ name: '__Directive', + isIntrospection: true, description: 'A Directive provides a way to describe alternate runtime execution and ' + 'type validation behavior in a GraphQL document.' + @@ -117,6 +119,7 @@ export const __Directive = new GraphQLObjectType({ export const __DirectiveLocation = new GraphQLEnumType({ name: '__DirectiveLocation', + isIntrospection: true, description: 'A Directive can be adjacent to many parts of the GraphQL language, a ' + '__DirectiveLocation describes one such possible adjacencies.', @@ -198,6 +201,7 @@ export const __DirectiveLocation = new GraphQLEnumType({ export const __Type = new GraphQLObjectType({ name: '__Type', + isIntrospection: true, description: 'The fundamental unit of any GraphQL Schema is the type. There are ' + 'many kinds of types in GraphQL as represented by the `__TypeKind` enum.' + @@ -299,6 +303,7 @@ export const __Type = new GraphQLObjectType({ export const __Field = new GraphQLObjectType({ name: '__Field', + isIntrospection: true, description: 'Object and Interface types are described by a list of Fields, each of ' + 'which has a name, potentially a list of arguments, and a return type.', @@ -320,6 +325,7 @@ export const __Field = new GraphQLObjectType({ export const __InputValue = new GraphQLObjectType({ name: '__InputValue', + isIntrospection: true, description: 'Arguments provided to Fields or Directives and the input fields of an ' + 'InputObject are represented as Input Values which describe their type ' + @@ -342,6 +348,7 @@ export const __InputValue = new GraphQLObjectType({ export const __EnumValue = new GraphQLObjectType({ name: '__EnumValue', + isIntrospection: true, description: 'One possible value for a given Enum. Enum values are unique values, not ' + 'a placeholder for a string or numeric value. However an Enum value is ' + @@ -369,6 +376,7 @@ export const TypeKind = { export const __TypeKind = new GraphQLEnumType({ name: '__TypeKind', + isIntrospection: true, description: 'An enum describing what kind of type a given `__Type` is.', values: { SCALAR: { diff --git a/src/utilities/assertValidName.js b/src/utilities/assertValidName.js index 8413767375..cd5d8881b8 100644 --- a/src/utilities/assertValidName.js +++ b/src/utilities/assertValidName.js @@ -8,15 +8,29 @@ * of patent rights can be found in the PATENTS file in the same directory. */ -import invariant from '../jsutils/invariant'; - - const NAME_RX = /^[_a-zA-Z][_a-zA-Z0-9]*$/; -// Helper to assert that provided names are valid. -export function assertValidName(name: string): void { - invariant( - NAME_RX.test(name), - `Names must match /^[_a-zA-Z][_a-zA-Z0-9]*$/ but "${name}" does not.` - ); +/** + * Upholds the spec rules about naming. + */ +export function assertValidName( + name: string, + isIntrospection?: boolean +): void { + if (!name || typeof name !== 'string') { + throw new Error( + `Must be named. Unexpected name: ${name}.` + ); + } + if (!isIntrospection && name.slice(0, 2) === '__') { + throw new Error( + `Name "${name}" must not begin with "__", which is reserved by ` + + 'GraphQL introspection.' + ); + } + if (!NAME_RX.test(name)) { + throw new Error( + `Names must match /^[_a-zA-Z][_a-zA-Z0-9]*$/ but "${name}" does not.` + ); + } }