From 4f8eaac7ff2b73dd18d6317c955d447011a69301 Mon Sep 17 00:00:00 2001 From: JoostK Date: Sat, 25 Sep 2021 12:50:41 +0200 Subject: [PATCH] fix(core): do not use Function constructors in development mode to avoid CSP violations (#43587) This commit removes the dynamic creation of named arrays for internal runtime storage arrays as they may cause CSP violations in development mode, when an application's CSP configuration does not include `unsafe-eval`. Named arrays for view data can still be enabled in development mode using the `ngDevMode=namedConstructors` query parameter when loading the application. The usage of native class syntax for named arrays does not have the desired effect when the code is downleveled to ES5. Since ES5 targets are becoming increasingly more rare this is considered less of a problem than the CSP violation. Fixes #43494 PR Close #43587 --- .../src/render3/instructions/lview_debug.ts | 52 ++++++++--------- .../core/src/render3/instructions/shared.ts | 5 +- .../test/acceptance/ngdevmode_debug_spec.ts | 58 +++++++++++++++++-- 3 files changed, 79 insertions(+), 36 deletions(-) diff --git a/packages/core/src/render3/instructions/lview_debug.ts b/packages/core/src/render3/instructions/lview_debug.ts index 1fd21dfbfdb..9d6b6d2af24 100644 --- a/packages/core/src/render3/instructions/lview_debug.ts +++ b/packages/core/src/render3/instructions/lview_debug.ts @@ -13,7 +13,6 @@ import {Sanitizer} from '../../sanitization/sanitizer'; import {KeyValueArray} from '../../util/array_utils'; import {assertDefined} from '../../util/assert'; import {createNamedArrayType} from '../../util/named_array_type'; -import {initNgDevMode} from '../../util/ng_dev_mode'; import {assertNodeInjector} from '../assert'; import {getInjectorIndex, getParentInjectorLocation} from '../di'; import {CONTAINER_HEADER_OFFSET, HAS_TRANSPLANTED_VIEWS, LContainer, MOVED_VIEWS, NATIVE} from '../interfaces/container'; @@ -30,8 +29,6 @@ import {attachDebugObject} from '../util/debug_utils'; import {getParentInjectorIndex, getParentInjectorView} from '../util/injector_utils'; import {unwrapRNode} from '../util/view_utils'; -const NG_DEV_MODE = ((typeof ngDevMode === 'undefined' || !!ngDevMode) && initNgDevMode()); - /* * This file contains conditionally attached classes which provide human readable (debug) level * information for `LView`, `LContainer` and other internal data structures. These data structures @@ -61,9 +58,11 @@ const NG_DEV_MODE = ((typeof ngDevMode === 'undefined' || !!ngDevMode) && initNg * ``` */ -let LVIEW_COMPONENT_CACHE!: Map>; -let LVIEW_EMBEDDED_CACHE!: Map>; -let LVIEW_ROOT!: Array; +let LVIEW_COMPONENT_CACHE: Map>|undefined; +let LVIEW_EMBEDDED_CACHE: Map>|undefined; +let LVIEW_ROOT: Array|undefined; +let LVIEW_COMPONENT: Array|undefined; +let LVIEW_EMBEDDED: Array|undefined; interface TViewDebug extends ITView { type: TViewType; @@ -80,12 +79,20 @@ export function cloneToLViewFromTViewBlueprint(tView: TView): LView { return lView.concat(tView.blueprint) as any; } +class LRootView extends Array {} +class LComponentView extends Array {} +class LEmbeddedView extends Array {} + function getLViewToClone(type: TViewType, name: string|null): Array { switch (type) { case TViewType.Root: - if (LVIEW_ROOT === undefined) LVIEW_ROOT = new (createNamedArrayType('LRootView'))(); + if (LVIEW_ROOT === undefined) LVIEW_ROOT = new LRootView(); return LVIEW_ROOT; case TViewType.Component: + if (!ngDevMode || !ngDevMode.namedConstructors) { + if (LVIEW_COMPONENT === undefined) LVIEW_COMPONENT = new LComponentView(); + return LVIEW_COMPONENT; + } if (LVIEW_COMPONENT_CACHE === undefined) LVIEW_COMPONENT_CACHE = new Map(); let componentArray = LVIEW_COMPONENT_CACHE.get(name); if (componentArray === undefined) { @@ -94,6 +101,10 @@ function getLViewToClone(type: TViewType, name: string|null): Array { } return componentArray; case TViewType.Embedded: + if (!ngDevMode || !ngDevMode.namedConstructors) { + if (LVIEW_EMBEDDED === undefined) LVIEW_EMBEDDED = new LEmbeddedView(); + return LVIEW_EMBEDDED; + } if (LVIEW_EMBEDDED_CACHE === undefined) LVIEW_EMBEDDED_CACHE = new Map(); let embeddedArray = LVIEW_EMBEDDED_CACHE.get(name); if (embeddedArray === undefined) { @@ -347,7 +358,7 @@ function processTNodeChildren(tNode: ITNode|null, buf: string[]) { } } -const TViewData = NG_DEV_MODE && createNamedArrayType('TViewData') || null! as ArrayConstructor; +class TViewData extends Array {} let TVIEWDATA_EMPTY: unknown[]; // can't initialize here or it will not be tree shaken, because // `LView` constructor could have side-effects. /** @@ -360,24 +371,13 @@ export function cloneToTViewData(list: any[]): TData { return TVIEWDATA_EMPTY.concat(list) as any; } -export const LViewBlueprint = - NG_DEV_MODE && createNamedArrayType('LViewBlueprint') || null! as ArrayConstructor; -export const MatchesArray = - NG_DEV_MODE && createNamedArrayType('MatchesArray') || null! as ArrayConstructor; -export const TViewComponents = - NG_DEV_MODE && createNamedArrayType('TViewComponents') || null! as ArrayConstructor; -export const TNodeLocalNames = - NG_DEV_MODE && createNamedArrayType('TNodeLocalNames') || null! as ArrayConstructor; -export const TNodeInitialInputs = - NG_DEV_MODE && createNamedArrayType('TNodeInitialInputs') || null! as ArrayConstructor; -export const TNodeInitialData = - NG_DEV_MODE && createNamedArrayType('TNodeInitialData') || null! as ArrayConstructor; -export const LCleanup = - NG_DEV_MODE && createNamedArrayType('LCleanup') || null! as ArrayConstructor; -export const TCleanup = - NG_DEV_MODE && createNamedArrayType('TCleanup') || null! as ArrayConstructor; - - +export class LViewBlueprint extends Array {} +export class MatchesArray extends Array {} +export class TViewComponents extends Array {} +export class TNodeLocalNames extends Array {} +export class TNodeInitialInputs extends Array {} +export class LCleanup extends Array {} +export class TCleanup extends Array {} export function attachLViewDebug(lView: LView) { attachDebugObject(lView, new LViewDebug(lView)); diff --git a/packages/core/src/render3/instructions/shared.ts b/packages/core/src/render3/instructions/shared.ts index a9214c4d73e..c05e733b5db 100644 --- a/packages/core/src/render3/instructions/shared.ts +++ b/packages/core/src/render3/instructions/shared.ts @@ -14,8 +14,6 @@ import {validateAgainstEventAttributes, validateAgainstEventProperties} from '.. import {Sanitizer} from '../../sanitization/sanitizer'; import {assertDefined, assertDomNode, assertEqual, assertGreaterThanOrEqual, assertIndexInRange, assertNotEqual, assertNotSame, assertSame, assertString} from '../../util/assert'; import {escapeCommentText} from '../../util/dom'; -import {createNamedArrayType} from '../../util/named_array_type'; -import {initNgDevMode} from '../../util/ng_dev_mode'; import {normalizeDebugBindingName, normalizeDebugBindingValue} from '../../util/ng_reflect'; import {stringify} from '../../util/stringify'; import {assertFirstCreatePass, assertFirstUpdatePass, assertLContainer, assertLView, assertTNodeForLView, assertTNodeForTView} from '../assert'; @@ -1623,8 +1621,7 @@ function generateInitialInputs(inputs: {[key: string]: string}, attrs: TAttribut ////////////////////////// // Not sure why I need to do `any` here but TS complains later. -const LContainerArray: any = ((typeof ngDevMode === 'undefined' || ngDevMode) && initNgDevMode()) && - createNamedArrayType('LContainer'); +const LContainerArray: any = class LContainer extends Array {}; /** * Creates a LContainer, either from a container instruction, or for a ViewContainerRef. diff --git a/packages/core/test/acceptance/ngdevmode_debug_spec.ts b/packages/core/test/acceptance/ngdevmode_debug_spec.ts index 64763f76509..8eef5a817cd 100644 --- a/packages/core/test/acceptance/ngdevmode_debug_spec.ts +++ b/packages/core/test/acceptance/ngdevmode_debug_spec.ts @@ -11,15 +11,28 @@ import {Component} from '@angular/core'; import {getLContext} from '@angular/core/src/render3/context_discovery'; import {getComponentLView} from '@angular/core/src/render3/util/discovery_utils'; import {createNamedArrayType} from '@angular/core/src/util/named_array_type'; +import {ngDevModeResetPerfCounters} from '@angular/core/src/util/ng_dev_mode'; import {TestBed} from '@angular/core/testing'; import {onlyInIvy} from '@angular/private/testing'; -const supportsArraySubclassing = - createNamedArrayType('SupportsArraySubclassing').name === 'SupportsArraySubclassing'; +class SupportsArraySubclassing extends Array {} + +// If this is being compiled to ES5 then the array subclass has `Array` as constructor +// instead of `SupportsArraySubclassing`. +const targetSupportsArraySubclassing = + (new SupportsArraySubclassing()).constructor.name === 'SupportsArraySubclassing'; + +// Creation of a named array types dynamically may succeed even in ES5 targets, as the named array +// is created using the Function constructor. +const runtimeSupportsArraySubclassing = + (new (createNamedArrayType('SupportsArraySubclassing'))).constructor.name === + 'SupportsArraySubclassing'; onlyInIvy('Debug information exist in ivy only').describe('ngDevMode debug', () => { describe('LViewDebug', () => { - supportsArraySubclassing && it('should name LView based on type', () => { + beforeEach(ngDevModeResetPerfCounters); + + runtimeSupportsArraySubclassing && it('should not name LView based on type by default', () => { @Component({ template: `
    @@ -33,16 +46,49 @@ onlyInIvy('Debug information exist in ivy only').describe('ngDevMode debug', () TestBed.configureTestingModule({declarations: [MyApp], imports: [CommonModule]}); const fixture = TestBed.createComponent(MyApp); const rootLView = getLContext(fixture.nativeElement)!.lView; - expect(rootLView.constructor.name).toEqual('LRootView'); + expect(rootLView.constructor.name) + .toEqual(targetSupportsArraySubclassing ? 'LRootView' : 'Array'); const componentLView = getComponentLView(fixture.componentInstance); - expect(componentLView.constructor.name).toEqual('LComponentView_MyApp'); + expect(componentLView.constructor.name) + .toEqual(targetSupportsArraySubclassing ? 'LComponentView' : 'Array'); const element: HTMLElement = fixture.nativeElement; fixture.detectChanges(); const li = element.querySelector('li')!; const embeddedLView = getLContext(li)!.lView; - expect(embeddedLView.constructor.name).toEqual('LEmbeddedView_MyApp_li_1'); + expect(embeddedLView.constructor.name) + .toEqual(targetSupportsArraySubclassing ? 'LEmbeddedView' : 'Array'); }); + + runtimeSupportsArraySubclassing && + it('should name LView based on type when namedConstructors is true', () => { + ngDevMode!.namedConstructors = true; + + @Component({ + template: ` +
      +
    • item
      • +
    + ` + }) + class MyApp { + } + + TestBed.configureTestingModule({declarations: [MyApp], imports: [CommonModule]}); + const fixture = TestBed.createComponent(MyApp); + const rootLView = getLContext(fixture.nativeElement)!.lView; + expect(rootLView.constructor.name) + .toEqual(targetSupportsArraySubclassing ? 'LRootView' : 'Array'); + + const componentLView = getComponentLView(fixture.componentInstance); + expect(componentLView.constructor.name).toEqual('LComponentView_MyApp'); + + const element: HTMLElement = fixture.nativeElement; + fixture.detectChanges(); + const li = element.querySelector('li')!; + const embeddedLView = getLContext(li)!.lView; + expect(embeddedLView.constructor.name).toEqual('LEmbeddedView_MyApp_li_1'); + }); }); });