Skip to content

Commit 757931d

Browse files
leebyronleebyron
committed
Input Value Validation
Depends on #3065 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
1 parent 30deb95 commit 757931d

File tree

14 files changed

+1459
-663
lines changed

14 files changed

+1459
-663
lines changed

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -643,7 +643,7 @@ describe('Execute: handles non-nullable types', () => {
643643
errors: [
644644
{
645645
message:
646-
'Argument Query.withNonNullArg(cannotBeNull:) of non-null type String! must not be null.',
646+
'Argument Query.withNonNullArg(cannotBeNull:) has invalid value: Expected value of non-null type String! not to be null.',
647647
locations: [{ line: 3, column: 42 }],
648648
path: ['withNonNullArg'],
649649
},
@@ -673,7 +673,7 @@ describe('Execute: handles non-nullable types', () => {
673673
errors: [
674674
{
675675
message:
676-
'Argument Query.withNonNullArg(cannotBeNull:) of required type String! was provided the variable "$testVar" which was not provided a runtime value.',
676+
'Argument Query.withNonNullArg(cannotBeNull:) has invalid value: Expected variable "$testVar" provided to type String! to provide a runtime value.',
677677
locations: [{ line: 3, column: 42 }],
678678
path: ['withNonNullArg'],
679679
},
@@ -701,7 +701,7 @@ describe('Execute: handles non-nullable types', () => {
701701
errors: [
702702
{
703703
message:
704-
'Argument Query.withNonNullArg(cannotBeNull:) of non-null type String! must not be null.',
704+
'Argument Query.withNonNullArg(cannotBeNull:) has invalid value: Expected variable "$testVar" provided to non-null type String! not to be null.',
705705
locations: [{ line: 3, column: 43 }],
706706
path: ['withNonNullArg'],
707707
},

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

Lines changed: 17 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -203,7 +203,7 @@ describe('Execute: Handles inputs', () => {
203203
errors: [
204204
{
205205
message:
206-
'Argument TestType.fieldWithObjectInput(input:) of type TestInputObject has invalid value ["foo", "bar", "baz"].',
206+
'Argument TestType.fieldWithObjectInput(input:) has invalid value: Expected value of type TestInputObject to be an object, found: ["foo", "bar", "baz"].',
207207
path: ['fieldWithObjectInput'],
208208
locations: [{ line: 3, column: 41 }],
209209
},
@@ -373,7 +373,7 @@ describe('Execute: Handles inputs', () => {
373373
errors: [
374374
{
375375
message:
376-
'Variable "$input" got invalid value null at "input.c"; Expected non-nullable type "String!" not to be null.',
376+
'Variable "$input" has invalid value at .c: Expected value of non-null type String! not to be null.',
377377
locations: [{ line: 2, column: 16 }],
378378
},
379379
],
@@ -387,7 +387,7 @@ describe('Execute: Handles inputs', () => {
387387
errors: [
388388
{
389389
message:
390-
'Variable "$input" got invalid value "foo bar"; Expected type "TestInputObject" to be an object.',
390+
'Variable "$input" has invalid value: Expected value of type TestInputObject to be an object, found: "foo bar".',
391391
locations: [{ line: 2, column: 16 }],
392392
},
393393
],
@@ -401,7 +401,7 @@ describe('Execute: Handles inputs', () => {
401401
errors: [
402402
{
403403
message:
404-
'Variable "$input" got invalid value { a: "foo", b: "bar" }; Field "c" of required type "String!" was not provided.',
404+
'Variable "$input" has invalid value: Expected value of type TestInputObject to include required field "c", found: { a: "foo", b: "bar" }.',
405405
locations: [{ line: 2, column: 16 }],
406406
},
407407
],
@@ -420,12 +420,12 @@ describe('Execute: Handles inputs', () => {
420420
errors: [
421421
{
422422
message:
423-
'Variable "$input" got invalid value { a: "foo" } at "input.na"; Field "c" of required type "String!" was not provided.',
423+
'Variable "$input" has invalid value at .na: Expected value of type TestInputObject to include required field "c", found: { a: "foo" }.',
424424
locations: [{ line: 2, column: 18 }],
425425
},
426426
{
427427
message:
428-
'Variable "$input" got invalid value { na: { a: "foo" } }; Field "nb" of required type "String!" was not provided.',
428+
'Variable "$input" has invalid value: Expected value of type TestNestedInputObject to include required field "nb", found: { na: { a: "foo" } }.',
429429
locations: [{ line: 2, column: 18 }],
430430
},
431431
],
@@ -442,7 +442,7 @@ describe('Execute: Handles inputs', () => {
442442
errors: [
443443
{
444444
message:
445-
'Variable "$input" got invalid value { a: "foo", b: "bar", c: "baz", extra: "dog" }; Field "extra" is not defined by type "TestInputObject".',
445+
'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" }.',
446446
locations: [{ line: 2, column: 16 }],
447447
},
448448
],
@@ -617,7 +617,7 @@ describe('Execute: Handles inputs', () => {
617617
errors: [
618618
{
619619
message:
620-
'Variable "$value" of required type String! was not provided.',
620+
'Variable "$value" has invalid value: Expected a value of non-null type String! to be provided.',
621621
locations: [{ line: 2, column: 16 }],
622622
},
623623
],
@@ -636,7 +636,7 @@ describe('Execute: Handles inputs', () => {
636636
errors: [
637637
{
638638
message:
639-
'Variable "$value" of non-null type String! must not be null.',
639+
'Variable "$value" has invalid value: Expected value of non-null type String! not to be null.',
640640
locations: [{ line: 2, column: 16 }],
641641
},
642642
],
@@ -702,7 +702,7 @@ describe('Execute: Handles inputs', () => {
702702
errors: [
703703
{
704704
message:
705-
'Variable "$value" got invalid value [1, 2, 3]; String cannot represent a non string value: [1, 2, 3]',
705+
'Variable "$value" has invalid value: String cannot represent a non string value: [1, 2, 3]',
706706
locations: [{ line: 2, column: 16 }],
707707
},
708708
],
@@ -730,7 +730,7 @@ describe('Execute: Handles inputs', () => {
730730
errors: [
731731
{
732732
message:
733-
'Argument TestType.fieldWithNonNullableStringInput(input:) of required type String! was provided the variable "$foo" which was not provided a runtime value.',
733+
'Argument TestType.fieldWithNonNullableStringInput(input:) has invalid value: Expected variable "$foo" provided to type String! to provide a runtime value.',
734734
locations: [{ line: 3, column: 50 }],
735735
path: ['fieldWithNonNullableStringInput'],
736736
},
@@ -785,7 +785,7 @@ describe('Execute: Handles inputs', () => {
785785
errors: [
786786
{
787787
message:
788-
'Variable "$input" of non-null type [String]! must not be null.',
788+
'Variable "$input" has invalid value: Expected value of non-null type [String]! not to be null.',
789789
locations: [{ line: 2, column: 16 }],
790790
},
791791
],
@@ -848,7 +848,7 @@ describe('Execute: Handles inputs', () => {
848848
errors: [
849849
{
850850
message:
851-
'Variable "$input" got invalid value null at "input[1]"; Expected non-nullable type "String!" not to be null.',
851+
'Variable "$input" has invalid value at [1]: Expected value of non-null type String! not to be null.',
852852
locations: [{ line: 2, column: 16 }],
853853
},
854854
],
@@ -867,7 +867,7 @@ describe('Execute: Handles inputs', () => {
867867
errors: [
868868
{
869869
message:
870-
'Variable "$input" of non-null type [String!]! must not be null.',
870+
'Variable "$input" has invalid value: Expected value of non-null type [String!]! not to be null.',
871871
locations: [{ line: 2, column: 16 }],
872872
},
873873
],
@@ -897,7 +897,7 @@ describe('Execute: Handles inputs', () => {
897897
errors: [
898898
{
899899
message:
900-
'Variable "$input" got invalid value null at "input[1]"; Expected non-nullable type "String!" not to be null.',
900+
'Variable "$input" has invalid value at [1]: Expected value of non-null type String! not to be null.',
901901
locations: [{ line: 2, column: 16 }],
902902
},
903903
],
@@ -982,7 +982,7 @@ describe('Execute: Handles inputs', () => {
982982
errors: [
983983
{
984984
message:
985-
'Argument TestType.fieldWithDefaultArgumentValue(input:) of type String has invalid value WRONG_TYPE.',
985+
'Argument TestType.fieldWithDefaultArgumentValue(input:) has invalid value: String cannot represent a non string value: WRONG_TYPE',
986986
locations: [{ line: 3, column: 48 }],
987987
path: ['fieldWithDefaultArgumentValue'],
988988
},
@@ -1022,7 +1022,7 @@ describe('Execute: Handles inputs', () => {
10221022

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

‎src/execution/values.ts‎

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

77
import { GraphQLError } from '../error/GraphQLError';
@@ -17,14 +17,22 @@ import { print } from '../language/printer';
1717
import type { GraphQLSchema } from '../type/schema';
1818
import type { GraphQLInputType, GraphQLField } from '../type/definition';
1919
import type { GraphQLDirective } from '../type/directives';
20-
import { isInputType, isNonNullType } from '../type/definition';
20+
import {
21+
isInputType,
22+
isNonNullType,
23+
isRequiredArgument,
24+
} from '../type/definition';
2125

2226
import { typeFromAST } from '../utilities/typeFromAST';
2327
import {
2428
coerceInputValue,
2529
coerceInputLiteral,
2630
coerceDefaultValue,
2731
} from '../utilities/coerceInputValue';
32+
import {
33+
validateInputValue,
34+
validateInputLiteral,
35+
} from '../utilities/validateInputValue';
2836

2937
export interface VariableValues {
3038
readonly sources: ReadOnlyObjMap<VariableValueSource>;
@@ -109,59 +117,38 @@ function coerceVariableValues(
109117
continue;
110118
}
111119

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

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

143-
sources[varName] = { variable: varDefNode, type: varType, value };
144-
coerced[varName] = coerceInputValue(
145-
value,
146-
varType,
147-
(path, invalidValue, error) => {
148-
let prefix =
149-
`Variable "$${varName}" got invalid value ` + inspect(invalidValue);
150-
if (path.length > 0) {
151-
prefix += ` at "${varName}${printPathArray(path)}"`;
152-
}
133+
const coercedValue = coerceInputValue(value, varType);
134+
if (coercedValue !== undefined) {
135+
coerced[varName] = coercedValue;
136+
} else {
137+
validateInputValue(value, varType, (error, path) => {
153138
onError(
154139
new GraphQLError(
155-
prefix + '; ' + error.message,
140+
`Variable "$${varName}" has invalid value${printPathArray(path)}: ${
141+
error.message
142+
}`,
156143
varDefNode,
157144
undefined,
158145
undefined,
159146
undefined,
160147
error.originalError,
161148
),
162149
);
163-
},
164-
);
150+
});
151+
}
165152
}
166153

167154
return { sources, coerced };
@@ -193,65 +180,52 @@ export function getArgumentValues(
193180
const argType = argDef.type;
194181
const argumentNode = argNodeMap[name];
195182

196-
if (!argumentNode) {
183+
if (!argumentNode && isRequiredArgument(argDef)) {
184+
// Note: ProvidedRequiredArgumentsRule validation should catch this before
185+
// execution. This is a runtime check to ensure execution does not
186+
// continue with an invalid argument value.
187+
throw new GraphQLError(
188+
`Argument ${argDef} of required type ${argType} was not provided.`,
189+
node,
190+
);
191+
}
192+
193+
// Variables without a value are treated as if no argument was provided if
194+
// the argument is not required.
195+
if (
196+
!argumentNode ||
197+
(argumentNode.value.kind === Kind.VARIABLE &&
198+
variableValues?.coerced[argumentNode.value.name.value] === undefined &&
199+
!isRequiredArgument(argDef))
200+
) {
197201
if (argDef.defaultValue) {
198202
coercedValues[name] = coerceDefaultValue(
199203
argDef.defaultValue,
200204
argDef.type,
201205
);
202-
} else if (isNonNullType(argType)) {
203-
throw new GraphQLError(
204-
`Argument ${argDef} of required type ${argType} was not provided.`,
205-
node,
206-
);
207206
}
208207
continue;
209208
}
210209

211210
const valueNode = argumentNode.value;
212-
let isNull = valueNode.kind === Kind.NULL;
213-
214-
if (valueNode.kind === Kind.VARIABLE) {
215-
const variableName = valueNode.name.value;
216-
if (
217-
variableValues == null ||
218-
variableValues.coerced[variableName] === undefined
219-
) {
220-
if (argDef.defaultValue) {
221-
coercedValues[name] = coerceDefaultValue(
222-
argDef.defaultValue,
223-
argDef.type,
224-
);
225-
} else if (isNonNullType(argType)) {
226-
throw new GraphQLError(
227-
`Argument ${argDef} of required type ${argType} ` +
228-
`was provided the variable "$${variableName}" which was not provided a runtime value.`,
229-
valueNode,
230-
);
231-
}
232-
continue;
233-
}
234-
isNull = variableValues.coerced[variableName] == null;
235-
}
236-
237-
if (isNull && isNonNullType(argType)) {
238-
throw new GraphQLError(
239-
`Argument ${argDef} of non-null type ${argType} must not be null.`,
240-
valueNode,
241-
);
242-
}
243-
244211
const coercedValue = coerceInputLiteral(valueNode, argType, variableValues);
245212
if (coercedValue === undefined) {
246213
// Note: ValuesOfCorrectTypeRule validation should catch this before
247214
// execution. This is a runtime check to ensure execution does not
248215
// continue with an invalid argument value.
249-
throw new GraphQLError(
250-
`Argument ${argDef} of type ${argType} has invalid value ${print(
251-
valueNode,
252-
)}.`,
216+
validateInputLiteral(
253217
valueNode,
218+
argType,
219+
variableValues,
220+
(error, path) => {
221+
error.message = `Argument ${argDef} has invalid value${printPathArray(
222+
path,
223+
)}: ${error.message}`;
224+
throw error;
225+
},
254226
);
227+
// istanbul ignore next (validateInputLiteral should throw)
228+
invariant(false, 'Invalid argument');
255229
}
256230
coercedValues[name] = coercedValue;
257231
}

0 commit comments

Comments
 (0)