From 463945003dcf253c64809ffdcddabedb87e78e06 Mon Sep 17 00:00:00 2001 From: JoostK Date: Sun, 21 Jul 2024 22:21:15 +0200 Subject: [PATCH] fix(compiler): limit the number of chained instructions (#57069) Some Angular template instructions that follow each other may be chained together in a single expressions statement, containing a deeply nested AST of call expressions. The number of chained instructions wasn't previously limited, so this could result in very deep ASTs that cause stack overflow errors during TypeScript emit. This commit introduces a limit to the number of chained instructions to avoid these problems. Closes #57066 PR Close #57069 --- .../GOLDEN_PARTIAL.js | 548 ++++++++++++++++++ .../r3_view_compiler_template/TEST_CASES.json | 11 + .../create_many_elements.js | 6 + .../create_many_elements.ts | 269 +++++++++ .../template/pipeline/src/phases/chaining.ts | 16 +- 5 files changed, 849 insertions(+), 1 deletion(-) create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/create_many_elements.js create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/create_many_elements.ts diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/GOLDEN_PARTIAL.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/GOLDEN_PARTIAL.js index 4458c430fed..c65ce590881 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/GOLDEN_PARTIAL.js +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/GOLDEN_PARTIAL.js @@ -1271,3 +1271,551 @@ export declare class MyComponent { static ɵcmp: i0.ɵɵComponentDeclaration; } +/**************************************************************************************************** + * PARTIAL FILE: create_many_elements.js + ****************************************************************************************************/ +import { Component } from '@angular/core'; +import * as i0 from "@angular/core"; +export class MyComponent { +} +MyComponent.ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyComponent, deps: [], target: i0.ɵɵFactoryTarget.Component }); +MyComponent.ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "14.0.0", version: "0.0.0-PLACEHOLDER", type: MyComponent, selector: "my-component", ngImport: i0, template: ` +
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+ `, isInline: true }); +i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: MyComponent, decorators: [{ + type: Component, + args: [{ + selector: 'my-component', + template: ` +
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+ `, + }] + }] }); + +/**************************************************************************************************** + * PARTIAL FILE: create_many_elements.d.ts + ****************************************************************************************************/ +import * as i0 from "@angular/core"; +export declare class MyComponent { + static ɵfac: i0.ɵɵFactoryDeclaration; + static ɵcmp: i0.ɵɵComponentDeclaration; +} + diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/TEST_CASES.json b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/TEST_CASES.json index 4dd2c950884..9a556531741 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/TEST_CASES.json +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/TEST_CASES.json @@ -398,6 +398,17 @@ "failureMessage": "Incorrect template" } ] + }, + { + "description": "should break large element creation chains", + "inputFiles": [ + "create_many_elements.ts" + ], + "expectations": [ + { + "failureMessage": "Incorrect template" + } + ] } ] } diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/create_many_elements.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/create_many_elements.js new file mode 100644 index 00000000000..5286f89c7dd --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/create_many_elements.js @@ -0,0 +1,6 @@ +function MyComponent_Template(rf, ctx) { + if (rf & 1) { + $r3$.ɵɵelement(0, "div")(1, "div")(2, "div")…(255, "div"); + $r3$.ɵɵelement(256, "div")(257, "div")(258, "div")(259, "div"); + } +} diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/create_many_elements.ts b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/create_many_elements.ts new file mode 100644 index 00000000000..9947c0a7924 --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_template/create_many_elements.ts @@ -0,0 +1,269 @@ +import { Component, NgModule } from '@angular/core'; + +@Component({ + selector: 'my-component', + template: ` +
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+
+ `, +}) +export class MyComponent { +} diff --git a/packages/compiler/src/template/pipeline/src/phases/chaining.ts b/packages/compiler/src/template/pipeline/src/phases/chaining.ts index 662064ed2e3..f35a94ae058 100644 --- a/packages/compiler/src/template/pipeline/src/phases/chaining.ts +++ b/packages/compiler/src/template/pipeline/src/phases/chaining.ts @@ -43,6 +43,13 @@ const CHAINABLE = new Set([ R3.declareLet, ]); +/** + * Chaining results in repeated call expressions, causing a deep AST of receiver expressions. To prevent running out of + * stack depth the maximum number of chained instructions is limited to this threshold, which has been selected + * arbitrarily. + */ +const MAX_CHAIN_LENGTH = 256; + /** * Post-process a reified view compilation and convert sequential calls to chainable instructions * into chain calls. @@ -93,7 +100,7 @@ function chainOperationsInList(opList: ir.OpList): vo // This instruction can be chained. It can either be added on to the previous chain (if // compatible) or it can be the start of a new chain. - if (chain !== null && chain.instruction === instruction) { + if (chain !== null && chain.instruction === instruction && chain.length < MAX_CHAIN_LENGTH) { // This instruction can be added onto the previous chain. const expression = chain.expression.callFn( op.statement.expr.args, @@ -102,6 +109,7 @@ function chainOperationsInList(opList: ir.OpList): vo ); chain.expression = expression; chain.op.statement = expression.toStmt(); + chain.length++; ir.OpList.remove(op as ir.Op); } else { // Leave this instruction alone for now, but consider it the start of a new chain. @@ -109,6 +117,7 @@ function chainOperationsInList(opList: ir.OpList): vo op, instruction, expression: op.statement.expr, + length: 1, }; } } @@ -135,4 +144,9 @@ interface Chain { * The instruction that is being chained. */ instruction: o.ExternalReference; + + /** + * The number of instructions that have been collected into this chain. + */ + length: number; }