Skip to content

Commit 597e4f5

Browse files
committed
Fixes for issue 1753. Adds ability to limit errors in getVariableValues() and coerceValue(). Errors are statically capped at 50
1 parent 49d86bb commit 597e4f5

File tree

4 files changed

+99
-8
lines changed

4 files changed

+99
-8
lines changed

src/execution/__tests__/variables-test.js

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -882,6 +882,32 @@ describe('Execute: Handles inputs', () => {
882882
});
883883
});
884884

885+
it('Limits errors when there are more than 50 errors of does not allow non-null lists of non-nulls to contain null.', () => {
886+
const doc = `
887+
query ($input: [String!]!) {
888+
nnListNN(input: $input)
889+
}
890+
`;
891+
const largeList = new Array(100).fill(null);
892+
const result = executeQuery(doc, { input: largeList });
893+
894+
const expectedErrors = [];
895+
for (let idx = 0; idx < 50; ++idx) {
896+
expectedErrors.push({
897+
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}].`,
898+
locations: [{ line: 2, column: 16 }],
899+
});
900+
}
901+
expectedErrors.push({
902+
message:
903+
'Too many errors processing variables, error limit reached. Execution aborted.',
904+
});
905+
906+
expect(result).to.deep.equal({
907+
errors: expectedErrors,
908+
});
909+
});
910+
885911
it('does not allow invalid types to be used as values', () => {
886912
const doc = `
887913
query ($input: TestType!) {

src/execution/values.js

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ export function getVariableValues(
4545
): CoercedVariableValues {
4646
const errors = [];
4747
const coercedValues = {};
48+
const maxErrors = 50;
49+
let errorLimitReached = false;
4850
for (let i = 0; i < varDefNodes.length; i++) {
4951
const varDefNode = varDefNodes[i];
5052
const varName = varDefNode.variable.name.value;
@@ -61,6 +63,10 @@ export function getVariableValues(
6163
varDefNode.type,
6264
),
6365
);
66+
if (errors.length === maxErrors) {
67+
errorLimitReached = true;
68+
break;
69+
}
6470
} else {
6571
const hasValue = hasOwnProperty(inputs, varName);
6672
const value = hasValue ? inputs[varName] : undefined;
@@ -81,6 +87,10 @@ export function getVariableValues(
8187
varDefNode,
8288
),
8389
);
90+
if (errors.length === maxErrors) {
91+
errorLimitReached = true;
92+
break;
93+
}
8494
} else if (hasValue) {
8595
if (value === null) {
8696
// If the explicit value `null` was provided, an entry in the coerced
@@ -92,19 +102,36 @@ export function getVariableValues(
92102
const coerced = coerceValue(value, varType, varDefNode);
93103
const coercionErrors = coerced.errors;
94104
if (coercionErrors) {
95-
for (const error of coercionErrors) {
105+
const reachesErrorLimit =
106+
coercionErrors.length + errors.length >= maxErrors;
107+
const publishedErrors = reachesErrorLimit
108+
? coercionErrors
109+
: coercionErrors.slice(0, maxErrors - errors.length);
110+
for (const error of publishedErrors) {
96111
error.message =
97112
`Variable "$${varName}" got invalid value ${inspect(value)}; ` +
98113
error.message;
99114
}
100-
errors.push(...coercionErrors);
115+
errors.push(...publishedErrors);
116+
if (reachesErrorLimit || coerced.errorLimitReached) {
117+
errorLimitReached = true;
118+
break;
119+
}
101120
} else {
102121
coercedValues[varName] = coerced.value;
103122
}
104123
}
105124
}
106125
}
107126
}
127+
if (errorLimitReached) {
128+
errors.push(
129+
new GraphQLError(
130+
'Too many errors processing variables, error limit reached. Execution aborted.',
131+
),
132+
);
133+
}
134+
108135
return errors.length === 0
109136
? { errors: undefined, coerced: coercedValues }
110137
: { errors, coerced: undefined };

src/utilities/__tests__/coerceValue-test.js

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,13 @@ import {
1616

1717
function expectValue(result) {
1818
expect(result.errors).to.equal(undefined);
19+
expect(result.errorLimitReached).to.equal(undefined);
1920
return expect(result.value);
2021
}
2122

2223
function expectErrors(result) {
2324
expect(result.value).to.equal(undefined);
25+
expect(result.errorLimitReached).to.not.equal(undefined);
2426
const messages = result.errors && result.errors.map(error => error.message);
2527
return expect(messages);
2628
}
@@ -334,5 +336,21 @@ describe('coerceValue', () => {
334336
const result = coerceValue([42, [null], null], TestNestedList);
335337
expectValue(result).to.deep.equal([[42], [null], null]);
336338
});
339+
340+
it('returns an error array limited to 50 errors and limit reached flag is true', () => {
341+
const value = [];
342+
const errors = [];
343+
for (let index = 0; index < 100; ++index) {
344+
value.push(['string']);
345+
if (index < 50) {
346+
errors.push(
347+
`Expected type Int at value[${index}][0]. Int cannot represent non-integer value: "string"`,
348+
);
349+
}
350+
}
351+
const result = coerceValue(value, TestNestedList);
352+
expectErrors(result).to.deep.equal(errors);
353+
expect(result.errorLimitReached).to.equal(true);
354+
});
337355
});
338356
});

