Skip to content

Commit

Permalink
Significant additional changes
Browse files Browse the repository at this point in the history
- Rewrites default value circular reference checking as "detectDefaultValueCycle()"
- Adds test suite for default value circular references
- Moves default value validation to utility "validateInputValue()"
- Adds "uncoerceDefaultValue()" to preserve behavior of existing programmatically provided default values
- Rewrites "astFromValue()" to remove "uncoerce" and type checking behavior. It used to operate on "internal" coerced values, now it operates on assumed uncoerced values. (also re-writes a bunch of its test suite).
- Extracts "validateInputValue()" from "coerceInputValue()" so it can be used separately
  • Loading branch information
leebyron committed Apr 28, 2021
1 parent 8ed299d commit d3e4125
Show file tree
Hide file tree
Showing 19 changed files with 692 additions and 515 deletions.
2 changes: 2 additions & 0 deletions src/index.d.ts
Expand Up @@ -407,6 +407,8 @@ export {
visitWithTypeInfo,
// Coerces a JavaScript value to a GraphQL type, or produces errors.
coerceInputValue,
// Validate a JavaScript value with a GraphQL type, collecting all errors.
validateInputValue,
// Concatenates multiple AST together.
concatAST,
// Separates an AST into an AST per Operation.
Expand Down
2 changes: 2 additions & 0 deletions src/index.js
Expand Up @@ -396,6 +396,8 @@ export {
visitWithTypeInfo,
// Coerces a JavaScript value to a GraphQL type, or produces errors.
coerceInputValue,
// Validate a JavaScript value with a GraphQL type, collecting all errors.
validateInputValue,
// Concatenates multiple AST together.
concatAST,
// Separates an AST into an AST per Operation.
Expand Down
98 changes: 98 additions & 0 deletions src/type/__tests__/validation-test.js
Expand Up @@ -867,6 +867,104 @@ describe('Type System: Input Objects must have fields', () => {
]);
});

it('rejects Input Objects with default value circular reference', () => {
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 = {}
}
input B2 {
x: B3 = {}
}
input B3 {
x: B = { x: { x: null } }
}
`);

expect(validateSchema(validSchema)).to.deep.equal([]);

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 }
}
`);

expect(validateSchema(invalidSchema)).to.deep.equal([
{
message:
'Cannot reference Input Object field A.x within itself through a series of default values: A.x.',
locations: [{ line: 7, column: 16 }],
},
{
message:
'Cannot reference Input Object field B.x within itself through a series of default values: B.x, B2.x, B3.x.',
locations: [
{ line: 11, column: 17 },
{ line: 15, column: 17 },
{ line: 19, column: 16 },
],
},
{
message:
'Cannot reference Input Object field C.x within itself through a series of default values: C.x.',
locations: [{ line: 23, column: 18 }],
},
{
message:
'Cannot reference Input Object field D.x within itself through a series of default values: D.x.',
locations: [{ line: 27, column: 16 }],
},
{
message:
'Cannot reference Input Object field E.x within itself through a series of default values: E.x, E.y.',
locations: [
{ line: 31, column: 16 },
{ line: 32, column: 16 },
],
},
]);
});

it('rejects an Input Object type with incorrectly typed fields', () => {
const schema = buildSchema(`
type Query {
Expand Down
74 changes: 71 additions & 3 deletions src/type/definition.js
Expand Up @@ -13,6 +13,7 @@ import { devAssert } from '../jsutils/devAssert';
import { keyValMap } from '../jsutils/keyValMap';
import { instanceOf } from '../jsutils/instanceOf';
import { didYouMean } from '../jsutils/didYouMean';
import { isIterableObject } from '../jsutils/isIterableObject';
import { isObjectLike } from '../jsutils/isObjectLike';
import { identityFunc } from '../jsutils/identityFunc';
import { suggestionList } from '../jsutils/suggestionList';
Expand Down Expand Up @@ -813,7 +814,10 @@ function defineFieldMap<TSource, TContext>(
name: argName,
description: argConfig.description,
type: argConfig.type,
defaultValue: argConfig.defaultValue,
defaultValue: uncoerceDefaultValue(
argConfig.defaultValue,
argConfig.type,
),
deprecationReason: argConfig.deprecationReason,
extensions: argConfig.extensions && toObjMap(argConfig.extensions),
astNode: argConfig.astNode,
Expand Down Expand Up @@ -1479,7 +1483,10 @@ export class GraphQLInputObjectType {

getFields(): GraphQLInputFieldMap {
if (typeof this._fields === 'function') {
this._fields = this._fields();
const _fields = this._fields;
// Assign before call to avoid potential infinite recursion.
this._fields = {};
this._fields = _fields();
}
return this._fields;
}
Expand Down Expand Up @@ -1535,7 +1542,10 @@ function defineInputFieldMap(
name: fieldName,
description: fieldConfig.description,
type: fieldConfig.type,
defaultValue: fieldConfig.defaultValue,
defaultValue: uncoerceDefaultValue(
fieldConfig.defaultValue,
fieldConfig.type,
),
deprecationReason: fieldConfig.deprecationReason,
extensions: fieldConfig.extensions && toObjMap(fieldConfig.extensions),
astNode: fieldConfig.astNode,
Expand Down Expand Up @@ -1587,3 +1597,61 @@ export function isRequiredInputField(
}

export type GraphQLInputFieldMap = ObjMap<GraphQLInputField>;

/**
* 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 avoid breaking existing clients.
*
* This performs the opposite of `coerceInputValue()`. Given an "internal"
* coerced value, reverse the process to provide an "external" uncoerced value.
*
* This function should not throw Errors on incorrectly shaped input. Instead
* it will simply pass through such values directly.
*
*/
function uncoerceDefaultValue(value: mixed, type: GraphQLInputType): mixed {
// Explicitly return the value null.
if (value === null) {
return null;
}

// Unwrap type
const namedType = getNamedType(type);

if (isIterableObject(value)) {
return Array.from(value, (itemValue) =>
uncoerceDefaultValue(itemValue, namedType),
);
}

if (isInputObjectType(namedType)) {
if (!isObjectLike(value)) {
return value;
}

const fieldDefs = namedType.getFields();
return mapValue(value, (fieldValue, fieldName) =>
fieldName in fieldDefs
? uncoerceDefaultValue(fieldValue, fieldDefs[fieldName].type)
: fieldValue,
);
}

if (isLeafType(namedType)) {
try {
// For leaf types (Scalars, Enums), serialize is the oppose of coercion
// (parseValue) and will produce an "external" value.
return namedType.serialize(value);
} catch (error) {
// Ingore any invalid data errors.
// istanbul ignore next - serialize should only throw GraphQLError
if (!(error instanceof GraphQLError)) {
throw error;
}
}
}

return value;
}
5 changes: 4 additions & 1 deletion src/type/introspection.js
Expand Up @@ -384,7 +384,10 @@ export const __InputValue: GraphQLObjectType = new GraphQLObjectType({
'A GraphQL-formatted string representing the default value for this input value.',
resolve(inputValue) {
const { type, defaultValue } = inputValue;
const valueAST = astFromValue(defaultValue, type);
const valueAST =
defaultValue !== undefined
? astFromValue(defaultValue, type)
: undefined;
return valueAST ? print(valueAST) : null;
},
},
Expand Down

0 comments on commit d3e4125

Please sign in to comment.