Skip to content

Commit 026cdd2

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 026cdd2

21 files changed

+1516
-657
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: 18 additions & 17 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
],
@@ -612,7 +612,7 @@ describe('Execute: Handles inputs', () => {
612612
errors: [
613613
{
614614
message:
615-
'Variable "$value" of required type "String!" was not provided.',
615+
'Variable "$value" has invalid value: Expected a value of non-null type "String!" to be provided.',
616616
locations: [{ line: 2, column: 16 }],
617617
},
618618
],
@@ -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: 58 additions & 88 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type { ReadOnlyObjMap, ReadOnlyObjMapLike } from '../jsutils/ObjMap';
2-
import { inspect } from '../jsutils/inspect';
2+
import { hasOwnProperty } from '../jsutils/hasOwnProperty';
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<{|
@@ -104,60 +111,38 @@ function coerceVariableValues(
104111
continue;
105112
}
106113

107-
if (!hasOwnProperty(inputs, varName)) {
114+
const value = hasOwnProperty(inputs, varName) ? inputs[varName] : undefined;
115+
sources[varName] = { variable: varDefNode, type: varType, value };
116+
117+
if (value === undefined) {
108118
if (varDefNode.defaultValue) {
109-
sources[varName] = {
110-
variable: varDefNode,
111-
type: varType,
112-
value: undefined,
113-
};
114119
coerced[varName] = valueFromAST(varDefNode.defaultValue, varType);
115-
} else if (isNonNullType(varType)) {
116-
const varTypeStr = inspect(varType);
117-
onError(
118-
new GraphQLError(
119-
`Variable "$${varName}" of required type "${varTypeStr}" was not provided.`,
120-
varDefNode,
121-
),
122-
);
120+
continue;
121+
} else if (!isNonNullType(varType)) {
122+
// Non-provided values for nullable variables are omitted.
123+
continue;
123124
}
124-
continue;
125125
}
126126

127-
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-
}
127+
const coercedValue = coerceInputValue(value, varType);
128+
if (coercedValue !== undefined) {
129+
coerced[varName] = coercedValue;
130+
} else {
131+
validateInputValue(value, varType, (error, path) => {
149132
onError(
150133
new GraphQLError(
151-
prefix + '; ' + error.message,
134+
`Variable "$${varName}" has invalid value${printPathArray(path)}: ${
135+
error.message
136+
}`,
152137
varDefNode,
153138
undefined,
154139
undefined,
155140
undefined,
156141
error.originalError,
157142
),
158143
);
159-
},
160-
);
144+
});
145+
}
161146
}
162147

163148
return { sources, coerced };
@@ -189,65 +174,54 @@ export function getArgumentValues(
189174
const argType = argDef.type;
190175
const argumentNode = argNodeMap[name];
191176

192-
if (!argumentNode) {
177+
if (!argumentNode && isRequiredInput(argDef)) {
178+
// Note: ProvidedRequiredArgumentsRule validation should catch this before
179+
// execution. This is a runtime check to ensure execution does not
180+
// continue with an invalid argument value.
181+
throw new GraphQLError(
182+
`Argument "${name}" of required type "${String(
183+
argType,
184+
)}" was not provided.`,
185+
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+
!isRequiredInput(argDef))
196+
) {
193197
if (argDef.defaultValue) {
194198
coercedValues[name] = getCoercedDefaultValue(
195199
argDef.defaultValue,
196200
argDef.type,
197201
);
198-
} else if (isNonNullType(argType)) {
199-
throw new GraphQLError(
200-
`Argument "${name}" of required type "${inspect(argType)}" ` +
201-
'was not provided.',
202-
node,
203-
);
204202
}
205203
continue;
206204
}
207205

208206
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-
242207
const coercedValue = valueFromAST(valueNode, argType, variableValues);
243208
if (coercedValue === undefined) {
244209
// Note: ValuesOfCorrectTypeRule validation should catch this before
245210
// execution. This is a runtime check to ensure execution does not
246211
// continue with an invalid argument value.
247-
throw new GraphQLError(
248-
`Argument "${name}" has invalid value ${print(valueNode)}.`,
212+
validateInputLiteral(
249213
valueNode,
214+
argType,
215+
variableValues,
216+
(error, path) => {
217+
error.message = `Argument "${name}" has invalid value${printPathArray(
218+
path,
219+
)}: ${error.message}`;
220+
throw error;
221+
},
250222
);
223+
// istanbul ignore next (validateInputLiteral should throw)
224+
invariant(false, 'Invalid argument');
251225
}
252226
coercedValues[name] = coercedValue;
253227
}
@@ -279,7 +253,3 @@ export function getDirectiveValues(
279253
return getArgumentValues(directiveDef, directiveNode, variableValues);
280254
}
281255
}
282-
283-
function hasOwnProperty(obj: mixed, prop: string): boolean {
284-
return Object.prototype.hasOwnProperty.call(obj, prop);
285-
}

0 commit comments

Comments
 (0)