src/utilities/coerceValue.js

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
// @flow strict
22

3-
import { forEach, isCollection } from 'iterall';
3+
import { isCollection } from 'iterall';
44
import objectValues from '../polyfills/objectValues';
55
import inspect from '../jsutils/inspect';
66
import isInvalid from '../jsutils/isInvalid';
@@ -19,6 +19,7 @@ import {
1919
} from '../type/definition';
2020

2121
type CoercedValue = {|
22+
+errorLimitReached: boolean | void,
2223
+errors: $ReadOnlyArray<GraphQLError> | void,
2324
+value: mixed,
2425
|};
@@ -38,6 +39,7 @@ export function coerceValue(
3839
blameNode?: ASTNode,
3940
path?: Path,
4041
): CoercedValue {
42+
const maxErrors = 50;
4143
// A value must be provided if the type is non-null.
4244
if (isNonNullType(type)) {
4345
if (value == null) {
@@ -108,7 +110,9 @@ export function coerceValue(
108110
if (isCollection(value)) {
109111
let errors;
110112
const coercedValue = [];
111-
forEach((value: any), (itemValue, index) => {
113+
const listValue: $ReadOnlyArray<mixed> = (value: any);
114+
for (let index = 0; index < listValue.length; ++index) {
115+
const itemValue = listValue[index];
112116
const coercedItem = coerceValue(
113117
itemValue,
114118
itemType,
@@ -117,10 +121,13 @@ export function coerceValue(
117121
);
118122
if (coercedItem.errors) {
119123
errors = add(errors, coercedItem.errors);
124+
if (errors.length >= maxErrors) {
125+
return ofErrors(errors.slice(0, maxErrors), true);
126+
}
120127
} else if (!errors) {
121128
coercedValue.push(coercedItem.value);
122129
}
123-
});
130+
}
124131
return errors ? ofErrors(errors) : ofValue(coercedValue);
125132
}
126133
// Lists accept a non-list value as a list of one.
@@ -157,6 +164,9 @@ export function coerceValue(
157164
blameNode,
158165
),
159166
);
167+
if (errors.length >= maxErrors) {
168+
return ofErrors(errors.slice(0, maxErrors), true);
169+
}
160170
}
161171
} else {
162172
const coercedField = coerceValue(
@@ -167,6 +177,9 @@ export function coerceValue(
167177
);
168178
if (coercedField.errors) {
169179
errors = add(errors, coercedField.errors);
180+
if (errors.length >= maxErrors) {
181+
return ofErrors(errors.slice(0, maxErrors), true);
182+
}
170183
} else if (!errors) {
171184
coercedValue[field.name] = coercedField.value;
172185
}
@@ -186,6 +199,9 @@ export function coerceValue(
186199
didYouMean(suggestions),
187200
),
188201
);
202+
if (errors.length >= maxErrors) {
203+
return ofErrors(errors.slice(0, maxErrors), true);
204+
}
189205
}
190206
}
191207

@@ -198,11 +214,15 @@ export function coerceValue(
198214
}
199215

200216
function ofValue(value) {
201-
return { errors: undefined, value };
217+
return { errorLimitReached: undefined, errors: undefined, value };
202218
}
203219

204-
function ofErrors(errors) {
205-
return { errors, value: undefined };
220+
function ofErrors(errors, errorLimitReached) {
221+
return {
222+
errorLimitReached: errorLimitReached == null ? false : errorLimitReached,
223+
errors,
224+
value: undefined,
225+
};
206226
}
207227

208228
function add(errors, moreErrors) {

0 commit comments

Comments
 (0)