Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Limits errors in getVariableValues() and coerceValue(). #2037

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
119 changes: 119 additions & 0 deletions src/execution/__tests__/variables-test.js
Expand Up @@ -80,6 +80,22 @@ function fieldWithInputArg(inputArg) {
};
}

function fieldsWithOneHundredInputArg(inputArg) {
const args = {};
for (let index = 0; index < 100; ++index) {
args[`input${index}`] = inputArg;
}
return {
type: GraphQLString,
args,
resolve(_, rargs) {
if ('input0' in rargs) {
return inspect(rargs.input0);
}
},
};
}

const TestType = new GraphQLObjectType({
name: 'TestType',
fields: {
Expand All @@ -104,6 +120,9 @@ const TestType = new GraphQLObjectType({
type: TestNestedInputObject,
defaultValue: 'Hello World',
}),
fieldWithOneHundredInputs: fieldsWithOneHundredInputArg({
type: GraphQLNonNull(GraphQLString),
}),
list: fieldWithInputArg({ type: GraphQLList(GraphQLString) }),
nnList: fieldWithInputArg({
type: GraphQLNonNull(GraphQLList(GraphQLString)),
Expand Down Expand Up @@ -715,6 +734,43 @@ describe('Execute: Handles inputs', () => {
],
});
});

it('limits errors when there are too many null values provided for non-nullable inputs', () => {
let queryFields = '';
let fieldArguments = '';
const valueObject = {};
const errors = [];
for (let index = 0; index < 100; ++index) {
queryFields += `$input${index}: String!`;
fieldArguments = `input${index}: $input${index}`;
if (index !== 99) {
queryFields += ',';
fieldArguments += ',';
}
valueObject[`input${index}`] = null;
if (index < 50) {
const column = index < 10 ? 16 + index * 17 : 16 + index * 18 - 10;
errors.push({
message: `Variable "$input${index}" of non-null type "String!" must not be null.`,
locations: [{ line: 2, column }],
});
}
}
errors.push({
message:
'Too many errors processing variables, error limit reached. Execution aborted.',
});
const doc = `
query (${queryFields}) {
fieldWithOneHundredInputs(${fieldArguments})
}
`;
const result = executeQuery(doc, valueObject);

expect(result).to.deep.equal({
errors,
});
});
});

describe('Handles lists and nullability', () => {
Expand Down Expand Up @@ -882,6 +938,32 @@ describe('Execute: Handles inputs', () => {
});
});

it('limits errors when there are more than 50 errors of does not allow non-null lists of non-nulls to contain null.', () => {
const doc = `
query ($input: [String!]!) {
nnListNN(input: $input)
}
`;
const largeList = new Array(100).fill(null);
const result = executeQuery(doc, { input: largeList });

const expectedErrors = [];
for (let idx = 0; idx < 50; ++idx) {
expectedErrors.push({
message: `Variable "$input" got invalid value [null, null, null, null, null, null, null, null, null, null, ... 90 more items]; Expected non-nullable type String! not to be null at value[${idx}].`,
locations: [{ line: 2, column: 16 }],
});
}
expectedErrors.push({
message:
'Too many errors processing variables, error limit reached. Execution aborted.',
});

expect(result).to.deep.equal({
errors: expectedErrors,
});
});

