Skip to content

Commit d3e4125

Browse files
committed
Significant additional changes
- 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
1 parent 8ed299d commit d3e4125

19 files changed

+692
-515
lines changed

src/index.d.ts

+2
Original file line numberDiff line numberDiff line change
@@ -407,6 +407,8 @@ export {
407407
visitWithTypeInfo,
408408
// Coerces a JavaScript value to a GraphQL type, or produces errors.
409409
coerceInputValue,
410+
// Validate a JavaScript value with a GraphQL type, collecting all errors.
411+
validateInputValue,
410412
// Concatenates multiple AST together.
411413
concatAST,
412414
// Separates an AST into an AST per Operation.

src/index.js

+2
Original file line numberDiff line numberDiff line change
@@ -396,6 +396,8 @@ export {
396396
visitWithTypeInfo,
397397
// Coerces a JavaScript value to a GraphQL type, or produces errors.
398398
coerceInputValue,
399+
// Validate a JavaScript value with a GraphQL type, collecting all errors.
400+
validateInputValue,
399401
// Concatenates multiple AST together.
400402
concatAST,
401403
// Separates an AST into an AST per Operation.

src/type/__tests__/validation-test.js

+98
Original file line numberDiff line numberDiff line change
@@ -867,6 +867,104 @@ describe('Type System: Input Objects must have fields', () => {
867867
]);
868868
});
869869

