diff --git a/packages/compiler-cli/test/compliance/r3_view_compiler_styling_spec.ts b/packages/compiler-cli/test/compliance/r3_view_compiler_styling_spec.ts index 4a9580256c..bb506bd473 100644 --- a/packages/compiler-cli/test/compliance/r3_view_compiler_styling_spec.ts +++ b/packages/compiler-cli/test/compliance/r3_view_compiler_styling_spec.ts @@ -7,7 +7,9 @@ */ import {AttributeMarker, ViewEncapsulation} from '@angular/compiler/src/core'; +import {CompilerStylingMode, compilerSetStylingMode} from '@angular/compiler/src/render3/view/styling_state'; import {setup} from '@angular/compiler/test/aot/test_util'; + import {compile, expectEmit} from './mock_compile'; describe('compiler compliance: styling', () => { @@ -1463,4 +1465,122 @@ describe('compiler compliance: styling', () => { const result = compile(files, angularFiles); expectEmit(result.source, template, 'Incorrect template'); }); + + describe('new styling refactor', () => { + beforeEach(() => { compilerSetStylingMode(CompilerStylingMode.UseNew); }); + + afterEach(() => { compilerSetStylingMode(CompilerStylingMode.UseOld); }); + + it('should generate a `styleSanitizer` instruction when one or more sanitizable style properties are statically detected', + () => { + const files = { + app: { + 'spec.ts': ` + import {Component, NgModule} from '@angular/core'; + + @Component({ + selector: 'my-app', + template: \` +
+ \` + }) + export class MyAppComp { + bgExp = ''; + } + ` + } + }; + + const template = ` + template: function MyAppComp_Template(rf, ctx) { + … + if (rf & 2) { + $r3$.ɵɵselect(0); + $r3$.ɵɵstyleSanitizer($r3$.ɵɵdefaultStyleSanitizer); + $r3$.ɵɵstyleProp(0, ctx.bgExp); + $r3$.ɵɵstylingApply(); + } + … + } + `; + + const result = compile(files, angularFiles); + expectEmit(result.source, template, 'Incorrect template'); + }); + + it('should generate a `styleSanitizer` instruction when a `styleMap` instruction is used', + () => { + const files = { + app: { + 'spec.ts': ` + import {Component, NgModule} from '@angular/core'; + + @Component({ + selector: 'my-app', + template: \` +
+ \` + }) + export class MyAppComp { + mapExp = {}; + } + ` + } + }; + + const template = ` + template: function MyAppComp_Template(rf, ctx) { + … + if (rf & 2) { + $r3$.ɵɵselect(0); + $r3$.ɵɵstyleSanitizer($r3$.ɵɵdefaultStyleSanitizer); + $r3$.ɵɵstyleMap(ctx.mapExp); + $r3$.ɵɵstylingApply(); + } + … + } + `; + + const result = compile(files, angularFiles); + expectEmit(result.source, template, 'Incorrect template'); + }); + + it('shouldn\'t generate a `styleSanitizer` instruction when class-based instructions are used', + () => { + const files = { + app: { + 'spec.ts': ` + import {Component, NgModule} from '@angular/core'; + + @Component({ + selector: 'my-app', + template: \` +
+ \` + }) + export class MyAppComp { + mapExp = {}; + nameExp = true; + } + ` + } + }; + + const template = ` + template: function MyAppComp_Template(rf, ctx) { + … + if (rf & 2) { + $r3$.ɵɵselect(0); + $r3$.ɵɵclassMap(ctx.mapExp); + $r3$.ɵɵclassProp(0, ctx.nameExp); + $r3$.ɵɵstylingApply(); + } + … + } + `; + + const result = compile(files, angularFiles); + expectEmit(result.source, template, 'Incorrect template'); + }); + }); }); diff --git a/packages/compiler/src/render3/r3_identifiers.ts b/packages/compiler/src/render3/r3_identifiers.ts index 7c8b0a061d..6979c3e140 100644 --- a/packages/compiler/src/render3/r3_identifiers.ts +++ b/packages/compiler/src/render3/r3_identifiers.ts @@ -80,6 +80,8 @@ export class Identifiers { static stylingApply: o.ExternalReference = {name: 'ɵɵstylingApply', moduleName: CORE}; + static styleSanitizer: o.ExternalReference = {name: 'ɵɵstyleSanitizer', moduleName: CORE}; + static elementHostAttrs: o.ExternalReference = {name: 'ɵɵelementHostAttrs', moduleName: CORE}; static containerCreate: o.ExternalReference = {name: 'ɵɵcontainer', moduleName: CORE}; diff --git a/packages/compiler/src/render3/view/styling_builder.ts b/packages/compiler/src/render3/view/styling_builder.ts index f9510aaac7..3ca675ecaf 100644 --- a/packages/compiler/src/render3/view/styling_builder.ts +++ b/packages/compiler/src/render3/view/styling_builder.ts @@ -89,6 +89,7 @@ export class StylingBuilder { /** an array of each [class.name] input */ private _singleClassInputs: BoundStylingEntry[]|null = null; private _lastStylingInput: BoundStylingEntry|null = null; + private _firstStylingInput: BoundStylingEntry|null = null; // maps are used instead of hash maps because a Map will // retain the ordering of the keys @@ -181,6 +182,7 @@ export class StylingBuilder { registerIntoMap(this._stylesIndex, property); } this._lastStylingInput = entry; + this._firstStylingInput = this._firstStylingInput || entry; this.hasBindings = true; return entry; } @@ -200,6 +202,7 @@ export class StylingBuilder { registerIntoMap(this._classesIndex, property); } this._lastStylingInput = entry; + this._firstStylingInput = this._firstStylingInput || entry; this.hasBindings = true; return entry; } @@ -453,6 +456,15 @@ export class StylingBuilder { }; } + private _buildSanitizerFn() { + return { + sourceSpan: this._firstStylingInput ? this._firstStylingInput.sourceSpan : null, + reference: R3.styleSanitizer, + allocateBindingSlots: 0, + buildParams: () => [o.importExpr(R3.defaultStyleSanitizer)] + }; + } + /** * Constructs all instructions which contain the expressions that will be placed * into the update block of a template function or a directive hostBindings function. @@ -460,6 +472,9 @@ export class StylingBuilder { buildUpdateLevelInstructions(valueConverter: ValueConverter) { const instructions: Instruction[] = []; if (this.hasBindings) { + if (compilerIsNewStylingInUse() && this._useDefaultSanitizer) { + instructions.push(this._buildSanitizerFn()); + } const styleMapInstruction = this.buildStyleMapInstruction(valueConverter); if (styleMapInstruction) { instructions.push(styleMapInstruction); diff --git a/packages/core/src/render3/debug.ts b/packages/core/src/render3/debug.ts index e8d9997b76..0d186a5568 100644 --- a/packages/core/src/render3/debug.ts +++ b/packages/core/src/render3/debug.ts @@ -195,8 +195,8 @@ export function toDebugNodes(tNode: TNode | null, lView: LView): DebugNode[]|nul let styles: DebugNewStyling|null = null; let classes: DebugNewStyling|null = null; if (runtimeIsNewStylingInUse()) { - styles = tNode.newStyles ? new NodeStylingDebug(tNode.newStyles, lView) : null; - classes = tNode.newClasses ? new NodeStylingDebug(tNode.newClasses, lView) : null; + styles = tNode.newStyles ? new NodeStylingDebug(tNode.newStyles, lView, false) : null; + classes = tNode.newClasses ? new NodeStylingDebug(tNode.newClasses, lView, true) : null; } debugNodes.push({ diff --git a/packages/core/src/render3/index.ts b/packages/core/src/render3/index.ts index 61c5c0daf7..a85c4255e2 100644 --- a/packages/core/src/render3/index.ts +++ b/packages/core/src/render3/index.ts @@ -102,6 +102,7 @@ export { ɵɵselect, ɵɵstyleMap, ɵɵstyleProp, + ɵɵstyleSanitizer, ɵɵstyling, ɵɵstylingApply, ɵɵtemplate, diff --git a/packages/core/src/render3/instructions/all.ts b/packages/core/src/render3/instructions/all.ts index b876f4980c..ab291b1cb4 100644 --- a/packages/core/src/render3/instructions/all.ts +++ b/packages/core/src/render3/instructions/all.ts @@ -45,5 +45,6 @@ export * from './property'; export * from './property_interpolation'; export * from './select'; export * from './styling'; +export {styleSanitizer as ɵɵstyleSanitizer} from '../styling_next/instructions'; export * from './text'; export * from './text_interpolation'; diff --git a/packages/core/src/render3/jit/environment.ts b/packages/core/src/render3/jit/environment.ts index b40dff1843..c48a3a02ae 100644 --- a/packages/core/src/render3/jit/environment.ts +++ b/packages/core/src/render3/jit/environment.ts @@ -121,6 +121,7 @@ export const angularCoreEnv: {[name: string]: Function} = 'ɵɵstyling': r3.ɵɵstyling, 'ɵɵstyleMap': r3.ɵɵstyleMap, 'ɵɵstyleProp': r3.ɵɵstyleProp, + 'ɵɵstyleSanitizer': r3.ɵɵstyleSanitizer, 'ɵɵstylingApply': r3.ɵɵstylingApply, 'ɵɵclassProp': r3.ɵɵclassProp, 'ɵɵselect': r3.ɵɵselect, diff --git a/packages/core/src/render3/styling/class_and_style_bindings.ts b/packages/core/src/render3/styling/class_and_style_bindings.ts index 4a30e99837..f90b1ca6f2 100644 --- a/packages/core/src/render3/styling/class_and_style_bindings.ts +++ b/packages/core/src/render3/styling/class_and_style_bindings.ts @@ -5,7 +5,7 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ -import {StyleSanitizeFn} from '../../sanitization/style_sanitizer'; +import {StyleSanitizeFn, StyleSanitizeMode} from '../../sanitization/style_sanitizer'; import {EMPTY_ARRAY, EMPTY_OBJ} from '../empty'; import {AttributeMarker, TAttributes} from '../interfaces/node'; import {BindingStore, BindingType, Player, PlayerBuilder, PlayerFactory, PlayerIndex} from '../interfaces/player'; @@ -943,7 +943,9 @@ function updateSingleStylingValue( if (currDirective !== directiveIndex) { const prop = getProp(context, singleIndex); const sanitizer = getStyleSanitizer(context, directiveIndex); - setSanitizeFlag(context, singleIndex, (sanitizer && sanitizer(prop)) ? true : false); + setSanitizeFlag( + context, singleIndex, + (sanitizer && sanitizer(prop, null, StyleSanitizeMode.ValidateProperty)) ? true : false); } // the value will always get updated (even if the dirty flag is skipped) @@ -1141,7 +1143,8 @@ export function setStyle( native: any, prop: string, value: string | null, renderer: Renderer3, sanitizer: StyleSanitizeFn | null, store?: BindingStore | null, playerBuilder?: ClassAndStylePlayerBuilder| null) { - value = sanitizer && value ? sanitizer(prop, value) : value; + value = + sanitizer && value ? sanitizer(prop, value, StyleSanitizeMode.ValidateAndSanitize) : value; if (store || playerBuilder) { if (store) { store.setValue(prop, value); @@ -1461,7 +1464,9 @@ function valueExists(value: string | null | boolean, isClassBased?: boolean) { function prepareInitialFlag( context: StylingContext, prop: string, entryIsClassBased: boolean, sanitizer?: StyleSanitizeFn | null) { - let flag = (sanitizer && sanitizer(prop)) ? StylingFlags.Sanitize : StylingFlags.None; + let flag = (sanitizer && sanitizer(prop, null, StyleSanitizeMode.ValidateProperty)) ? + StylingFlags.Sanitize : + StylingFlags.None; let initialIndex: number; if (entryIsClassBased) { diff --git a/packages/core/src/render3/styling_next/bindings.ts b/packages/core/src/render3/styling_next/bindings.ts index ab69e63da8..728a64f2d3 100644 --- a/packages/core/src/render3/styling_next/bindings.ts +++ b/packages/core/src/render3/styling_next/bindings.ts @@ -5,10 +5,11 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ +import {StyleSanitizeFn, StyleSanitizeMode} from '../../sanitization/style_sanitizer'; import {ProceduralRenderer3, RElement, Renderer3, RendererStyleFlags3, isProceduralRenderer} from '../interfaces/renderer'; -import {ApplyStylingFn, LStylingData, LStylingMap, StylingMapsSyncMode, SyncStylingMapsFn, TStylingContext, TStylingContextIndex} from './interfaces'; -import {allowStylingFlush, getBindingValue, getGuardMask, getProp, getPropValuesStartPosition, getValuesCount, hasValueChanged, isContextLocked, isStylingValueDefined, lockContext} from './util'; +import {ApplyStylingFn, LStylingData, LStylingMap, StylingMapsSyncMode, SyncStylingMapsFn, TStylingContext, TStylingContextIndex, TStylingContextPropConfigFlags} from './interfaces'; +import {allowStylingFlush, getBindingValue, getGuardMask, getProp, getPropValuesStartPosition, getValuesCount, hasValueChanged, isContextLocked, isSanitizationRequired, isStylingValueDefined, lockContext, setGuardMask} from './util'; /** @@ -49,7 +50,7 @@ let currentStyleIndex = STYLING_INDEX_START_VALUE; let currentClassIndex = STYLING_INDEX_START_VALUE; let stylesBitMask = 0; let classesBitMask = 0; -let deferredBindingQueue: (TStylingContext | number | string | null)[] = []; +let deferredBindingQueue: (TStylingContext | number | string | null | boolean)[] = []; /** * Visits a class-based binding and updates the new value (if changed). @@ -64,11 +65,11 @@ let deferredBindingQueue: (TStylingContext | number | string | null)[] = []; export function updateClassBinding( context: TStylingContext, data: LStylingData, prop: string | null, bindingIndex: number, value: boolean | string | null | undefined | LStylingMap, deferRegistration: boolean, - forceUpdate?: boolean): void { + forceUpdate: boolean): void { const isMapBased = !prop; const index = isMapBased ? STYLING_INDEX_FOR_MAP_BINDING : currentClassIndex++; const updated = updateBindingData( - context, data, index, prop, bindingIndex, value, deferRegistration, forceUpdate); + context, data, index, prop, bindingIndex, value, deferRegistration, forceUpdate, false); if (updated || forceUpdate) { classesBitMask |= 1 << index; } @@ -86,12 +87,16 @@ export function updateClassBinding( */ export function updateStyleBinding( context: TStylingContext, data: LStylingData, prop: string | null, bindingIndex: number, - value: String | string | number | null | undefined | LStylingMap, deferRegistration: boolean, - forceUpdate?: boolean): void { + value: String | string | number | null | undefined | LStylingMap, + sanitizer: StyleSanitizeFn | null, deferRegistration: boolean, forceUpdate: boolean): void { const isMapBased = !prop; const index = isMapBased ? STYLING_INDEX_FOR_MAP_BINDING : currentStyleIndex++; + const sanitizationRequired = isMapBased ? + true : + (sanitizer ? sanitizer(prop !, null, StyleSanitizeMode.ValidateProperty) : false); const updated = updateBindingData( - context, data, index, prop, bindingIndex, value, deferRegistration, forceUpdate); + context, data, index, prop, bindingIndex, value, deferRegistration, forceUpdate, + sanitizationRequired); if (updated || forceUpdate) { stylesBitMask |= 1 << index; } @@ -114,10 +119,10 @@ function updateBindingData( context: TStylingContext, data: LStylingData, counterIndex: number, prop: string | null, bindingIndex: number, value: string | String | number | boolean | null | undefined | LStylingMap, - deferRegistration?: boolean, forceUpdate?: boolean): boolean { + deferRegistration: boolean, forceUpdate: boolean, sanitizationRequired: boolean): boolean { if (!isContextLocked(context)) { if (deferRegistration) { - deferBindingRegistration(context, counterIndex, prop, bindingIndex); + deferBindingRegistration(context, counterIndex, prop, bindingIndex, sanitizationRequired); } else { deferredBindingQueue.length && flushDeferredBindings(); @@ -127,7 +132,7 @@ function updateBindingData( // update pass is executed (remember that all styling instructions // are run in the update phase, and, as a result, are no more // styling instructions that are run in the creation phase). - registerBinding(context, counterIndex, prop, bindingIndex); + registerBinding(context, counterIndex, prop, bindingIndex, sanitizationRequired); } } @@ -150,8 +155,9 @@ function updateBindingData( * after the inheritance chain exits. */ function deferBindingRegistration( - context: TStylingContext, counterIndex: number, prop: string | null, bindingIndex: number) { - deferredBindingQueue.splice(0, 0, context, counterIndex, prop, bindingIndex); + context: TStylingContext, counterIndex: number, prop: string | null, bindingIndex: number, + sanitizationRequired: boolean) { + deferredBindingQueue.unshift(context, counterIndex, prop, bindingIndex, sanitizationRequired); } /** @@ -165,7 +171,8 @@ function flushDeferredBindings() { const count = deferredBindingQueue[i++] as number; const prop = deferredBindingQueue[i++] as string; const bindingIndex = deferredBindingQueue[i++] as number | null; - registerBinding(context, count, prop, bindingIndex); + const sanitizationRequired = deferredBindingQueue[i++] as boolean; + registerBinding(context, count, prop, bindingIndex, sanitizationRequired); } deferredBindingQueue.length = 0; } @@ -208,7 +215,7 @@ function flushDeferredBindings() { */ export function registerBinding( context: TStylingContext, countId: number, prop: string | null, - bindingValue: number | null | string | boolean) { + bindingValue: number | null | string | boolean, sanitizationRequired?: boolean) { // prop-based bindings (e.g `
`) if (prop) { let found = false; @@ -220,7 +227,7 @@ export function registerBinding( if (found) { // all style/class bindings are sorted by property name if (prop < p) { - allocateNewContextEntry(context, i, prop); + allocateNewContextEntry(context, i, prop, sanitizationRequired); } addBindingIntoContext(context, false, i, bindingValue, countId); break; @@ -229,7 +236,7 @@ export function registerBinding( } if (!found) { - allocateNewContextEntry(context, context.length, prop); + allocateNewContextEntry(context, context.length, prop, sanitizationRequired); addBindingIntoContext(context, false, i, bindingValue, countId); } } else { @@ -241,15 +248,18 @@ export function registerBinding( } } -function allocateNewContextEntry(context: TStylingContext, index: number, prop: string) { +function allocateNewContextEntry( + context: TStylingContext, index: number, prop: string, sanitizationRequired?: boolean) { // 1,2: splice index locations - // 3: each entry gets a guard mask value that is used to check against updates + // 3: each entry gets a config value (guard mask + flags) // 4. each entry gets a size value (which is always one because there is always a default binding // value) // 5. the property that is getting allocated into the context // 6. the default binding value (usually `null`) - context.splice( - index, 0, DEFAULT_GUARD_MASK_VALUE, DEFAULT_SIZE_VALUE, prop, DEFAULT_BINDING_VALUE); + const config = sanitizationRequired ? TStylingContextPropConfigFlags.SanitizationRequired : + TStylingContextPropConfigFlags.Default; + context.splice(index, 0, config, DEFAULT_SIZE_VALUE, prop, DEFAULT_BINDING_VALUE); + setGuardMask(context, index, DEFAULT_GUARD_MASK_VALUE); } /** @@ -285,7 +295,12 @@ function addBindingIntoContext( if (typeof bindingValue === 'number') { context.splice(lastValueIndex, 0, bindingValue); (context[index + TStylingContextIndex.ValuesCountOffset] as number)++; - (context[index + TStylingContextIndex.GuardOffset] as number) |= 1 << countId; + + // now that a new binding index has been added to the property + // the guard mask bit value (at the `countId` position) needs + // to be included into the existing mask value. + const guardMask = getGuardMask(context, index) | (1 << countId); + setGuardMask(context, index, guardMask); } else if (typeof bindingValue === 'string' && context[lastValueIndex] == null) { context[lastValueIndex] = bindingValue; } @@ -294,37 +309,49 @@ function addBindingIntoContext( /** * Applies all class entries in the provided context to the provided element and resets * any counter and/or bitMask values associated with class bindings. + * + * @returns whether or not the classes were flushed to the element. */ export function applyClasses( renderer: Renderer3 | ProceduralRenderer3 | null, data: LStylingData, context: TStylingContext, - element: RElement, directiveIndex: number) { + element: RElement, directiveIndex: number): boolean { + let classesFlushed = false; if (allowStylingFlush(context, directiveIndex)) { const isFirstPass = !isContextLocked(context); isFirstPass && lockContext(context); if (classesBitMask) { - applyStyling(context, renderer, element, data, classesBitMask, setClass); + // there is no way to sanitize a class value therefore `sanitizer=null` + applyStyling(context, renderer, element, data, classesBitMask, setClass, null); classesBitMask = 0; + classesFlushed = true; } currentClassIndex = STYLING_INDEX_START_VALUE; } + return classesFlushed; } /** * Applies all style entries in the provided context to the provided element and resets * any counter and/or bitMask values associated with style bindings. + * + * @returns whether or not the styles were flushed to the element. */ export function applyStyles( renderer: Renderer3 | ProceduralRenderer3 | null, data: LStylingData, context: TStylingContext, - element: RElement, directiveIndex: number) { + element: RElement, directiveIndex: number, sanitizer: StyleSanitizeFn | null): boolean { + let stylesFlushed = false; if (allowStylingFlush(context, directiveIndex)) { const isFirstPass = !isContextLocked(context); isFirstPass && lockContext(context); if (stylesBitMask) { - applyStyling(context, renderer, element, data, stylesBitMask, setStyle); + applyStyling(context, renderer, element, data, stylesBitMask, setStyle, sanitizer); stylesBitMask = 0; + stylesFlushed = true; } currentStyleIndex = STYLING_INDEX_START_VALUE; + return true; } + return stylesFlushed; } /** @@ -355,7 +382,8 @@ export function applyStyles( */ export function applyStyling( context: TStylingContext, renderer: Renderer3 | ProceduralRenderer3 | null, element: RElement, - bindingData: LStylingData, bitMaskValue: number | boolean, applyStylingFn: ApplyStylingFn) { + bindingData: LStylingData, bitMaskValue: number | boolean, applyStylingFn: ApplyStylingFn, + sanitizer: StyleSanitizeFn | null) { deferredBindingQueue.length && flushDeferredBindings(); const bitMask = normalizeBitMaskValue(bitMaskValue); @@ -380,9 +408,12 @@ export function applyStyling( // value gets set for the styling binding for (let j = 0; j < valuesCountUpToDefault; j++) { const bindingIndex = getBindingValue(context, i, j) as number; - const valueToApply = bindingData[bindingIndex]; - if (isStylingValueDefined(valueToApply)) { - applyStylingFn(renderer, element, prop, valueToApply, bindingIndex); + const value = bindingData[bindingIndex]; + if (isStylingValueDefined(value)) { + const finalValue = sanitizer && isSanitizationRequired(context, i) ? + sanitizer(prop, value, StyleSanitizeMode.SanitizeOnly) : + value; + applyStylingFn(renderer, element, prop, finalValue, bindingIndex); valueApplied = true; break; } @@ -397,7 +428,8 @@ export function applyStyling( const mode = mapsMode | (valueApplied ? StylingMapsSyncMode.SkipTargetProp : StylingMapsSyncMode.ApplyTargetProp); const valueAppliedWithinMap = stylingMapsSyncFn( - context, renderer, element, bindingData, applyStylingFn, mode, prop, defaultValue); + context, renderer, element, bindingData, applyStylingFn, sanitizer, mode, prop, + defaultValue); valueApplied = valueApplied || valueAppliedWithinMap; } @@ -417,7 +449,7 @@ export function applyStyling( // values. For this reason, one more call to the sync function // needs to be issued at the end. if (stylingMapsSyncFn) { - stylingMapsSyncFn(context, renderer, element, bindingData, applyStylingFn, mapsMode); + stylingMapsSyncFn(context, renderer, element, bindingData, applyStylingFn, sanitizer, mapsMode); } } diff --git a/packages/core/src/render3/styling_next/instructions.ts b/packages/core/src/render3/styling_next/instructions.ts index 902a71a661..f565579a2b 100644 --- a/packages/core/src/render3/styling_next/instructions.ts +++ b/packages/core/src/render3/styling_next/instructions.ts @@ -5,20 +5,24 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ +import {Sanitizer} from '../../sanitization/security'; +import {StyleSanitizeFn} from '../../sanitization/style_sanitizer'; import {LContainer} from '../interfaces/container'; import {AttributeMarker, TAttributes, TNode, TNodeType} from '../interfaces/node'; import {RElement} from '../interfaces/renderer'; import {StylingContext as OldStylingContext, StylingIndex as OldStylingIndex} from '../interfaces/styling'; -import {BINDING_INDEX, HEADER_OFFSET, HOST, LView, RENDERER} from '../interfaces/view'; +import {BINDING_INDEX, HEADER_OFFSET, HOST, LView, RENDERER, SANITIZER} from '../interfaces/view'; import {getActiveDirectiveId, getActiveDirectiveSuperClassDepth, getActiveDirectiveSuperClassHeight, getLView, getSelectedIndex} from '../state'; import {NO_CHANGE} from '../tokens'; +import {renderStringify} from '../util/misc_utils'; import {getTNode, isStylingContext as isOldStylingContext} from '../util/view_utils'; import {applyClasses, applyStyles, registerBinding, updateClassBinding, updateStyleBinding} from './bindings'; import {TStylingContext} from './interfaces'; import {activeStylingMapFeature, normalizeIntoStylingMap} from './map_based_bindings'; +import {getCurrentStyleSanitizer, setCurrentStyleSanitizer} from './state'; import {attachStylingDebugObject} from './styling_debug'; -import {allocStylingContext, hasValueChanged, updateContextDirectiveIndex} from './util'; +import {allocTStylingContext, getCurrentOrLViewSanitizer, hasValueChanged, updateContextDirectiveIndex} from './util'; @@ -51,12 +55,32 @@ export function stylingInit() { updateLastDirectiveIndex(tNode, getActiveDirectiveStylingIndex()); } +/** + * Sets the current style sanitizer function which will then be used + * within all follow-up prop and map-based style binding instructions + * for the given element. + * + * Note that once styling has been applied to the element (i.e. once + * `select(n)` is executed or the hostBindings/template function exits) + * then the active `sanitizerFn` will be set to `null`. This means that + * once styling is applied to another element then a another call to + * `styleSanitizer` will need to be made. + * + * @param sanitizerFn The sanitization function that will be used to + * process style prop/value entries. + * + * @codeGenApi + */ +export function styleSanitizer(sanitizer: Sanitizer | StyleSanitizeFn | null): void { + setCurrentStyleSanitizer(sanitizer); +} + /** * Mirror implementation of the `styleProp()` instruction (found in `instructions/styling.ts`). */ export function styleProp( prop: string, value: string | number | String | null, suffix?: string | null): void { - _stylingProp(prop, value, false); + _stylingProp(prop, resolveStylePropValue(value, suffix), false); } /** @@ -79,10 +103,12 @@ function _stylingProp( if (isClassBased) { updateClassBinding( getClassesContext(tNode), lView, prop, bindingIndex, value as string | boolean | null, - defer); + defer, false); } else { + const sanitizer = getCurrentOrLViewSanitizer(lView); updateStyleBinding( - getStylesContext(tNode), lView, prop, bindingIndex, value as string | null, defer); + getStylesContext(tNode), lView, prop, bindingIndex, value as string | null, sanitizer, + defer, false); } } @@ -122,8 +148,10 @@ function _stylingMap(value: {[key: string]: any} | string | null, isClassBased: updateClassBinding( getClassesContext(tNode), lView, null, bindingIndex, lStylingMap, defer, valueHasChanged); } else { + const sanitizer = getCurrentOrLViewSanitizer(lView); updateStyleBinding( - getStylesContext(tNode), lView, null, bindingIndex, lStylingMap, defer, valueHasChanged); + getStylesContext(tNode), lView, null, bindingIndex, lStylingMap, sanitizer, defer, + valueHasChanged); } } } @@ -151,7 +179,11 @@ export function stylingApply() { const native = getNativeFromLView(index, lView); const directiveIndex = getActiveDirectiveStylingIndex(); applyClasses(renderer, lView, getClassesContext(tNode), native, directiveIndex); - applyStyles(renderer, lView, getStylesContext(tNode), native, directiveIndex); + + const sanitizer = getCurrentOrLViewSanitizer(lView); + applyStyles(renderer, lView, getStylesContext(tNode), native, directiveIndex, sanitizer); + + setCurrentStyleSanitizer(null); } /** @@ -202,10 +234,10 @@ export function registerInitialStylingIntoContext( mode = attr; } else if (mode == AttributeMarker.Classes) { classesContext = classesContext || getClassesContext(tNode); - registerBinding(classesContext, -1, attr as string, true); + registerBinding(classesContext, -1, attr as string, true, false); } else if (mode == AttributeMarker.Styles) { stylesContext = stylesContext || getStylesContext(tNode); - registerBinding(stylesContext, -1, attr as string, attrs[++i] as string); + registerBinding(stylesContext, -1, attr as string, attrs[++i] as string, false); } } } @@ -254,7 +286,7 @@ function getClassesContext(tNode: TNode): TStylingContext { function getContext(tNode: TNode, isClassBased: boolean) { let context = isClassBased ? tNode.newClasses : tNode.newStyles; if (!context) { - context = allocStylingContext(); + context = allocTStylingContext(); if (ngDevMode) { attachStylingDebugObject(context); } @@ -266,3 +298,22 @@ function getContext(tNode: TNode, isClassBased: boolean) { } return context; } + +function resolveStylePropValue( + value: string | number | String | null, suffix: string | null | undefined) { + let resolvedValue: string|null = null; + if (value !== null) { + if (suffix) { + // when a suffix is applied then it will bypass + // sanitization entirely (b/c a new string is created) + resolvedValue = renderStringify(value) + suffix; + } else { + // sanitization happens by dealing with a String value + // this means that the string value will be passed through + // into the style rendering later (which is where the value + // will be sanitized before it is applied) + resolvedValue = value as any as string; + } + } + return resolvedValue; +} diff --git a/packages/core/src/render3/styling_next/interfaces.ts b/packages/core/src/render3/styling_next/interfaces.ts index 3887474f23..3d99d9cbd5 100644 --- a/packages/core/src/render3/styling_next/interfaces.ts +++ b/packages/core/src/render3/styling_next/interfaces.ts @@ -5,6 +5,7 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ +import {StyleSanitizeFn} from '../../sanitization/style_sanitizer'; import {ProceduralRenderer3, RElement, Renderer3} from '../interfaces/renderer'; import {LView} from '../interfaces/view'; @@ -200,16 +201,16 @@ import {LView} from '../interfaces/view'; * In order to figure out which value to apply, the following * binding prioritization is adhered to: * - * 1. First template-level styling bindings are applied (if present). - * This includes things like `[style.width]` and `[class.active]`. + * 1. First template-level styling bindings are applied (if present). + * This includes things like `[style.width]` and `[class.active]`. * - * 2. Second are styling-level host bindings present in directives. - * (if there are sub/super directives present then the sub directives - * are applied first). + * 2. Second are styling-level host bindings present in directives. + * (if there are sub/super directives present then the sub directives + * are applied first). * - * 3. Third are styling-level host bindings present in components. - * (if there are sub/super components present then the sub directives - * are applied first). + * 3. Third are styling-level host bindings present in components. + * (if there are sub/super components present then the sub directives + * are applied first). * * This means that in the code above the styling binding present in the * template is applied first and, only if its falsy, then the directive @@ -225,7 +226,39 @@ import {LView} from '../interfaces/view'; * For the algorithm to apply styling values efficiently, the * styling map entries must be applied in sync (property by property) * with prop-based bindings. (The map-based algorithm is described - * more inside of the `render3/stlying_next/map_based_bindings.ts` file.) + * more inside of the `render3/styling_next/map_based_bindings.ts` file.) + * + * ## Sanitization + * Sanitization is used to prevent invalid style values from being applied to + * the element. + * + * It is enabled in two cases: + * + * 1. The `styleSanitizer(sanitizerFn)` instruction was called (just before any other + * styling instructions are run). + * + * 2. The component/directive `LView` instance has a sanitizer object attached to it + * (this happens when `renderComponent` is executed with a `sanitizer` value or + * if the ngModule contains a sanitizer provider attached to it). + * + * If and when sanitization is active then all property/value entries will be evaluated + * through the active sanitizer before they are applied to the element (or the styling + * debug handler). + * + * If a `Sanitizer` object is used (via the `LView[SANITIZER]` value) then that object + * will be used for every property. + * + * If a `StyleSanitizerFn` function is used (via the `styleSanitizer`) then it will be + * called in two ways: + * + * 1. property validation mode: this will be called early to mark whether a property + * should be sanitized or not at during the flushing stage. + * + * 2. value sanitization mode: this will be called during the flushing stage and will + * run the sanitizer function against the value before applying it to the element. + * + * If sanitization returns an empty value then that empty value will be applied + * to the element. */ export interface TStylingContext extends Array { /** Configuration data for the context */ @@ -289,12 +322,22 @@ export const enum TStylingContextIndex { // each tuple entry in the context // (mask, count, prop, ...bindings||default-value) - GuardOffset = 0, + ConfigAndGuardOffset = 0, ValuesCountOffset = 1, PropOffset = 2, BindingsStartOffset = 3, } +/** + * A series of flags used for each property entry within the `TStylingContext`. + */ +export const enum TStylingContextPropConfigFlags { + Default = 0b0, + SanitizationRequired = 0b1, + TotalBits = 1, + Mask = 0b1, +} + /** * A function used to apply or remove styling from an element for a given property. */ @@ -370,8 +413,8 @@ export const enum LStylingMapIndex { */ export interface SyncStylingMapsFn { (context: TStylingContext, renderer: Renderer3|ProceduralRenderer3|null, element: RElement, - data: LStylingData, applyStylingFn: ApplyStylingFn, mode: StylingMapsSyncMode, - targetProp?: string|null, defaultValue?: string|null): boolean; + data: LStylingData, applyStylingFn: ApplyStylingFn, sanitizer: StyleSanitizeFn|null, + mode: StylingMapsSyncMode, targetProp?: string|null, defaultValue?: string|null): boolean; } /** diff --git a/packages/core/src/render3/styling_next/map_based_bindings.ts b/packages/core/src/render3/styling_next/map_based_bindings.ts index b0c5573abd..d24f67eec4 100644 --- a/packages/core/src/render3/styling_next/map_based_bindings.ts +++ b/packages/core/src/render3/styling_next/map_based_bindings.ts @@ -5,6 +5,7 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ +import {StyleSanitizeFn, StyleSanitizeMode} from '../../sanitization/style_sanitizer'; import {ProceduralRenderer3, RElement, Renderer3} from '../interfaces/renderer'; import {setStylingMapsSyncFn} from './bindings'; @@ -103,8 +104,9 @@ import {getBindingValue, getValuesCount, isStylingValueDefined} from './util'; */ export const syncStylingMap: SyncStylingMapsFn = (context: TStylingContext, renderer: Renderer3 | ProceduralRenderer3 | null, element: RElement, - data: LStylingData, applyStylingFn: ApplyStylingFn, mode: StylingMapsSyncMode, - targetProp?: string | null, defaultValue?: string | null): boolean => { + data: LStylingData, applyStylingFn: ApplyStylingFn, sanitizer: StyleSanitizeFn | null, + mode: StylingMapsSyncMode, targetProp?: string | null, + defaultValue?: string | null): boolean => { let targetPropValueWasApplied = false; // once the map-based styling code is activate it is never deactivated. For this reason a @@ -125,8 +127,8 @@ export const syncStylingMap: SyncStylingMapsFn = if (runTheSyncAlgorithm) { targetPropValueWasApplied = innerSyncStylingMap( - context, renderer, element, data, applyStylingFn, mode, targetProp || null, 0, - defaultValue || null); + context, renderer, element, data, applyStylingFn, sanitizer, mode, targetProp || null, + 0, defaultValue || null); } if (loopUntilEnd) { @@ -148,8 +150,9 @@ export const syncStylingMap: SyncStylingMapsFn = */ function innerSyncStylingMap( context: TStylingContext, renderer: Renderer3 | ProceduralRenderer3 | null, element: RElement, - data: LStylingData, applyStylingFn: ApplyStylingFn, mode: StylingMapsSyncMode, - targetProp: string | null, currentMapIndex: number, defaultValue: string | null): boolean { + data: LStylingData, applyStylingFn: ApplyStylingFn, sanitizer: StyleSanitizeFn | null, + mode: StylingMapsSyncMode, targetProp: string | null, currentMapIndex: number, + defaultValue: string | null): boolean { let targetPropValueWasApplied = false; const totalMaps = getValuesCount(context, TStylingContextIndex.MapBindingsPosition); @@ -176,7 +179,7 @@ function innerSyncStylingMap( iteratedTooFar ? mode : resolveInnerMapMode(mode, valueIsDefined, isTargetPropMatched); const innerProp = iteratedTooFar ? targetProp : prop; let valueApplied = innerSyncStylingMap( - context, renderer, element, data, applyStylingFn, innerMode, innerProp, + context, renderer, element, data, applyStylingFn, sanitizer, innerMode, innerProp, currentMapIndex + 1, defaultValue); if (iteratedTooFar) { @@ -187,7 +190,10 @@ function innerSyncStylingMap( const useDefault = isTargetPropMatched && !valueIsDefined; const valueToApply = useDefault ? defaultValue : value; const bindingIndexToApply = useDefault ? bindingIndex : null; - applyStylingFn(renderer, element, prop, valueToApply, bindingIndexToApply); + const finalValue = sanitizer ? + sanitizer(prop, valueToApply, StyleSanitizeMode.ValidateAndSanitize) : + valueToApply; + applyStylingFn(renderer, element, prop, finalValue, bindingIndexToApply); valueApplied = true; } diff --git a/packages/core/src/render3/styling_next/state.ts b/packages/core/src/render3/styling_next/state.ts index d4c5d281e2..50e42c6807 100644 --- a/packages/core/src/render3/styling_next/state.ts +++ b/packages/core/src/render3/styling_next/state.ts @@ -5,6 +5,21 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ +import {Sanitizer} from '../../sanitization/security'; +import {StyleSanitizeFn} from '../../sanitization/style_sanitizer'; + +/** + * -------- + * + * This file contains temporary code to incorporate the new styling refactor + * code to work alongside the existing instruction set. + * + * This file will be removed once `select(n)` is fully functional (once + * it is able to evaluate host bindings in sync element-by-element + * with template code). + * + * -------- + */ /** * A temporary enum of states that inform the core whether or not @@ -35,3 +50,12 @@ export function runtimeIsNewStylingInUse() { export function runtimeAllowOldStyling() { return _stylingMode < RuntimeStylingMode.UseNew; } + +let _currentSanitizer: Sanitizer|StyleSanitizeFn|null; +export function setCurrentStyleSanitizer(sanitizer: Sanitizer | StyleSanitizeFn | null) { + _currentSanitizer = sanitizer; +} + +export function getCurrentStyleSanitizer() { + return _currentSanitizer; +} diff --git a/packages/core/src/render3/styling_next/styling_debug.ts b/packages/core/src/render3/styling_next/styling_debug.ts index ac54022e93..2f78274321 100644 --- a/packages/core/src/render3/styling_next/styling_debug.ts +++ b/packages/core/src/render3/styling_next/styling_debug.ts @@ -5,13 +5,19 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ +import {Sanitizer} from '../../sanitization/security'; +import {StyleSanitizeFn} from '../../sanitization/style_sanitizer'; import {RElement} from '../interfaces/renderer'; +import {LView, SANITIZER} from '../interfaces/view'; import {attachDebugObject} from '../util/debug_utils'; import {applyStyling} from './bindings'; import {ApplyStylingFn, LStylingData, TStylingContext, TStylingContextIndex} from './interfaces'; import {activeStylingMapFeature} from './map_based_bindings'; -import {getDefaultValue, getGuardMask, getProp, getValuesCount, isContextLocked, isMapBased} from './util'; +import {getCurrentStyleSanitizer} from './state'; +import {getCurrentOrLViewSanitizer, getDefaultValue, getGuardMask, getProp, getValuesCount, isContextLocked, isMapBased, isSanitizationRequired} from './util'; + + /** * -------- @@ -59,6 +65,11 @@ export interface DebugStyling { * runtime values. */ values: {[key: string]: string | number | null | boolean}; + + /** + * Overrides the sanitizer used to process styles. + */ + overrideSanitizer(sanitizer: StyleSanitizeFn|null): void; } /** @@ -77,6 +88,11 @@ export interface TStylingTupleSummary { */ guardMask: number; + /** + * Whether or not the entry requires sanitization + */ + sanitizationRequired: boolean; + /** * The default value that will be applied if any bindings are falsy. */ @@ -127,6 +143,7 @@ class TStylingContextDebug { const prop = getProp(context, i); const guardMask = getGuardMask(context, i); const defaultValue = getDefaultValue(context, i); + const sanitizationRequired = isSanitizationRequired(context, i); const bindingsStartPosition = i + TStylingContextIndex.BindingsStartOffset; const sources: (number | string | null)[] = []; @@ -134,7 +151,7 @@ class TStylingContextDebug { sources.push(context[bindingsStartPosition + j] as number | string | null); } - entries[prop] = {prop, guardMask, valuesCount, defaultValue, sources}; + entries[prop] = {prop, guardMask, sanitizationRequired, valuesCount, defaultValue, sources}; } i += TStylingContextIndex.BindingsStartOffset + valuesCount; @@ -150,7 +167,16 @@ class TStylingContextDebug { * application has `ngDevMode` activated. */ export class NodeStylingDebug implements DebugStyling { - constructor(public context: TStylingContext, private _data: LStylingData) {} + private _sanitizer: StyleSanitizeFn|null = null; + + constructor( + public context: TStylingContext, private _data: LStylingData, + private _isClassBased?: boolean) {} + + /** + * Overrides the sanitizer used to process styles. + */ + overrideSanitizer(sanitizer: StyleSanitizeFn|null) { this._sanitizer = sanitizer; } /** * Returns a detailed summary of each styling entry in the context and @@ -190,6 +216,8 @@ export class NodeStylingDebug implements DebugStyling { fn(prop, value, bindingIndex || null); }; - applyStyling(this.context, null, mockElement, this._data, true, mapFn); + const sanitizer = this._isClassBased ? null : (this._sanitizer || + getCurrentOrLViewSanitizer(this._data as LView)); + applyStyling(this.context, null, mockElement, this._data, true, mapFn, sanitizer); } } diff --git a/packages/core/src/render3/styling_next/util.ts b/packages/core/src/render3/styling_next/util.ts index 3a21ba5fbb..494738403a 100644 --- a/packages/core/src/render3/styling_next/util.ts +++ b/packages/core/src/render3/styling_next/util.ts @@ -5,10 +5,14 @@ * Use of this source code is governed by an MIT-style license that can be * found in the LICENSE file at https://angular.io/license */ +import {Sanitizer, SecurityContext} from '../../sanitization/security'; +import {StyleSanitizeFn, StyleSanitizeMode} from '../../sanitization/style_sanitizer'; import {StylingContext} from '../interfaces/styling'; +import {LView, SANITIZER} from '../interfaces/view'; import {getProp as getOldProp, getSinglePropIndexValue as getOldSinglePropIndexValue} from '../styling/class_and_style_bindings'; -import {LStylingMap, LStylingMapIndex, TStylingConfigFlags, TStylingContext, TStylingContextIndex} from './interfaces'; +import {LStylingMap, LStylingMapIndex, TStylingConfigFlags, TStylingContext, TStylingContextIndex, TStylingContextPropConfigFlags} from './interfaces'; +import {getCurrentStyleSanitizer, setCurrentStyleSanitizer} from './state'; const MAP_BASED_ENTRY_PROP_NAME = '--MAP--'; @@ -18,8 +22,14 @@ const MAP_BASED_ENTRY_PROP_NAME = '--MAP--'; * This function will also pre-fill the context with data * for map-based bindings. */ -export function allocStylingContext(): TStylingContext { - return [TStylingConfigFlags.Initial, 0, 0, 0, MAP_BASED_ENTRY_PROP_NAME]; +export function allocTStylingContext(): TStylingContext { + // because map-based bindings deal with a dynamic set of values, there + // is no way to know ahead of time whether or not sanitization is required. + // For this reason the configuration will always mark sanitization as active + // (this means that when map-based values are applied then sanitization will + // be checked against each property). + const mapBasedConfig = TStylingContextPropConfigFlags.SanitizationRequired; + return [TStylingConfigFlags.Initial, 0, mapBasedConfig, 0, MAP_BASED_ENTRY_PROP_NAME]; } /** @@ -53,8 +63,24 @@ export function getProp(context: TStylingContext, index: number) { return context[index + TStylingContextIndex.PropOffset] as string; } +function getPropConfig(context: TStylingContext, index: number): number { + return (context[index + TStylingContextIndex.ConfigAndGuardOffset] as number) & + TStylingContextPropConfigFlags.Mask; +} + +export function isSanitizationRequired(context: TStylingContext, index: number) { + return (getPropConfig(context, index) & TStylingContextPropConfigFlags.SanitizationRequired) > 0; +} + export function getGuardMask(context: TStylingContext, index: number) { - return context[index + TStylingContextIndex.GuardOffset] as number; + const configGuardValue = context[index + TStylingContextIndex.ConfigAndGuardOffset] as number; + return configGuardValue >> TStylingContextPropConfigFlags.TotalBits; +} + +export function setGuardMask(context: TStylingContext, index: number, maskValue: number) { + const config = getPropConfig(context, index); + const guardMask = maskValue << TStylingContextPropConfigFlags.TotalBits; + context[index + TStylingContextIndex.ConfigAndGuardOffset] = config | guardMask; } export function getValuesCount(context: TStylingContext, index: number) { @@ -115,3 +141,36 @@ export function isStylingValueDefined(value: any) { // set a value to an empty string to remove it. return value != null && value !== ''; } + +/** + * Returns the current style sanitizer function for the given view. + * + * The default style sanitizer (which lives inside of `LView`) will + * be returned depending on whether the `styleSanitizer` instruction + * was called or not prior to any styling instructions running. + */ +export function getCurrentOrLViewSanitizer(lView: LView): StyleSanitizeFn|null { + const sanitizer: StyleSanitizeFn|null = (getCurrentStyleSanitizer() || lView[SANITIZER]) as any; + if (sanitizer && typeof sanitizer !== 'function') { + setCurrentStyleSanitizer(sanitizer); + return sanitizeUsingSanitizerObject; + } + return sanitizer; +} + +/** + * Style sanitization function that internally uses a `Sanitizer` instance to handle style + * sanitization. + */ +const sanitizeUsingSanitizerObject: StyleSanitizeFn = + (prop: string, value: string, mode: StyleSanitizeMode) => { + const sanitizer = getCurrentStyleSanitizer() as Sanitizer; + if (sanitizer) { + if (mode & StyleSanitizeMode.SanitizeOnly) { + return sanitizer.sanitize(SecurityContext.STYLE, value); + } else { + return true; + } + } + return value; + }; diff --git a/packages/core/src/sanitization/sanitization.ts b/packages/core/src/sanitization/sanitization.ts index 1d05760c6d..92953d694a 100644 --- a/packages/core/src/sanitization/sanitization.ts +++ b/packages/core/src/sanitization/sanitization.ts @@ -13,7 +13,7 @@ import {renderStringify} from '../render3/util/misc_utils'; import {BypassType, allowSanitizationBypass} from './bypass'; import {_sanitizeHtml as _sanitizeHtml} from './html_sanitizer'; import {Sanitizer, SecurityContext} from './security'; -import {StyleSanitizeFn, _sanitizeStyle as _sanitizeStyle} from './style_sanitizer'; +import {StyleSanitizeFn, StyleSanitizeMode, _sanitizeStyle as _sanitizeStyle} from './style_sanitizer'; import {_sanitizeUrl as _sanitizeUrl} from './url_sanitizer'; @@ -183,15 +183,22 @@ export function ɵɵsanitizeUrlOrResourceUrl(unsafeUrl: any, tag: string, prop: * * @publicApi */ -export const ɵɵdefaultStyleSanitizer = (function(prop: string, value?: string): string | boolean { - if (value === undefined) { - return prop === 'background-image' || prop === 'background' || prop === 'border-image' || - prop === 'filter' || prop === 'list-style' || prop === 'list-style-image' || - prop === 'clip-path'; - } +export const ɵɵdefaultStyleSanitizer = + (function(prop: string, value: string|null, mode?: StyleSanitizeMode): string | boolean | null { + mode = mode || StyleSanitizeMode.ValidateAndSanitize; + let doSanitizeValue = true; + if (mode & StyleSanitizeMode.ValidateProperty) { + doSanitizeValue = prop === 'background-image' || prop === 'background' || + prop === 'border-image' || prop === 'filter' || prop === 'list-style' || + prop === 'list-style-image' || prop === 'clip-path'; + } - return ɵɵsanitizeStyle(value); -} as StyleSanitizeFn); + if (mode & StyleSanitizeMode.SanitizeOnly) { + return doSanitizeValue ? ɵɵsanitizeStyle(value) : value; + } else { + return doSanitizeValue; + } + } as StyleSanitizeFn); export function validateAgainstEventProperties(name: string) { if (name.toLowerCase().startsWith('on')) { diff --git a/packages/core/src/sanitization/style_sanitizer.ts b/packages/core/src/sanitization/style_sanitizer.ts index 62a6ecbe59..9303a167a1 100644 --- a/packages/core/src/sanitization/style_sanitizer.ts +++ b/packages/core/src/sanitization/style_sanitizer.ts @@ -103,6 +103,30 @@ export function _sanitizeStyle(value: string): string { } +/** + * A series of flags to instruct a style sanitizer to either validate + * or sanitize a value. + * + * Because sanitization is dependent on the style property (i.e. style + * sanitization for `width` is much different than for `background-image`) + * the sanitization function (e.g. `StyleSanitizerFn`) needs to check a + * property value first before it actually sanitizes any values. + * + * This enum exist to allow a style sanitization function to either only + * do validation (check the property to see whether a value will be + * sanitized or not) or to sanitize the value (or both). + * + * @publicApi + */ +export const enum StyleSanitizeMode { + /** Just check to see if the property is required to be sanitized or not */ + ValidateProperty = 0b01, + /** Skip checking the property; just sanitize the value */ + SanitizeOnly = 0b10, + /** Check the property and (if true) then sanitize the value */ + ValidateAndSanitize = 0b11, +} + /** * Used to intercept and sanitize style values before they are written to the renderer. * @@ -111,9 +135,5 @@ export function _sanitizeStyle(value: string): string { * If a value is provided then the sanitized version of that will be returned. */ export interface StyleSanitizeFn { - /** This mode is designed to instruct whether the property will be used for sanitization - * at a later point */ - (prop: string): boolean; - /** This mode is designed to sanitize the provided value */ - (prop: string, value: string): string; + (prop: string, value: string|null, mode?: StyleSanitizeMode): any; } diff --git a/packages/core/test/acceptance/styling_next_spec.ts b/packages/core/test/acceptance/styling_next_spec.ts index 73bd958a35..8186a241e8 100644 --- a/packages/core/test/acceptance/styling_next_spec.ts +++ b/packages/core/test/acceptance/styling_next_spec.ts @@ -7,11 +7,15 @@ */ import {CompilerStylingMode, compilerSetStylingMode} from '@angular/compiler/src/render3/view/styling_state'; import {Component, Directive, HostBinding, Input, ViewChild} from '@angular/core'; +import {SecurityContext} from '@angular/core/src/core'; +import {getLContext} from '@angular/core/src/render3/context_discovery'; import {DebugNode, LViewDebug, toDebug} from '@angular/core/src/render3/debug'; +import {SANITIZER} from '@angular/core/src/render3/interfaces/view'; import {RuntimeStylingMode, runtimeSetStylingMode} from '@angular/core/src/render3/styling_next/state'; import {loadLContextFromNode} from '@angular/core/src/render3/util/discovery_utils'; import {ngDevModeResetPerfCounters as resetStylingCounters} from '@angular/core/src/util/ng_dev_mode'; import {TestBed} from '@angular/core/testing'; +import {DomSanitizer, SafeStyle} from '@angular/platform-browser'; import {expect} from '@angular/platform-browser/testing/src/matchers'; import {onlyInIvy} from '@angular/private/testing'; @@ -574,6 +578,161 @@ describe('new styling integration', () => { assertStyle(element, 'color', 'blue'); assertStyle(element, 'opacity', ''); }); + + onlyInIvy('only ivy has style/class bindings debugging support') + .it('should sanitize style values before writing them', () => { + @Component({ + template: ` +
+ ` + }) + class Cmp { + widthExp = ''; + bgImageExp = ''; + styleMapExp: any = {}; + } + + TestBed.configureTestingModule({declarations: [Cmp]}); + const fixture = TestBed.createComponent(Cmp); + const comp = fixture.componentInstance; + fixture.detectChanges(); + + const element = fixture.nativeElement.querySelector('div'); + const node = getDebugNode(element) !; + const styles = node.styles !; + + const lastSanitizedProps: any[] = []; + styles.overrideSanitizer((prop, value) => { + lastSanitizedProps.push(prop); + return value; + }); + + comp.bgImageExp = '123'; + fixture.detectChanges(); + + expect(styles.values).toEqual({ + 'background-image': '123', + 'width': null, + }); + + expect(lastSanitizedProps).toEqual(['background-image']); + lastSanitizedProps.length = 0; + + comp.styleMapExp = {'clip-path': '456'}; + fixture.detectChanges(); + + expect(styles.values).toEqual({ + 'background-image': '123', + 'clip-path': '456', + 'width': null, + }); + + expect(lastSanitizedProps).toEqual(['background-image', 'clip-path']); + lastSanitizedProps.length = 0; + + comp.widthExp = '789px'; + fixture.detectChanges(); + + expect(styles.values).toEqual({ + 'background-image': '123', + 'clip-path': '456', + 'width': '789px', + }); + + expect(lastSanitizedProps).toEqual(['background-image', 'clip-path']); + lastSanitizedProps.length = 0; + }); + + onlyInIvy('only ivy has style/class bindings debugging support') + .it('should apply a unit to a style before writing it', () => { + @Component({ + template: ` +
+ ` + }) + class Cmp { + widthExp: string|number|null = ''; + heightExp: string|number|null = ''; + } + + TestBed.configureTestingModule({declarations: [Cmp]}); + const fixture = TestBed.createComponent(Cmp); + const comp = fixture.componentInstance; + fixture.detectChanges(); + + const element = fixture.nativeElement.querySelector('div'); + const node = getDebugNode(element) !; + const styles = node.styles !; + + comp.widthExp = '200'; + comp.heightExp = 10; + fixture.detectChanges(); + + expect(styles.values).toEqual({ + 'width': '200px', + 'height': '10em', + }); + + comp.widthExp = 0; + comp.heightExp = null; + fixture.detectChanges(); + + expect(styles.values).toEqual({ + 'width': '0px', + 'height': null, + }); + }); + + onlyInIvy('only ivy has style/class bindings debugging support') + .it('should pick up and use the sanitizer present in the lView', () => { + @Component({ + template: ` +
+ ` + }) + class Cmp { + w = '100px'; + } + + TestBed.configureTestingModule({declarations: [Cmp]}); + const fixture = TestBed.createComponent(Cmp); + const comp = fixture.componentInstance; + fixture.detectChanges(); + + const element = fixture.nativeElement.querySelector('div'); + const lView = getLContext(element) !.lView; + lView[SANITIZER] = new MockSanitizer(value => { return `${value}-safe`; }); + + comp.w = '200px'; + fixture.detectChanges(); + + const node = getDebugNode(element) !; + const styles = node.styles !; + expect(styles.values['width']).toEqual('200px-safe'); + }); + + it('should be able to bind a SafeValue to clip-path', () => { + @Component({template: '
'}) + class Cmp { + path !: SafeStyle; + } + + TestBed.configureTestingModule({declarations: [Cmp]}); + const fixture = TestBed.createComponent(Cmp); + const sanitizer: DomSanitizer = TestBed.get(DomSanitizer); + + fixture.componentInstance.path = sanitizer.bypassSecurityTrustStyle('url("#test")'); + fixture.detectChanges(); + + const html = fixture.nativeElement.innerHTML; + + // Note that check the raw HTML, because (at the time of writing) the Node-based renderer + // that we use to run tests doesn't support `clip-path` in `CSSStyleDeclaration`. + expect(html).toMatch(/style=["|']clip-path:\s*url\(.*#test.*\)/); + }); }); function assertStyleCounters(countForSet: number, countForRemove: number) { @@ -597,3 +756,8 @@ function getDebugNode(element: Node): DebugNode|null { } return null; } + +class MockSanitizer { + constructor(private _interceptorFn: ((value: any) => any)) {} + sanitize(context: SecurityContext, value: any): string|null { return this._interceptorFn(value); } +} diff --git a/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json b/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json index 0488845316..9fa46ba84c 100644 --- a/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json +++ b/packages/core/test/bundling/cyclic_import/bundle.golden_symbols.json @@ -192,7 +192,7 @@ "name": "allocStylingContext" }, { - "name": "allocStylingContext" + "name": "allocTStylingContext" }, { "name": "allocateNewContextEntry" @@ -362,6 +362,9 @@ { "name": "getGlobal" }, + { + "name": "getGuardMask" + }, { "name": "getHighestElementOrICUContainer" }, @@ -434,6 +437,9 @@ { "name": "getProp" }, + { + "name": "getPropConfig" + }, { "name": "getPropValuesStartPosition" }, @@ -686,6 +692,9 @@ { "name": "setCurrentQueryIndex" }, + { + "name": "setGuardMask" + }, { "name": "setHostBindings" }, diff --git a/packages/core/test/bundling/todo/bundle.golden_symbols.json b/packages/core/test/bundling/todo/bundle.golden_symbols.json index 216119ff48..494d759a4f 100644 --- a/packages/core/test/bundling/todo/bundle.golden_symbols.json +++ b/packages/core/test/bundling/todo/bundle.golden_symbols.json @@ -230,6 +230,9 @@ { "name": "SWITCH_VIEW_CONTAINER_REF_FACTORY" }, + { + "name": "SecurityContext" + }, { "name": "SkipSelf" }, @@ -444,7 +447,7 @@ "name": "allocStylingContext" }, { - "name": "allocStylingContext" + "name": "allocTStylingContext" }, { "name": "allocateNewContextEntry" @@ -785,6 +788,12 @@ { "name": "getContextLView" }, + { + "name": "getCurrentOrLViewSanitizer" + }, + { + "name": "getCurrentStyleSanitizer" + }, { "name": "getDebugContext" }, @@ -935,6 +944,9 @@ { "name": "getProp" }, + { + "name": "getPropConfig" + }, { "name": "getPropValuesStartPosition" }, @@ -1151,6 +1163,9 @@ { "name": "isRootView" }, + { + "name": "isSanitizationRequired" + }, { "name": "isStylingContext" }, @@ -1343,6 +1358,9 @@ { "name": "runtimeIsNewStylingInUse" }, + { + "name": "sanitizeUsingSanitizerObject" + }, { "name": "saveNameToExportMap" }, @@ -1391,12 +1409,18 @@ { "name": "setCurrentQueryIndex" }, + { + "name": "setCurrentStyleSanitizer" + }, { "name": "setDirty" }, { "name": "setFlag" }, + { + "name": "setGuardMask" + }, { "name": "setHostBindings" }, diff --git a/packages/core/test/render3/styling/class_and_style_bindings_spec.ts b/packages/core/test/render3/styling/class_and_style_bindings_spec.ts index 3dfecb239f..4f6120edd2 100644 --- a/packages/core/test/render3/styling/class_and_style_bindings_spec.ts +++ b/packages/core/test/render3/styling/class_and_style_bindings_spec.ts @@ -23,7 +23,7 @@ import {registerHostDirective} from '../../../src/render3/styling/host_instructi import {BoundPlayerFactory, bindPlayerFactory} from '../../../src/render3/styling/player_factory'; import {allocStylingContext, createEmptyStylingContext} from '../../../src/render3/styling/util'; import {ɵɵdefaultStyleSanitizer} from '../../../src/sanitization/sanitization'; -import {StyleSanitizeFn} from '../../../src/sanitization/style_sanitizer'; +import {StyleSanitizeFn, StyleSanitizeMode} from '../../../src/sanitization/style_sanitizer'; import {ComponentFixture, renderToHtml} from '../render_util'; import {MockPlayer} from './mock_player'; @@ -2991,11 +2991,16 @@ describe('style and class based bindings', () => { }); it('should sanitize styles before they are passed into the player', () => { - const sanitizer = (function(prop: string, value?: string): string | boolean { - if (value === undefined) { - return prop === 'width' || prop === 'height'; + const sanitizer = (function(prop: string, value: string, mode: StyleSanitizeMode): any { + let allow = true; + if (mode & StyleSanitizeMode.ValidateProperty) { + allow = prop === 'width' || prop === 'height'; + } + + if (mode & StyleSanitizeMode.SanitizeOnly) { + return allow ? `${value}-safe!` : value; } else { - return `${value}-safe!`; + return allow; } }) as StyleSanitizeFn; diff --git a/packages/core/test/render3/styling_next/styling_context_spec.ts b/packages/core/test/render3/styling_next/styling_context_spec.ts index f3788528c1..e84137ba6b 100644 --- a/packages/core/test/render3/styling_next/styling_context_spec.ts +++ b/packages/core/test/render3/styling_next/styling_context_spec.ts @@ -7,8 +7,7 @@ */ import {DEFAULT_GUARD_MASK_VALUE, registerBinding} from '@angular/core/src/render3/styling_next/bindings'; import {attachStylingDebugObject} from '@angular/core/src/render3/styling_next/styling_debug'; - -import {allocStylingContext} from '../../../src/render3/styling_next/util'; +import {allocTStylingContext} from '../../../src/render3/styling_next/util'; describe('styling context', () => { it('should register a series of entries into the context', () => { @@ -20,6 +19,7 @@ describe('styling context', () => { expect(debug.entries['width']).toEqual({ prop: 'width', valuesCount: 1, + sanitizationRequired: false, guardMask: buildGuardMask(), defaultValue: '100px', sources: ['100px'], @@ -28,6 +28,7 @@ describe('styling context', () => { registerBinding(context, 2, 'width', 20); expect(debug.entries['width']).toEqual({ prop: 'width', + sanitizationRequired: false, valuesCount: 2, guardMask: buildGuardMask(2), defaultValue: '100px', @@ -39,6 +40,7 @@ describe('styling context', () => { expect(debug.entries['height']).toEqual({ prop: 'height', valuesCount: 3, + sanitizationRequired: false, guardMask: buildGuardMask(3, 4), defaultValue: null, sources: [10, 15, null], @@ -50,9 +52,11 @@ describe('styling context', () => { const context = debug.context; registerBinding(context, 1, 'width', null); + const x = debug.entries['width']; expect(debug.entries['width']).toEqual({ prop: 'width', valuesCount: 1, + sanitizationRequired: false, guardMask: buildGuardMask(), defaultValue: null, sources: [null] @@ -62,6 +66,7 @@ describe('styling context', () => { expect(debug.entries['width']).toEqual({ prop: 'width', valuesCount: 1, + sanitizationRequired: false, guardMask: buildGuardMask(), defaultValue: '100px', sources: ['100px'] @@ -71,6 +76,7 @@ describe('styling context', () => { expect(debug.entries['width']).toEqual({ prop: 'width', valuesCount: 1, + sanitizationRequired: false, guardMask: buildGuardMask(), defaultValue: '100px', sources: ['100px'] @@ -79,7 +85,7 @@ describe('styling context', () => { }); function makeContextWithDebug() { - const ctx = allocStylingContext(); + const ctx = allocTStylingContext(); return attachStylingDebugObject(ctx); } diff --git a/packages/core/test/render3/styling_next/styling_debug_spec.ts b/packages/core/test/render3/styling_next/styling_debug_spec.ts index 499685974e..138def5d09 100644 --- a/packages/core/test/render3/styling_next/styling_debug_spec.ts +++ b/packages/core/test/render3/styling_next/styling_debug_spec.ts @@ -7,7 +7,7 @@ */ import {registerBinding} from '@angular/core/src/render3/styling_next/bindings'; import {NodeStylingDebug, attachStylingDebugObject} from '@angular/core/src/render3/styling_next/styling_debug'; -import {allocStylingContext} from '@angular/core/src/render3/styling_next/util'; +import {allocTStylingContext} from '@angular/core/src/render3/styling_next/util'; describe('styling debugging tools', () => { describe('NodeStylingDebug', () => { @@ -64,6 +64,6 @@ describe('styling debugging tools', () => { }); function makeContextWithDebug() { - const ctx = allocStylingContext(); + const ctx = allocTStylingContext(); return attachStylingDebugObject(ctx); }