it('does not allow invalid types to be used as values', () => {
const doc = `
query ($input: TestType!) {
Expand Down Expand Up @@ -919,6 +1001,43 @@ describe('Execute: Handles inputs', () => {
],
});
});

it('limits errors when there are too many unknown types to be used as values', () => {
let queryFields = '';
let fieldArguments = '';
const valueObject = {};
const errors = [];
for (let index = 0; index < 100; ++index) {
queryFields += `$input${index}: UnknownType!`;
fieldArguments = `input${index}: $input${index}`;
if (index !== 99) {
queryFields += ',';
fieldArguments += ',';
}
valueObject[`input${index}`] = 'whoknows';
if (index < 50) {
const column = index < 10 ? 25 + index * 22 : 25 + index * 23 - 9;
errors.push({
message: `Variable "$input${index}" expected value of type "UnknownType!" which cannot be used as an input type.`,
locations: [{ line: 2, column }],
});
}
}
errors.push({
message:
'Too many errors processing variables, error limit reached. Execution aborted.',
});
const doc = `
query (${queryFields}) {
fieldWithOneHundredInputs(${fieldArguments})
}
`;
const result = executeQuery(doc, valueObject);

expect(result).to.deep.equal({
errors,
});
});
});

describe('Execute: Uses argument default values', () => {
Expand Down
31 changes: 29 additions & 2 deletions src/execution/values.js
Expand Up @@ -45,6 +45,8 @@ export function getVariableValues(
): CoercedVariableValues {
const errors = [];
const coercedValues = {};
const maxErrors = 50;
let errorLimitReached = false;
for (let i = 0; i < varDefNodes.length; i++) {
const varDefNode = varDefNodes[i];
const varName = varDefNode.variable.name.value;
Expand All @@ -61,6 +63,10 @@ export function getVariableValues(
varDefNode.type,
),
);
if (errors.length === maxErrors) {
errorLimitReached = true;
break;
}
} else {
const hasValue = hasOwnProperty(inputs, varName);
const value = hasValue ? inputs[varName] : undefined;
Expand All @@ -81,6 +87,10 @@ export function getVariableValues(
varDefNode,
),
);
if (errors.length === maxErrors) {
errorLimitReached = true;
break;
}
} else if (hasValue) {
if (value === null) {
// If the explicit value `null` was provided, an entry in the coerced
Expand All @@ -92,19 +102,36 @@ export function getVariableValues(
const coerced = coerceValue(value, varType, varDefNode);
const coercionErrors = coerced.errors;
if (coercionErrors) {
for (const error of coercionErrors) {
const reachesErrorLimit =
coercionErrors.length + errors.length >= maxErrors;
const publishedErrors = reachesErrorLimit
? coercionErrors
: coercionErrors.slice(0, maxErrors - errors.length);
for (const error of publishedErrors) {
error.message =
`Variable "$${varName}" got invalid value ${inspect(value)}; ` +
error.message;
}
errors.push(...coercionErrors);
errors.push(...publishedErrors);
if (reachesErrorLimit || coerced.errorLimitReached) {
errorLimitReached = true;
break;
}
} else {
coercedValues[varName] = coerced.value;
}
}
}
}
}
if (errorLimitReached) {
errors.push(
new GraphQLError(
'Too many errors processing variables, error limit reached. Execution aborted.',
),
);
}

return errors.length === 0
? { errors: undefined, coerced: coercedValues }
: { errors, coerced: undefined };
Expand Down
84 changes: 84 additions & 0 deletions src/utilities/__tests__/coerceValue-test.js
Expand Up @@ -16,11 +16,13 @@ import {

function expectValue(result) {
expect(result.errors).to.equal(undefined);
expect(result.errorLimitReached).to.equal(undefined);
return expect(result.value);
}

function expectErrors(result) {
expect(result.value).to.equal(undefined);
expect(result.errorLimitReached).to.not.equal(undefined);
const messages = result.errors && result.errors.map(error => error.message);
return expect(messages);
}
Expand Down Expand Up @@ -255,13 +257,63 @@ describe('coerceValue', () => {
]);
});

it('limits errors for too many invalid fields', () => {
const TestInputBigObjectConfig = {
name: 'TestInputBigObject',
fields: {},
};
const valueObject = {};
const errors = [];
for (let index = 0; index < 100; ++index) {
TestInputBigObjectConfig.fields[`field${index}`] = {
type: GraphQLNonNull(GraphQLInt),
};
valueObject[`field${index}`] = 'abc';
if (index < 50) {
errors.push(
`Expected type Int at value.field${index}. Int cannot represent non-integer value: "abc"`,
);
}
}
const TestInputBigObject = new GraphQLInputObjectType(
TestInputBigObjectConfig,
);
const result = coerceValue(valueObject, TestInputBigObject);
expectErrors(result).to.deep.equal(errors);
expect(result.errorLimitReached).to.equal(true);
});

it('returns error for a missing required field', () => {
const result = coerceValue({ bar: 123 }, TestInputObject);
expectErrors(result).to.deep.equal([
'Field value.foo of required type Int! was not provided.',
]);
});

it('limits errors for too many missing required fields', () => {
const TestInputBigObjectConfig = {
name: 'TestInputBigObject',
fields: {},
};
const errors = [];
for (let index = 0; index < 100; ++index) {
TestInputBigObjectConfig.fields[`field${index}`] = {
type: GraphQLNonNull(GraphQLInt),
};
if (index < 50) {
errors.push(
`Field value.field${index} of required type Int! was not provided.`,
);
}
}
const TestInputBigObject = new GraphQLInputObjectType(
TestInputBigObjectConfig,
);
const result = coerceValue({}, TestInputBigObject);
expectErrors(result).to.deep.equal(errors);
expect(result.errorLimitReached).to.equal(true);
});

it('returns error for an unknown field', () => {
const result = coerceValue(
{ foo: 123, unknownField: 123 },
Expand All @@ -272,6 +324,22 @@ describe('coerceValue', () => {
]);
});

it('limits errors for too many unkown fields', () => {
const valueObject = { foo: 123 };
const errors = [];
for (let index = 0; index < 100; ++index) {
valueObject[`field${index}`] = 'string';
if (index < 50) {
errors.push(
`Field "field${index}" is not defined by type TestInputObject.`,
);
}
}
const result = coerceValue(valueObject, TestInputObject);
expectErrors(result).to.deep.equal(errors);
expect(result.errorLimitReached).to.equal(true);
});

it('returns error for a misspelled field', () => {
const result = coerceValue({ foo: 123, bart: 123 }, TestInputObject);
expectErrors(result).to.deep.equal([
Expand Down Expand Up @@ -334,5 +402,21 @@ describe('coerceValue', () => {
const result = coerceValue([42, [null], null], TestNestedList);
expectValue(result).to.deep.equal([[42], [null], null]);
});

it('returns an error array limited to 50 errors and limit reached flag is true', () => {
const value = [];
const errors = [];
for (let index = 0; index < 100; ++index) {
value.push(['string']);
if (index < 50) {
errors.push(
`Expected type Int at value[${index}][0]. Int cannot represent non-integer value: "string"`,
);
}
}
const result = coerceValue(value, TestNestedList);
expectErrors(result).to.deep.equal(errors);
expect(result.errorLimitReached).to.equal(true);
});
});
});