Skip to content

Commit eeba30a

Browse files
authored
Fix infinite loop: module.exports alias detection (microsoft#31436)
* Fix infinite loop: module.exports alias detection Previously, module.exports alias detection in the binder could enter an infinite recursion. Now it does not. Notably, there are *two* safeguards: a counter limiter that I set at 100, and an already-seen set. I actually prefer the counter limiter code because it's foolproof and uses less memory. But it takes 100 iterations to escape from loops. * fix space lint * Remove already-seen map
1 parent f4b83ef commit eeba30a

File tree

4 files changed

+61
-16
lines changed

4 files changed

+61
-16
lines changed

src/compiler/binder.ts

Lines changed: 22 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2581,7 +2581,7 @@ namespace ts {
25812581
// Fix up parent pointers since we're going to use these nodes before we bind into them
25822582
node.left.parent = node;
25832583
node.right.parent = node;
2584-
if (isIdentifier(lhs.expression) && container === file && isNameOfExportsOrModuleExportsAliasDeclaration(file, lhs.expression)) {
2584+
if (isIdentifier(lhs.expression) && container === file && isExportsOrModuleExportsOrAlias(file, lhs.expression)) {
25852585
// This can be an alias for the 'exports' or 'module.exports' names, e.g.
25862586
// var util = module.exports;
25872587
// util.property = function ...
@@ -2975,21 +2975,27 @@ namespace ts {
29752975
}
29762976

29772977
export function isExportsOrModuleExportsOrAlias(sourceFile: SourceFile, node: Expression): boolean {
2978-
return isExportsIdentifier(node) ||
2979-
isModuleExportsPropertyAccessExpression(node) ||
2980-
isIdentifier(node) && isNameOfExportsOrModuleExportsAliasDeclaration(sourceFile, node);
2981-
}
2982-
2983-
function isNameOfExportsOrModuleExportsAliasDeclaration(sourceFile: SourceFile, node: Identifier): boolean {
2984-
const symbol = lookupSymbolForNameWorker(sourceFile, node.escapedText);
2985-
return !!symbol && !!symbol.valueDeclaration && isVariableDeclaration(symbol.valueDeclaration) &&
2986-
!!symbol.valueDeclaration.initializer && isExportsOrModuleExportsOrAliasOrAssignment(sourceFile, symbol.valueDeclaration.initializer);
2987-
}
2988-
2989-
function isExportsOrModuleExportsOrAliasOrAssignment(sourceFile: SourceFile, node: Expression): boolean {
2990-
return isExportsOrModuleExportsOrAlias(sourceFile, node) ||
2991-
(isAssignmentExpression(node, /*excludeCompoundAssignment*/ true) && (
2992-
isExportsOrModuleExportsOrAliasOrAssignment(sourceFile, node.left) || isExportsOrModuleExportsOrAliasOrAssignment(sourceFile, node.right)));
2978+
let i = 0;
2979+
const q = [node];
2980+
while (q.length && i < 100) {
2981+
i++;
2982+
node = q.shift()!;
2983+
if (isExportsIdentifier(node) || isModuleExportsPropertyAccessExpression(node)) {
2984+
return true;
2985+
}
2986+
else if (isIdentifier(node)) {
2987+
const symbol = lookupSymbolForNameWorker(sourceFile, node.escapedText);
2988+
if (!!symbol && !!symbol.valueDeclaration && isVariableDeclaration(symbol.valueDeclaration) && !!symbol.valueDeclaration.initializer) {
2989+
const init = symbol.valueDeclaration.initializer;
2990+
q.push(init);
2991+
if (isAssignmentExpression(init, /*excludeCompoundAssignment*/ true)) {
2992+
q.push(init.left);
2993+
q.push(init.right);
2994+
}
2995+
}
2996+
}
2997+
}
2998+
return false;
29932999
}
29943000

29953001
function lookupSymbolForNameWorker(container: Node, name: __String): Symbol | undefined {
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
=== tests/cases/conformance/salsa/loop.js ===
2+
var loop1 = loop2;
3+
>loop1 : Symbol(loop1, Decl(loop.js, 0, 3))
4+
>loop2 : Symbol(loop2, Decl(loop.js, 1, 3))
5+
6+
var loop2 = loop1;
7+
>loop2 : Symbol(loop2, Decl(loop.js, 1, 3))
8+
>loop1 : Symbol(loop1, Decl(loop.js, 0, 3))
9+
10+
module.exports = loop2;
11+
>module.exports : Symbol("tests/cases/conformance/salsa/loop", Decl(loop.js, 0, 0))
12+
>module : Symbol(export=, Decl(loop.js, 1, 18))
13+
>exports : Symbol(export=, Decl(loop.js, 1, 18))
14+
>loop2 : Symbol(loop2, Decl(loop.js, 1, 3))
15+
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
=== tests/cases/conformance/salsa/loop.js ===
2+
var loop1 = loop2;
3+
>loop1 : any
4+
>loop2 : any
5+
6+
var loop2 = loop1;
7+
>loop2 : any
8+
>loop1 : any
9+
10+
module.exports = loop2;
11+
>module.exports = loop2 : any
12+
>module.exports : any
13+
>module : { "tests/cases/conformance/salsa/loop": any; }
14+
>exports : any
15+
>loop2 : any
16+
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
// @noEmit: true
2+
// @allowJs: true
3+
// @checkJs: true
4+
// @Filename: loop.js
5+
var loop1 = loop2;
6+
var loop2 = loop1;
7+
8+
module.exports = loop2;

0 commit comments

Comments
 (0)