From 5dbd8de7295b98811eaa1e469d514eab89c4f579 Mon Sep 17 00:00:00 2001 From: Lucas Mirelmann Date: Tue, 26 Jan 2016 15:11:52 +0100 Subject: [PATCH] fix($parse): Preserve expensive checks when runnning $eval inside an expression When running an expression with expensive checks, there is a call to `$eval` or `$evalAsync` then that expression is also evaluated using expensive checks --- src/ng/parse.js | 39 ++++++++++++++++++-- src/ng/rootScope.js | 3 +- test/ng/parseSpec.js | 78 +++++++++++++++++++++++++++++++++++++++- test/ng/rootScopeSpec.js | 8 ++--- 4 files changed, 120 insertions(+), 8 deletions(-) diff --git a/src/ng/parse.js b/src/ng/parse.js index cddde467bc59..7905c206a985 100644 --- a/src/ng/parse.js +++ b/src/ng/parse.js @@ -1757,10 +1757,19 @@ function $ParseProvider() { csp: noUnsafeEval, expensiveChecks: true }; + var runningChecksEnabled = false; - return function $parse(exp, interceptorFn, expensiveChecks) { + $parse.$$runningExpensiveChecks = function() { + return runningChecksEnabled; + }; + + return $parse; + + function $parse(exp, interceptorFn, expensiveChecks) { var parsedExpression, oneTime, cacheKey; + expensiveChecks = expensiveChecks || runningChecksEnabled; + switch (typeof exp) { case 'string': exp = exp.trim(); @@ -1786,6 +1795,9 @@ function $ParseProvider() { } else if (parsedExpression.inputs) { parsedExpression.$$watchDelegate = inputsWatchDelegate; } + if (expensiveChecks) { + parsedExpression = expensiveChecksInterceptor(parsedExpression); + } cache[cacheKey] = parsedExpression; } return addInterceptor(parsedExpression, interceptorFn); @@ -1796,7 +1808,30 @@ function $ParseProvider() { default: return addInterceptor(noop, interceptorFn); } - }; + } + + function expensiveChecksInterceptor(fn) { + if (!fn) return fn; + expensiveCheckFn.$$watchDelegate = fn.$$watchDelegate; + expensiveCheckFn.assign = expensiveChecksInterceptor(fn.assign); + expensiveCheckFn.constant = fn.constant; + expensiveCheckFn.literal = fn.literal; + for (var i = 0; fn.inputs && i < fn.inputs.length; ++i) { + fn.inputs[i] = expensiveChecksInterceptor(fn.inputs[i]); + } + + return expensiveCheckFn; + + function expensiveCheckFn(scope, locals, assign, inputs) { + var expensiveCheckOldValue = runningChecksEnabled; + runningChecksEnabled = true; + try { + return fn(scope, locals, assign, inputs); + } finally { + runningChecksEnabled = expensiveCheckOldValue; + } + } + } function expressionInputDirtyCheck(newValue, oldValueOfValue) { diff --git a/src/ng/rootScope.js b/src/ng/rootScope.js index 3658d376e0a1..8c8b2b7bdfab 100644 --- a/src/ng/rootScope.js +++ b/src/ng/rootScope.js @@ -998,7 +998,7 @@ function $RootScopeProvider() { }); } - asyncQueue.push({scope: this, expression: expr, locals: locals}); + asyncQueue.push({scope: this, expression: $parse(expr), locals: locals}); }, $$postDigest: function(fn) { @@ -1090,6 +1090,7 @@ function $RootScopeProvider() { $applyAsync: function(expr) { var scope = this; expr && applyAsyncQueue.push($applyAsyncExpression); + expr = $parse(expr); scheduleApplyAsync(); function $applyAsyncExpression() { diff --git a/test/ng/parseSpec.js b/test/ng/parseSpec.js index a11cb6c06d0b..355c922436c4 100644 --- a/test/ng/parseSpec.js +++ b/test/ng/parseSpec.js @@ -1681,7 +1681,6 @@ describe('parser', function() { $filterProvider = filterProvider; }])); - forEach([true, false], function(cspEnabled) { describe('csp: ' + cspEnabled, function() { @@ -2400,6 +2399,64 @@ describe('parser', function() { '$parse', 'isecwindow', 'Referencing the Window in Angular expressions is disallowed! ' + 'Expression: foo.w = 1'); })); + + they('should propagate expensive checks when calling $prop', + ['foo.w && true', + '$eval("foo.w && true")', + 'this["$eval"]("foo.w && true")', + 'bar;$eval("foo.w && true")', + '$eval("foo.w && true");bar', + '$eval("foo.w && true", null, false)', + '$eval("foo");$eval("foo.w && true")', + '$eval("$eval(\\"foo.w && true\\")")', + '$eval("foo.e()")', + '$evalAsync("foo.w && true")', + 'this["$evalAsync"]("foo.w && true")', + 'bar;$evalAsync("foo.w && true")', + '$evalAsync("foo.w && true");bar', + '$evalAsync("foo.w && true", null, false)', + '$evalAsync("foo");$evalAsync("foo.w && true")', + '$evalAsync("$evalAsync(\\"foo.w && true\\")")', + '$evalAsync("foo.e()")', + '$evalAsync("$eval(\\"foo.w && true\\")")', + '$eval("$evalAsync(\\"foo.w && true\\")")', + '$watch("foo.w && true")', + '$watchCollection("foo.w && true", foo.f)', + '$watchGroup(["foo.w && true"])', + '$applyAsync("foo.w && true")'], function(expression) { + inject(function($parse, $window) { + scope.foo = { + w: $window, + bar: 'bar', + e: function() { scope.$eval("foo.w && true"); }, + f: function() {} + }; + expect($parse.$$runningExpensiveChecks()).toEqual(false); + expect(function() { + scope.$eval($parse(expression, null, true)); + scope.$digest(); + }).toThrowMinErr( + '$parse', 'isecwindow', 'Referencing the Window in Angular expressions is disallowed! ' + + 'Expression: foo.w && true'); + expect($parse.$$runningExpensiveChecks()).toEqual(false); + }); + }); + + they('should restore the state of $$runningExpensiveChecks when the expression $prop throws', + ['$eval("foo.t()")', + '$evalAsync("foo.t()", {foo: foo})'], function(expression) { + inject(function($parse, $window) { + scope.foo = { + t: function() { throw new Error(); } + }; + expect($parse.$$runningExpensiveChecks()).toEqual(false); + expect(function() { + scope.$eval($parse(expression, null, true)); + scope.$digest(); + }).toThrow(); + expect($parse.$$runningExpensiveChecks()).toEqual(false); + }); + }); }); }); @@ -2966,6 +3023,25 @@ describe('parser', function() { expect(log).toEqual(''); })); + it('should work with expensive checks', inject(function($parse, $rootScope, log) { + var fn = $parse('::foo', null, true); + $rootScope.$watch(fn, function(value, old) { if (value !== old) log(value); }); + + $rootScope.$digest(); + expect($rootScope.$$watchers.length).toBe(1); + + $rootScope.foo = 'bar'; + $rootScope.$digest(); + expect($rootScope.$$watchers.length).toBe(0); + expect(log).toEqual('bar'); + log.reset(); + + $rootScope.foo = 'man'; + $rootScope.$digest(); + expect($rootScope.$$watchers.length).toBe(0); + expect(log).toEqual(''); + })); + it('should have a stable value if at the end of a $digest it has a defined value', inject(function($parse, $rootScope, log) { var fn = $parse('::foo'); $rootScope.$watch(fn, function(value, old) { if (value !== old) log(value); }); diff --git a/test/ng/rootScopeSpec.js b/test/ng/rootScopeSpec.js index 015db507ebb5..7d9d6ecf3de1 100644 --- a/test/ng/rootScopeSpec.js +++ b/test/ng/rootScopeSpec.js @@ -1387,7 +1387,7 @@ describe('Scope', function() { expect(child.log).toBe('child context'); })); - it('should operate only with a single queue across all child and isolate scopes', inject(function($rootScope) { + it('should operate only with a single queue across all child and isolate scopes', inject(function($rootScope, $parse) { var childScope = $rootScope.$new(); var isolateScope = $rootScope.$new(true); @@ -1398,9 +1398,9 @@ describe('Scope', function() { expect(childScope.$$asyncQueue).toBe($rootScope.$$asyncQueue); expect(isolateScope.$$asyncQueue).toBeUndefined(); expect($rootScope.$$asyncQueue).toEqual([ - {scope: $rootScope, expression: 'rootExpression'}, - {scope: childScope, expression: 'childExpression'}, - {scope: isolateScope, expression: 'isolateExpression'} + {scope: $rootScope, expression: $parse('rootExpression')}, + {scope: childScope, expression: $parse('childExpression')}, + {scope: isolateScope, expression: $parse('isolateExpression')} ]); }));