Skip to content

Commit

Permalink
Input Value Validation
Browse files Browse the repository at this point in the history 
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 graphql#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.
  • Loading branch information
@leebyron @yaacovCR
leebyron authored and yaacovCR committed Jan 31, 2023
1 parent 39b8d09 commit 568fc02
Show file tree
Hide file tree
Showing 14 changed files with 1,465 additions and 659 deletions.
6 changes: 3 additions & 3 deletions src/execution/__tests__/nonnull-test.ts
View file
Expand Up @@ -647,7 +647,7 @@ describe('Execute: handles non-nullable types', () => {
errors: [
{
message:
'Argument Query.withNonNullArg(cannotBeNull:) of non-null type String! must not be null.',
'Argument Query.withNonNullArg(cannotBeNull:) has invalid value: Expected value of non-null type String! not to be null.',
locations: [{ line: 3, column: 42 }],
path: ['withNonNullArg'],
},
Expand Down Expand Up @@ -677,7 +677,7 @@ describe('Execute: handles non-nullable types', () => {
errors: [
{
message:
'Argument Query.withNonNullArg(cannotBeNull:) of required type String! was provided the variable "$testVar" which was not provided a runtime value.',
'Argument Query.withNonNullArg(cannotBeNull:) has invalid value: Expected variable "$testVar" provided to type String! to provide a runtime value.',
locations: [{ line: 3, column: 42 }],
path: ['withNonNullArg'],
},
Expand Down Expand Up @@ -705,7 +705,7 @@ describe('Execute: handles non-nullable types', () => {
errors: [
{
message:
'Argument Query.withNonNullArg(cannotBeNull:) of non-null type String! must not be null.',
'Argument Query.withNonNullArg(cannotBeNull:) has invalid value: Expected variable "$testVar" provided to non-null type String! not to be null.',
locations: [{ line: 3, column: 43 }],
path: ['withNonNullArg'],
},
Expand Down
2 changes: 1 addition & 1 deletion src/execution/__tests__/subscribe-test.ts
View file
Expand Up @@ -523,7 +523,7 @@ describe('Subscription Initialization Phase', () => {
errors: [
{
message:
'Variable "$arg" got invalid value "meow"; Int cannot represent non-integer value: "meow"',
'Variable "$arg" has invalid value: Int cannot represent non-integer value: "meow"',
locations: [{ line: 2, column: 21 }],
},
],
Expand Down
34 changes: 17 additions & 17 deletions src/execution/__tests__/variables-test.ts
View file
Expand Up @@ -204,7 +204,7 @@ describe('Execute: Handles inputs', () => {
errors: [
{
message:
'Argument TestType.fieldWithObjectInput(input:) of type TestInputObject has invalid value ["foo", "bar", "baz"].',
'Argument TestType.fieldWithObjectInput(input:) has invalid value: Expected value of type TestInputObject to be an object, found: ["foo", "bar", "baz"].',
path: ['fieldWithObjectInput'],
locations: [{ line: 3, column: 41 }],
},
Expand Down Expand Up @@ -374,7 +374,7 @@ describe('Execute: Handles inputs', () => {
errors: [
{
message:
'Variable "$input" got invalid value null at "input.c"; Expected non-nullable type "String!" not to be null.',
'Variable "$input" has invalid value at .c: Expected value of non-null type String! not to be null.',
locations: [{ line: 2, column: 16 }],
},
],
Expand All @@ -388,7 +388,7 @@ describe('Execute: Handles inputs', () => {
errors: [
{
message:
'Variable "$input" got invalid value "foo bar"; Expected type "TestInputObject" to be an object.',
'Variable "$input" has invalid value: Expected value of type TestInputObject to be an object, found: "foo bar".',
locations: [{ line: 2, column: 16 }],
},
],
Expand All @@ -402,7 +402,7 @@ describe('Execute: Handles inputs', () => {
errors: [
{
message:
'Variable "$input" got invalid value { a: "foo", b: "bar" }; Field "c" of required type "String!" was not provided.',
'Variable "$input" has invalid value: Expected value of type TestInputObject to include required field "c", found: { a: "foo", b: "bar" }.',
locations: [{ line: 2, column: 16 }],
},
],
Expand All @@ -421,12 +421,12 @@ describe('Execute: Handles inputs', () => {
errors: [
{
message:
'Variable "$input" got invalid value { a: "foo" } at "input.na"; Field "c" of required type "String!" was not provided.',
'Variable "$input" has invalid value at .na: Expected value of type TestInputObject to include required field "c", found: { a: "foo" }.',
locations: [{ line: 2, column: 18 }],
},
{
message:
'Variable "$input" got invalid value { na: { a: "foo" } }; Field "nb" of required type "String!" was not provided.',
'Variable "$input" has invalid value: Expected value of type TestNestedInputObject to include required field "nb", found: { na: { a: "foo" } }.',
locations: [{ line: 2, column: 18 }],
},
],
Expand All @@ -443,7 +443,7 @@ describe('Execute: Handles inputs', () => {
errors: [
{
message:
'Variable "$input" got invalid value { a: "foo", b: "bar", c: "baz", extra: "dog" }; Field "extra" is not defined by type "TestInputObject".',
'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" }.',
locations: [{ line: 2, column: 16 }],
},
],
Expand Down Expand Up @@ -618,7 +618,7 @@ describe('Execute: Handles inputs', () => {
errors: [
{
message:
'Variable "$value" of required type String! was not provided.',
'Variable "$value" has invalid value: Expected a value of non-null type String! to be provided.',
locations: [{ line: 2, column: 16 }],
},
],
Expand All @@ -637,7 +637,7 @@ describe('Execute: Handles inputs', () => {
errors: [
{
message:
'Variable "$value" of non-null type String! must not be null.',
'Variable "$value" has invalid value: Expected value of non-null type String! not to be null.',
locations: [{ line: 2, column: 16 }],
},
],
Expand Down Expand Up @@ -703,7 +703,7 @@ describe('Execute: Handles inputs', () => {
errors: [
{
message:
'Variable "$value" got invalid value [1, 2, 3]; String cannot represent a non string value: [1, 2, 3]',
'Variable "$value" has invalid value: String cannot represent a non string value: [1, 2, 3]',
locations: [{ line: 2, column: 16 }],
},
],
Expand Down Expand Up @@ -731,7 +731,7 @@ describe('Execute: Handles inputs', () => {
errors: [
{
message:
'Argument TestType.fieldWithNonNullableStringInput(input:) of required type String! was provided the variable "$foo" which was not provided a runtime value.',
'Argument TestType.fieldWithNonNullableStringInput(input:) has invalid value: Expected variable "$foo" provided to type String! to provide a runtime value.',
locations: [{ line: 3, column: 50 }],
path: ['fieldWithNonNullableStringInput'],
},
Expand Down Expand Up @@ -786,7 +786,7 @@ describe('Execute: Handles inputs', () => {
errors: [
{
message:
'Variable "$input" of non-null type [String]! must not be null.',
'Variable "$input" has invalid value: Expected value of non-null type [String]! not to be null.',
locations: [{ line: 2, column: 16 }],
},
],
Expand Down Expand Up @@ -849,7 +849,7 @@ describe('Execute: Handles inputs', () => {
errors: [
{
message:
'Variable "$input" got invalid value null at "input[1]"; Expected non-nullable type "String!" not to be null.',
'Variable "$input" has invalid value at [1]: Expected value of non-null type String! not to be null.',
locations: [{ line: 2, column: 16 }],
},
],
Expand All @@ -868,7 +868,7 @@ describe('Execute: Handles inputs', () => {
errors: [
{
message:
'Variable "$input" of non-null type [String!]! must not be null.',
'Variable "$input" has invalid value: Expected value of non-null type [String!]! not to be null.',
locations: [{ line: 2, column: 16 }],
},
],
Expand Down Expand Up @@ -898,7 +898,7 @@ describe('Execute: Handles inputs', () => {
errors: [
{
message:
'Variable "$input" got invalid value null at "input[1]"; Expected non-nullable type "String!" not to be null.',
'Variable "$input" has invalid value at [1]: Expected value of non-null type String! not to be null.',
locations: [{ line: 2, column: 16 }],
},
],
Expand Down Expand Up @@ -983,7 +983,7 @@ describe('Execute: Handles inputs', () => {
errors: [
{
message:
'Argument TestType.fieldWithDefaultArgumentValue(input:) of type String has invalid value WRONG_TYPE.',
'Argument TestType.fieldWithDefaultArgumentValue(input:) has invalid value: String cannot represent a non string value: WRONG_TYPE',
locations: [{ line: 3, column: 48 }],
path: ['fieldWithDefaultArgumentValue'],
},
Expand Down Expand Up @@ -1023,7 +1023,7 @@ describe('Execute: Handles inputs', () => {

function invalidValueError(value: number, index: number) {
return {
message: `Variable "$input" got invalid value ${value} at "input[${index}]"; String cannot represent a non string value: ${value}`,
message: `Variable "$input" has invalid value at [${index}]: String cannot represent a non string value: ${value}`,
locations: [{ line: 2, column: 14 }],
};
}
Expand Down
157 changes: 67 additions & 90 deletions src/execution/values.ts
View file
@@ -1,7 +1,7 @@
import { inspect } from '../jsutils/inspect.js';
import { invariant } from '../jsutils/invariant.js';
import { keyMap } from '../jsutils/keyMap.js';
import type { Maybe } from '../jsutils/Maybe.js';
import type { ObjMap, ReadOnlyObjMap } from '../jsutils/ObjMap';
import type { ObjMap, ReadOnlyObjMap } from '../jsutils/ObjMap.js';
import { printPathArray } from '../jsutils/printPathArray.js';

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

import type { GraphQLField, GraphQLInputType } from '../type/definition.js';
import { isInputType, isNonNullType } from '../type/definition.js';
import {
isInputType,
isNonNullType,
isRequiredArgument,
} from '../type/definition.js';
import type { GraphQLDirective } from '../type/directives.js';
import type { GraphQLSchema } from '../type/schema.js';

Expand All @@ -25,6 +29,10 @@ import {
coerceInputValue,
} from '../utilities/coerceInputValue.js';
import { typeFromAST } from '../utilities/typeFromAST.js';
import {
validateInputLiteral,
validateInputValue,
} from '../utilities/validateInputValue.js';

export interface VariableValues {
readonly sources: ReadOnlyObjMap<VariableValueSource>;
Expand Down Expand Up @@ -107,55 +115,37 @@ function coerceVariableValues(
continue;
}

if (!hasOwnProperty(inputs, varName)) {
const defaultValue = varDefNode.defaultValue;
if (defaultValue) {
sources[varName] = {
variable: varDefNode,
type: varType,
value: undefined,
};
coerced[varName] = coerceInputLiteral(defaultValue, varType);
} else if (isNonNullType(varType)) {
onError(
new GraphQLError(
`Variable "$${varName}" of required type ${varType} was not provided.`,
{ nodes: varDefNode },
),
);
}
continue;
}
const value = hasOwnProperty(inputs, varName) ? inputs[varName] : undefined;
sources[varName] = { variable: varDefNode, type: varType, value };

const value = inputs[varName];
if (value === null && isNonNullType(varType)) {
onError(
new GraphQLError(
`Variable "$${varName}" of non-null type ${varType} must not be null.`,
{ nodes: varDefNode },
),
);
continue;
if (value === undefined) {
if (varDefNode.defaultValue) {
coerced[varName] = coerceInputLiteral(varDefNode.defaultValue, varType);
continue;
} else if (!isNonNullType(varType)) {
// Non-provided values for nullable variables are omitted.
continue;
}
}

sources[varName] = { variable: varDefNode, type: varType, value };
coerced[varName] = coerceInputValue(
value,
varType,
(path, invalidValue, error) => {
let prefix =
`Variable "$${varName}" got invalid value ` + inspect(invalidValue);
if (path.length > 0) {
prefix += ` at "${varName}${printPathArray(path)}"`;
}
const coercedValue = coerceInputValue(value, varType);
if (coercedValue !== undefined) {
coerced[varName] = coercedValue;
} else {
validateInputValue(value, varType, (error, path) => {
onError(
new GraphQLError(prefix + '; ' + error.message, {
nodes: varDefNode,
originalError: error.originalError,
}),
new GraphQLError(
`Variable "$${varName}" has invalid value${printPathArray(path)}: ${
error.message
}`,
{
nodes: varDefNode,
originalError: error.originalError,
},
),
);
},
);
});
}
}

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

if (!argumentNode) {
if (!argumentNode && isRequiredArgument(argDef)) {
// Note: ProvidedRequiredArgumentsRule validation should catch this before
// execution. This is a runtime check to ensure execution does not
// continue with an invalid argument value.
throw new GraphQLError(
`Argument ${argDef} of required type ${argType} was not provided.`,
{ nodes: node },
);
}

// Variables without a value are treated as if no argument was provided if
// the argument is not required.
if (
!argumentNode ||
(argumentNode.value.kind === Kind.VARIABLE &&
variableValues?.coerced[argumentNode.value.name.value] === undefined &&
!isRequiredArgument(argDef))
) {
if (argDef.defaultValue) {
coercedValues[name] = coerceDefaultValue(
argDef.defaultValue,
argDef.type,
);
} else if (isNonNullType(argType)) {
throw new GraphQLError(
`Argument ${argDef} of required type ${argType} was not provided.`,
{ nodes: node },
);
}
continue;
}

const valueNode = argumentNode.value;
let isNull = valueNode.kind === Kind.NULL;

if (valueNode.kind === Kind.VARIABLE) {
const variableName = valueNode.name.value;
if (
variableValues == null ||
variableValues.coerced[variableName] === undefined
) {
if (argDef.defaultValue) {
coercedValues[name] = coerceDefaultValue(
argDef.defaultValue,
argDef.type,
);
} else if (isNonNullType(argType)) {
throw new GraphQLError(
`Argument ${argDef} of required type ${argType} ` +
`was provided the variable "$${variableName}" which was not provided a runtime value.`,
{ nodes: valueNode },
);
}
continue;
}
isNull = variableValues.coerced[variableName] == null;
}

if (isNull && isNonNullType(argType)) {
throw new GraphQLError(
`Argument ${argDef} of non-null type ${argType} must not be null.`,
{ nodes: valueNode },
);
}

const coercedValue = coerceInputLiteral(valueNode, argType, variableValues);
if (coercedValue === undefined) {
// Note: ValuesOfCorrectTypeRule validation should catch this before
// execution. This is a runtime check to ensure execution does not
// continue with an invalid argument value.
throw new GraphQLError(
`Argument ${argDef} of type ${argType} has invalid value ${print(
valueNode,
)}.`,
{ nodes: valueNode },
validateInputLiteral(
valueNode,
argType,
variableValues,
(error, path) => {
error.message = `Argument ${argDef} has invalid value${printPathArray(
path,
)}: ${error.message}`;
throw error;
},
);
// istanbul ignore next (validateInputLiteral should throw)
invariant(false, 'Invalid argument');
}
coercedValues[name] = coercedValue;
}
Expand Down

0 comments on commit 568fc02

Please sign in to comment.