Skip to content

Commit e833a5a

Browse files
leebyronyaacovCR
leebyron
authored and
yaacovCR
committed
Input Value Validation
Factors out input validation to reusable functions: * Introduces `validateInputLiteral` by extracting this behavior from `ValuesOfCorrectTypeRule`. * Introduces `validateInputValue` by extracting this behavior from `coerceInputValue` * Simplifies `coerceInputValue` to return early on validation error * Unifies error reporting between `validateInputValue` and `validateInputLiteral`, causing some error message strings to change, but error data (eg locations) are preserved. These two parallel functions will be used to validate default values in #3049 Potentially breaking if you rely on the existing behavior of `coerceInputValue` to call a callback function, as the call signature has changed. GraphQL behavior should not change, though error messages are now slightly different.
1 parent f03f2c5 commit e833a5a

File tree

14 files changed

+1465
-659
lines changed

14 files changed

+1465
-659
lines changed

‎src/execution/__tests__/nonnull-test.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -647,7 +647,7 @@ describe('Execute: handles non-nullable types', () => {
647647
errors: [
648648
{
649649
message:
650-
'Argument Query.withNonNullArg(cannotBeNull:) of non-null type String! must not be null.',
650+
'Argument Query.withNonNullArg(cannotBeNull:) has invalid value: Expected value of non-null type String! not to be null.',
651651
locations: [{ line: 3, column: 42 }],
652652
path: ['withNonNullArg'],
653653
},
@@ -677,7 +677,7 @@ describe('Execute: handles non-nullable types', () => {
677677
errors: [
678678
{
679679
message:
680-
'Argument Query.withNonNullArg(cannotBeNull:) of required type String! was provided the variable "$testVar" which was not provided a runtime value.',
680+
'Argument Query.withNonNullArg(cannotBeNull:) has invalid value: Expected variable "$testVar" provided to type String! to provide a runtime value.',
681681
locations: [{ line: 3, column: 42 }],
682682
path: ['withNonNullArg'],
683683
},
@@ -705,7 +705,7 @@ describe('Execute: handles non-nullable types', () => {
705705
errors: [
706706
{
707707
message:
708-
'Argument Query.withNonNullArg(cannotBeNull:) of non-null type String! must not be null.',
708+
'Argument Query.withNonNullArg(cannotBeNull:) has invalid value: Expected variable "$testVar" provided to non-null type String! not to be null.',
709709
locations: [{ line: 3, column: 43 }],
710710
path: ['withNonNullArg'],
711711
},

‎src/execution/__tests__/subscribe-test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -523,7 +523,7 @@ describe('Subscription Initialization Phase', () => {
523523
errors: [
524524
{
525525
message:
526-
'Variable "$arg" got invalid value "meow"; Int cannot represent non-integer value: "meow"',
526+
'Variable "$arg" has invalid value: Int cannot represent non-integer value: "meow"',
527527
locations: [{ line: 2, column: 21 }],
528528
},
529529
],

‎src/execution/__tests__/variables-test.ts‎

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ describe('Execute: Handles inputs', () => {
204204
errors: [
205205
{
206206
message:
207-
'Argument TestType.fieldWithObjectInput(input:) of type TestInputObject has invalid value ["foo", "bar", "baz"].',
207+
'Argument TestType.fieldWithObjectInput(input:) has invalid value: Expected value of type TestInputObject to be an object, found: ["foo", "bar", "baz"].',
208208
path: ['fieldWithObjectInput'],
209209
locations: [{ line: 3, column: 41 }],
210210
},
@@ -374,7 +374,7 @@ describe('Execute: Handles inputs', () => {
374374
errors: [
375375
{
376376
message:
377-
'Variable "$input" got invalid value null at "input.c"; Expected non-nullable type "String!" not to be null.',
377+
'Variable "$input" has invalid value at .c: Expected value of non-null type String! not to be null.',
378378
locations: [{ line: 2, column: 16 }],
379379
},
380380
],
@@ -388,7 +388,7 @@ describe('Execute: Handles inputs', () => {
388388
errors: [
389389
{
390390
message:
391-
'Variable "$input" got invalid value "foo bar"; Expected type "TestInputObject" to be an object.',
391+
'Variable "$input" has invalid value: Expected value of type TestInputObject to be an object, found: "foo bar".',
392392
locations: [{ line: 2, column: 16 }],
393393
},
394394
],
@@ -402,7 +402,7 @@ describe('Execute: Handles inputs', () => {
402402
errors: [
403403
{
404404
message:
405-
'Variable "$input" got invalid value { a: "foo", b: "bar" }; Field "c" of required type "String!" was not provided.',
405+
'Variable "$input" has invalid value: Expected value of type TestInputObject to include required field "c", found: { a: "foo", b: "bar" }.',
406406
locations: [{ line: 2, column: 16 }],
407407
},
408408
],
@@ -421,12 +421,12 @@ describe('Execute: Handles inputs', () => {
421421
errors: [
422422
{
423423
message:
424-
'Variable "$input" got invalid value { a: "foo" } at "input.na"; Field "c" of required type "String!" was not provided.',
424+
'Variable "$input" has invalid value at .na: Expected value of type TestInputObject to include required field "c", found: { a: "foo" }.',
425425
locations: [{ line: 2, column: 18 }],
426426
},
427427
{
428428
message:
429-
'Variable "$input" got invalid value { na: { a: "foo" } }; Field "nb" of required type "String!" was not provided.',
429+
'Variable "$input" has invalid value: Expected value of type TestNestedInputObject to include required field "nb", found: { na: { a: "foo" } }.',
430430
locations: [{ line: 2, column: 18 }],
431431
},
432432
],
@@ -443,7 +443,7 @@ describe('Execute: Handles inputs', () => {
443443
errors: [
444444
{
445445
message:
446-
'Variable "$input" got invalid value { a: "foo", b: "bar", c: "baz", extra: "dog" }; Field "extra" is not defined by type "TestInputObject".',
446+
'Variable "$input" has invalid value: Expected value of type TestInputObject not to include unknown field "extra", found: { a: "foo", b: "bar", c: "baz", extra: "dog" }.',
447447
locations: [{ line: 2, column: 16 }],
448448
},
449449
],
@@ -618,7 +618,7 @@ describe('Execute: Handles inputs', () => {
618618
errors: [
619619
{
620620
message:
621-
'Variable "$value" of required type String! was not provided.',
621+
'Variable "$value" has invalid value: Expected a value of non-null type String! to be provided.',
622622
locations: [{ line: 2, column: 16 }],
623623
},
624624
],
@@ -637,7 +637,7 @@ describe('Execute: Handles inputs', () => {
637637
errors: [
638638
{
639639
message:
640-
'Variable "$value" of non-null type String! must not be null.',
640+
'Variable "$value" has invalid value: Expected value of non-null type String! not to be null.',
641641
locations: [{ line: 2, column: 16 }],
642642
},
643643
],
@@ -703,7 +703,7 @@ describe('Execute: Handles inputs', () => {
703703
errors: [
704704
{
705705
message:
706-
'Variable "$value" got invalid value [1, 2, 3]; String cannot represent a non string value: [1, 2, 3]',
706+
'Variable "$value" has invalid value: String cannot represent a non string value: [1, 2, 3]',
707707
locations: [{ line: 2, column: 16 }],
708708
},
709709
],
@@ -731,7 +731,7 @@ describe('Execute: Handles inputs', () => {
731731
errors: [
732732
{
733733
message:
734-
'Argument TestType.fieldWithNonNullableStringInput(input:) of required type String! was provided the variable "$foo" which was not provided a runtime value.',
734+
'Argument TestType.fieldWithNonNullableStringInput(input:) has invalid value: Expected variable "$foo" provided to type String! to provide a runtime value.',
735735
locations: [{ line: 3, column: 50 }],
736736
path: ['fieldWithNonNullableStringInput'],
737737
},
@@ -786,7 +786,7 @@ describe('Execute: Handles inputs', () => {
786786
errors: [
787787
{
788788
message:
789-
'Variable "$input" of non-null type [String]! must not be null.',
789+
'Variable "$input" has invalid value: Expected value of non-null type [String]! not to be null.',
790790
locations: [{ line: 2, column: 16 }],
791791
},
792792
],
@@ -849,7 +849,7 @@ describe('Execute: Handles inputs', () => {
849849
errors: [
850850
{
851851
message:
852-
'Variable "$input" got invalid value null at "input[1]"; Expected non-nullable type "String!" not to be null.',
852+
'Variable "$input" has invalid value at [1]: Expected value of non-null type String! not to be null.',
853853
locations: [{ line: 2, column: 16 }],
854854
},
855855
],
@@ -868,7 +868,7 @@ describe('Execute: Handles inputs', () => {
868868
errors: [
869869
{
870870
message:
871-
'Variable "$input" of non-null type [String!]! must not be null.',
871+
'Variable "$input" has invalid value: Expected value of non-null type [String!]! not to be null.',
872872
locations: [{ line: 2, column: 16 }],
873873
},
874874
],
@@ -898,7 +898,7 @@ describe('Execute: Handles inputs', () => {
898898
errors: [
899899
{
900900
message:
901-
'Variable "$input" got invalid value null at "input[1]"; Expected non-nullable type "String!" not to be null.',
901+
'Variable "$input" has invalid value at [1]: Expected value of non-null type String! not to be null.',
902902
locations: [{ line: 2, column: 16 }],
903903
},
904904
],
@@ -983,7 +983,7 @@ describe('Execute: Handles inputs', () => {
983983
errors: [
984984
{
985985
message:
986-
'Argument TestType.fieldWithDefaultArgumentValue(input:) of type String has invalid value WRONG_TYPE.',
986+
'Argument TestType.fieldWithDefaultArgumentValue(input:) has invalid value: String cannot represent a non string value: WRONG_TYPE',
987987
locations: [{ line: 3, column: 48 }],
988988
path: ['fieldWithDefaultArgumentValue'],
989989
},
@@ -1023,7 +1023,7 @@ describe('Execute: Handles inputs', () => {
10231023

10241024
function invalidValueError(value: number, index: number) {
10251025
return {
1026-
message: `Variable "$input" got invalid value ${value} at "input[${index}]"; String cannot represent a non string value: ${value}`,
1026+
message: `Variable "$input" has invalid value at [${index}]: String cannot represent a non string value: ${value}`,
10271027
locations: [{ line: 2, column: 14 }],
10281028
};
10291029
}

‎src/execution/values.ts‎

Lines changed: 67 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
1-
import { inspect } from '../jsutils/inspect.js';
1+
import { invariant } from '../jsutils/invariant.js';
22
import { keyMap } from '../jsutils/keyMap.js';
33
import type { Maybe } from '../jsutils/Maybe.js';
4-
import type { ObjMap, ReadOnlyObjMap } from '../jsutils/ObjMap';
4+
import type { ObjMap, ReadOnlyObjMap } from '../jsutils/ObjMap.js';
55
import { printPathArray } from '../jsutils/printPathArray.js';
66

77
import { GraphQLError } from '../error/GraphQLError.js';
@@ -15,7 +15,11 @@ import { Kind } from '../language/kinds.js';
1515
import { print } from '../language/printer.js';
1616

1717
import type { GraphQLField, GraphQLInputType } from '../type/definition.js';
18-
import { isInputType, isNonNullType } from '../type/definition.js';
18+
import {
19+
isInputType,
20+
isNonNullType,
21+
isRequiredArgument,
22+
} from '../type/definition.js';
1923
import type { GraphQLDirective } from '../type/directives.js';
2024
import type { GraphQLSchema } from '../type/schema.js';
2125

@@ -25,6 +29,10 @@ import {
2529
coerceInputValue,
2630
} from '../utilities/coerceInputValue.js';
2731
import { typeFromAST } from '../utilities/typeFromAST.js';
32+
import {
33+
validateInputLiteral,
34+
validateInputValue,
35+
} from '../utilities/validateInputValue.js';
2836

2937
export interface VariableValues {
3038
readonly sources: ReadOnlyObjMap<VariableValueSource>;
@@ -107,55 +115,37 @@ function coerceVariableValues(
107115
continue;
108116
}
109117

110-
if (!hasOwnProperty(inputs, varName)) {
111-
const defaultValue = varDefNode.defaultValue;
112-
if (defaultValue) {
113-
sources[varName] = {
114-
variable: varDefNode,
115-
type: varType,
116-
value: undefined,
117-
};
118-
coerced[varName] = coerceInputLiteral(defaultValue, varType);
119-
} else if (isNonNullType(varType)) {
120-
onError(
121-
new GraphQLError(
122-
`Variable "$${varName}" of required type ${varType} was not provided.`,
123-
{ nodes: varDefNode },
124-
),
125-
);
126-
}
127-
continue;
128-
}
118+
const value = hasOwnProperty(inputs, varName) ? inputs[varName] : undefined;
119+
sources[varName] = { variable: varDefNode, type: varType, value };
129120

130-
const value = inputs[varName];
131-
if (value === null && isNonNullType(varType)) {
132-
onError(
133-
new GraphQLError(
134-
`Variable "$${varName}" of non-null type ${varType} must not be null.`,
135-
{ nodes: varDefNode },
136-
),
137-
);
138-
continue;
121+
if (value === undefined) {
122+
if (varDefNode.defaultValue) {
123+
coerced[varName] = coerceInputLiteral(varDefNode.defaultValue, varType);
124+
continue;
125+
} else if (!isNonNullType(varType)) {
126+
// Non-provided values for nullable variables are omitted.
127+
continue;
128+
}
139129
}
140130

141-
sources[varName] = { variable: varDefNode, type: varType, value };
142-
coerced[varName] = coerceInputValue(
143-
value,
144-
varType,
145-
(path, invalidValue, error) => {
146-
let prefix =
147-
`Variable "$${varName}" got invalid value ` + inspect(invalidValue);
148-
if (path.length > 0) {
149-
prefix += ` at "${varName}${printPathArray(path)}"`;
150-
}
131+
const coercedValue = coerceInputValue(value, varType);
132+
if (coercedValue !== undefined) {
133+
coerced[varName] = coercedValue;
134+
} else {
135+
validateInputValue(value, varType, (error, path) => {
151136
onError(
152-
new GraphQLError(prefix + '; ' + error.message, {
153-
nodes: varDefNode,
154-
originalError: error.originalError,
155-
}),
137+
new GraphQLError(
138+
`Variable "$${varName}" has invalid value${printPathArray(path)}: ${
139+
error.message
140+
}`,
141+
{
142+
nodes: varDefNode,
143+
originalError: error.originalError,
144+
},
145+
),
156146
);
157-
},
158-
);
147+
});
148+
}
159149
}
160150

161151
return { sources, coerced };
@@ -186,65 +176,52 @@ export function getArgumentValues(
186176
const argType = argDef.type;
187177
const argumentNode = argNodeMap[name];
188178

189-
if (!argumentNode) {
179+
if (!argumentNode && isRequiredArgument(argDef)) {
180+
// Note: ProvidedRequiredArgumentsRule validation should catch this before
181+
// execution. This is a runtime check to ensure execution does not
182+
// continue with an invalid argument value.
183+
throw new GraphQLError(
184+
`Argument ${argDef} of required type ${argType} was not provided.`,
185+
{ nodes: node },
186+
);
187+
}
188+
189+
// Variables without a value are treated as if no argument was provided if
190+
// the argument is not required.
191+
if (
192+
!argumentNode ||
193+
(argumentNode.value.kind === Kind.VARIABLE &&
194+
variableValues?.coerced[argumentNode.value.name.value] === undefined &&
195+
!isRequiredArgument(argDef))
196+
) {
190197
if (argDef.defaultValue) {
191198
coercedValues[name] = coerceDefaultValue(
192199
argDef.defaultValue,
193200
argDef.type,
194201
);
195-
} else if (isNonNullType(argType)) {
196-
throw new GraphQLError(
197-
`Argument ${argDef} of required type ${argType} was not provided.`,
198-
{ nodes: node },
199-
);
200202
}
201203
continue;
202204
}
203205

204206
const valueNode = argumentNode.value;
205-
let isNull = valueNode.kind === Kind.NULL;
206-
207-
if (valueNode.kind === Kind.VARIABLE) {
208-
const variableName = valueNode.name.value;
209-
if (
210-
variableValues == null ||
211-
variableValues.coerced[variableName] === undefined
212-
) {
213-
if (argDef.defaultValue) {
214-
coercedValues[name] = coerceDefaultValue(
215-
argDef.defaultValue,
216-
argDef.type,
217-
);
218-
} else if (isNonNullType(argType)) {
219-
throw new GraphQLError(
220-
`Argument ${argDef} of required type ${argType} ` +
221-
`was provided the variable "$${variableName}" which was not provided a runtime value.`,
222-
{ nodes: valueNode },
223-
);
224-
}
225-
continue;
226-
}
227-
isNull = variableValues.coerced[variableName] == null;
228-
}
229-
230-
if (isNull && isNonNullType(argType)) {
231-
throw new GraphQLError(
232-
`Argument ${argDef} of non-null type ${argType} must not be null.`,
233-
{ nodes: valueNode },
234-
);
235-
}
236-
237207
const coercedValue = coerceInputLiteral(valueNode, argType, variableValues);
238208
if (coercedValue === undefined) {
239209
// Note: ValuesOfCorrectTypeRule validation should catch this before
240210
// execution. This is a runtime check to ensure execution does not
241211
// continue with an invalid argument value.
242-
throw new GraphQLError(
243-
`Argument ${argDef} of type ${argType} has invalid value ${print(
244-
valueNode,
245-
)}.`,
246-
{ nodes: valueNode },
212+
validateInputLiteral(
213+
valueNode,
214+
argType,
215+
variableValues,
216+
(error, path) => {
217+
error.message = `Argument ${argDef} has invalid value${printPathArray(
218+
path,
219+
)}: ${error.message}`;
220+
throw error;
221+
},
247222
);
223+
// istanbul ignore next (validateInputLiteral should throw)
224+
invariant(false, 'Invalid argument');
248225
}
249226
coercedValues[name] = coercedValue;
250227
}

0 commit comments

Comments
 (0)