fix(compiler): don't access view local variables nor pipes in host expressions (#12396)

Fixes #12004
Closes #12071

modules/@angular/compiler/src/expression_parser/parser.ts

@@ -60,10 +60,11 @@ export class Parser {
   parseSimpleBinding(
       input: string, location: string,
       interpolationConfig: InterpolationConfig = DEFAULT_INTERPOLATION_CONFIG): ASTWithSource {
-    var ast = this._parseBindingAst(input, location, interpolationConfig);
+    const ast = this._parseBindingAst(input, location, interpolationConfig);
-    if (!SimpleExpressionChecker.check(ast)) {
+    const errors = SimpleExpressionChecker.check(ast);
+    if (errors.length > 0) {
       this._reportError(
-          'Host binding expression can only contain field access and constants', input, location);
+          `Host binding expression cannot contain ${errors.join(' ')}`, input, location);
     }
     return new ASTWithSource(ast, input, location, this.errors);
   }
@@ -751,51 +752,51 @@ export class _ParseAST {
 }
 
 class SimpleExpressionChecker implements AstVisitor {
-  static check(ast: AST): boolean {
+  static check(ast: AST): string[] {
     var s = new SimpleExpressionChecker();
     ast.visit(s);
-    return s.simple;
+    return s.errors;
   }
 
-  simple = true;
+  errors: string[] = [];
 
   visitImplicitReceiver(ast: ImplicitReceiver, context: any) {}
 
-  visitInterpolation(ast: Interpolation, context: any) { this.simple = false; }
+  visitInterpolation(ast: Interpolation, context: any) {}
 
   visitLiteralPrimitive(ast: LiteralPrimitive, context: any) {}
 
   visitPropertyRead(ast: PropertyRead, context: any) {}
 
-  visitPropertyWrite(ast: PropertyWrite, context: any) { this.simple = false; }
+  visitPropertyWrite(ast: PropertyWrite, context: any) {}
 
-  visitSafePropertyRead(ast: SafePropertyRead, context: any) { this.simple = false; }
+  visitSafePropertyRead(ast: SafePropertyRead, context: any) {}
 
-  visitMethodCall(ast: MethodCall, context: any) { this.simple = false; }
+  visitMethodCall(ast: MethodCall, context: any) {}
 
-  visitSafeMethodCall(ast: SafeMethodCall, context: any) { this.simple = false; }
+  visitSafeMethodCall(ast: SafeMethodCall, context: any) {}
 
-  visitFunctionCall(ast: FunctionCall, context: any) { this.simple = false; }
+  visitFunctionCall(ast: FunctionCall, context: any) {}
 
   visitLiteralArray(ast: LiteralArray, context: any) { this.visitAll(ast.expressions); }
 
   visitLiteralMap(ast: LiteralMap, context: any) { this.visitAll(ast.values); }
 
-  visitBinary(ast: Binary, context: any) { this.simple = false; }
+  visitBinary(ast: Binary, context: any) {}
 
-  visitPrefixNot(ast: PrefixNot, context: any) { this.simple = false; }
+  visitPrefixNot(ast: PrefixNot, context: any) {}
 
-  visitConditional(ast: Conditional, context: any) { this.simple = false; }
+  visitConditional(ast: Conditional, context: any) {}
 
-  visitPipe(ast: BindingPipe, context: any) { this.simple = false; }
+  visitPipe(ast: BindingPipe, context: any) { this.errors.push('pipes'); }
 
-  visitKeyedRead(ast: KeyedRead, context: any) { this.simple = false; }
+  visitKeyedRead(ast: KeyedRead, context: any) {}
 
-  visitKeyedWrite(ast: KeyedWrite, context: any) { this.simple = false; }
+  visitKeyedWrite(ast: KeyedWrite, context: any) {}
 
   visitAll(asts: any[]): any[] { return asts.map(node => node.visit(this)); }
 
-  visitChain(ast: Chain, context: any) { this.simple = false; }
+  visitChain(ast: Chain, context: any) {}
 
-  visitQuote(ast: Quote, context: any) { this.simple = false; }
+  visitQuote(ast: Quote, context: any) {}
 }

modules/@angular/compiler/src/template_parser/template_parser.ts

@@ -277,11 +277,14 @@ class TemplateParseVisitor implements html.Visitor {
     }
   }
 
-  private _parseBinding(value: string, sourceSpan: ParseSourceSpan): ASTWithSource {
+  private _parseBinding(value: string, isHostBinding: boolean, sourceSpan: ParseSourceSpan):
+      ASTWithSource {
     const sourceInfo = sourceSpan.start.toString();
 
     try {
-      const ast = this._exprParser.parseBinding(value, sourceInfo, this._interpolationConfig);
+      const ast = isHostBinding ?
+          this._exprParser.parseSimpleBinding(value, sourceInfo, this._interpolationConfig) :
+          this._exprParser.parseBinding(value, sourceInfo, this._interpolationConfig);
       if (ast) this._reportParserErrors(ast.errors, sourceSpan);
       this._checkPipes(ast, sourceSpan);
       return ast;
@@ -668,7 +671,7 @@ class TemplateParseVisitor implements html.Visitor {
           targetAnimationProps);
     } else {
       this._parsePropertyAst(
-          name, this._parseBinding(expression, sourceSpan), sourceSpan, targetMatchableAttrs,
+          name, this._parseBinding(expression, false, sourceSpan), sourceSpan, targetMatchableAttrs,
           targetProps);
     }
   }
@@ -683,7 +686,7 @@ class TemplateParseVisitor implements html.Visitor {
       expression = 'null';
     }
 
-    const ast = this._parseBinding(expression, sourceSpan);
+    const ast = this._parseBinding(expression, false, sourceSpan);
     targetMatchableAttrs.push([name, ast.source]);
     targetAnimationProps.push(new BoundElementPropertyAst(
         name, PropertyBindingType.Animation, SecurityContext.NONE, ast, null, sourceSpan));
@@ -846,7 +849,7 @@ class TemplateParseVisitor implements html.Visitor {
       Object.keys(hostProps).forEach(propName => {
         const expression = hostProps[propName];
         if (typeof expression === 'string') {
-          const exprAst = this._parseBinding(expression, sourceSpan);
+          const exprAst = this._parseBinding(expression, true, sourceSpan);
           targetPropertyAsts.push(
               this._createElementPropertyAst(elementName, propName, exprAst, sourceSpan));
         } else {

modules/@angular/compiler/src/view_compiler/event_binder.ts

@@ -16,7 +16,7 @@ import {CompileBinding} from './compile_binding';
 import {CompileElement} from './compile_element';
 import {CompileMethod} from './compile_method';
 import {EventHandlerVars, ViewProperties} from './constants';
-import {convertCdStatementToIr} from './expression_converter';
+import {NoLocalsNameResolver, convertCdStatementToIr} from './expression_converter';
 
 export class CompileEventListener {
   private _method: CompileMethod;
@@ -62,7 +62,8 @@ export class CompileEventListener {
     this._method.resetDebugInfo(this.compileElement.nodeIndex, hostEvent);
     var context = directiveInstance || this.compileElement.view.componentContext;
     var actionStmts = convertCdStatementToIr(
-        this.compileElement.view, context, hostEvent.handler, this.compileElement.nodeIndex);
+        directive ? new NoLocalsNameResolver(this.compileElement.view) : this.compileElement.view,
+        context, hostEvent.handler, this.compileElement.nodeIndex);
     var lastIndex = actionStmts.length - 1;
     if (lastIndex >= 0) {
       var lastStatement = actionStmts[lastIndex];

modules/@angular/compiler/src/view_compiler/expression_converter.ts

@@ -11,6 +11,7 @@ import * as cdAst from '../expression_parser/ast';
 import {isBlank, isPresent} from '../facade/lang';
 import {Identifiers, resolveIdentifier} from '../identifiers';
 import * as o from '../output/output_ast';
+import {EventHandlerVars} from './constants';
 
 export interface NameResolver {
   callPipe(name: string, input: o.Expression, args: o.Expression[]): o.Expression;
@@ -19,6 +20,26 @@ export interface NameResolver {
   createLiteralMap(values: Array<Array<string|o.Expression>>): o.Expression;
 }
 
+/**
+ * A wrapper around another NameResolver that removes all locals and pipes.
+ */
+export class NoLocalsNameResolver implements NameResolver {
+  constructor(private _delegate: NameResolver) {}
+  callPipe(name: string, input: o.Expression, args: o.Expression[]): o.Expression { return null; }
+  getLocal(name: string): o.Expression {
+    if (name == EventHandlerVars.event.name) {
+      return EventHandlerVars.event;
+    }
+    return null;
+  }
+  createLiteralArray(values: o.Expression[]): o.Expression {
+    return this._delegate.createLiteralArray(values);
+  }
+  createLiteralMap(values: Array<Array<string|o.Expression>>): o.Expression {
+    return this._delegate.createLiteralMap(values);
+  }
+}
+
 export class ExpressionWithWrappedValueInfo {
   constructor(
       public expression: o.Expression, public needsValueUnwrapper: boolean,
@@ -170,6 +191,9 @@ class _AstToIrVisitor implements cdAst.AstVisitor {
     const input = this.visit(ast.exp, _Mode.Expression);
     const args = this.visitAll(ast.args, _Mode.Expression);
     const value = this._nameResolver.callPipe(ast.name, input, args);
+    if (!value) {
+      throw new Error(`Illegal state: Pipe ${ast.name} is not allowed here!`);
+    }
     this.needsValueUnwrapper = true;
     return convertToStatementIfNeeded(mode, this._valueUnwrapper.callMethod('unwrap', [value]));
   }

modules/@angular/compiler/src/view_compiler/property_binder.ts

@@ -22,7 +22,7 @@ import {CompileMethod} from './compile_method';
 import {CompileView} from './compile_view';
 import {DetectChangesVars, ViewProperties} from './constants';
 import {CompileEventListener} from './event_binder';
-import {convertCdExpressionToIr, temporaryDeclaration} from './expression_converter';
+import {NameResolver, NoLocalsNameResolver, convertCdExpressionToIr, temporaryDeclaration} from './expression_converter';
 
 function createBindFieldExpr(exprIndex: number): o.ReadPropExpr {
   return o.THIS_EXPR.prop(`_expr_${exprIndex}`);
@@ -38,9 +38,10 @@ class EvalResult {
 
 function evalCdAst(
     view: CompileView, currValExpr: o.ReadVarExpr, parsedExpression: cdAst.AST,
-    context: o.Expression, method: CompileMethod, bindingIndex: number): EvalResult {
+    context: o.Expression, nameResolver: NameResolver, method: CompileMethod,
+    bindingIndex: number): EvalResult {
   var checkExpression = convertCdExpressionToIr(
-      view, context, parsedExpression, DetectChangesVars.valUnwrapper, bindingIndex);
+      nameResolver, context, parsedExpression, DetectChangesVars.valUnwrapper, bindingIndex);
   if (!checkExpression.expression) {
     // e.g. an empty expression was given
     return null;
@@ -67,9 +68,10 @@ function evalCdAst(
 
 function bind(
     view: CompileView, currValExpr: o.ReadVarExpr, fieldExpr: o.ReadPropExpr,
-    parsedExpression: cdAst.AST, context: o.Expression, actions: o.Statement[],
+    parsedExpression: cdAst.AST, context: o.Expression, nameResolver: NameResolver,
-    method: CompileMethod, bindingIndex: number) {
+    actions: o.Statement[], method: CompileMethod, bindingIndex: number) {
-  const evalResult = evalCdAst(view, currValExpr, parsedExpression, context, method, bindingIndex);
+  const evalResult =
+      evalCdAst(view, currValExpr, parsedExpression, context, nameResolver, method, bindingIndex);
   if (!evalResult) {
     return;
   }
@@ -100,7 +102,7 @@ export function bindRenderText(
   view.detectChangesRenderPropertiesMethod.resetDebugInfo(compileNode.nodeIndex, boundText);
 
   bind(
-      view, currValExpr, valueField, boundText.value, view.componentContext,
+      view, currValExpr, valueField, boundText.value, view.componentContext, view,
       [o.THIS_EXPR.prop('renderer')
            .callMethod('setText', [compileNode.renderNode, currValExpr])
            .toStmt()],
@@ -205,7 +207,8 @@ function bindAndWriteToRenderer(
     }
 
     bind(
-        view, currValExpr, fieldExpr, boundProp.value, context, updateStmts, compileMethod,
+        view, currValExpr, fieldExpr, boundProp.value, context,
+        isHostProp ? new NoLocalsNameResolver(view) : view, updateStmts, compileMethod,
         view.bindings.length);
   });
 }
@@ -267,7 +270,7 @@ export function bindDirectiveInputs(
     detectChangesInInputsMethod.resetDebugInfo(compileElement.nodeIndex, input);
     var currValExpr = createCurrValueExpr(bindingIndex);
     const evalResult = evalCdAst(
-        view, currValExpr, input.value, view.componentContext, detectChangesInInputsMethod,
+        view, currValExpr, input.value, view.componentContext, view, detectChangesInInputsMethod,
         bindingIndex);
     if (!evalResult) {
       return;

modules/@angular/compiler/test/expression_parser/parser_spec.ts

@@ -74,7 +74,10 @@ export function main() {
         return;
       }
     }
-    throw Error(`Expected an error containing "${message}" to be reported`);
+    const errMsgs = ast.errors.map(err => err.message).join('\n');
+    throw Error(
+        `Expected an error containing "${message}" to be reported, but got the errors:\n` +
+        errMsgs);
   }
 
   function expectActionError(text: string, message: string) {
@@ -504,16 +507,10 @@ export function main() {
         validate(p);
       });
 
-      it('should parse a constant', () => {
+      it('should report when encountering pipes', () => {
-        var p = parseSimpleBinding('[1, 2]');
-        expect(unparse(p)).toEqual('[1, 2]');
-        validate(p);
-      });
-
-      it('should report when the given expression is not just a field name', () => {
         expectError(
-            validate(parseSimpleBinding('name + 1')),
+            validate(parseSimpleBinding('a | somePipe')),
-            'Host binding expression can only contain field access and constants');
+            'Host binding expression cannot contain pipes');
       });
 
       it('should report when encountering interpolation', () => {
@@ -521,6 +518,10 @@ export function main() {
             validate(parseSimpleBinding('{{exp}}')),
             'Got interpolation ({{}}) where expression was expected');
       });
+
+      it('should report when encountering field write', () => {
+        expectError(validate(parseSimpleBinding('a = b')), 'Bindings cannot contain assignments');
+      });
     });
 
     describe('wrapLiteralPrimitive', () => {

modules/@angular/core/test/linker/integration_spec.ts

@@ -826,6 +826,73 @@ function declareTests({useJit}: {useJit: boolean}) {
         expect(tc.nativeElement.id).toEqual('newId');
       });
 
+      it('should not use template variables for expressions in hostProperties', () => {
+        @Directive({selector: '[host-properties]', host: {'[id]': 'id', '[title]': 'unknownProp'}})
+        class DirectiveWithHostProps {
+          id = 'one';
+        }
+
+        const fixture =
+            TestBed.configureTestingModule({declarations: [MyComp, DirectiveWithHostProps]})
+                .overrideComponent(
+                    MyComp,
+                    {set: {template: `<div *ngFor="let id of ['forId']" host-properties></div>`}})
+                .createComponent(MyComp);
+        fixture.detectChanges();
+
+        var tc = fixture.debugElement.children[0];
+        expect(tc.properties['id']).toBe('one');
+        expect(tc.properties['title']).toBe(undefined);
+      });
+
+      it('should not allow pipes in hostProperties', () => {
+        @Directive({selector: '[host-properties]', host: {'[id]': 'id | uppercase'}})
+        class DirectiveWithHostProps {
+        }
+
+        TestBed.configureTestingModule({declarations: [MyComp, DirectiveWithHostProps]});
+        const template = '<div host-properties></div>';
+        TestBed.overrideComponent(MyComp, {set: {template}});
+        expect(() => TestBed.createComponent(MyComp))
+            .toThrowError(/Host binding expression cannot contain pipes/);
+      });
+
+      it('should not use template variables for expressions in hostListeners', () => {
+        @Directive({selector: '[host-listener]', host: {'(click)': 'doIt(id, unknownProp)'}})
+        class DirectiveWithHostListener {
+          id = 'one';
+          receivedArgs: any[];
+
+          doIt(...args: any[]) { this.receivedArgs = args; }
+        }
+
+        const fixture =
+            TestBed.configureTestingModule({declarations: [MyComp, DirectiveWithHostListener]})
+                .overrideComponent(
+                    MyComp,
+                    {set: {template: `<div *ngFor="let id of ['forId']" host-listener></div>`}})
+                .createComponent(MyComp);
+        fixture.detectChanges();
+        var tc = fixture.debugElement.children[0];
+        tc.triggerEventHandler('click', {});
+        var dir: DirectiveWithHostListener = tc.injector.get(DirectiveWithHostListener);
+        expect(dir.receivedArgs).toEqual(['one', undefined]);
+      });
+
+      it('should not allow pipes in hostListeners', () => {
+        @Directive({selector: '[host-listener]', host: {'(click)': 'doIt() | somePipe'}})
+        class DirectiveWithHostListener {
+        }
+
+        TestBed.configureTestingModule({declarations: [MyComp, DirectiveWithHostListener]});
+        const template = '<div host-listener></div>';
+        TestBed.overrideComponent(MyComp, {set: {template}});
+        expect(() => TestBed.createComponent(MyComp))
+            .toThrowError(/Cannot have a pipe in an action expression/);
+      });
+
+
+
       if (getDOM().supportsDOMEvents()) {
         it('should support preventing default on render events', () => {
           TestBed.configureTestingModule({