870+
it('rejects Input Objects with default value circular reference', () => {
871+
const validSchema = buildSchema(`
872+
type Query {
873+
field(arg1: A, arg2: B): String
874+
}
875+
876+
input A {
877+
x: A = null
878+
y: A = { x: null, y: null }
879+
z: [A] = []
880+
}
881+
882+
input B {
883+
x: B2 = {}
884+
}
885+
886+
input B2 {
887+
x: B3 = {}
888+
}
889+
890+
input B3 {
891+
x: B = { x: { x: null } }
892+
}
893+
`);
894+
895+
expect(validateSchema(validSchema)).to.deep.equal([]);
896+
897+
const invalidSchema = buildSchema(`
898+
type Query {
899+
field(arg1: A, arg2: B, arg3: C, arg4: D, arg5: E): String
900+
}
901+
902+
input A {
903+
x: A = {}
904+
}
905+
906+
input B {
907+
x: B2 = {}
908+
}
909+
910+
input B2 {
911+
x: B3 = {}
912+
}
913+
914+
input B3 {
915+
x: B = {}
916+
}
917+
918+
input C {
919+
x: [C] = [{}]
920+
}
921+
922+
input D {
923+
x: D = { x: { x: {} } }
924+
}
925+
926+
input E {
927+
x: E = { x: null }
928+
y: E = { y: null }
929+
}
930+
`);
931+
932+
expect(validateSchema(invalidSchema)).to.deep.equal([
933+
{
934+
message:
935+
'Cannot reference Input Object field A.x within itself through a series of default values: A.x.',
936+
locations: [{ line: 7, column: 16 }],
937+
},
938+
{
939+
message:
940+
'Cannot reference Input Object field B.x within itself through a series of default values: B.x, B2.x, B3.x.',
941+
locations: [
942+
{ line: 11, column: 17 },
943+
{ line: 15, column: 17 },
944+
{ line: 19, column: 16 },
945+
],
946+
},
947+
{
948+
message:
949+
'Cannot reference Input Object field C.x within itself through a series of default values: C.x.',
950+
locations: [{ line: 23, column: 18 }],
951+
},
952+
{
953+
message:
954+
'Cannot reference Input Object field D.x within itself through a series of default values: D.x.',
955+
locations: [{ line: 27, column: 16 }],
956+
},
957+
{
958+
message:
959+
'Cannot reference Input Object field E.x within itself through a series of default values: E.x, E.y.',
960+
locations: [
961+
{ line: 31, column: 16 },
962+
{ line: 32, column: 16 },
963+
],
964+
},
965+
]);
966+
});
967+
870968
it('rejects an Input Object type with incorrectly typed fields', () => {
871969
const schema = buildSchema(`
872970
type Query {

src/type/definition.js

+71-3
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ import { devAssert } from '../jsutils/devAssert';
1313
import { keyValMap } from '../jsutils/keyValMap';
1414
import { instanceOf } from '../jsutils/instanceOf';
1515
import { didYouMean } from '../jsutils/didYouMean';
16+
import { isIterableObject } from '../jsutils/isIterableObject';
1617
import { isObjectLike } from '../jsutils/isObjectLike';
1718
import { identityFunc } from '../jsutils/identityFunc';
1819
import { suggestionList } from '../jsutils/suggestionList';
@@ -813,7 +814,10 @@ function defineFieldMap<TSource, TContext>(
813814
name: argName,
814815
description: argConfig.description,
815816
type: argConfig.type,
816-
defaultValue: argConfig.defaultValue,
817+
defaultValue: uncoerceDefaultValue(
818+
argConfig.defaultValue,
819+
argConfig.type,
820+
),
817821
deprecationReason: argConfig.deprecationReason,
818822
extensions: argConfig.extensions && toObjMap(argConfig.extensions),
819823
astNode: argConfig.astNode,
@@ -1479,7 +1483,10 @@ export class GraphQLInputObjectType {
14791483

14801484
getFields(): GraphQLInputFieldMap {
14811485
if (typeof this._fields === 'function') {
1482-
this._fields = this._fields();
1486+
const _fields = this._fields;
1487+
// Assign before call to avoid potential infinite recursion.
1488+
this._fields = {};
1489+
this._fields = _fields();
14831490
}
14841491
return this._fields;
14851492
}
@@ -1535,7 +1542,10 @@ function defineInputFieldMap(
15351542
name: fieldName,
15361543
description: fieldConfig.description,
15371544
type: fieldConfig.type,
1538-
defaultValue: fieldConfig.defaultValue,
1545+
defaultValue: uncoerceDefaultValue(
1546+
fieldConfig.defaultValue,
1547+
fieldConfig.type,
1548+
),
15391549
deprecationReason: fieldConfig.deprecationReason,
15401550
extensions: fieldConfig.extensions && toObjMap(fieldConfig.extensions),
15411551
astNode: fieldConfig.astNode,
@@ -1587,3 +1597,61 @@ export function isRequiredInputField(
15871597
}
15881598

15891599
export type GraphQLInputFieldMap = ObjMap<GraphQLInputField>;
1600+
1601+
/**
1602+
* Historically GraphQL.js allowed default values to be provided as
1603+
* assumed-coerced "internal" values, however default values should be provided
1604+
* as "external" pre-coerced values. `uncoerceDefaultValue()` will convert such
1605+
* "internal" values to "external" values to avoid breaking existing clients.
1606+
*
1607+
* This performs the opposite of `coerceInputValue()`. Given an "internal"
1608+
* coerced value, reverse the process to provide an "external" uncoerced value.
1609+
*
1610+
* This function should not throw Errors on incorrectly shaped input. Instead
1611+
* it will simply pass through such values directly.
1612+
*
1613+
*/
1614+
function uncoerceDefaultValue(value: mixed, type: GraphQLInputType): mixed {
1615+
// Explicitly return the value null.
1616+
if (value === null) {
1617+
return null;
1618+
}
1619+
1620+
// Unwrap type
1621+
const namedType = getNamedType(type);
1622+
1623+
if (isIterableObject(value)) {
1624+
return Array.from(value, (itemValue) =>
1625+
uncoerceDefaultValue(itemValue, namedType),
1626+
);
1627+
}
1628+
1629+
if (isInputObjectType(namedType)) {
1630+
if (!isObjectLike(value)) {
1631+
return value;
1632+
}
1633+
1634+
const fieldDefs = namedType.getFields();
1635+
return mapValue(value, (fieldValue, fieldName) =>
1636+
fieldName in fieldDefs
1637+
? uncoerceDefaultValue(fieldValue, fieldDefs[fieldName].type)
1638+
: fieldValue,
1639+
);
1640+
}
1641+
1642+
if (isLeafType(namedType)) {
1643+
try {
1644+
// For leaf types (Scalars, Enums), serialize is the oppose of coercion
1645+
// (parseValue) and will produce an "external" value.
1646+
return namedType.serialize(value);
1647+
} catch (error) {
1648+
// Ingore any invalid data errors.
1649+
// istanbul ignore next - serialize should only throw GraphQLError
1650+
if (!(error instanceof GraphQLError)) {
1651+
throw error;
1652+
}
1653+
}
1654+
}
1655+
1656+
return value;
1657+
}

src/type/introspection.js

+4-1
Original file line numberDiff line numberDiff line change
@@ -384,7 +384,10 @@ export const __InputValue: GraphQLObjectType = new GraphQLObjectType({
384384
'A GraphQL-formatted string representing the default value for this input value.',
385385
resolve(inputValue) {
386386
const { type, defaultValue } = inputValue;
387-
const valueAST = astFromValue(defaultValue, type);
387+
const valueAST =
388+
defaultValue !== undefined
389+
? astFromValue(defaultValue, type)
390+
: undefined;
388391
return valueAST ? print(valueAST) : null;
389392
},
390393
},

0 commit comments

Comments
 (0)