Skip to content

Commit 0bd6afb

Browse files
ivanstanevdarscan
authored andcommitted
fix: avoid stack overflow with spread operator
A customer cannot scan their project with a supposedly very large dependency graph because as soon as it reaches our back-end, we get a stack overflow error. It was identified that the error is due to the combination of Array.push() and the spread operator. For reference: nodejs/node#16870 To reproduce, create an array of size 2^17 then do [].push(...myArray); This fix replaces all uses of push() and spread with a simple loop.
1 parent fd8b897 commit 0bd6afb

File tree

2 files changed

+43
-2
lines changed

2 files changed

+43
-2
lines changed

src/core/dep-graph.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,10 @@ class DepGraphImpl implements types.DepGraphInternal {
125125

126126
const pathsToRoot: types.PkgInfo[][] = [];
127127
for (const id of this.getPkgNodeIds(pkg)) {
128-
pathsToRoot.push(...this.pathsFromNodeToRoot(id));
128+
const paths = this.pathsFromNodeToRoot(id);
129+
for (const path of paths) {
130+
pathsToRoot.push(path);
131+
}
129132
}
130133
// note: sorting to get shorter paths first -
131134
// it's nicer - and better resembles older behaviour
@@ -281,7 +284,9 @@ class DepGraphImpl implements types.DepGraphInternal {
281284
const out = this.pathsFromNodeToRoot(id).map((path) => {
282285
return [this.getNodePkg(nodeId)].concat(path);
283286
});
284-
allPaths.push(...out);
287+
for (const path of out) {
288+
allPaths.push(path);
289+
}
285290
});
286291

287292
return allPaths;

test/core/stress.test.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
import * as depGraphLib from '../../src';
2+
3+
const dependencyName = 'needle';
4+
5+
async function generateLargeGraph(width: number) {
6+
const builder = new depGraphLib.DepGraphBuilder(
7+
{ name: 'npm' },
8+
{ name: 'root', version: '1.2.3' },
9+
);
10+
const rootNodeId = 'root-node';
11+
12+
const deepDependency = { name: dependencyName, version: '1.2.3' };
13+
14+
builder.addPkgNode(deepDependency, dependencyName);
15+
builder.connectDep(rootNodeId, dependencyName);
16+
17+
for (let j = 0; j < width; j++) {
18+
const shallowName = `id-${j}`;
19+
const shallowDependency = { name: shallowName, version: '1.2.3' };
20+
21+
builder.addPkgNode(shallowDependency, shallowName);
22+
builder.connectDep(rootNodeId, shallowName);
23+
builder.connectDep(shallowName, dependencyName);
24+
}
25+
26+
return builder.build();
27+
}
28+
29+
describe('stress tests', () => {
30+
test('pkgPathsToRoot() does not cause stack overflow for large dep-graphs', async () => {
31+
const graph = await generateLargeGraph(125000);
32+
33+
const result = graph.pkgPathsToRoot({ name: dependencyName, version: '1.2.3' });
34+
expect(result).toBeDefined();
35+
});
36+
});

0 commit comments

Comments
 (0)