From 178fb79b5cafe36c993bb1f9f9a63e8bac162c66 Mon Sep 17 00:00:00 2001 From: Tobias Bosch Date: Mon, 24 Oct 2016 09:58:52 -0700 Subject: [PATCH] refactor(compiler): move host properties into DirectiveWrapper Part of #11683 --- modules/@angular/compiler-cli/src/codegen.ts | 4 +- modules/@angular/compiler/index.ts | 2 +- .../src/compiler_util/identifier_util.ts | 11 +- .../compiler/src/compiler_util/render_util.ts | 63 +++++------ .../src/directive_wrapper_compiler.ts | 101 +++++++++++++++++- .../src/schema/dom_element_schema_registry.ts | 9 +- .../src/schema/element_schema_registry.ts | 6 +- .../src/template_parser/binding_parser.ts | 88 ++++++++------- .../src/template_parser/template_ast.ts | 4 +- .../src/template_parser/template_parser.ts | 29 ++++- .../compiler/src/view_compiler/constants.ts | 56 ++-------- .../src/view_compiler/property_binder.ts | 39 ++++++- .../compiler/src/view_compiler/view_binder.ts | 14 ++- .../src/view_compiler/view_compiler.ts | 5 +- .../dom_element_schema_registry_spec.ts | 23 ++-- .../template_parser/binding_parser_spec.ts | 59 ++++++++++ .../template_parser/template_parser_spec.ts | 4 +- .../compiler/testing/schema_registry_mock.ts | 4 +- .../test/linker/security_integration_spec.ts | 63 +++++++++-- 19 files changed, 409 insertions(+), 175 deletions(-) create mode 100644 modules/@angular/compiler/test/template_parser/binding_parser_spec.ts diff --git a/modules/@angular/compiler-cli/src/codegen.ts b/modules/@angular/compiler-cli/src/codegen.ts index 2aedf26ed2..ab0f393b05 100644 --- a/modules/@angular/compiler-cli/src/codegen.ts +++ b/modules/@angular/compiler-cli/src/codegen.ts @@ -133,7 +133,9 @@ export class CodeGenerator { // TODO(vicb): do not pass cliOptions.i18nFormat here const offlineCompiler = new compiler.OfflineCompiler( resolver, normalizer, tmplParser, new compiler.StyleCompiler(urlResolver), - new compiler.ViewCompiler(config), new compiler.DirectiveWrapperCompiler(config), + new compiler.ViewCompiler(config, elementSchemaRegistry), + new compiler.DirectiveWrapperCompiler( + config, expressionParser, elementSchemaRegistry, console), new compiler.NgModuleCompiler(), new compiler.TypeScriptEmitter(reflectorHost), cliOptions.locale, cliOptions.i18nFormat); diff --git a/modules/@angular/compiler/index.ts b/modules/@angular/compiler/index.ts index fc5517df20..edbaa486d9 100644 --- a/modules/@angular/compiler/index.ts +++ b/modules/@angular/compiler/index.ts @@ -34,7 +34,7 @@ export {DirectiveResolver} from './src/directive_resolver'; export {PipeResolver} from './src/pipe_resolver'; export {NgModuleResolver} from './src/ng_module_resolver'; export {DEFAULT_INTERPOLATION_CONFIG, InterpolationConfig} from './src/ml_parser/interpolation_config'; -export {ElementSchemaRegistry} from './src/schema/element_schema_registry'; +export * from './src/schema/element_schema_registry'; export * from './src/i18n/index'; export * from './src/template_parser/template_ast'; export * from './src/directive_normalizer'; diff --git a/modules/@angular/compiler/src/compiler_util/identifier_util.ts b/modules/@angular/compiler/src/compiler_util/identifier_util.ts index 3b48dbdc89..cc4df049ea 100644 --- a/modules/@angular/compiler/src/compiler_util/identifier_util.ts +++ b/modules/@angular/compiler/src/compiler_util/identifier_util.ts @@ -8,7 +8,7 @@ import {CompileTokenMetadata} from '../compile_metadata'; import {isPresent} from '../facade/lang'; -import {Identifiers, resolveIdentifier} from '../identifiers'; +import {IdentifierSpec, Identifiers, resolveEnumIdentifier, resolveIdentifier} from '../identifiers'; import * as o from '../output/output_ast'; export function createDiTokenExpression(token: CompileTokenMetadata): o.Expression { @@ -49,3 +49,12 @@ export function createPureProxy( .set(o.importExpr(resolveIdentifier(pureProxyId)).callFn([fn])) .toStmt()); } + +export function createEnumExpression(enumType: IdentifierSpec, enumValue: any): o.Expression { + const enumName = + Object.keys(enumType.runtime).find((propName) => enumType.runtime[propName] === enumValue); + if (!enumName) { + throw new Error(`Unknown enum value ${enumValue} in ${enumType.name}`); + } + return o.importExpr(resolveEnumIdentifier(resolveIdentifier(enumType), enumName)); +} diff --git a/modules/@angular/compiler/src/compiler_util/render_util.ts b/modules/@angular/compiler/src/compiler_util/render_util.ts index 4c76e17a4d..e0beeef6ac 100644 --- a/modules/@angular/compiler/src/compiler_util/render_util.ts +++ b/modules/@angular/compiler/src/compiler_util/render_util.ts @@ -12,23 +12,27 @@ import {Identifiers, resolveIdentifier} from '../identifiers'; import * as o from '../output/output_ast'; import {BoundElementPropertyAst, PropertyBindingType} from '../template_parser/template_ast'; +import {createEnumExpression} from './identifier_util'; + export function writeToRenderer( - view: o.Expression, boundProp: BoundElementPropertyAst, renderNode: o.Expression, - renderValue: o.Expression, logBindingUpdate: boolean): o.Statement[] { + view: o.Expression, boundProp: BoundElementPropertyAst, renderElement: o.Expression, + renderValue: o.Expression, logBindingUpdate: boolean, + securityContextExpression?: o.Expression): o.Statement[] { const updateStmts: o.Statement[] = []; const renderer = view.prop('renderer'); - renderValue = sanitizedValue(view, boundProp, renderValue); + renderValue = sanitizedValue(view, boundProp, renderValue, securityContextExpression); switch (boundProp.type) { case PropertyBindingType.Property: if (logBindingUpdate) { - updateStmts.push(o.importExpr(resolveIdentifier(Identifiers.setBindingDebugInfo)) - .callFn([renderer, renderNode, o.literal(boundProp.name), renderValue]) - .toStmt()); + updateStmts.push( + o.importExpr(resolveIdentifier(Identifiers.setBindingDebugInfo)) + .callFn([renderer, renderElement, o.literal(boundProp.name), renderValue]) + .toStmt()); } updateStmts.push( renderer .callMethod( - 'setElementProperty', [renderNode, o.literal(boundProp.name), renderValue]) + 'setElementProperty', [renderElement, o.literal(boundProp.name), renderValue]) .toStmt()); break; case PropertyBindingType.Attribute: @@ -37,13 +41,14 @@ export function writeToRenderer( updateStmts.push( renderer .callMethod( - 'setElementAttribute', [renderNode, o.literal(boundProp.name), renderValue]) + 'setElementAttribute', [renderElement, o.literal(boundProp.name), renderValue]) .toStmt()); break; case PropertyBindingType.Class: updateStmts.push( renderer - .callMethod('setElementClass', [renderNode, o.literal(boundProp.name), renderValue]) + .callMethod( + 'setElementClass', [renderElement, o.literal(boundProp.name), renderValue]) .toStmt()); break; case PropertyBindingType.Style: @@ -55,7 +60,8 @@ export function writeToRenderer( renderValue = renderValue.isBlank().conditional(o.NULL_EXPR, strValue); updateStmts.push( renderer - .callMethod('setElementStyle', [renderNode, o.literal(boundProp.name), renderValue]) + .callMethod( + 'setElementStyle', [renderElement, o.literal(boundProp.name), renderValue]) .toStmt()); break; case PropertyBindingType.Animation: @@ -65,32 +71,19 @@ export function writeToRenderer( } function sanitizedValue( - view: o.Expression, boundProp: BoundElementPropertyAst, - renderValue: o.Expression): o.Expression { - let enumValue: string; - switch (boundProp.securityContext) { - case SecurityContext.NONE: - return renderValue; // No sanitization needed. - case SecurityContext.HTML: - enumValue = 'HTML'; - break; - case SecurityContext.STYLE: - enumValue = 'STYLE'; - break; - case SecurityContext.SCRIPT: - enumValue = 'SCRIPT'; - break; - case SecurityContext.URL: - enumValue = 'URL'; - break; - case SecurityContext.RESOURCE_URL: - enumValue = 'RESOURCE_URL'; - break; - default: - throw new Error(`internal error, unexpected SecurityContext ${boundProp.securityContext}.`); + view: o.Expression, boundProp: BoundElementPropertyAst, renderValue: o.Expression, + securityContextExpression?: o.Expression): o.Expression { + if (boundProp.securityContext === SecurityContext.NONE) { + return renderValue; // No sanitization needed. + } + if (!boundProp.needsRuntimeSecurityContext) { + securityContextExpression = + createEnumExpression(Identifiers.SecurityContext, boundProp.securityContext); + } + if (!securityContextExpression) { + throw new Error(`internal error, no SecurityContext given ${boundProp.name}`); } let ctx = view.prop('viewUtils').prop('sanitizer'); - let args = - [o.importExpr(resolveIdentifier(Identifiers.SecurityContext)).prop(enumValue), renderValue]; + let args = [securityContextExpression, renderValue]; return ctx.callMethod('sanitize', args); } diff --git a/modules/@angular/compiler/src/directive_wrapper_compiler.ts b/modules/@angular/compiler/src/directive_wrapper_compiler.ts index 4c598b8b9f..32c987b0eb 100644 --- a/modules/@angular/compiler/src/directive_wrapper_compiler.ts +++ b/modules/@angular/compiler/src/directive_wrapper_compiler.ts @@ -10,11 +10,19 @@ import {Injectable} from '@angular/core'; import {CompileDirectiveMetadata, CompileIdentifierMetadata} from './compile_metadata'; import {createCheckBindingField, createCheckBindingStmt} from './compiler_util/binding_util'; +import {convertPropertyBinding} from './compiler_util/expression_converter'; +import {writeToRenderer} from './compiler_util/render_util'; import {CompilerConfig} from './config'; +import {Parser} from './expression_parser/parser'; import {Identifiers, resolveIdentifier} from './identifiers'; +import {DEFAULT_INTERPOLATION_CONFIG} from './ml_parser/interpolation_config'; import {ClassBuilder, createClassStmt} from './output/class_builder'; import * as o from './output/output_ast'; -import {LifecycleHooks, isDefaultChangeDetectionStrategy} from './private_import_core'; +import {ParseError, ParseErrorLevel, ParseLocation, ParseSourceFile, ParseSourceSpan} from './parse_util'; +import {Console, LifecycleHooks, isDefaultChangeDetectionStrategy} from './private_import_core'; +import {ElementSchemaRegistry} from './schema/element_schema_registry'; +import {BindingParser} from './template_parser/binding_parser'; +import {BoundElementPropertyAst, BoundEventAst} from './template_parser/template_ast'; export class DirectiveWrapperCompileResult { constructor(public statements: o.Statement[], public dirWrapperClassVar: string) {} @@ -44,14 +52,28 @@ const RESET_CHANGES_STMT = o.THIS_EXPR.prop(CHANGES_FIELD_NAME).set(o.literalMap export class DirectiveWrapperCompiler { static dirWrapperClassName(id: CompileIdentifierMetadata) { return `Wrapper_${id.name}`; } - constructor(private compilerConfig: CompilerConfig) {} + constructor( + private compilerConfig: CompilerConfig, private _exprParser: Parser, + private _schemaRegistry: ElementSchemaRegistry, private _console: Console) {} compile(dirMeta: CompileDirectiveMetadata): DirectiveWrapperCompileResult { const builder = new DirectiveWrapperBuilder(this.compilerConfig, dirMeta); Object.keys(dirMeta.inputs).forEach((inputFieldName) => { addCheckInputMethod(inputFieldName, builder); }); - addDetectChangesInternalMethod(builder); + addDetectChangesInInputPropsMethod(builder); + + const hostParseResult = parseHostBindings(dirMeta, this._exprParser, this._schemaRegistry); + reportParseErrors(hostParseResult.errors, this._console); + // host properties are change detected by the DirectiveWrappers, + // except for the animation properties as they need close integration with animation events + // and DirectiveWrappers don't support + // event listeners right now. + addDetectChangesInHostPropsMethod( + hostParseResult.hostProps.filter(hostProp => !hostProp.isAnimation), builder); + + // TODO(tbosch): implement hostListeners via DirectiveWrapper as well! + const classStmt = builder.build(); return new DirectiveWrapperCompileResult([classStmt], classStmt.name); } @@ -108,7 +130,7 @@ class DirectiveWrapperBuilder implements ClassBuilder { } } -function addDetectChangesInternalMethod(builder: DirectiveWrapperBuilder) { +function addDetectChangesInInputPropsMethod(builder: DirectiveWrapperBuilder) { const changedVar = o.variable('changed'); const stmts: o.Statement[] = [ changedVar.set(o.THIS_EXPR.prop(CHANGED_FIELD_NAME)).toDeclStmt(), @@ -148,7 +170,7 @@ function addDetectChangesInternalMethod(builder: DirectiveWrapperBuilder) { stmts.push(new o.ReturnStatement(changedVar)); builder.methods.push(new o.ClassMethod( - 'detectChangesInternal', + 'detectChangesInInputProps', [ new o.FnParam( VIEW_VAR.name, o.importType(resolveIdentifier(Identifiers.AppView), [o.DYNAMIC_TYPE])), @@ -184,3 +206,72 @@ function addCheckInputMethod(input: string, builder: DirectiveWrapperBuilder) { ], methodBody)); } + +function addDetectChangesInHostPropsMethod( + hostProps: BoundElementPropertyAst[], builder: DirectiveWrapperBuilder) { + const stmts: o.Statement[] = []; + const methodParams: o.FnParam[] = [ + new o.FnParam( + VIEW_VAR.name, o.importType(resolveIdentifier(Identifiers.AppView), [o.DYNAMIC_TYPE])), + new o.FnParam(RENDER_EL_VAR.name, o.DYNAMIC_TYPE), + new o.FnParam(THROW_ON_CHANGE_VAR.name, o.BOOL_TYPE), + ]; + hostProps.forEach((hostProp) => { + const field = createCheckBindingField(builder); + const evalResult = convertPropertyBinding( + builder, null, o.THIS_EXPR.prop(CONTEXT_FIELD_NAME), hostProp.value, field.bindingId); + if (!evalResult) { + return; + } + let securityContextExpr: o.ReadVarExpr; + if (hostProp.needsRuntimeSecurityContext) { + securityContextExpr = o.variable(`secCtx_${methodParams.length}`); + methodParams.push(new o.FnParam( + securityContextExpr.name, o.importType(resolveIdentifier(Identifiers.SecurityContext)))); + } + stmts.push(...createCheckBindingStmt( + evalResult, field.expression, THROW_ON_CHANGE_VAR, + writeToRenderer( + VIEW_VAR, hostProp, RENDER_EL_VAR, evalResult.currValExpr, + builder.compilerConfig.logBindingUpdate, securityContextExpr))); + }); + builder.methods.push(new o.ClassMethod('detectChangesInHostProps', methodParams, stmts)); +} + +class ParseResult { + constructor( + public hostProps: BoundElementPropertyAst[], public hostListeners: BoundEventAst[], + public errors: ParseError[]) {} +} + +function parseHostBindings( + dirMeta: CompileDirectiveMetadata, exprParser: Parser, + schemaRegistry: ElementSchemaRegistry): ParseResult { + const errors: ParseError[] = []; + const parser = + new BindingParser(exprParser, DEFAULT_INTERPOLATION_CONFIG, schemaRegistry, [], errors); + const sourceFileName = dirMeta.type.moduleUrl ? + `in Directive ${dirMeta.type.name} in ${dirMeta.type.moduleUrl}` : + `in Directive ${dirMeta.type.name}`; + const sourceFile = new ParseSourceFile('', sourceFileName); + const sourceSpan = new ParseSourceSpan( + new ParseLocation(sourceFile, null, null, null), + new ParseLocation(sourceFile, null, null, null)); + const parsedHostProps = parser.createDirectiveHostPropertyAsts(dirMeta, sourceSpan); + const parsedHostListeners = parser.createDirectiveHostEventAsts(dirMeta, sourceSpan); + + return new ParseResult(parsedHostProps, parsedHostListeners, errors); +} + +function reportParseErrors(parseErrors: ParseError[], console: Console) { + const warnings = parseErrors.filter(error => error.level === ParseErrorLevel.WARNING); + const errors = parseErrors.filter(error => error.level === ParseErrorLevel.FATAL); + + if (warnings.length > 0) { + this._console.warn(`Directive parse warnings:\n${warnings.join('\n')}`); + } + + if (errors.length > 0) { + throw new Error(`Directive parse errors:\n${errors.join('\n')}`); + } +} \ No newline at end of file diff --git a/modules/@angular/compiler/src/schema/dom_element_schema_registry.ts b/modules/@angular/compiler/src/schema/dom_element_schema_registry.ts index 929b1f40aa..64059d7d92 100644 --- a/modules/@angular/compiler/src/schema/dom_element_schema_registry.ts +++ b/modules/@angular/compiler/src/schema/dom_element_schema_registry.ts @@ -328,7 +328,12 @@ export class DomElementSchemaRegistry extends ElementSchemaRegistry { * 'NONE' security context, i.e. that they are safe inert string values. Only specific well known * attack vectors are assigned their appropriate context. */ - securityContext(tagName: string, propName: string): SecurityContext { + securityContext(tagName: string, propName: string, isAttribute: boolean): SecurityContext { + if (isAttribute) { + // NB: For security purposes, use the mapped property name, not the attribute name. + propName = this.getMappedPropName(propName); + } + // Make sure comparisons are case insensitive, so that case differences between attribute and // property names do not have a security impact. tagName = tagName.toLowerCase(); @@ -366,4 +371,6 @@ export class DomElementSchemaRegistry extends ElementSchemaRegistry { return {error: false}; } } + + allKnownElementNames(): string[] { return Object.keys(this._schema); } } diff --git a/modules/@angular/compiler/src/schema/element_schema_registry.ts b/modules/@angular/compiler/src/schema/element_schema_registry.ts index 52ed080d96..d76eaa6991 100644 --- a/modules/@angular/compiler/src/schema/element_schema_registry.ts +++ b/modules/@angular/compiler/src/schema/element_schema_registry.ts @@ -6,12 +6,14 @@ * found in the LICENSE file at https://angular.io/license */ -import {SchemaMetadata} from '@angular/core'; +import {SchemaMetadata, SecurityContext} from '@angular/core'; export abstract class ElementSchemaRegistry { abstract hasProperty(tagName: string, propName: string, schemaMetas: SchemaMetadata[]): boolean; abstract hasElement(tagName: string, schemaMetas: SchemaMetadata[]): boolean; - abstract securityContext(tagName: string, propName: string): any; + abstract securityContext(elementName: string, propName: string, isAttribute: boolean): + SecurityContext; + abstract allKnownElementNames(): string[]; abstract getMappedPropName(propName: string): string; abstract getDefaultComponentElementName(): string; abstract validateProperty(name: string): {error: boolean, msg?: string}; diff --git a/modules/@angular/compiler/src/template_parser/binding_parser.ts b/modules/@angular/compiler/src/template_parser/binding_parser.ts index 0e0cc3d648..1f4b7535c1 100644 --- a/modules/@angular/compiler/src/template_parser/binding_parser.ts +++ b/modules/@angular/compiler/src/template_parser/binding_parser.ts @@ -6,17 +6,18 @@ * found in the LICENSE file at https://angular.io/license */ -import {SchemaMetadata, SecurityContext} from '@angular/core'; +import {SecurityContext} from '@angular/core'; -import {CompilePipeMetadata} from '../compile_metadata'; +import {CompileDirectiveMetadata, CompilePipeMetadata} from '../compile_metadata'; import {AST, ASTWithSource, BindingPipe, EmptyExpr, Interpolation, LiteralPrimitive, ParserError, RecursiveAstVisitor, TemplateBinding} from '../expression_parser/ast'; import {Parser} from '../expression_parser/parser'; import {isPresent} from '../facade/lang'; -import {InterpolationConfig} from '../ml_parser/interpolation_config'; +import {DEFAULT_INTERPOLATION_CONFIG, InterpolationConfig} from '../ml_parser/interpolation_config'; import {mergeNsAndName} from '../ml_parser/tags'; import {ParseError, ParseErrorLevel, ParseSourceSpan} from '../parse_util'; import {view_utils} from '../private_import_core'; import {ElementSchemaRegistry} from '../schema/element_schema_registry'; +import {CssSelector} from '../selector'; import {splitAtColon, splitAtPeriod} from '../util'; import {BoundElementPropertyAst, BoundEventAst, PropertyBindingType, VariableAst} from './template_ast'; @@ -55,18 +56,17 @@ export class BindingParser { constructor( private _exprParser: Parser, private _interpolationConfig: InterpolationConfig, - private _schemaRegistry: ElementSchemaRegistry, private _schemas: SchemaMetadata[], - pipes: CompilePipeMetadata[], private _targetErrors: ParseError[]) { + private _schemaRegistry: ElementSchemaRegistry, pipes: CompilePipeMetadata[], + private _targetErrors: ParseError[]) { pipes.forEach(pipe => this.pipesByName.set(pipe.name, pipe)); } - createDirectiveHostPropertyAsts( - elementName: string, hostProps: {[key: string]: string}, - sourceSpan: ParseSourceSpan): BoundElementPropertyAst[] { - if (hostProps) { + createDirectiveHostPropertyAsts(dirMeta: CompileDirectiveMetadata, sourceSpan: ParseSourceSpan): + BoundElementPropertyAst[] { + if (dirMeta.hostProperties) { const boundProps: BoundProperty[] = []; - Object.keys(hostProps).forEach(propName => { - const expression = hostProps[propName]; + Object.keys(dirMeta.hostProperties).forEach(propName => { + const expression = dirMeta.hostProperties[propName]; if (typeof expression === 'string') { this.parsePropertyBinding(propName, expression, true, sourceSpan, [], boundProps); } else { @@ -75,16 +75,16 @@ export class BindingParser { sourceSpan); } }); - return boundProps.map((prop) => this.createElementPropertyAst(elementName, prop)); + return boundProps.map((prop) => this.createElementPropertyAst(dirMeta.selector, prop)); } } - createDirectiveHostEventAsts(hostListeners: {[key: string]: string}, sourceSpan: ParseSourceSpan): + createDirectiveHostEventAsts(dirMeta: CompileDirectiveMetadata, sourceSpan: ParseSourceSpan): BoundEventAst[] { - if (hostListeners) { + if (dirMeta.hostListeners) { const targetEventAsts: BoundEventAst[] = []; - Object.keys(hostListeners).forEach(propName => { - const expression = hostListeners[propName]; + Object.keys(dirMeta.hostListeners).forEach(propName => { + const expression = dirMeta.hostListeners[propName]; if (typeof expression === 'string') { this.parseEvent(propName, expression, sourceSpan, [], targetEventAsts); } else { @@ -240,42 +240,33 @@ export class BindingParser { } } - createElementPropertyAst(elementName: string, boundProp: BoundProperty): BoundElementPropertyAst { + createElementPropertyAst(elementSelector: string, boundProp: BoundProperty): + BoundElementPropertyAst { if (boundProp.isAnimation) { return new BoundElementPropertyAst( - boundProp.name, PropertyBindingType.Animation, SecurityContext.NONE, boundProp.expression, - null, boundProp.sourceSpan); + boundProp.name, PropertyBindingType.Animation, SecurityContext.NONE, false, + boundProp.expression, null, boundProp.sourceSpan); } let unit: string = null; let bindingType: PropertyBindingType; let boundPropertyName: string; const parts = boundProp.name.split(PROPERTY_PARTS_SEPARATOR); - let securityContext: SecurityContext; + let securityContexts: SecurityContext[]; if (parts.length === 1) { var partValue = parts[0]; boundPropertyName = this._schemaRegistry.getMappedPropName(partValue); - securityContext = this._schemaRegistry.securityContext(elementName, boundPropertyName); + securityContexts = calcPossibleSecurityContexts( + this._schemaRegistry, elementSelector, boundPropertyName, false); bindingType = PropertyBindingType.Property; this._validatePropertyOrAttributeName(boundPropertyName, boundProp.sourceSpan, false); - if (!this._schemaRegistry.hasProperty(elementName, boundPropertyName, this._schemas)) { - let errorMsg = - `Can't bind to '${boundPropertyName}' since it isn't a known property of '${elementName}'.`; - if (elementName.indexOf('-') > -1) { - errorMsg += - `\n1. If '${elementName}' is an Angular component and it has '${boundPropertyName}' input, then verify that it is part of this module.` + - `\n2. If '${elementName}' is a Web Component then add "CUSTOM_ELEMENTS_SCHEMA" to the '@NgModule.schemas' of this component to suppress this message.\n`; - } - this._reportError(errorMsg, boundProp.sourceSpan); - } } else { if (parts[0] == ATTRIBUTE_PREFIX) { boundPropertyName = parts[1]; this._validatePropertyOrAttributeName(boundPropertyName, boundProp.sourceSpan, true); - // NB: For security purposes, use the mapped property name, not the attribute name. - const mapPropName = this._schemaRegistry.getMappedPropName(boundPropertyName); - securityContext = this._schemaRegistry.securityContext(elementName, mapPropName); + securityContexts = calcPossibleSecurityContexts( + this._schemaRegistry, elementSelector, boundPropertyName, true); const nsSeparatorIdx = boundPropertyName.indexOf(':'); if (nsSeparatorIdx > -1) { @@ -288,22 +279,21 @@ export class BindingParser { } else if (parts[0] == CLASS_PREFIX) { boundPropertyName = parts[1]; bindingType = PropertyBindingType.Class; - securityContext = SecurityContext.NONE; + securityContexts = [SecurityContext.NONE]; } else if (parts[0] == STYLE_PREFIX) { unit = parts.length > 2 ? parts[2] : null; boundPropertyName = parts[1]; bindingType = PropertyBindingType.Style; - securityContext = SecurityContext.STYLE; + securityContexts = [SecurityContext.STYLE]; } else { this._reportError(`Invalid property name '${boundProp.name}'`, boundProp.sourceSpan); bindingType = null; - securityContext = null; + securityContexts = []; } } - return new BoundElementPropertyAst( - boundPropertyName, bindingType, securityContext, boundProp.expression, unit, - boundProp.sourceSpan); + boundPropertyName, bindingType, securityContexts.length === 1 ? securityContexts[0] : null, + securityContexts.length > 1, boundProp.expression, unit, boundProp.sourceSpan); } parseEvent( @@ -429,3 +419,21 @@ export class PipeCollector extends RecursiveAstVisitor { function _isAnimationLabel(name: string): boolean { return name[0] == '@'; } + +export function calcPossibleSecurityContexts( + registry: ElementSchemaRegistry, selector: string, propName: string, + isAttribute: boolean): SecurityContext[] { + const ctxs: SecurityContext[] = []; + CssSelector.parse(selector).forEach((selector) => { + const elementNames = selector.element ? [selector.element] : registry.allKnownElementNames(); + const notElementNames = + new Set(selector.notSelectors.filter(selector => selector.isElementSelector()) + .map((selector) => selector.element)); + const possibleElementNames = + elementNames.filter(elementName => !notElementNames.has(elementName)); + + ctxs.push(...possibleElementNames.map( + elementName => registry.securityContext(elementName, propName, isAttribute))); + }); + return ctxs.length === 0 ? [SecurityContext.NONE] : Array.from(new Set(ctxs)).sort(); +} diff --git a/modules/@angular/compiler/src/template_parser/template_ast.ts b/modules/@angular/compiler/src/template_parser/template_ast.ts index 4fcf948009..c56d11177e 100644 --- a/modules/@angular/compiler/src/template_parser/template_ast.ts +++ b/modules/@angular/compiler/src/template_parser/template_ast.ts @@ -63,8 +63,8 @@ export class AttrAst implements TemplateAst { export class BoundElementPropertyAst implements TemplateAst { constructor( public name: string, public type: PropertyBindingType, - public securityContext: SecurityContext, public value: AST, public unit: string, - public sourceSpan: ParseSourceSpan) {} + public securityContext: SecurityContext, public needsRuntimeSecurityContext: boolean, + public value: AST, public unit: string, public sourceSpan: ParseSourceSpan) {} visit(visitor: TemplateAstVisitor, context: any): any { return visitor.visitElementProperty(this, context); } diff --git a/modules/@angular/compiler/src/template_parser/template_parser.ts b/modules/@angular/compiler/src/template_parser/template_parser.ts index 8cef2dd4f9..10d68d4de1 100644 --- a/modules/@angular/compiler/src/template_parser/template_parser.ts +++ b/modules/@angular/compiler/src/template_parser/template_parser.ts @@ -137,7 +137,7 @@ export class TemplateParser { }; } const bindingParser = new BindingParser( - this._exprParser, interpolationConfig, this._schemaRegistry, schemas, uniqPipes, errors); + this._exprParser, interpolationConfig, this._schemaRegistry, uniqPipes, errors); const parseVisitor = new TemplateParseVisitor( providerViewContext, uniqDirectives, bindingParser, this._schemaRegistry, schemas, errors); @@ -549,10 +549,12 @@ class TemplateParseVisitor implements html.Visitor { component = directive; } const directiveProperties: BoundDirectivePropertyAst[] = []; - const hostProperties = this._bindingParser.createDirectiveHostPropertyAsts( - elementName, directive.hostProperties, sourceSpan); - const hostEvents = - this._bindingParser.createDirectiveHostEventAsts(directive.hostListeners, sourceSpan); + const hostProperties = + this._bindingParser.createDirectiveHostPropertyAsts(directive, sourceSpan); + // Note: We need to check the host properties here as well, + // as we don't know the element name in the DirectiveWrapperCompiler yet. + this._checkPropertiesInSchema(elementName, hostProperties); + const hostEvents = this._bindingParser.createDirectiveHostEventAsts(directive, sourceSpan); this._createDirectivePropertyAsts(directive.inputs, props, directiveProperties); elementOrDirectiveRefs.forEach((elOrDirRef) => { if ((elOrDirRef.value.length === 0 && directive.isComponent) || @@ -626,6 +628,7 @@ class TemplateParseVisitor implements html.Visitor { boundElementProps.push(this._bindingParser.createElementPropertyAst(elementName, prop)); } }); + this._checkPropertiesInSchema(elementName, boundElementProps); return boundElementProps; } @@ -700,6 +703,22 @@ class TemplateParseVisitor implements html.Visitor { }); } + private _checkPropertiesInSchema(elementName: string, boundProps: BoundElementPropertyAst[]) { + boundProps.forEach((boundProp) => { + if (boundProp.type === PropertyBindingType.Property && + !this._schemaRegistry.hasProperty(elementName, boundProp.name, this._schemas)) { + let errorMsg = + `Can't bind to '${boundProp.name}' since it isn't a known property of '${elementName}'.`; + if (elementName.indexOf('-') > -1) { + errorMsg += + `\n1. If '${elementName}' is an Angular component and it has '${boundProp.name}' input, then verify that it is part of this module.` + + `\n2. If '${elementName}' is a Web Component then add "CUSTOM_ELEMENTS_SCHEMA" to the '@NgModule.schemas' of this component to suppress this message.\n`; + } + this._reportError(errorMsg, boundProp.sourceSpan); + } + }); + } + private _reportError( message: string, sourceSpan: ParseSourceSpan, level: ParseErrorLevel = ParseErrorLevel.FATAL) { diff --git a/modules/@angular/compiler/src/view_compiler/constants.ts b/modules/@angular/compiler/src/view_compiler/constants.ts index 72cea01dee..03e242f9b7 100644 --- a/modules/@angular/compiler/src/view_compiler/constants.ts +++ b/modules/@angular/compiler/src/view_compiler/constants.ts @@ -9,9 +9,9 @@ import {ChangeDetectionStrategy, ViewEncapsulation} from '@angular/core'; import {CompileIdentifierMetadata} from '../compile_metadata'; -import {Identifiers, resolveEnumIdentifier, resolveIdentifier} from '../identifiers'; +import {createEnumExpression} from '../compiler_util/identifier_util'; +import {Identifiers, resolveEnumIdentifier} from '../identifiers'; import * as o from '../output/output_ast'; - import {ChangeDetectorStatus, ViewType} from '../private_import_core'; function _enumExpression(classIdentifier: CompileIdentifierMetadata, name: string): o.Expression { @@ -20,69 +20,25 @@ function _enumExpression(classIdentifier: CompileIdentifierMetadata, name: strin export class ViewTypeEnum { static fromValue(value: ViewType): o.Expression { - const viewType = resolveIdentifier(Identifiers.ViewType); - switch (value) { - case ViewType.HOST: - return _enumExpression(viewType, 'HOST'); - case ViewType.COMPONENT: - return _enumExpression(viewType, 'COMPONENT'); - case ViewType.EMBEDDED: - return _enumExpression(viewType, 'EMBEDDED'); - default: - throw Error(`Inavlid ViewType value: ${value}`); - } + return createEnumExpression(Identifiers.ViewType, value); } } export class ViewEncapsulationEnum { static fromValue(value: ViewEncapsulation): o.Expression { - const viewEncapsulation = resolveIdentifier(Identifiers.ViewEncapsulation); - switch (value) { - case ViewEncapsulation.Emulated: - return _enumExpression(viewEncapsulation, 'Emulated'); - case ViewEncapsulation.Native: - return _enumExpression(viewEncapsulation, 'Native'); - case ViewEncapsulation.None: - return _enumExpression(viewEncapsulation, 'None'); - default: - throw Error(`Inavlid ViewEncapsulation value: ${value}`); - } + return createEnumExpression(Identifiers.ViewEncapsulation, value); } } export class ChangeDetectionStrategyEnum { static fromValue(value: ChangeDetectionStrategy): o.Expression { - const changeDetectionStrategy = resolveIdentifier(Identifiers.ChangeDetectionStrategy); - switch (value) { - case ChangeDetectionStrategy.OnPush: - return _enumExpression(changeDetectionStrategy, 'OnPush'); - case ChangeDetectionStrategy.Default: - return _enumExpression(changeDetectionStrategy, 'Default'); - default: - throw Error(`Inavlid ChangeDetectionStrategy value: ${value}`); - } + return createEnumExpression(Identifiers.ChangeDetectionStrategy, value); } } export class ChangeDetectorStatusEnum { static fromValue(value: ChangeDetectorStatusEnum): o.Expression { - const changeDetectorStatus = resolveIdentifier(Identifiers.ChangeDetectorStatus); - switch (value) { - case ChangeDetectorStatus.CheckOnce: - return _enumExpression(changeDetectorStatus, 'CheckOnce'); - case ChangeDetectorStatus.Checked: - return _enumExpression(changeDetectorStatus, 'Checked'); - case ChangeDetectorStatus.CheckAlways: - return _enumExpression(changeDetectorStatus, 'CheckAlways'); - case ChangeDetectorStatus.Detached: - return _enumExpression(changeDetectorStatus, 'Detached'); - case ChangeDetectorStatus.Errored: - return _enumExpression(changeDetectorStatus, 'Errored'); - case ChangeDetectorStatus.Destroyed: - return _enumExpression(changeDetectorStatus, 'Destroyed'); - default: - throw Error(`Inavlid ChangeDetectorStatus value: ${value}`); - } + return createEnumExpression(Identifiers.ChangeDetectorStatus, value); } } diff --git a/modules/@angular/compiler/src/view_compiler/property_binder.ts b/modules/@angular/compiler/src/view_compiler/property_binder.ts index 1d66730b49..510fcfa098 100644 --- a/modules/@angular/compiler/src/view_compiler/property_binder.ts +++ b/modules/@angular/compiler/src/view_compiler/property_binder.ts @@ -10,12 +10,14 @@ import {SecurityContext} from '@angular/core'; import {createCheckBindingField, createCheckBindingStmt} from '../compiler_util/binding_util'; import {ConvertPropertyBindingResult, convertPropertyBinding} from '../compiler_util/expression_converter'; +import {createEnumExpression} from '../compiler_util/identifier_util'; import {writeToRenderer} from '../compiler_util/render_util'; import * as cdAst from '../expression_parser/ast'; import {isPresent} from '../facade/lang'; import {Identifiers, resolveIdentifier} from '../identifiers'; import * as o from '../output/output_ast'; import {EMPTY_STATE as EMPTY_ANIMATION_STATE, LifecycleHooks, isDefaultChangeDetectionStrategy} from '../private_import_core'; +import {ElementSchemaRegistry} from '../schema/element_schema_registry'; import {BoundElementPropertyAst, BoundTextAst, DirectiveAst, PropertyBindingType} from '../template_parser/template_ast'; import {camelCaseToDashCase} from '../util'; @@ -121,10 +123,39 @@ export function bindRenderInputs( } export function bindDirectiveHostProps( - directiveAst: DirectiveAst, directiveInstance: o.Expression, compileElement: CompileElement, - eventListeners: CompileEventListener[]): void { + directiveAst: DirectiveAst, directiveWrapperInstance: o.Expression, + compileElement: CompileElement, eventListeners: CompileEventListener[], elementName: string, + schemaRegistry: ElementSchemaRegistry): void { + // host properties are change detected by the DirectiveWrappers, + // except for the animation properties as they need close integration with animation events + // and DirectiveWrappers don't support + // event listeners right now. bindAndWriteToRenderer( - directiveAst.hostProperties, directiveInstance, compileElement, true, eventListeners); + directiveAst.hostProperties.filter(boundProp => boundProp.isAnimation), + directiveWrapperInstance.prop('context'), compileElement, true, eventListeners); + + + const methodArgs: o.Expression[] = + [o.THIS_EXPR, compileElement.renderNode, DetectChangesVars.throwOnChange]; + // We need to provide the SecurityContext for properties that could need sanitization. + directiveAst.hostProperties.filter(boundProp => boundProp.needsRuntimeSecurityContext) + .forEach((boundProp) => { + let ctx: SecurityContext; + switch (boundProp.type) { + case PropertyBindingType.Property: + ctx = schemaRegistry.securityContext(elementName, boundProp.name, false); + break; + case PropertyBindingType.Attribute: + ctx = schemaRegistry.securityContext(elementName, boundProp.name, true); + break; + default: + throw new Error( + `Illegal state: Only property / attribute bindings can have an unknown security context! Binding ${boundProp.name}`); + } + methodArgs.push(createEnumExpression(Identifiers.SecurityContext, ctx)); + }); + compileElement.view.detectChangesRenderPropertiesMethod.addStmt( + directiveWrapperInstance.callMethod('detectChangesInHostProps', methodArgs).toStmt()); } export function bindDirectiveInputs( @@ -157,7 +188,7 @@ export function bindDirectiveInputs( var isOnPushComp = directiveAst.directive.isComponent && !isDefaultChangeDetectionStrategy(directiveAst.directive.changeDetection); let directiveDetectChangesExpr = directiveWrapperInstance.callMethod( - 'detectChangesInternal', + 'detectChangesInInputProps', [o.THIS_EXPR, compileElement.renderNode, DetectChangesVars.throwOnChange]); const directiveDetectChangesStmt = isOnPushComp ? new o.IfStmt(directiveDetectChangesExpr, [compileElement.appElement.prop('componentView') diff --git a/modules/@angular/compiler/src/view_compiler/view_binder.ts b/modules/@angular/compiler/src/view_compiler/view_binder.ts index 691de894ea..c8444b00d4 100644 --- a/modules/@angular/compiler/src/view_compiler/view_binder.ts +++ b/modules/@angular/compiler/src/view_compiler/view_binder.ts @@ -6,6 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ +import {ElementSchemaRegistry} from '../schema/element_schema_registry'; import {AttrAst, BoundDirectivePropertyAst, BoundElementPropertyAst, BoundEventAst, BoundTextAst, DirectiveAst, ElementAst, EmbeddedTemplateAst, NgContentAst, ReferenceAst, TemplateAst, TemplateAstVisitor, TextAst, VariableAst, templateVisitAll} from '../template_parser/template_ast'; import {CompileElement} from './compile_element'; @@ -14,8 +15,9 @@ import {CompileEventListener, bindDirectiveOutputs, bindRenderOutputs, collectEv import {bindDirectiveAfterContentLifecycleCallbacks, bindDirectiveAfterViewLifecycleCallbacks, bindInjectableDestroyLifecycleCallbacks, bindPipeDestroyLifecycleCallbacks} from './lifecycle_binder'; import {bindDirectiveHostProps, bindDirectiveInputs, bindRenderInputs, bindRenderText} from './property_binder'; -export function bindView(view: CompileView, parsedTemplate: TemplateAst[]): void { - var visitor = new ViewBinderVisitor(view); +export function bindView( + view: CompileView, parsedTemplate: TemplateAst[], schemaRegistry: ElementSchemaRegistry): void { + var visitor = new ViewBinderVisitor(view, schemaRegistry); templateVisitAll(visitor, parsedTemplate); view.pipes.forEach( (pipe) => { bindPipeDestroyLifecycleCallbacks(pipe.meta, pipe.instance, pipe.view); }); @@ -24,7 +26,7 @@ export function bindView(view: CompileView, parsedTemplate: TemplateAst[]): void class ViewBinderVisitor implements TemplateAstVisitor { private _nodeIndex: number = 0; - constructor(public view: CompileView) {} + constructor(public view: CompileView, private _schemaRegistry: ElementSchemaRegistry) {} visitBoundText(ast: BoundTextAst, parent: CompileElement): any { var node = this.view.nodes[this._nodeIndex++]; @@ -52,7 +54,9 @@ class ViewBinderVisitor implements TemplateAstVisitor { compileElement.directiveWrapperInstance.get(directiveAst.directive.type.reference); bindDirectiveInputs(directiveAst, directiveWrapperInstance, dirIndex, compileElement); - bindDirectiveHostProps(directiveAst, directiveInstance, compileElement, eventListeners); + bindDirectiveHostProps( + directiveAst, directiveWrapperInstance, compileElement, eventListeners, ast.name, + this._schemaRegistry); bindDirectiveOutputs(directiveAst, directiveInstance, eventListeners); }); templateVisitAll(this, ast.children, compileElement); @@ -91,7 +95,7 @@ class ViewBinderVisitor implements TemplateAstVisitor { var providerInstance = compileElement.instances.get(providerAst.token.reference); bindInjectableDestroyLifecycleCallbacks(providerAst, providerInstance, compileElement); }); - bindView(compileElement.embeddedView, ast.children); + bindView(compileElement.embeddedView, ast.children, this._schemaRegistry); return null; } diff --git a/modules/@angular/compiler/src/view_compiler/view_compiler.ts b/modules/@angular/compiler/src/view_compiler/view_compiler.ts index 60d06e0777..875e2bb3ac 100644 --- a/modules/@angular/compiler/src/view_compiler/view_compiler.ts +++ b/modules/@angular/compiler/src/view_compiler/view_compiler.ts @@ -12,6 +12,7 @@ import {AnimationEntryCompileResult} from '../animation/animation_compiler'; import {CompileDirectiveMetadata, CompilePipeMetadata} from '../compile_metadata'; import {CompilerConfig} from '../config'; import * as o from '../output/output_ast'; +import {ElementSchemaRegistry} from '../schema/element_schema_registry'; import {TemplateAst} from '../template_parser/template_ast'; import {CompileElement} from './compile_element'; @@ -31,7 +32,7 @@ export class ViewCompileResult { @Injectable() export class ViewCompiler { - constructor(private _genConfig: CompilerConfig) {} + constructor(private _genConfig: CompilerConfig, private _schemaRegistry: ElementSchemaRegistry) {} compileComponent( component: CompileDirectiveMetadata, template: TemplateAst[], styles: o.Expression, @@ -47,7 +48,7 @@ export class ViewCompiler { buildView(view, template, dependencies); // Need to separate binding from creation to be able to refer to // variables that have been declared after usage. - bindView(view, template); + bindView(view, template, this._schemaRegistry); finishView(view, statements); return new ViewCompileResult(statements, view.viewFactory.name, dependencies); diff --git a/modules/@angular/compiler/test/schema/dom_element_schema_registry_spec.ts b/modules/@angular/compiler/test/schema/dom_element_schema_registry_spec.ts index dee9bd9b40..e373466f52 100644 --- a/modules/@angular/compiler/test/schema/dom_element_schema_registry_spec.ts +++ b/modules/@angular/compiler/test/schema/dom_element_schema_registry_spec.ts @@ -147,12 +147,12 @@ If 'onAnything' is a directive input, make sure the directive is imported by the }); it('should return security contexts for elements', () => { - expect(registry.securityContext('iframe', 'srcdoc')).toBe(SecurityContext.HTML); - expect(registry.securityContext('p', 'innerHTML')).toBe(SecurityContext.HTML); - expect(registry.securityContext('a', 'href')).toBe(SecurityContext.URL); - expect(registry.securityContext('a', 'style')).toBe(SecurityContext.STYLE); - expect(registry.securityContext('ins', 'cite')).toBe(SecurityContext.URL); - expect(registry.securityContext('base', 'href')).toBe(SecurityContext.RESOURCE_URL); + expect(registry.securityContext('iframe', 'srcdoc', false)).toBe(SecurityContext.HTML); + expect(registry.securityContext('p', 'innerHTML', false)).toBe(SecurityContext.HTML); + expect(registry.securityContext('a', 'href', false)).toBe(SecurityContext.URL); + expect(registry.securityContext('a', 'style', false)).toBe(SecurityContext.STYLE); + expect(registry.securityContext('ins', 'cite', false)).toBe(SecurityContext.URL); + expect(registry.securityContext('base', 'href', false)).toBe(SecurityContext.RESOURCE_URL); }); it('should detect properties on namespaced elements', () => { @@ -162,9 +162,14 @@ If 'onAnything' is a directive input, make sure the directive is imported by the }); it('should check security contexts case insensitive', () => { - expect(registry.securityContext('p', 'iNnErHtMl')).toBe(SecurityContext.HTML); - expect(registry.securityContext('p', 'formaction')).toBe(SecurityContext.URL); - expect(registry.securityContext('p', 'formAction')).toBe(SecurityContext.URL); + expect(registry.securityContext('p', 'iNnErHtMl', false)).toBe(SecurityContext.HTML); + expect(registry.securityContext('p', 'formaction', false)).toBe(SecurityContext.URL); + expect(registry.securityContext('p', 'formAction', false)).toBe(SecurityContext.URL); + }); + + it('should check security contexts for attributes', () => { + expect(registry.securityContext('p', 'innerHtml', true)).toBe(SecurityContext.HTML); + expect(registry.securityContext('p', 'formaction', true)).toBe(SecurityContext.URL); }); describe('Angular custom elements', () => { diff --git a/modules/@angular/compiler/test/template_parser/binding_parser_spec.ts b/modules/@angular/compiler/test/template_parser/binding_parser_spec.ts new file mode 100644 index 0000000000..50c1991ce4 --- /dev/null +++ b/modules/@angular/compiler/test/template_parser/binding_parser_spec.ts @@ -0,0 +1,59 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * 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 {SecurityContext} from '@angular/core'; +import {inject} from '@angular/core/testing'; + +import {ElementSchemaRegistry} from '../../src/schema/element_schema_registry'; +import {calcPossibleSecurityContexts} from '../../src/template_parser/binding_parser'; + +export function main() { + describe('BindingParser', () => { + let registry: ElementSchemaRegistry; + + beforeEach(inject( + [ElementSchemaRegistry], (_registry: ElementSchemaRegistry) => { registry = _registry; })); + + describe('possibleSecurityContexts', () => { + function hrefSecurityContexts(selector: string) { + return calcPossibleSecurityContexts(registry, selector, 'href', false); + } + + it('should return a single security context if the selector as an element name', + () => { expect(hrefSecurityContexts('a')).toEqual([SecurityContext.URL]); }); + + it('should return the possible security contexts if the selector has no element name', () => { + expect(hrefSecurityContexts('[myDir]')).toEqual([ + SecurityContext.NONE, SecurityContext.URL, SecurityContext.RESOURCE_URL + ]); + }); + + it('should exclude possible elements via :not', () => { + expect(hrefSecurityContexts('[myDir]:not(link):not(base)')).toEqual([ + SecurityContext.NONE, SecurityContext.URL + ]); + }); + + it('should not exclude possible narrowed elements via :not', () => { + expect(hrefSecurityContexts('[myDir]:not(link.someClass):not(base.someClass)')).toEqual([ + SecurityContext.NONE, SecurityContext.URL, SecurityContext.RESOURCE_URL + ]); + }); + + it('should return SecurityContext.NONE if there are no possible elements', + () => { expect(hrefSecurityContexts('img:not(img)')).toEqual([SecurityContext.NONE]); }); + + it('should return the union of the possible security contexts if multiple selectors are specified', + () => { + expect(calcPossibleSecurityContexts(registry, 'a,link', 'href', false)).toEqual([ + SecurityContext.URL, SecurityContext.RESOURCE_URL + ]); + }); + }); + }); +} \ No newline at end of file diff --git a/modules/@angular/compiler/test/template_parser/template_parser_spec.ts b/modules/@angular/compiler/test/template_parser/template_parser_spec.ts index 058616a14e..6b32b3c154 100644 --- a/modules/@angular/compiler/test/template_parser/template_parser_spec.ts +++ b/modules/@angular/compiler/test/template_parser/template_parser_spec.ts @@ -124,7 +124,7 @@ export function main() { new class extends NullVisitor{ visitElementProperty(ast: BoundElementPropertyAst, context: any): any{return ast;} }, - new BoundElementPropertyAst('foo', null, null, null, 'bar', null)); + new BoundElementPropertyAst('foo', null, null, false, null, 'bar', null)); }); it('should visit AttrAst', () => { @@ -171,7 +171,7 @@ export function main() { new ElementAst('foo', [], [], [], [], [], [], false, [], 0, null, null), new ReferenceAst('foo', null, null), new VariableAst('foo', 'bar', null), new BoundEventAst('foo', 'bar', 'goo', null, null), - new BoundElementPropertyAst('foo', null, null, null, 'bar', null), + new BoundElementPropertyAst('foo', null, null, false, null, 'bar', null), new AttrAst('foo', 'bar', null), new BoundTextAst(null, 0, null), new TextAst('foo', 0, null), new DirectiveAst(null, [], [], [], null), new BoundDirectivePropertyAst('foo', 'bar', null, null) diff --git a/modules/@angular/compiler/testing/schema_registry_mock.ts b/modules/@angular/compiler/testing/schema_registry_mock.ts index bd11e80e5f..45b7d0885a 100644 --- a/modules/@angular/compiler/testing/schema_registry_mock.ts +++ b/modules/@angular/compiler/testing/schema_registry_mock.ts @@ -26,7 +26,9 @@ export class MockSchemaRegistry implements ElementSchemaRegistry { return value === void 0 ? true : value; } - securityContext(tagName: string, property: string): SecurityContext { + allKnownElementNames(): string[] { return Object.keys(this.existingElements); } + + securityContext(selector: string, property: string, isAttribute: boolean): SecurityContext { return SecurityContext.NONE; } diff --git a/modules/@angular/core/test/linker/security_integration_spec.ts b/modules/@angular/core/test/linker/security_integration_spec.ts index 46670b3d7a..8b84172a4d 100644 --- a/modules/@angular/core/test/linker/security_integration_spec.ts +++ b/modules/@angular/core/test/linker/security_integration_spec.ts @@ -6,9 +6,9 @@ * found in the LICENSE file at https://angular.io/license */ -import {Component, Directive, Input, NO_ERRORS_SCHEMA} from '@angular/core'; +import {Component, Directive, HostBinding, Input, NO_ERRORS_SCHEMA} from '@angular/core'; import {ComponentFixture, TestBed, getTestBed} from '@angular/core/testing'; -import {afterEach, beforeEach, describe, expect, it} from '@angular/core/testing/testing_internal'; +import {afterEach, beforeEach, describe, expect, iit, it} from '@angular/core/testing/testing_internal'; import {getDOM} from '@angular/platform-browser/src/dom/dom_adapter'; import {DomSanitizer} from '@angular/platform-browser/src/security/dom_sanitization_service'; @@ -133,22 +133,67 @@ function declareTests({useJit}: {useJit: boolean}) { }); describe('sanitizing', () => { - it('should escape unsafe attributes', () => { - const template = `Link Title`; - TestBed.overrideComponent(SecuredComponent, {set: {template}}); - const fixture = TestBed.createComponent(SecuredComponent); - + function checkEscapeOfHrefProperty(fixture: ComponentFixture, isAttribute: boolean) { let e = fixture.debugElement.children[0].nativeElement; let ci = fixture.componentInstance; ci.ctxProp = 'hello'; fixture.detectChanges(); // In the browser, reading href returns an absolute URL. On the server side, // it just echoes back the property. - expect(getDOM().getProperty(e, 'href')).toMatch(/.*\/?hello$/); + let value = + isAttribute ? getDOM().getAttribute(e, 'href') : getDOM().getProperty(e, 'href'); + expect(value).toMatch(/.*\/?hello$/); ci.ctxProp = 'javascript:alert(1)'; fixture.detectChanges(); - expect(getDOM().getProperty(e, 'href')).toEqual('unsafe:javascript:alert(1)'); + value = isAttribute ? getDOM().getAttribute(e, 'href') : getDOM().getProperty(e, 'href'); + expect(value).toEqual('unsafe:javascript:alert(1)'); + } + + it('should escape unsafe properties', () => { + const template = `Link Title`; + TestBed.overrideComponent(SecuredComponent, {set: {template}}); + const fixture = TestBed.createComponent(SecuredComponent); + + checkEscapeOfHrefProperty(fixture, false); + }); + + it('should escape unsafe attributes', () => { + const template = `Link Title`; + TestBed.overrideComponent(SecuredComponent, {set: {template}}); + const fixture = TestBed.createComponent(SecuredComponent); + + checkEscapeOfHrefProperty(fixture, true); + }); + + it('should escape unsafe properties if they are used in host bindings', () => { + @Directive({selector: '[dirHref]'}) + class HrefDirective { + @HostBinding('href') @Input() + dirHref: string; + } + + const template = `Link Title`; + TestBed.configureTestingModule({declarations: [HrefDirective]}); + TestBed.overrideComponent(SecuredComponent, {set: {template}}); + const fixture = TestBed.createComponent(SecuredComponent); + + checkEscapeOfHrefProperty(fixture, false); + }); + + it('should escape unsafe attributes if they are used in host bindings', () => { + @Directive({selector: '[dirHref]'}) + class HrefDirective { + @HostBinding('attr.href') @Input() + dirHref: string; + } + + const template = `Link Title`; + TestBed.configureTestingModule({declarations: [HrefDirective]}); + TestBed.overrideComponent(SecuredComponent, {set: {template}}); + const fixture = TestBed.createComponent(SecuredComponent); + + checkEscapeOfHrefProperty(fixture, true); }); it('should escape unsafe style values', () => {