Skip to content

Commit 77ffb26

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 d3aaf3e commit 77ffb26

21 files changed

+1499
-635
lines changed

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

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 "cannotBeNull" of non-null type "String!" must not be null.',
646+
'Argument "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 "cannotBeNull" of required type "String!" was provided the variable "$testVar" which was not provided a runtime value.',
676+
'Argument "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 "cannotBeNull" of non-null type "String!" must not be null.',
704+
'Argument "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.js

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,7 @@ describe('Execute: Handles inputs', () => {
198198
errors: [
199199
{
200200
message:
201-
'Argument "input" has invalid value ["foo", "bar", "baz"].',
201+
'Argument "input" has invalid value: Expected value of type "TestInputObject" to be an object, found ["foo", "bar", "baz"].',
202202
path: ['fieldWithObjectInput'],
203203
locations: [{ line: 3, column: 41 }],
204204
},
@@ -368,7 +368,7 @@ describe('Execute: Handles inputs', () => {
368368
errors: [
369369
{
370370
message:
371-
'Variable "$input" got invalid value null at "input.c"; Expected non-nullable type "String!" not to be null.',
371+
'Variable "$input" has invalid value at .c: Expected value of non-null type "String!" not to be null.',
372372
locations: [{ line: 2, column: 16 }],
373373
},
374374
],
@@ -382,7 +382,7 @@ describe('Execute: Handles inputs', () => {
382382
errors: [
383383
{
384384
message:
385-
'Variable "$input" got invalid value "foo bar"; Expected type "TestInputObject" to be an object.',
385+
'Variable "$input" has invalid value: Expected value of type "TestInputObject" to be an object, found "foo bar".',
386386
locations: [{ line: 2, column: 16 }],
387387
},
388388
],
@@ -396,7 +396,7 @@ describe('Execute: Handles inputs', () => {
396396
errors: [
397397
{
398398
message:
399-
'Variable "$input" got invalid value { a: "foo", b: "bar" }; Field "c" of required type "String!" was not provided.',
399+
'Variable "$input" has invalid value: Expected value of type "TestInputObject" to include required field "c", found { a: "foo", b: "bar" }',
400400
locations: [{ line: 2, column: 16 }],
401401
},
402402
],
@@ -415,12 +415,12 @@ describe('Execute: Handles inputs', () => {
415415
errors: [
416416
{
417417
message:
418-
'Variable "$input" got invalid value { a: "foo" } at "input.na"; Field "c" of required type "String!" was not provided.',
418+
'Variable "$input" has invalid value at .na: Expected value of type "TestInputObject" to include required field "c", found { a: "foo" }',
419419
locations: [{ line: 2, column: 18 }],
420420
},
421421
{
422422
message:
423-
'Variable "$input" got invalid value { na: { a: "foo" } }; Field "nb" of required type "String!" was not provided.',
423+
'Variable "$input" has invalid value: Expected value of type "TestNestedInputObject" to include required field "nb", found { na: { a: "foo" } }',
424424
locations: [{ line: 2, column: 18 }],
425425
},
426426
],
@@ -437,7 +437,7 @@ describe('Execute: Handles inputs', () => {
437437
errors: [
438438
{
439439
message:
440-
'Variable "$input" got invalid value { a: "foo", b: "bar", c: "baz", extra: "dog" }; Field "extra" is not defined by type "TestInputObject".',
440+
'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" }',
441441
locations: [{ line: 2, column: 16 }],
442442
},
443443
],
@@ -631,7 +631,7 @@ describe('Execute: Handles inputs', () => {
631631
errors: [
632632
{
633633
message:
634-
'Variable "$value" of non-null type "String!" must not be null.',
634+
'Variable "$value" has invalid value: Expected value of non-null type "String!" not to be null.',
635635
locations: [{ line: 2, column: 16 }],
636636
},
637637
],
@@ -697,7 +697,7 @@ describe('Execute: Handles inputs', () => {
697697
errors: [
698698
{
699699
message:
700-
'Variable "$value" got invalid value [1, 2, 3]; String cannot represent a non string value: [1, 2, 3]',
700+
'Variable "$value" has invalid value: String cannot represent a non string value: [1, 2, 3]',
701701
locations: [{ line: 2, column: 16 }],
702702
},
703703
],
@@ -725,7 +725,7 @@ describe('Execute: Handles inputs', () => {
725725
errors: [
726726
{
727727
message:
728-
'Argument "input" of required type "String!" was provided the variable "$foo" which was not provided a runtime value.',
728+
'Argument "input" has invalid value: Expected variable "$foo" provided to type "String!" to provide a runtime value.',
729729
locations: [{ line: 3, column: 50 }],
730730
path: ['fieldWithNonNullableStringInput'],
731731
},
@@ -780,7 +780,7 @@ describe('Execute: Handles inputs', () => {
780780
errors: [
781781
{
782782
message:
783-
'Variable "$input" of non-null type "[String]!" must not be null.',
783+
'Variable "$input" has invalid value: Expected value of non-null type "[String]!" not to be null.',
784784
locations: [{ line: 2, column: 16 }],
785785
},
786786
],
@@ -843,7 +843,7 @@ describe('Execute: Handles inputs', () => {
843843
errors: [
844844
{
845845
message:
846-
'Variable "$input" got invalid value null at "input[1]"; Expected non-nullable type "String!" not to be null.',
846+
'Variable "$input" has invalid value at [1]: Expected value of non-null type "String!" not to be null.',
847847
locations: [{ line: 2, column: 16 }],
848848
},
849849
],
@@ -862,7 +862,7 @@ describe('Execute: Handles inputs', () => {
862862
errors: [
863863
{
864864
message:
865-
'Variable "$input" of non-null type "[String!]!" must not be null.',
865+
'Variable "$input" has invalid value: Expected value of non-null type "[String!]!" not to be null.',
866866
locations: [{ line: 2, column: 16 }],
867867
},
868868
],
@@ -892,7 +892,7 @@ describe('Execute: Handles inputs', () => {
892892
errors: [
893893
{
894894
message:
895-
'Variable "$input" got invalid value null at "input[1]"; Expected non-nullable type "String!" not to be null.',
895+
'Variable "$input" has invalid value at [1]: Expected value of non-null type "String!" not to be null.',
896896
locations: [{ line: 2, column: 16 }],
897897
},
898898
],
@@ -976,7 +976,8 @@ describe('Execute: Handles inputs', () => {
976976
},
977977
errors: [
978978
{
979-
message: 'Argument "input" has invalid value WRONG_TYPE.',
979+
message:
980+
'Argument "input" has invalid value: String cannot represent a non string value: WRONG_TYPE',
980981
locations: [{ line: 3, column: 48 }],
981982
path: ['fieldWithDefaultArgumentValue'],
982983
},
@@ -1016,7 +1017,7 @@ describe('Execute: Handles inputs', () => {
10161017

10171018
function invalidValueError(value: number, index: number) {
10181019
return {
1019-
message: `Variable "$input" got invalid value ${value} at "input[${index}]"; String cannot represent a non string value: ${value}`,
1020+
message: `Variable "$input" has invalid value at [${index}]: String cannot represent a non string value: ${value}`,
10201021
locations: [{ line: 2, column: 14 }],
10211022
};
10221023
}

‎src/execution/values.js

Lines changed: 50 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type { ReadOnlyObjMap, ReadOnlyObjMapLike } from '../jsutils/ObjMap';
22
import { inspect } from '../jsutils/inspect';
3+
import { invariant } from '../jsutils/invariant';
34
import { keyMap } from '../jsutils/keyMap';
45
import { printPathArray } from '../jsutils/printPathArray';
56

@@ -16,12 +17,18 @@ import { print } from '../language/printer';
1617
import type { GraphQLSchema } from '../type/schema';
1718
import type { GraphQLInputType, GraphQLField } from '../type/definition';
1819
import type { GraphQLDirective } from '../type/directives';
19-
import { isInputType, isNonNullType } from '../type/definition';
20+
import {
21+
isInputType,
22+
isNonNullType,
23+
isRequiredInput,
24+
} from '../type/definition';
2025
import { getCoercedDefaultValue } from '../type/defaultValues';
2126

2227
import { typeFromAST } from '../utilities/typeFromAST';
2328
import { valueFromAST } from '../utilities/valueFromAST';
2429
import { coerceInputValue } from '../utilities/coerceInputValue';
30+
import { validateInputValue } from '../utilities/validateInputValue';
31+
import { validateInputLiteral } from '../utilities/validateInputLiteral';
2532

2633
export type VariableValues = {|
2734
+sources: ReadOnlyObjMap<{|
@@ -125,39 +132,26 @@ function coerceVariableValues(
125132
}
126133

127134
const value = inputs[varName];
128-
if (value === null && isNonNullType(varType)) {
129-
const varTypeStr = inspect(varType);
130-
onError(
131-
new GraphQLError(
132-
`Variable "$${varName}" of non-null type "${varTypeStr}" must not be null.`,
133-
varDefNode,
134-
),
135-
);
136-
continue;
137-
}
138-
139-
sources[varName] = { variable: varDefNode, type: varType, value };
140-
coerced[varName] = coerceInputValue(
141-
value,
142-
varType,
143-
(path, invalidValue, error) => {
144-
let prefix =
145-
`Variable "$${varName}" got invalid value ` + inspect(invalidValue);
146-
if (path.length > 0) {
147-
prefix += ` at "${varName}${printPathArray(path)}"`;
148-
}
135+
const coercedValue = coerceInputValue(value, varType);
136+
if (coercedValue !== undefined) {
137+
sources[varName] = { variable: varDefNode, type: varType, value };
138+
coerced[varName] = coercedValue;
139+
} else {
140+
validateInputValue(value, varType, (error, path) => {
149141
onError(
150142
new GraphQLError(
151-
prefix + '; ' + error.message,
143+
`Variable "$${varName}" has invalid value${printPathArray(path)}: ${
144+
error.message
145+
}`,
152146
varDefNode,
153147
undefined,
154148
undefined,
155149
undefined,
156150
error.originalError,
157151
),
158152
);
159-
},
160-
);
153+
});
154+
}
161155
}
162156

163157
return { sources, coerced };
@@ -189,65 +183,54 @@ export function getArgumentValues(
189183
const argType = argDef.type;
190184
const argumentNode = argNodeMap[name];
191185

192-
if (!argumentNode) {
186+
if (!argumentNode && isRequiredInput(argDef)) {
187+
// Note: ProvidedRequiredArgumentsRule validation should catch this before
188+
// execution. This is a runtime check to ensure execution does not
189+
// continue with an invalid argument value.
190+
throw new GraphQLError(
191+
`Argument "${name}" of required type "${String(
192+
argType,
193+
)}" was not provided.`,
194+
node,
195+
);
196+
}
197+
198+
// Variables without a value are treated as if no argument was provided if
199+
// the argument is not required.
200+
if (
201+
!argumentNode ||
202+
(argumentNode.value.kind === Kind.VARIABLE &&
203+
variableValues?.coerced[argumentNode.value.name.value] === undefined &&
204+
!isRequiredInput(argDef))
205+
) {
193206
if (argDef.defaultValue) {
194207
coercedValues[name] = getCoercedDefaultValue(
195208
argDef.defaultValue,
196209
argDef.type,
197210
);
198-
} else if (isNonNullType(argType)) {
199-
throw new GraphQLError(
200-
`Argument "${name}" of required type "${inspect(argType)}" ` +
201-
'was not provided.',
202-
node,
203-
);
204211
}
205212
continue;
206213
}
207214

208215
const valueNode = argumentNode.value;
209-
let isNull = valueNode.kind === Kind.NULL;
210-
211-
if (valueNode.kind === Kind.VARIABLE) {
212-
const variableName = valueNode.name.value;
213-
if (
214-
variableValues == null ||
215-
variableValues.coerced[variableName] === undefined
216-
) {
217-
if (argDef.defaultValue) {
218-
coercedValues[name] = getCoercedDefaultValue(
219-
argDef.defaultValue,
220-
argDef.type,
221-
);
222-
} else if (isNonNullType(argType)) {
223-
throw new GraphQLError(
224-
`Argument "${name}" of required type "${inspect(argType)}" ` +
225-
`was provided the variable "$${variableName}" which was not provided a runtime value.`,
226-
valueNode,
227-
);
228-
}
229-
continue;
230-
}
231-
isNull = variableValues.coerced[variableName] == null;
232-
}
233-
234-
if (isNull && isNonNullType(argType)) {
235-
throw new GraphQLError(
236-
`Argument "${name}" of non-null type "${inspect(argType)}" ` +
237-
'must not be null.',
238-
valueNode,
239-
);
240-
}
241-
242216
const coercedValue = valueFromAST(valueNode, argType, variableValues);
243217
if (coercedValue === undefined) {
244218
// Note: ValuesOfCorrectTypeRule validation should catch this before
245219
// execution. This is a runtime check to ensure execution does not
246220
// continue with an invalid argument value.
247-
throw new GraphQLError(
248-
`Argument "${name}" has invalid value ${print(valueNode)}.`,
221+
validateInputLiteral(
249222
valueNode,
223+
argType,
224+
variableValues,
225+
(error, path) => {
226+
error.message = `Argument "${name}" has invalid value${printPathArray(
227+
path,
228+
)}: ${error.message}`;
229+
throw error;
230+
},
250231
);
232+
// istanbul ignore next (validateInputLiteral should throw)
233+
invariant(false, 'Invalid argument');
251234
}
252235
coercedValues[name] = coercedValue;
253236
}

‎src/index.d.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -423,6 +423,10 @@ export {
423423
valueToLiteral,
424424
// Coerces a JavaScript value to a GraphQL type, or produces errors.
425425
coerceInputValue,
426+
// Validate a JavaScript value with a GraphQL type, collecting all errors.
427+
validateInputValue,
428+
// Validate a GraphQL Literal AST with a GraphQL type, collecting all errors.
429+
validateInputLiteral,
426430
// Concatenates multiple AST together.
427431
concatAST,
428432
// Separates an AST into an AST per Operation.

‎src/index.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -412,6 +412,10 @@ export {
412412
valueToLiteral,
413413
// Coerces a JavaScript value to a GraphQL type, or produces errors.
414414
coerceInputValue,
415+
// Validate a JavaScript value with a GraphQL type, collecting all errors.
416+
validateInputValue,
417+
// Validate a GraphQL Literal AST with a GraphQL type, collecting all errors.
418+
validateInputLiteral,
415419
// Concatenates multiple AST together.
416420
concatAST,
417421
// Separates an AST into an AST per Operation.

‎src/jsutils/printPathArray.js

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,10 @@
22
* Build a string describing the path.
33
*/
44
export function printPathArray(path: $ReadOnlyArray<string | number>): string {
5-
return path
6-
.map((key) =>
7-
typeof key === 'number' ? '[' + key.toString() + ']' : '.' + key,
8-
)
9-
.join('');
5+
if (path.length === 0) {
6+
return '';
7+
}
8+
return ` at ${path
9+
.map((key) => (typeof key === 'number' ? `[${key}]` : `.${key}`))
10+
.join('')}`;
1011
}

‎src/subscription/__tests__/subscribe-test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -472,7 +472,7 @@ describe('Subscription Initialization Phase', () => {
472472
errors: [
473473
{
474474
message:
475-
'Variable "$arg" got invalid value "meow"; Int cannot represent non-integer value: "meow"',
475+
'Variable "$arg" has invalid value: Int cannot represent non-integer value: "meow"',
476476
locations: [{ line: 2, column: 21 }],
477477
},
478478
],

‎src/type/__tests__/enumType-test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -263,7 +263,7 @@ describe('Type System: Enum Values', () => {
263263
errors: [
264264
{
265265
message:
266-
'Variable "$color" got invalid value 2; Enum "Color" cannot represent non-string value: 2.',
266+
'Variable "$color" has invalid value: Enum "Color" cannot represent non-string value: 2.',
267267
locations: [{ line: 1, column: 8 }],
268268
},
269269
],

0 commit comments

Comments
 (0)