Skip to content

Commit

Permalink
Fixes for issue 1753. Adds ability to limit errors in getVariableValu…
Browse files Browse the repository at this point in the history 
…es() and coerceValue(). Errors are statically capped at 50
  • Loading branch information
@SoyYoRafa
SoyYoRafa committed Jul 14, 2019
1 parent 49d86bb commit 62f5356
Show file tree
Hide file tree
Showing 4 changed files with 258 additions and 8 deletions.
119 changes: 119 additions & 0 deletions src/execution/__tests__/variables-test.js
View file
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
View file
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
View file
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);
});
});
});

0 comments on commit 62f5356

Please sign in to comment.