From 3af103aa61d2963fe8c8b83cc23a2102ce650b9d Mon Sep 17 00:00:00 2001 From: Misko Hevery Date: Wed, 19 Feb 2020 13:15:36 -0800 Subject: [PATCH] fix(core): support sanitizer value in the [style] bindings (#35564) When binding to `[style]` we correctly sanitized/unwrapped properties but we did not do it for the object itself. ``` @HostBinding("style") style: SafeStyle = this.sanitizer.bypassSecurityTrustStyle( "background: red; color: white; display: block;" ); ``` Above code would fail since the `[style]` would not unwrap the `SafeValue` and would treat it as object resulting in incorrect behavior. Fix #35476 (FW-1875) PR Close #35564 --- .../core/src/render3/instructions/styling.ts | 24 +++++---- packages/core/test/acceptance/styling_spec.ts | 51 ++++++++++++++++++- 2 files changed, 63 insertions(+), 12 deletions(-) diff --git a/packages/core/src/render3/instructions/styling.ts b/packages/core/src/render3/instructions/styling.ts index c7c58041e3..6370ebd952 100644 --- a/packages/core/src/render3/instructions/styling.ts +++ b/packages/core/src/render3/instructions/styling.ts @@ -625,23 +625,25 @@ export function getHostDirectiveDef(tData: TData): DirectiveDef|null { export function toStylingKeyValueArray( keyValueArraySet: (keyValueArray: KeyValueArray, key: string, value: any) => void, stringParser: (styleKeyValueArray: KeyValueArray, text: string) => void, - value: string|string[]|{[key: string]: any}|null|undefined): KeyValueArray { + value: string|string[]|{[key: string]: any}|SafeValue|null|undefined): KeyValueArray { if (value == null /*|| value === undefined */ || value === '') return EMPTY_ARRAY as any; const styleKeyValueArray: KeyValueArray = [] as any; - if (Array.isArray(value)) { - for (let i = 0; i < value.length; i++) { - keyValueArraySet(styleKeyValueArray, value[i], true); + const unwrappedValue = unwrapSafeValue(value) as string | string[] | {[key: string]: any}; + if (Array.isArray(unwrappedValue)) { + for (let i = 0; i < unwrappedValue.length; i++) { + keyValueArraySet(styleKeyValueArray, unwrappedValue[i], true); } - } else if (typeof value === 'object') { - for (const key in value) { - if (value.hasOwnProperty(key)) { - keyValueArraySet(styleKeyValueArray, key, value[key]); + } else if (typeof unwrappedValue === 'object') { + for (const key in unwrappedValue) { + if (unwrappedValue.hasOwnProperty(key)) { + keyValueArraySet(styleKeyValueArray, key, unwrappedValue[key]); } } - } else if (typeof value === 'string') { - stringParser(styleKeyValueArray, value); + } else if (typeof unwrappedValue === 'string') { + stringParser(styleKeyValueArray, unwrappedValue); } else { - ngDevMode && throwError('Unsupported styling type ' + typeof value + ': ' + value); + ngDevMode && + throwError('Unsupported styling type ' + typeof unwrappedValue + ': ' + unwrappedValue); } return styleKeyValueArray; } diff --git a/packages/core/test/acceptance/styling_spec.ts b/packages/core/test/acceptance/styling_spec.ts index be35c567ef..63278e8bcf 100644 --- a/packages/core/test/acceptance/styling_spec.ts +++ b/packages/core/test/acceptance/styling_spec.ts @@ -3384,6 +3384,55 @@ describe('styling', () => { }); describe('regression', () => { + it('should support sanitizer value in the [style] bindings', () => { + if (!ivyEnabled && !supportsWritingStringsToStyleProperty()) { + // VE does not treat `[style]` as anything special, instead it simply writes to the + // `style` property on the element like so `element.style=value`. This seems to work fine + // every where except ie10, where it throws an error and as a consequence this test fails in + // VE on ie10. + return; + } + @Component({template: `
`}) + class HostBindingTestComponent { + style: SafeStyle = this.sanitizer.bypassSecurityTrustStyle('color: white; display: block;'); + constructor(private sanitizer: DomSanitizer) {} + } + TestBed.configureTestingModule({declarations: [HostBindingTestComponent]}); + const fixture = TestBed.createComponent(HostBindingTestComponent); + fixture.detectChanges(); + const div: HTMLElement = fixture.nativeElement.querySelector('div'); + expectStyle(div).toEqual({color: 'white', display: 'block'}); + }); + + /** + * Tests to see if the current browser supports non standard way of writing into styles. + * + * This is not the correct way to write to style and is not supported in ie10. + * ``` + * div.style = 'color: white'; + * ``` + * + * This is the correct way to write to styles: + * ``` + * div.style.cssText = 'color: white'; + * ``` + * + * Even though writing to `div.style` is not officially supported, it works in all + * browsers except ie10. + * + * This function detects this condition and allows us to skip the test. + */ + function supportsWritingStringsToStyleProperty() { + const div = document.createElement('div'); + const CSS = 'color: white;'; + try { + (div as any).style = CSS; + } catch (e) { + return false; + } + return div.style.cssText === CSS; + } + onlyInIvy('styling priority resolution is Ivy only feature.') .it('should allow lookahead binding on second pass #35118', () => { @Component({ @@ -3445,4 +3494,4 @@ function expectStyle(element: HTMLElement) { function expectClass(element: HTMLElement) { return expect(getElementClasses(element)); -} \ No newline at end of file +}