From 33c0ee3441dc6b3e2e097d87640f9b163abd6950 Mon Sep 17 00:00:00 2001 From: Chuck Jazdzewski Date: Fri, 15 Dec 2017 16:25:04 -0800 Subject: [PATCH] fix(compiler): report an error for recursive module references Modules that directly or indirectly export or imported themselves reports an error instead of generating a stack fault. Fixes: #19979 --- packages/compiler-cli/test/ngc_spec.ts | 62 ++++++++++++++++++++++ packages/compiler/src/metadata_resolver.ts | 34 ++++++++++-- 2 files changed, 91 insertions(+), 5 deletions(-) diff --git a/packages/compiler-cli/test/ngc_spec.ts b/packages/compiler-cli/test/ngc_spec.ts index 7008ae3e0c..a7cf6a2c2b 100644 --- a/packages/compiler-cli/test/ngc_spec.ts +++ b/packages/compiler-cli/test/ngc_spec.ts @@ -1636,6 +1636,68 @@ describe('ngc transformer command-line', () => { expect(messages[0]).toContain('Parser Error: Unexpected token'); }); + // Regression test for #19979 + it('should not stack overflow on a recursive module export', () => { + write('src/tsconfig.json', `{ + "extends": "../tsconfig-base.json", + "files": ["test-module.ts"] + }`); + + write('src/test-module.ts', ` + import {Component, NgModule} from '@angular/core'; + + @Component({ + template: 'Hello' + }) + export class MyFaultyComponent {} + + @NgModule({ + exports: [MyFaultyModule], + declarations: [MyFaultyComponent], + providers: [], + }) + export class MyFaultyModule { } + `); + const messages: string[] = []; + expect( + main(['-p', path.join(basePath, 'src/tsconfig.json')], message => messages.push(message))) + .toBe(1, 'Compile was expected to fail'); + expect(messages[0]).toContain(`module 'MyFaultyModule' is exported recursively`); + }); + + // Regression test for #19979 + it('should not stack overflow on a recursive module import', () => { + write('src/tsconfig.json', `{ + "extends": "../tsconfig-base.json", + "files": ["test-module.ts"] + }`); + + write('src/test-module.ts', ` + import {Component, NgModule, forwardRef} from '@angular/core'; + + @Component({ + template: 'Hello' + }) + export class MyFaultyComponent {} + + @NgModule({ + imports: [forwardRef(() => MyFaultyModule)] + }) + export class MyFaultyImport {} + + @NgModule({ + imports: [MyFaultyImport], + declarations: [MyFaultyComponent] + }) + export class MyFaultyModule { } + `); + const messages: string[] = []; + expect( + main(['-p', path.join(basePath, 'src/tsconfig.json')], message => messages.push(message))) + .toBe(1, 'Compile was expected to fail'); + expect(messages[0]).toContain(`is imported recursively by the module 'MyFaultyImport`); + }); + it('should allow using 2 classes with the same name in declarations with noEmitOnError=true', () => { write('src/tsconfig.json', `{ diff --git a/packages/compiler/src/metadata_resolver.ts b/packages/compiler/src/metadata_resolver.ts index 15920b776a..8f8fd4e887 100644 --- a/packages/compiler/src/metadata_resolver.ts +++ b/packages/compiler/src/metadata_resolver.ts @@ -444,11 +444,12 @@ export class CompileMetadataResolver { this._ngModuleResolver.isNgModule(type); } - getNgModuleSummary(moduleType: any): cpl.CompileNgModuleSummary|null { + getNgModuleSummary(moduleType: any, alreadyCollecting: Set|null = null): + cpl.CompileNgModuleSummary|null { let moduleSummary: cpl.CompileNgModuleSummary|null = this._loadSummary(moduleType, cpl.CompileSummaryKind.NgModule); if (!moduleSummary) { - const moduleMeta = this.getNgModuleMetadata(moduleType, false); + const moduleMeta = this.getNgModuleMetadata(moduleType, false, alreadyCollecting); moduleSummary = moduleMeta ? moduleMeta.toSummary() : null; if (moduleSummary) { this._summaryCache.set(moduleType, moduleSummary); @@ -476,7 +477,9 @@ export class CompileMetadataResolver { return Promise.all(loading); } - getNgModuleMetadata(moduleType: any, throwIfNotFound = true): cpl.CompileNgModuleMetadata|null { + getNgModuleMetadata( + moduleType: any, throwIfNotFound = true, + alreadyCollecting: Set|null = null): cpl.CompileNgModuleMetadata|null { moduleType = resolveForwardRef(moduleType); let compileMeta = this._ngModuleCache.get(moduleType); if (compileMeta) { @@ -514,7 +517,18 @@ export class CompileMetadataResolver { if (importedModuleType) { if (this._checkSelfImport(moduleType, importedModuleType)) return; - const importedModuleSummary = this.getNgModuleSummary(importedModuleType); + if (!alreadyCollecting) alreadyCollecting = new Set(); + if (alreadyCollecting.has(importedModuleType)) { + this._reportError( + syntaxError( + `${this._getTypeDescriptor(importedModuleType)} '${stringifyType(importedType)}' is imported recursively by the module '${stringifyType(moduleType)}'.`), + moduleType); + return; + } + alreadyCollecting.add(importedModuleType); + const importedModuleSummary = + this.getNgModuleSummary(importedModuleType, alreadyCollecting); + alreadyCollecting.delete(importedModuleType); if (!importedModuleSummary) { this._reportError( syntaxError( @@ -542,7 +556,17 @@ export class CompileMetadataResolver { moduleType); return; } - const exportedModuleSummary = this.getNgModuleSummary(exportedType); + if (!alreadyCollecting) alreadyCollecting = new Set(); + if (alreadyCollecting.has(exportedType)) { + this._reportError( + syntaxError( + `${this._getTypeDescriptor(exportedType)} '${stringify(exportedType)}' is exported recursively by the module '${stringifyType(moduleType)}'`), + moduleType); + return; + } + alreadyCollecting.add(exportedType); + const exportedModuleSummary = this.getNgModuleSummary(exportedType, alreadyCollecting); + alreadyCollecting.delete(exportedType); if (exportedModuleSummary) { exportedModules.push(exportedModuleSummary); } else {