From 99526d5f9af8c410b46f34875068df4024be7e5b Mon Sep 17 00:00:00 2001 From: Lee Byron Date: Wed, 21 Apr 2021 00:27:35 +0900 Subject: [PATCH 1/2] RFC: Default value validation & coercion Implements https://github.com/graphql/graphql-spec/pull/793/ * Adds validation of default values during schema validation. * Adds coercion of default values anywhere a default value is used at runtime Potentially breaking: * Deprecates `astFromValue` * Changes type of `defaultValue` provided during type configuration from an "internal" to an "external" value. --- cspell.yml | 2 + src/execution/__tests__/variables-test.ts | 1 - src/index.ts | 1 + src/type/__tests__/enumType-test.ts | 4 +- src/type/__tests__/validation-test.ts | 555 ++++++++++++++++++- src/type/introspection.ts | 4 +- src/type/validate.ts | 351 +++++++++++- src/utilities/__tests__/astFromValue-test.ts | 1 + src/utilities/astFromValue.ts | 1 + src/utilities/coerceInputValue.ts | 4 +- src/utilities/findSchemaChanges.ts | 4 +- src/utilities/index.ts | 5 +- src/utilities/printSchema.ts | 4 +- 13 files changed, 900 insertions(+), 37 deletions(-) diff --git a/cspell.yml b/cspell.yml index 9b20fded97..962dfd83b0 100644 --- a/cspell.yml +++ b/cspell.yml @@ -34,6 +34,8 @@ ignoreRegExpList: words: - graphiql + - uncoerce + - uncoerced # Different names used inside tests - Skywalker diff --git a/src/execution/__tests__/variables-test.ts b/src/execution/__tests__/variables-test.ts index a6a9d2c43f..ca729d0248 100644 --- a/src/execution/__tests__/variables-test.ts +++ b/src/execution/__tests__/variables-test.ts @@ -150,7 +150,6 @@ const TestType = new GraphQLObjectType({ }), fieldWithNestedInputObject: fieldWithInputArg({ type: TestNestedInputObject, - defaultValue: 'Hello World', }), list: fieldWithInputArg({ type: new GraphQLList(GraphQLString) }), nested: { diff --git a/src/index.ts b/src/index.ts index 66efef71e6..aef9d75b16 100644 --- a/src/index.ts +++ b/src/index.ts @@ -448,6 +448,7 @@ export { // Create a JavaScript value from a GraphQL language AST without a Type. valueFromASTUntyped, // Create a GraphQL language AST from a JavaScript value. + /** @deprecated use `valueToLiteral()` instead with care to operate on external values - `astFromValue()` will be removed in v18 */ astFromValue, // A helper to use within recursive-descent visitors which need to be aware of the GraphQL type system. TypeInfo, diff --git a/src/type/__tests__/enumType-test.ts b/src/type/__tests__/enumType-test.ts index d9888f5199..e4f7219908 100644 --- a/src/type/__tests__/enumType-test.ts +++ b/src/type/__tests__/enumType-test.ts @@ -71,9 +71,7 @@ const QueryType = new GraphQLObjectType({ args: { fromEnum: { type: ComplexEnum, - // Note: defaultValue is provided an *internal* representation for - // Enums, rather than the string name. - defaultValue: Complex1, + defaultValue: 'ONE', }, provideGoodValue: { type: GraphQLBoolean }, provideBadValue: { type: GraphQLBoolean }, diff --git a/src/type/__tests__/validation-test.ts b/src/type/__tests__/validation-test.ts index ca4763595b..71bfba3527 100644 --- a/src/type/__tests__/validation-test.ts +++ b/src/type/__tests__/validation-test.ts @@ -33,10 +33,11 @@ import { GraphQLList, GraphQLNonNull, GraphQLObjectType, + GraphQLScalarType, GraphQLUnionType, } from '../definition.js'; import { assertDirective, GraphQLDirective } from '../directives.js'; -import { GraphQLString } from '../scalars.js'; +import { GraphQLInt, GraphQLString } from '../scalars.js'; import { GraphQLSchema } from '../schema.js'; import { assertValidSchema, validateSchema } from '../validate.js'; @@ -923,7 +924,7 @@ describe('Type System: Input Objects must have fields', () => { expectJSON(validateSchema(schema)).toDeepEqual([ { message: - 'Cannot reference Input Object "SomeInputObject" within itself through a series of non-null fields: "nonNullSelf".', + 'Invalid circular reference. The Input Object SomeInputObject references itself in the non-null field SomeInputObject.nonNullSelf.', locations: [{ line: 7, column: 9 }], }, ]); @@ -951,7 +952,7 @@ describe('Type System: Input Objects must have fields', () => { expectJSON(validateSchema(schema)).toDeepEqual([ { message: - 'Cannot reference Input Object "SomeInputObject" within itself through a series of non-null fields: "startLoop.nextInLoop.closeLoop".', + 'Invalid circular reference. The Input Object SomeInputObject references itself via the non-null fields: SomeInputObject.startLoop, AnotherInputObject.nextInLoop, YetAnotherInputObject.closeLoop.', locations: [ { line: 7, column: 9 }, { line: 11, column: 9 }, @@ -985,7 +986,7 @@ describe('Type System: Input Objects must have fields', () => { expectJSON(validateSchema(schema)).toDeepEqual([ { message: - 'Cannot reference Input Object "SomeInputObject" within itself through a series of non-null fields: "startLoop.closeLoop".', + 'Invalid circular reference. The Input Object SomeInputObject references itself via the non-null fields: SomeInputObject.startLoop, AnotherInputObject.closeLoop.', locations: [ { line: 7, column: 9 }, { line: 11, column: 9 }, @@ -993,7 +994,7 @@ describe('Type System: Input Objects must have fields', () => { }, { message: - 'Cannot reference Input Object "AnotherInputObject" within itself through a series of non-null fields: "startSecondLoop.closeSecondLoop".', + 'Invalid circular reference. The Input Object AnotherInputObject references itself via the non-null fields: AnotherInputObject.startSecondLoop, YetAnotherInputObject.closeSecondLoop.', locations: [ { line: 12, column: 9 }, { line: 16, column: 9 }, @@ -1001,12 +1002,294 @@ describe('Type System: Input Objects must have fields', () => { }, { message: - 'Cannot reference Input Object "YetAnotherInputObject" within itself through a series of non-null fields: "nonNullSelf".', + 'Invalid circular reference. The Input Object YetAnotherInputObject references itself in the non-null field YetAnotherInputObject.nonNullSelf.', locations: [{ line: 17, column: 9 }], }, ]); }); + it('accepts Input Objects with default values without circular references (SDL)', () => { + const validSchema = buildSchema(` + type Query { + field(arg1: A, arg2: B): String + } + + input A { + x: A = null + y: A = { x: null, y: null } + z: [A] = [] + } + + input B { + x: B2! = {} + y: String = "abc" + z: Custom = {} + } + + input B2 { + x: B3 = {} + } + + input B3 { + x: B = { x: { x: null } } + } + + scalar Custom + `); + + expect(validateSchema(validSchema)).to.deep.equal([]); + }); + + it('accepts Input Objects with default values without circular references (programmatic)', () => { + const AType: GraphQLInputObjectType = new GraphQLInputObjectType({ + name: 'A', + fields: () => ({ + x: { type: AType, defaultValue: null }, + y: { type: AType, defaultValue: { x: null, y: null } }, + z: { type: new GraphQLList(AType), defaultValue: [] }, + }), + }); + + const BType: GraphQLInputObjectType = new GraphQLInputObjectType({ + name: 'B', + fields: () => ({ + x: { type: new GraphQLNonNull(B2Type), defaultValue: {} }, + y: { type: GraphQLString, defaultValue: 'abc' }, + z: { type: CustomType, defaultValue: {} }, + }), + }); + + const B2Type: GraphQLInputObjectType = new GraphQLInputObjectType({ + name: 'B2', + fields: () => ({ + x: { type: B3Type, defaultValue: {} }, + }), + }); + + const B3Type: GraphQLInputObjectType = new GraphQLInputObjectType({ + name: 'B3', + fields: () => ({ + x: { type: BType, defaultValue: { x: { x: null } } }, + }), + }); + + const CustomType = new GraphQLScalarType({ name: 'Custom' }); + + const validSchema = new GraphQLSchema({ + query: new GraphQLObjectType({ + name: 'Query', + fields: { + field: { + type: GraphQLString, + args: { + arg1: { type: AType }, + arg2: { type: BType }, + }, + }, + }, + }), + }); + + expect(validateSchema(validSchema)).to.deep.equal([]); + }); + + it('rejects Input Objects with default value circular reference (SDL)', () => { + const invalidSchema = buildSchema(` + type Query { + field(arg1: A, arg2: B, arg3: C, arg4: D, arg5: E): String + } + + input A { + x: A = {} + } + + input B { + x: B2 = {} + } + + input B2 { + x: B3 = {} + } + + input B3 { + x: B = {} + } + + input C { + x: [C] = [{}] + } + + input D { + x: D = { x: { x: {} } } + } + + input E { + x: E = { x: null } + y: E = { y: null } + } + + input F { + x: F2! = {} + } + + input F2 { + x: F = { x: {} } + } + `); + + expectJSON(validateSchema(invalidSchema)).toDeepEqual([ + { + message: + 'Invalid circular reference. The default value of Input Object field A.x references itself.', + locations: [{ line: 7, column: 16 }], + }, + { + message: + 'Invalid circular reference. The default value of Input Object field B.x references itself via the default values of: B2.x, B3.x.', + locations: [ + { line: 11, column: 17 }, + { line: 15, column: 17 }, + { line: 19, column: 16 }, + ], + }, + { + message: + 'Invalid circular reference. The default value of Input Object field C.x references itself.', + locations: [{ line: 23, column: 18 }], + }, + { + message: + 'Invalid circular reference. The default value of Input Object field D.x references itself.', + locations: [{ line: 27, column: 16 }], + }, + { + message: + 'Invalid circular reference. The default value of Input Object field E.x references itself via the default values of: E.y.', + locations: [ + { line: 31, column: 16 }, + { line: 32, column: 16 }, + ], + }, + { + message: + 'Invalid circular reference. The default value of Input Object field F2.x references itself.', + locations: [{ line: 40, column: 16 }], + }, + ]); + }); + + it('rejects Input Objects with default value circular reference (programmatic)', () => { + const AType: GraphQLInputObjectType = new GraphQLInputObjectType({ + name: 'A', + fields: () => ({ + x: { type: AType, defaultValue: {} }, + }), + }); + + const BType: GraphQLInputObjectType = new GraphQLInputObjectType({ + name: 'B', + fields: () => ({ + x: { type: B2Type, defaultValue: {} }, + }), + }); + + const B2Type: GraphQLInputObjectType = new GraphQLInputObjectType({ + name: 'B2', + fields: () => ({ + x: { type: B3Type, defaultValue: {} }, + }), + }); + + const B3Type: GraphQLInputObjectType = new GraphQLInputObjectType({ + name: 'B3', + fields: () => ({ + x: { type: BType, defaultValue: {} }, + }), + }); + + const CType: GraphQLInputObjectType = new GraphQLInputObjectType({ + name: 'C', + fields: () => ({ + x: { type: new GraphQLList(CType), defaultValue: [{}] }, + }), + }); + + const DType: GraphQLInputObjectType = new GraphQLInputObjectType({ + name: 'D', + fields: () => ({ + x: { type: DType, defaultValue: { x: { x: {} } } }, + }), + }); + + const EType: GraphQLInputObjectType = new GraphQLInputObjectType({ + name: 'E', + fields: () => ({ + x: { type: EType, defaultValue: { x: null } }, + y: { type: EType, defaultValue: { y: null } }, + }), + }); + + const FType: GraphQLInputObjectType = new GraphQLInputObjectType({ + name: 'F', + fields: () => ({ + x: { type: new GraphQLNonNull(F2Type), defaultValue: {} }, + }), + }); + + const F2Type: GraphQLInputObjectType = new GraphQLInputObjectType({ + name: 'F2', + fields: () => ({ + x: { type: FType, defaultValue: { x: {} } }, + }), + }); + + const invalidSchema = new GraphQLSchema({ + query: new GraphQLObjectType({ + name: 'Query', + fields: { + field: { + type: GraphQLString, + args: { + arg1: { type: AType }, + arg2: { type: BType }, + arg3: { type: CType }, + arg4: { type: DType }, + arg5: { type: EType }, + arg6: { type: FType }, + }, + }, + }, + }), + }); + + expectJSON(validateSchema(invalidSchema)).toDeepEqual([ + { + message: + 'Invalid circular reference. The default value of Input Object field A.x references itself.', + }, + { + message: + 'Invalid circular reference. The default value of Input Object field B.x references itself via the default values of: B2.x, B3.x.', + }, + { + message: + 'Invalid circular reference. The default value of Input Object field C.x references itself.', + }, + { + message: + 'Invalid circular reference. The default value of Input Object field D.x references itself.', + }, + { + message: + 'Invalid circular reference. The default value of Input Object field E.x references itself via the default values of: E.y.', + }, + { + message: + 'Invalid circular reference. The default value of Input Object field F2.x references itself.', + }, + ]); + }); + it('rejects an Input Object type with incorrectly typed fields', () => { const schema = buildSchema(` type Query { @@ -1039,7 +1322,7 @@ describe('Type System: Input Objects must have fields', () => { ]); }); - it('rejects an Input Object type with required argument that is deprecated', () => { + it('rejects an Input Object type with required field that is deprecated', () => { const schema = buildSchema(` type Query { field(arg: SomeInputObject): String @@ -1733,6 +2016,209 @@ describe('Type System: Arguments must have input types', () => { }); }); +describe('Type System: Argument default values must be valid', () => { + it('rejects an argument with invalid default values (SDL)', () => { + const schema = buildSchema(` + type Query { + field(arg: Int = 3.14): Int + } + + directive @bad(arg: Int = 2.718) on FIELD + `); + + expectJSON(validateSchema(schema)).toDeepEqual([ + { + message: + '@bad(arg:) has invalid default value: Int cannot represent non-integer value: 2.718', + locations: [{ line: 6, column: 33 }], + }, + { + message: + 'Query.field(arg:) has invalid default value: Int cannot represent non-integer value: 3.14', + locations: [{ line: 3, column: 26 }], + }, + ]); + }); + + it('rejects an argument with invalid default values (programmatic)', () => { + const schema = new GraphQLSchema({ + query: new GraphQLObjectType({ + name: 'Query', + fields: { + field: { + type: GraphQLInt, + args: { + arg: { type: GraphQLInt, defaultValue: 3.14 }, + }, + }, + }, + }), + directives: [ + new GraphQLDirective({ + name: 'bad', + args: { + arg: { type: GraphQLInt, defaultValue: 2.718 }, + }, + locations: [DirectiveLocation.FIELD], + }), + ], + }); + + expectJSON(validateSchema(schema)).toDeepEqual([ + { + message: + '@bad(arg:) has invalid default value: Int cannot represent non-integer value: 2.718', + }, + { + message: + 'Query.field(arg:) has invalid default value: Int cannot represent non-integer value: 3.14', + }, + ]); + }); + + it('Attempts to offer a suggested fix if possible (programmatic)', () => { + const Exotic = Symbol('Exotic'); + + const testEnum = new GraphQLEnumType({ + name: 'TestEnum', + values: { + ONE: { value: 1 }, + TWO: { value: Exotic }, + }, + }); + + const testInput: GraphQLInputObjectType = new GraphQLInputObjectType({ + name: 'TestInput', + fields: () => ({ + self: { type: testInput }, + string: { type: new GraphQLNonNull(new GraphQLList(GraphQLString)) }, + enum: { type: new GraphQLList(testEnum) }, + }), + }); + + const schema = new GraphQLSchema({ + query: new GraphQLObjectType({ + name: 'Query', + fields: { + field: { + type: GraphQLInt, + args: { + argWithPossibleFix: { + type: testInput, + defaultValue: { self: null, string: [1], enum: Exotic }, + }, + argWithInvalidPossibleFix: { + type: testInput, + defaultValue: { string: null }, + }, + argWithoutPossibleFix: { + type: testInput, + defaultValue: { enum: 'Exotic' }, + }, + }, + }, + }, + }), + }); + + expectJSON(validateSchema(schema)).toDeepEqual([ + { + message: + 'Query.field(argWithPossibleFix:) has invalid default value: { self: null, string: [1], enum: Symbol(Exotic) }. Did you mean: { self: null, string: ["1"], enum: ["TWO"] }?', + }, + { + message: + 'Query.field(argWithInvalidPossibleFix:) has invalid default value at .string: Expected value of non-null type "[String]!" not to be null.', + }, + { + message: + 'Query.field(argWithoutPossibleFix:) has invalid default value: Expected value of type "TestInput" to include required field "string", found: { enum: "Exotic" }.', + }, + { + message: + 'Query.field(argWithoutPossibleFix:) has invalid default value at .enum: Value "Exotic" does not exist in "TestEnum" enum.', + }, + ]); + }); + + it('Attempts to offer a suggested fix if possible (SDL)', () => { + const originalSchema = buildSchema(` + enum TestEnum { + ONE + TWO + } + + input TestInput { + self: TestInput + string: [String]! + enum: [TestEnum] + } + + type Query { + field( + argWithPossibleFix: TestInput + argWithInvalidPossibleFix: TestInput + argWithoutPossibleFix: TestInput + ): Int + } + `); + + const Exotic = Symbol('Exotic'); + + // workaround as we cannot inject custom internal values into enums defined in SDL + const testEnum = new GraphQLEnumType({ + name: 'TestEnum', + values: { + ONE: { value: 1 }, + TWO: { value: Exotic }, + }, + }); + + const testInput = assertInputObjectType( + originalSchema.getType('TestInput'), + ); + testInput.getFields().enum.type = new GraphQLList(testEnum); + + // workaround as we cannot inject exotic default values into arguments defined in SDL + const QueryType = assertObjectType(originalSchema.getType('Query')); + for (const arg of QueryType.getFields().field.args) { + arg.type = testInput; + switch (arg.name) { + case 'argWithPossibleFix': + arg.defaultValue = { + value: { self: null, string: [1], enum: Exotic }, + }; + break; + case 'argWithInvalidPossibleFix': + arg.defaultValue = { value: { string: null } }; + break; + case 'argWithoutPossibleFix': + arg.defaultValue = { value: { enum: 'Exotic' } }; + break; + } + } + + expectJSON(validateSchema(originalSchema)).toDeepEqual([ + { + message: + 'Query.field(argWithPossibleFix:) has invalid default value: { self: null, string: [1], enum: Symbol(Exotic) }. Did you mean: { self: null, string: ["1"], enum: ["TWO"] }?', + }, + { + message: + 'Query.field(argWithInvalidPossibleFix:) has invalid default value at .string: Expected value of non-null type "[String]!" not to be null.', + }, + { + message: + 'Query.field(argWithoutPossibleFix:) has invalid default value: Expected value of type "TestInput" to include required field "string", found: { enum: "Exotic" }.', + }, + { + message: + 'Query.field(argWithoutPossibleFix:) has invalid default value at .enum: Value "Exotic" does not exist in "TestEnum" enum.', + }, + ]); + }); +}); + describe('Type System: Input Object fields must have input types', () => { function schemaWithInputField( inputFieldConfig: GraphQLInputFieldConfig, @@ -1829,6 +2315,61 @@ describe('Type System: Input Object fields must have input types', () => { }); }); +describe('Type System: Input Object field default values must be valid', () => { + it('rejects an Input Object field with invalid default values (SDL)', () => { + const schema = buildSchema(` + type Query { + field(arg: SomeInputObject): Int + } + + input SomeInputObject { + field: Int = 3.14 + } + `); + + expectJSON(validateSchema(schema)).toDeepEqual([ + { + message: + 'SomeInputObject.field has invalid default value: Int cannot represent non-integer value: 3.14', + locations: [{ line: 7, column: 20 }], + }, + ]); + }); + + it('rejects an Input Object field with invalid default values (programmatic)', () => { + const someInputObject = new GraphQLInputObjectType({ + name: 'SomeInputObject', + fields: { + field: { + type: GraphQLInt, + defaultValue: 3.14, + }, + }, + }); + + const schema = new GraphQLSchema({ + query: new GraphQLObjectType({ + name: 'Query', + fields: { + field: { + type: GraphQLInt, + args: { + arg: { type: someInputObject }, + }, + }, + }, + }), + }); + + expectJSON(validateSchema(schema)).toDeepEqual([ + { + message: + 'SomeInputObject.field has invalid default value: Int cannot represent non-integer value: 3.14', + }, + ]); + }); +}); + describe('Type System: OneOf Input Object fields must be nullable', () => { it('rejects non-nullable fields', () => { const schema = buildSchema(` diff --git a/src/type/introspection.ts b/src/type/introspection.ts index b72b162cbb..8dfacb7933 100644 --- a/src/type/introspection.ts +++ b/src/type/introspection.ts @@ -4,7 +4,7 @@ import { invariant } from '../jsutils/invariant.js'; import { DirectiveLocation } from '../language/directiveLocation.js'; import { print } from '../language/printer.js'; -import { astFromValue } from '../utilities/astFromValue.js'; +import { valueToLiteral } from '../utilities/valueToLiteral.js'; import type { GraphQLEnumValue, @@ -411,7 +411,7 @@ export const __InputValue: GraphQLObjectType = new GraphQLObjectType({ return null; } const literal = - defaultValue.literal ?? astFromValue(defaultValue.value, type); + defaultValue.literal ?? valueToLiteral(defaultValue.value, type); invariant(literal != null, 'Invalid default value'); return print(literal); }, diff --git a/src/type/validate.ts b/src/type/validate.ts index 9fe052635a..9c31094949 100644 --- a/src/type/validate.ts +++ b/src/type/validate.ts @@ -2,12 +2,20 @@ import { AccumulatorMap } from '../jsutils/AccumulatorMap.js'; import { capitalize } from '../jsutils/capitalize.js'; import { andList } from '../jsutils/formatList.js'; import { inspect } from '../jsutils/inspect.js'; +import { invariant } from '../jsutils/invariant.js'; +import { isIterableObject } from '../jsutils/isIterableObject.js'; +import { isObjectLike } from '../jsutils/isObjectLike.js'; +import { keyMap } from '../jsutils/keyMap.js'; +import { mapValue } from '../jsutils/mapValue.js'; import type { Maybe } from '../jsutils/Maybe.js'; +import type { ObjMap } from '../jsutils/ObjMap.js'; +import { printPathArray } from '../jsutils/printPathArray.js'; import { GraphQLError } from '../error/GraphQLError.js'; import type { ASTNode, + ConstValueNode, DirectiveNode, InterfaceTypeDefinitionNode, InterfaceTypeExtensionNode, @@ -18,22 +26,32 @@ import type { UnionTypeExtensionNode, } from '../language/ast.js'; import { OperationTypeNode } from '../language/ast.js'; +import { Kind } from '../language/kinds.js'; import { isEqualType, isTypeSubTypeOf } from '../utilities/typeComparators.js'; +import { + validateInputLiteral, + validateInputValue, +} from '../utilities/validateInputValue.js'; import type { + GraphQLArgument, GraphQLEnumType, GraphQLInputField, GraphQLInputObjectType, + GraphQLInputType, GraphQLInterfaceType, GraphQLObjectType, GraphQLUnionType, } from './definition.js'; import { + assertLeafType, + getNamedType, isEnumType, isInputObjectType, isInputType, isInterfaceType, + isListType, isNamedType, isNonNullType, isObjectType, @@ -192,10 +210,12 @@ function validateDirectives(context: SchemaValidationContext): void { // Ensure they are named correctly. validateName(context, arg); + const argStr = `${directive}(${arg.name}:)`; + // Ensure the type is an input type. if (!isInputType(arg.type)) { context.reportError( - `The type of ${directive}(${arg.name}:) must be Input Type ` + + `The type of ${argStr} must be Input Type ` + `but got: ${inspect(arg.type)}.`, arg.astNode, ); @@ -203,12 +223,130 @@ function validateDirectives(context: SchemaValidationContext): void { if (isRequiredArgument(arg) && arg.deprecationReason != null) { context.reportError( - `Required argument ${directive}(${arg.name}:) cannot be deprecated.`, + `Required argument ${argStr} cannot be deprecated.`, [getDeprecatedDirectiveNode(arg.astNode), arg.astNode?.type], ); } + + validateDefaultValue(context, arg, argStr); + } + } +} + +function validateDefaultValue( + context: SchemaValidationContext, + inputValue: GraphQLArgument | GraphQLInputField, + argStr: string, +): void { + const defaultValue = inputValue.defaultValue; + + if (!defaultValue) { + return; + } + + if (defaultValue.literal) { + validateInputLiteral( + defaultValue.literal, + inputValue.type, + (error, path) => { + context.reportError( + `${argStr} has invalid default value${printPathArray(path)}: ${ + error.message + }`, + error.nodes, + ); + }, + ); + } else { + const errors: Array<[GraphQLError, ReadonlyArray]> = []; + validateInputValue(defaultValue.value, inputValue.type, (error, path) => { + errors.push([error, path]); + }); + + // If there were validation errors, check to see if it can be "uncoerced" + // and then correctly validated. If so, report a clear error with a path + // to resolution. + if (errors.length > 0) { + try { + const uncoercedValue = uncoerceDefaultValue( + defaultValue.value, + inputValue.type, + ); + + const uncoercedErrors = []; + validateInputValue(uncoercedValue, inputValue.type, (error, path) => { + uncoercedErrors.push([error, path]); + }); + + if (uncoercedErrors.length === 0) { + context.reportError( + `${argStr} has invalid default value: ${inspect( + defaultValue.value, + )}. Did you mean: ${inspect(uncoercedValue)}?`, + inputValue.astNode?.defaultValue, + ); + return; + } + } catch (_error) { + // ignore + } + } + + // Otherwise report the original set of errors. + for (const [error, path] of errors) { + context.reportError( + `${argStr} has invalid default value${printPathArray(path)}: ${ + error.message + }`, + inputValue.astNode?.defaultValue, + ); + } + } +} + +/** + * Historically GraphQL.js allowed default values to be provided as + * assumed-coerced "internal" values, however default values should be provided + * as "external" pre-coerced values. `uncoerceDefaultValue()` will convert such + * "internal" values to "external" values to display as part of validation. + * + * This performs the "opposite" of `coerceInputValue()`. Given an "internal" + * coerced value, reverse the process to provide an "external" uncoerced value. + */ +function uncoerceDefaultValue(value: unknown, type: GraphQLInputType): unknown { + if (isNonNullType(type)) { + return uncoerceDefaultValue(value, type.ofType); + } + + if (value === null) { + return null; + } + + if (isListType(type)) { + if (isIterableObject(value)) { + return Array.from(value, (itemValue) => + uncoerceDefaultValue(itemValue, type.ofType), + ); } + return [uncoerceDefaultValue(value, type.ofType)]; } + + if (isInputObjectType(type)) { + invariant(isObjectLike(value)); + const fieldDefs = type.getFields(); + return mapValue(value, (fieldValue, fieldName) => { + invariant(fieldName in fieldDefs); + return uncoerceDefaultValue(fieldValue, fieldDefs[fieldName].type); + }); + } + + assertLeafType(type); + + // For most leaf types (Scalars, Enums), result coercion ("serialize") is + // the inverse of input coercion ("parseValue") and will produce an + // "external" value. Historically, this method was also used as part of the + // now-removed "astFromValue" to perform the same behavior. + return type.serialize(value); } function validateName( @@ -225,8 +363,11 @@ function validateName( } function validateTypes(context: SchemaValidationContext): void { - const validateInputObjectCircularRefs = - createInputObjectCircularRefsValidator(context); + // Ensure Input Objects do not contain non-nullable circular references. + const validateInputObjectNonNullCircularRefs = + createInputObjectNonNullCircularRefsValidator(context); + const validateInputObjectDefaultValueCircularRefs = + createInputObjectDefaultValueCircularRefsValidator(context); const typeMap = context.schema.getTypeMap(); for (const type of Object.values(typeMap)) { // Ensure all provided types are in fact GraphQL type. @@ -265,8 +406,12 @@ function validateTypes(context: SchemaValidationContext): void { // Ensure Input Object fields are valid. validateInputFields(context, type); - // Ensure Input Objects do not contain non-nullable circular references - validateInputObjectCircularRefs(type); + // Ensure Input Objects do not contain invalid field circular references. + // Ensure Input Objects do not contain non-nullable circular references. + validateInputObjectNonNullCircularRefs(type); + + // Ensure Input Objects do not contain invalid default value circular references. + validateInputObjectDefaultValueCircularRefs(type); } } } @@ -305,11 +450,12 @@ function validateFields( // Ensure they are named correctly. validateName(context, arg); + const argStr = `${type}.${field.name}(${argName}:)`; + // Ensure the type is an input type if (!isInputType(arg.type)) { context.reportError( - `The type of ${type}.${field.name}(${argName}:) must be Input ` + - `Type but got: ${inspect(arg.type)}.`, + `The type of ${argStr} must be Input Type but got: ${inspect(arg.type)}.`, arg.astNode?.type, ); } @@ -320,6 +466,8 @@ function validateFields( [getDeprecatedDirectiveNode(arg.astNode), arg.astNode?.type], ); } + + validateDefaultValue(context, arg, argStr); } } } @@ -420,8 +568,6 @@ function validateTypeImplementsInterface( [ifaceArg.astNode?.type, typeArg.astNode?.type], ); } - - // TODO: validate default values? } // Assert additional arguments must not be required. @@ -528,7 +674,7 @@ function validateInputFields( ); } - // Ensure the arguments are valid + // Ensure the input fields are valid for (const field of fields) { // Ensure they are named correctly. validateName(context, field); @@ -542,13 +688,17 @@ function validateInputFields( ); } + const fieldStr = `${inputObj.name}.${field.name}`; + if (isRequiredInputField(field) && field.deprecationReason != null) { context.reportError( - `Required input field ${inputObj.name}.${field.name} cannot be deprecated.`, + `Required input field ${fieldStr} cannot be deprecated.`, [getDeprecatedDirectiveNode(field.astNode), field.astNode?.type], ); } + validateDefaultValue(context, field, fieldStr); + if (inputObj.isOneOf) { validateOneOfInputObjectField(inputObj, field, context); } @@ -575,7 +725,7 @@ function validateOneOfInputObjectField( } } -function createInputObjectCircularRefsValidator( +function createInputObjectNonNullCircularRefsValidator( context: SchemaValidationContext, ): (inputObj: GraphQLInputObjectType) => void { // Modified copy of algorithm from 'src/validation/rules/NoFragmentCycles.js'. @@ -584,10 +734,11 @@ function createInputObjectCircularRefsValidator( const visitedTypes = new Set(); // Array of types nodes used to produce meaningful errors - const fieldPath: Array = []; + const fieldPath: Array<{ fieldStr: string; astNode: Maybe }> = []; // Position in the type path - const fieldPathIndexByTypeName = Object.create(null); + const fieldPathIndexByTypeName: ObjMap = + Object.create(null); return detectCycleRecursive; @@ -608,14 +759,23 @@ function createInputObjectCircularRefsValidator( const fieldType = field.type.ofType; const cycleIndex = fieldPathIndexByTypeName[fieldType.name]; - fieldPath.push(field); + fieldPath.push({ + fieldStr: `${inputObj}.${field.name}`, + astNode: field.astNode, + }); if (cycleIndex === undefined) { detectCycleRecursive(fieldType); } else { const cyclePath = fieldPath.slice(cycleIndex); - const pathStr = cyclePath.map((fieldObj) => fieldObj.name).join('.'); + const pathStr = cyclePath + .map((fieldObj) => fieldObj.fieldStr) + .join(', '); context.reportError( - `Cannot reference Input Object "${fieldType}" within itself through a series of non-null fields: "${pathStr}".`, + `Invalid circular reference. The Input Object ${fieldType} references itself ${ + cyclePath.length > 1 + ? 'via the non-null fields:' + : 'in the non-null field' + } ${pathStr}.`, cyclePath.map((fieldObj) => fieldObj.astNode), ); } @@ -627,6 +787,161 @@ function createInputObjectCircularRefsValidator( } } +function createInputObjectDefaultValueCircularRefsValidator( + context: SchemaValidationContext, +): (inputObj: GraphQLInputObjectType) => 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 visitedFields = Object.create(null); + + // Array of keys for fields and default values used to produce meaningful errors. + const fieldPath: Array< + [fieldStr: string, defaultValue: ConstValueNode | undefined] + > = []; + + // Position in the path + const fieldPathIndex: ObjMap = Object.create(null); + + // 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. + return function validateInputObjectDefaultValueCircularRefs( + inputObj: GraphQLInputObjectType, + ): void { + // Start with an empty object as a way to visit every field in this input + // object type and apply every default value. + return detectValueDefaultValueCycle(inputObj, {}); + }; + + function detectValueDefaultValueCycle( + inputObj: GraphQLInputObjectType, + defaultValue: unknown, + ): void { + // If the value is a List, recursively check each entry for a cycle. + // Otherwise, only object values can contain a cycle. + if (isIterableObject(defaultValue)) { + for (const itemValue of defaultValue) { + detectValueDefaultValueCycle(inputObj, itemValue); + } + return; + } else if (!isObjectLike(defaultValue)) { + return; + } + + // Check each defined field for a cycle. + for (const field of Object.values(inputObj.getFields())) { + const namedFieldType = getNamedType(field.type); + + // Only input object type fields can result in a cycle. + if (!isInputObjectType(namedFieldType)) { + continue; + } + + if (Object.hasOwn(defaultValue, field.name)) { + // If the provided value has this field defined, recursively check it + // for cycles. + detectValueDefaultValueCycle(namedFieldType, defaultValue[field.name]); + } else { + // Otherwise check this field's default value for cycles. + detectFieldDefaultValueCycle( + field, + namedFieldType, + `${inputObj}.${field.name}`, + ); + } + } + } + + function detectLiteralDefaultValueCycle( + inputObj: GraphQLInputObjectType, + defaultValue: ConstValueNode, + ): void { + // If the value is a List, recursively check each entry for a cycle. + // Otherwise, only object values can contain a cycle. + if (defaultValue.kind === Kind.LIST) { + for (const itemLiteral of defaultValue.values) { + detectLiteralDefaultValueCycle(inputObj, itemLiteral); + } + return; + } else if (defaultValue.kind !== Kind.OBJECT) { + return; + } + + // Check each defined field for a cycle. + const fieldNodes = keyMap(defaultValue.fields, (field) => field.name.value); + for (const field of Object.values(inputObj.getFields())) { + const namedFieldType = getNamedType(field.type); + + // Only input object type fields can result in a cycle. + if (!isInputObjectType(namedFieldType)) { + continue; + } + + if (Object.hasOwn(fieldNodes, field.name)) { + // If the provided value has this field defined, recursively check it + // for cycles. + detectLiteralDefaultValueCycle( + namedFieldType, + fieldNodes[field.name].value, + ); + } else { + // Otherwise check this field's default value for cycles. + detectFieldDefaultValueCycle( + field, + namedFieldType, + `${inputObj}.${field.name}`, + ); + } + } + } + + function detectFieldDefaultValueCycle( + field: GraphQLInputField, + fieldType: GraphQLInputObjectType, + fieldStr: string, + ): void { + // Only a field with a default value can result in a cycle. + const defaultValue = field.defaultValue; + if (defaultValue === undefined) { + return; + } + + // Check to see if there is cycle. + const cycleIndex = fieldPathIndex[fieldStr]; + if (cycleIndex !== undefined && cycleIndex > 0) { + context.reportError( + `Invalid circular reference. The default value of Input Object field ${fieldStr} references itself${ + cycleIndex < fieldPath.length + ? ` via the default values of: ${fieldPath + .slice(cycleIndex) + .map(([stringForMessage]) => stringForMessage) + .join(', ')}` + : '' + }.`, + fieldPath.slice(cycleIndex - 1).map(([, node]) => node), + ); + return; + } + + // Recurse into this field's default value once, tracking the path. + if (visitedFields[fieldStr] === undefined) { + visitedFields[fieldStr] = true; + fieldPathIndex[fieldStr] = fieldPath.push([ + fieldStr, + field.astNode?.defaultValue, + ]); + if (defaultValue.literal) { + detectLiteralDefaultValueCycle(fieldType, defaultValue.literal); + } else { + detectValueDefaultValueCycle(fieldType, defaultValue.value); + } + fieldPath.pop(); + fieldPathIndex[fieldStr] = undefined; + } + } +} + function getAllImplementsInterfaceNodes( type: GraphQLObjectType | GraphQLInterfaceType, iface: GraphQLInterfaceType, diff --git a/src/utilities/__tests__/astFromValue-test.ts b/src/utilities/__tests__/astFromValue-test.ts index 46a5beaea6..c5aba4a956 100644 --- a/src/utilities/__tests__/astFromValue-test.ts +++ b/src/utilities/__tests__/astFromValue-test.ts @@ -18,6 +18,7 @@ import { import { astFromValue } from '../astFromValue.js'; +/** @deprecated use `valueToLiteral()` instead with care to operate on external values - `astFromValue()` will be removed in v18 */ describe('astFromValue', () => { it('converts boolean values to ASTs', () => { expect(astFromValue(true, GraphQLBoolean)).to.deep.equal({ diff --git a/src/utilities/astFromValue.ts b/src/utilities/astFromValue.ts index 46758d7734..3ace598d91 100644 --- a/src/utilities/astFromValue.ts +++ b/src/utilities/astFromValue.ts @@ -37,6 +37,7 @@ import { GraphQLID } from '../type/scalars.js'; * | Unknown | Enum Value | * | null | NullValue | * + * @deprecated use `valueToLiteral()` instead with care to operate on external values - `astFromValue()` will be removed in v18 */ export function astFromValue( value: unknown, diff --git a/src/utilities/coerceInputValue.ts b/src/utilities/coerceInputValue.ts index 903733d967..a946884bb0 100644 --- a/src/utilities/coerceInputValue.ts +++ b/src/utilities/coerceInputValue.ts @@ -1,3 +1,4 @@ +import { invariant } from '../jsutils/invariant.js'; import { isIterableObject } from '../jsutils/isIterableObject.js'; import { isObjectLike } from '../jsutils/isObjectLike.js'; import type { Maybe } from '../jsutils/Maybe.js'; @@ -308,7 +309,8 @@ export function coerceDefaultValue( if (coercedValue === undefined) { coercedValue = defaultValue.literal ? coerceInputLiteral(defaultValue.literal, type) - : defaultValue.value; + : coerceInputValue(defaultValue.value, type); + invariant(coercedValue !== undefined); (defaultValue as any)._memoizedCoercedValue = coercedValue; } return coercedValue; diff --git a/src/utilities/findSchemaChanges.ts b/src/utilities/findSchemaChanges.ts index 909e3161e6..ab08896f11 100644 --- a/src/utilities/findSchemaChanges.ts +++ b/src/utilities/findSchemaChanges.ts @@ -32,8 +32,8 @@ import { import { isSpecifiedScalarType } from '../type/scalars.js'; import type { GraphQLSchema } from '../type/schema.js'; -import { astFromValue } from './astFromValue.js'; import { sortValueNode } from './sortValueNode.js'; +import { valueToLiteral } from './valueToLiteral.js'; enum BreakingChangeType { TYPE_REMOVED = 'TYPE_REMOVED', @@ -635,7 +635,7 @@ function stringifyValue( defaultValue: GraphQLDefaultValueUsage, type: GraphQLInputType, ): string { - const ast = defaultValue.literal ?? astFromValue(defaultValue.value, type); + const ast = defaultValue.literal ?? valueToLiteral(defaultValue.value, type); invariant(ast != null); return print(sortValueNode(ast)); } diff --git a/src/utilities/index.ts b/src/utilities/index.ts index a9691f6125..5b891cded1 100644 --- a/src/utilities/index.ts +++ b/src/utilities/index.ts @@ -66,7 +66,10 @@ export { export { valueFromASTUntyped } from './valueFromASTUntyped.js'; // Create a GraphQL language AST from a JavaScript value. -export { astFromValue } from './astFromValue.js'; +export { + /** @deprecated use `valueToLiteral()` instead with care to operate on external values - `astFromValue()` will be removed in v18 */ + astFromValue, +} from './astFromValue.js'; // A helper to use within recursive-descent visitors which need to be aware of the GraphQL type system. export { TypeInfo, visitWithTypeInfo } from './TypeInfo.js'; diff --git a/src/utilities/printSchema.ts b/src/utilities/printSchema.ts index b02580c6fa..52fc342626 100644 --- a/src/utilities/printSchema.ts +++ b/src/utilities/printSchema.ts @@ -34,7 +34,7 @@ import { isIntrospectionType } from '../type/introspection.js'; import { isSpecifiedScalarType } from '../type/scalars.js'; import type { GraphQLSchema } from '../type/schema.js'; -import { astFromValue } from './astFromValue.js'; +import { valueToLiteral } from './valueToLiteral.js'; export function printSchema(schema: GraphQLSchema): string { return printFilteredSchema( @@ -263,7 +263,7 @@ function printInputValue(arg: GraphQLInputField): string { if (arg.defaultValue) { const literal = arg.defaultValue.literal ?? - astFromValue(arg.defaultValue.value, arg.type); + valueToLiteral(arg.defaultValue.value, arg.type); invariant(literal != null, 'Invalid default value'); argDecl += ` = ${print(literal)}`; } From cda413d17956e94293352a0dc427517f4dfedb92 Mon Sep 17 00:00:00 2001 From: Yaacov Rydzinski Date: Sun, 27 Oct 2024 21:20:54 +0200 Subject: [PATCH 2/2] update terms with recent scalar changes --- src/type/validate.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/type/validate.ts b/src/type/validate.ts index 9c31094949..48cc70299a 100644 --- a/src/type/validate.ts +++ b/src/type/validate.ts @@ -342,11 +342,11 @@ function uncoerceDefaultValue(value: unknown, type: GraphQLInputType): unknown { assertLeafType(type); - // For most leaf types (Scalars, Enums), result coercion ("serialize") is + // For most leaf types (Scalars, Enums), output value coercion ("serialize") is // the inverse of input coercion ("parseValue") and will produce an // "external" value. Historically, this method was also used as part of the - // now-removed "astFromValue" to perform the same behavior. - return type.serialize(value); + // now-deprecated "astFromValue" to perform the same behavior. + return type.coerceOutputValue(value); } function validateName(