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

fix(16.x.x.): avoid stack size limit in overlapping field rules as well as execute #4116

Closed
41 changes: 41 additions & 0 deletions src/execution/__tests__/executor-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,47 @@ describe('Execute: Handles basic execution tasks', () => {
);
});

it('works with deeply nested fragments', async () => {
const DataType: GraphQLObjectType = new GraphQLObjectType({
name: 'Query',
fields: () => ({
a: { type: GraphQLString, resolve: () => 'Apple' },
}),
});

const n = 10000;
const fragments = Array.from(Array(n).keys()).reduce(
(acc, next) =>
acc.concat(`\n
fragment X${next + 1} on Query {
...X${next}
}
`),
'',
);

const document = parse(`
query {
...X${n}
}
${fragments}
fragment X0 on Query {
a
}
`);

const result = await execute({
schema: new GraphQLSchema({ query: DataType }),
document,
});

expect(result).to.deep.equal({
data: {
a: 'Apple',
},
});
});

it('executes arbitrary code', async () => {
const data = {
a: () => 'Apple',
Expand Down
103 changes: 66 additions & 37 deletions src/execution/collectFields.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,18 @@ import { typeFromAST } from '../utilities/typeFromAST';

import { getDirectiveValues } from './values';

interface FieldEntry {
selection: FieldNode;
name: string;
}

interface EntryWithSelectionset {
selectionSet: SelectionSetNode;
runtimeType: GraphQLObjectType;
}

type StackEntry = EntryWithSelectionset | FieldEntry;

/**
* Given a selectionSet, collects all of the fields and returns them.
*
Expand All @@ -37,16 +49,32 @@ export function collectFields(
runtimeType: GraphQLObjectType,
selectionSet: SelectionSetNode,
): Map<string, ReadonlyArray<FieldNode>> {
const stack: Array<StackEntry> = [{ selectionSet, runtimeType }];
const fields = new Map();
collectFieldsImpl(
schema,
fragments,
variableValues,
runtimeType,
selectionSet,
fields,
new Set(),
);
const visited = new Set<string>();

let entry;
while ((entry = stack.shift()) !== undefined) {
if ('selectionSet' in entry) {
collectFieldsImpl(
schema,
fragments,
variableValues,
entry.runtimeType,
entry.selectionSet,
visited,
stack,
);
} else {
const fieldList = fields.get(entry.name);
if (fieldList !== undefined) {
fieldList.push(entry.selection);
} else {
fields.set(entry.name, [entry.selection]);
}
}
}

return fields;
}

Expand All @@ -68,20 +96,36 @@ export function collectSubfields(
fieldNodes: ReadonlyArray<FieldNode>,
): Map<string, ReadonlyArray<FieldNode>> {
const subFieldNodes = new Map();
const stack: Array<StackEntry> = [];
const visitedFragmentNames = new Set<string>();
for (const node of fieldNodes) {
if (node.selectionSet) {
stack.push({ selectionSet: node.selectionSet, runtimeType: returnType });
}
}

let entry;
while ((entry = stack.shift()) !== undefined) {
if ('selectionSet' in entry) {
collectFieldsImpl(
schema,
fragments,
variableValues,
returnType,
node.selectionSet,
subFieldNodes,
entry.runtimeType,
entry.selectionSet,
visitedFragmentNames,
stack,
);
} else {
const fieldList = subFieldNodes.get(entry.name);
if (fieldList !== undefined) {
fieldList.push(entry.selection);
} else {
subFieldNodes.set(entry.name, [entry.selection]);
}
}
}

return subFieldNodes;
}

Expand All @@ -91,22 +135,18 @@ function collectFieldsImpl(
variableValues: { [variable: string]: unknown },
runtimeType: GraphQLObjectType,
selectionSet: SelectionSetNode,
fields: Map<string, Array<FieldNode>>,
visitedFragmentNames: Set<string>,
stack: Array<StackEntry>,
): void {
const discovered = [];
for (const selection of selectionSet.selections) {
switch (selection.kind) {
case Kind.FIELD: {
if (!shouldIncludeNode(variableValues, selection)) {
continue;
}
const name = getFieldEntryKey(selection);
const fieldList = fields.get(name);
if (fieldList !== undefined) {
fieldList.push(selection);
} else {
fields.set(name, [selection]);
}
discovered.push({ selection, name });
break;
}
case Kind.INLINE_FRAGMENT: {
Expand All @@ -116,15 +156,7 @@ function collectFieldsImpl(
) {
continue;
}
collectFieldsImpl(
schema,
fragments,
variableValues,
runtimeType,
selection.selectionSet,
fields,
visitedFragmentNames,
);
discovered.push({ selectionSet: selection.selectionSet, runtimeType });
break;
}
case Kind.FRAGMENT_SPREAD: {
Expand All @@ -143,19 +175,16 @@ function collectFieldsImpl(
) {
continue;
}
collectFieldsImpl(
schema,
fragments,
variableValues,
runtimeType,
fragment.selectionSet,
fields,
visitedFragmentNames,
);

discovered.push({ selectionSet: fragment.selectionSet, runtimeType });
break;
}
}
}

if (discovered.length !== 0) {
stack.unshift(...discovered);
}
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1138,4 +1138,70 @@ describe('Validate: Overlapping fields can be merged', () => {
},
]);
});

it('does not hit stack size limits', () => {
const n = 10000;
JoviDeCroock marked this conversation as resolved.
Show resolved Hide resolved
const fragments = Array.from(Array(n).keys()).reduce(
(acc, next) =>
acc.concat(`\n
fragment X${next + 1} on Query {
...X${next}
}
`),
'',
);

const query = `
query Test {
...X${n}
}
${fragments}
fragment X0 on Query {
__typename
}
`;

expectErrors(query).toDeepEqual([]);
});

it('finds conflicts in nested fragments', () => {
const n = 10000;
const fragments = Array.from(Array(n).keys()).reduce(
(acc, next) =>
acc.concat(`\n
fragment X${next + 1} on Query {
...X${next}
}
`),
'',
);

const query = `
query Test {
type: conflict
...X${n}
}
${fragments}
fragment X0 on Query {
type: conflict2
__typename
}
`;
expectErrors(query).toDeepEqual([
{
locations: [
{
column: 9,
line: 3,
},
{
column: 9,
line: 50008,
},
],
message:
'Fields "type" conflict because "conflict" and "conflict2" are different fields. Use different aliases on the fields to fetch both if this was intentional.',
},
]);
});
});
Loading
Loading