Skip to content

Commit dab9e3d

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 803d83b commit dab9e3d

19 files changed

+692
-515
lines changed

src/index.d.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -415,6 +415,8 @@ export {
415415
visitWithTypeInfo,
416416
// Coerces a JavaScript value to a GraphQL type, or produces errors.
417417
coerceInputValue,
418+
// Validate a JavaScript value with a GraphQL type, collecting all errors.
419+
validateInputValue,
418420
// Concatenates multiple AST together.
419421
concatAST,
420422
// Separates an AST into an AST per Operation.

src/index.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -404,6 +404,8 @@ export {
404404
visitWithTypeInfo,
405405
// Coerces a JavaScript value to a GraphQL type, or produces errors.
406406
coerceInputValue,
407+
// Validate a JavaScript value with a GraphQL type, collecting all errors.
408+
validateInputValue,
407409
// Concatenates multiple AST together.
408410
concatAST,
409411
// Separates an AST into an AST per Operation.

src/type/__tests__/validation-test.js

Lines changed: 98 additions & 0 deletions
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

Lines changed: 71 additions & 3 deletions
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';
@@ -830,7 +831,10 @@ export function defineArguments(
830831
name: argName,
831832
description: argConfig.description,
832833
type: argConfig.type,
833-
defaultValue: argConfig.defaultValue,
834+
defaultValue: uncoerceDefaultValue(
835+
argConfig.defaultValue,
836+
argConfig.type,
837+
),
834838
deprecationReason: argConfig.deprecationReason,
835839
extensions: argConfig.extensions && toObjMap(argConfig.extensions),
836840
astNode: argConfig.astNode,
@@ -1483,7 +1487,10 @@ export class GraphQLInputObjectType {
14831487

14841488
getFields(): GraphQLInputFieldMap {
14851489
if (typeof this._fields === 'function') {
1486-
this._fields = this._fields();
1490+
const _fields = this._fields;
1491+
// Assign before call to avoid potential infinite recursion.
1492+
this._fields = {};
1493+
this._fields = _fields();
14871494
}
14881495
return this._fields;
14891496
}
@@ -1539,7 +1546,10 @@ function defineInputFieldMap(
15391546
name: fieldName,
15401547
description: fieldConfig.description,
15411548
type: fieldConfig.type,
1542-
defaultValue: fieldConfig.defaultValue,
1549+
defaultValue: uncoerceDefaultValue(
1550+
fieldConfig.defaultValue,
1551+
fieldConfig.type,
1552+
),
15431553
deprecationReason: fieldConfig.deprecationReason,
15441554
extensions: fieldConfig.extensions && toObjMap(fieldConfig.extensions),
15451555
astNode: fieldConfig.astNode,
@@ -1591,3 +1601,61 @@ export function isRequiredInputField(
15911601
}
15921602

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

src/type/introspection.js

Lines changed: 4 additions & 1 deletion
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)