Skip to content

Commit

Permalink
fix($parse): validate assignment lval in parser phase
Browse files Browse the repository at this point in the history
The parser always threw an error in the case of an invalid left-value
assignment but it was an unhelpful:

```
Cannot set property 'undefined' of undefined
```

This commit provides a more meaningful error message, so it is not a
breaking change.

Closes angular#15234
  • Loading branch information
jbedard authored and petebacondarwin committed Oct 10, 2016
1 parent faf0c3e commit a02c886
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 3 deletions.
7 changes: 4 additions & 3 deletions src/ng/parse.js
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,10 @@ AST.prototype = {
assignment: function() {
var result = this.ternary();
if (this.expect('=')) {
if (!isAssignable(result)) {
throw $parseMinErr('lval', 'Trying to assign a value to a non l-value');
}

result = { type: AST.AssignmentExpression, left: result, right: this.assignment(), operator: '='};
}
return result;
Expand Down Expand Up @@ -1024,9 +1028,6 @@ ASTCompiler.prototype = {
case AST.AssignmentExpression:
right = this.nextId();
left = {};
if (!isAssignable(ast.left)) {
throw $parseMinErr('lval', 'Trying to assign a value to a non l-value');
}
this.recurse(ast.left, undefined, left, function() {
self.if_(self.notNull(left.context), function() {
self.recurse(ast.right, right);
Expand Down
14 changes: 14 additions & 0 deletions test/ng/parseSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2129,6 +2129,20 @@ describe('parser', function() {
expect(scope.b).toEqual(234);
});

it('should throw with invalid left-val in assignments', function() {
expect(function() { scope.$eval('1 = 1'); }).toThrowMinErr('$parse', 'lval');
expect(function() { scope.$eval('{} = 1'); }).toThrowMinErr('$parse', 'lval');
expect(function() { scope.$eval('[] = 1'); }).toThrowMinErr('$parse', 'lval');
expect(function() { scope.$eval('true = 1'); }).toThrowMinErr('$parse', 'lval');
expect(function() { scope.$eval('(a=b) = 1'); }).toThrowMinErr('$parse', 'lval');
expect(function() { scope.$eval('(1<2) = 1'); }).toThrowMinErr('$parse', 'lval');
expect(function() { scope.$eval('(1+2) = 1'); }).toThrowMinErr('$parse', 'lval');
expect(function() { scope.$eval('!v = 1'); }).toThrowMinErr('$parse', 'lval');
expect(function() { scope.$eval('this = 1'); }).toThrowMinErr('$parse', 'lval');
expect(function() { scope.$eval('+v = 1'); }).toThrowMinErr('$parse', 'lval');
expect(function() { scope.$eval('(1?v1:v2) = 1'); }).toThrowMinErr('$parse', 'lval');
});

it('should evaluate assignments in ternary operator', function() {
scope.$eval('a = 1 ? 2 : 3');
expect(scope.a).toBe(2);
Expand Down

0 comments on commit a02c886

Please sign in to comment.