Skip to content

Commit 96d62cc

Browse files
lgalfasogkalpak
lgalfaso
authored and
gkalpak
committed
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 Closes: angular#13850
1 parent 5cb7d0e commit 96d62cc

4 files changed

Lines changed: 120 additions & 8 deletions

File tree

‎src/ng/parse.js‎

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1735,10 +1735,19 @@ function $ParseProvider() {
17351735
csp: noUnsafeEval,
17361736
expensiveChecks: true
17371737
};
1738+
var runningChecksEnabled = false;
17381739

1739-
return function $parse(exp, interceptorFn, expensiveChecks) {
1740+
$parse.$$runningExpensiveChecks = function() {
1741+
return runningChecksEnabled;
1742+
};
1743+
1744+
return $parse;
1745+
1746+
function $parse(exp, interceptorFn, expensiveChecks) {
17401747
var parsedExpression, oneTime, cacheKey;
17411748

1749+
expensiveChecks = expensiveChecks || runningChecksEnabled;
1750+
17421751
switch (typeof exp) {
17431752
case 'string':
17441753
exp = exp.trim();
@@ -1764,6 +1773,9 @@ function $ParseProvider() {
17641773
} else if (parsedExpression.inputs) {
17651774
parsedExpression.$$watchDelegate = inputsWatchDelegate;
17661775
}
1776+
if (expensiveChecks) {
1777+
parsedExpression = expensiveChecksInterceptor(parsedExpression);
1778+
}
17671779
cache[cacheKey] = parsedExpression;
17681780
}
17691781
return addInterceptor(parsedExpression, interceptorFn);
@@ -1774,7 +1786,30 @@ function $ParseProvider() {
17741786
default:
17751787
return addInterceptor(noop, interceptorFn);
17761788
}
1777-
};
1789+
}
1790+
1791+
function expensiveChecksInterceptor(fn) {
1792+
if (!fn) return fn;
1793+
expensiveCheckFn.$$watchDelegate = fn.$$watchDelegate;
1794+
expensiveCheckFn.assign = expensiveChecksInterceptor(fn.assign);
1795+
expensiveCheckFn.constant = fn.constant;
1796+
expensiveCheckFn.literal = fn.literal;
1797+
for (var i = 0; fn.inputs && i < fn.inputs.length; ++i) {
1798+
fn.inputs[i] = expensiveChecksInterceptor(fn.inputs[i]);
1799+
}
1800+
1801+
return expensiveCheckFn;
1802+
1803+
function expensiveCheckFn(scope, locals, assign, inputs) {
1804+
var expensiveCheckOldValue = runningChecksEnabled;
1805+
runningChecksEnabled = true;
1806+
try {
1807+
return fn(scope, locals, assign, inputs);
1808+
} finally {
1809+
runningChecksEnabled = expensiveCheckOldValue;
1810+
}
1811+
}
1812+
}
17781813

17791814
function expressionInputDirtyCheck(newValue, oldValueOfValue) {
17801815

‎src/ng/rootScope.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -998,7 +998,7 @@ function $RootScopeProvider() {
998998
});
999999
}
10001000

1001-
asyncQueue.push({scope: this, expression: expr, locals: locals});
1001+
asyncQueue.push({scope: this, expression: $parse(expr), locals: locals});
10021002
},
10031003

10041004
$$postDigest: function(fn) {
@@ -1090,6 +1090,7 @@ function $RootScopeProvider() {
10901090
$applyAsync: function(expr) {
10911091
var scope = this;
10921092
expr && applyAsyncQueue.push($applyAsyncExpression);
1093+
expr = $parse(expr);
10931094
scheduleApplyAsync();
10941095

10951096
function $applyAsyncExpression() {

‎test/ng/parseSpec.js‎

Lines changed: 77 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1666,7 +1666,6 @@ describe('parser', function() {
16661666
$filterProvider = filterProvider;
16671667
}]));
16681668

1669-
16701669
forEach([true, false], function(cspEnabled) {
16711670
describe('csp: ' + cspEnabled, function() {
16721671

@@ -2372,6 +2371,64 @@ describe('parser', function() {
23722371
'$parse', 'isecwindow', 'Referencing the Window in Angular expressions is disallowed! ' +
23732372
'Expression: foo.w = 1');
23742373
}));
2374+
2375+
they('should propagate expensive checks when calling $prop',
2376+
['foo.w && true',
2377+
'$eval("foo.w && true")',
2378+
'this["$eval"]("foo.w && true")',
2379+
'bar;$eval("foo.w && true")',
2380+
'$eval("foo.w && true");bar',
2381+
'$eval("foo.w && true", null, false)',
2382+
'$eval("foo");$eval("foo.w && true")',
2383+
'$eval("$eval(\\"foo.w && true\\")")',
2384+
'$eval("foo.e()")',
2385+
'$evalAsync("foo.w && true")',
2386+
'this["$evalAsync"]("foo.w && true")',
2387+
'bar;$evalAsync("foo.w && true")',
2388+
'$evalAsync("foo.w && true");bar',
2389+
'$evalAsync("foo.w && true", null, false)',
2390+
'$evalAsync("foo");$evalAsync("foo.w && true")',
2391+
'$evalAsync("$evalAsync(\\"foo.w && true\\")")',
2392+
'$evalAsync("foo.e()")',
2393+
'$evalAsync("$eval(\\"foo.w && true\\")")',
2394+
'$eval("$evalAsync(\\"foo.w && true\\")")',
2395+
'$watch("foo.w && true")',
2396+
'$watchCollection("foo.w && true", foo.f)',
2397+
'$watchGroup(["foo.w && true"])',
2398+
'$applyAsync("foo.w && true")'], function(expression) {
2399+
inject(function($parse, $window) {
2400+
scope.foo = {
2401+
w: $window,
2402+
bar: 'bar',
2403+
e: function() { scope.$eval("foo.w && true"); },
2404+
f: function() {}
2405+
};
2406+
expect($parse.$$runningExpensiveChecks()).toEqual(false);
2407+
expect(function() {
2408+
scope.$eval($parse(expression, null, true));
2409+
scope.$digest();
2410+
}).toThrowMinErr(
2411+
'$parse', 'isecwindow', 'Referencing the Window in Angular expressions is disallowed! ' +
2412+
'Expression: foo.w && true');
2413+
expect($parse.$$runningExpensiveChecks()).toEqual(false);
2414+
});
2415+
});
2416+
2417+
they('should restore the state of $$runningExpensiveChecks when the expression $prop throws',
2418+
['$eval("foo.t()")',
2419+
'$evalAsync("foo.t()", {foo: foo})'], function(expression) {
2420+
inject(function($parse, $window) {
2421+
scope.foo = {
2422+
t: function() { throw new Error(); }
2423+
};
2424+
expect($parse.$$runningExpensiveChecks()).toEqual(false);
2425+
expect(function() {
2426+
scope.$eval($parse(expression, null, true));
2427+
scope.$digest();
2428+
}).toThrow();
2429+
expect($parse.$$runningExpensiveChecks()).toEqual(false);
2430+
});
2431+
});
23752432
});
23762433
});
23772434

@@ -2932,6 +2989,25 @@ describe('parser', function() {
29322989
expect(log).toEqual('');
29332990
}));
29342991

2992+
it('should work with expensive checks', inject(function($parse, $rootScope, log) {
2993+
var fn = $parse('::foo', null, true);
2994+
$rootScope.$watch(fn, function(value, old) { if (value !== old) log(value); });
2995+
2996+
$rootScope.$digest();
2997+
expect($rootScope.$$watchers.length).toBe(1);
2998+
2999+
$rootScope.foo = 'bar';
3000+
$rootScope.$digest();
3001+
expect($rootScope.$$watchers.length).toBe(0);
3002+
expect(log).toEqual('bar');
3003+
log.reset();
3004+
3005+
$rootScope.foo = 'man';
3006+
$rootScope.$digest();
3007+
expect($rootScope.$$watchers.length).toBe(0);
3008+
expect(log).toEqual('');
3009+
}));
3010+
29353011
it('should have a stable value if at the end of a $digest it has a defined value', inject(function($parse, $rootScope, log) {
29363012
var fn = $parse('::foo');
29373013
$rootScope.$watch(fn, function(value, old) { if (value !== old) log(value); });

‎test/ng/rootScopeSpec.js‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1387,7 +1387,7 @@ describe('Scope', function() {
13871387
expect(child.log).toBe('child context');
13881388
}));
13891389

1390-
it('should operate only with a single queue across all child and isolate scopes', inject(function($rootScope) {
1390+
it('should operate only with a single queue across all child and isolate scopes', inject(function($rootScope, $parse) {
13911391
var childScope = $rootScope.$new();
13921392
var isolateScope = $rootScope.$new(true);
13931393

@@ -1398,9 +1398,9 @@ describe('Scope', function() {
13981398
expect(childScope.$$asyncQueue).toBe($rootScope.$$asyncQueue);
13991399
expect(isolateScope.$$asyncQueue).toBeUndefined();
14001400
expect($rootScope.$$asyncQueue).toEqual([
1401-
{scope: $rootScope, expression: 'rootExpression'},
1402-
{scope: childScope, expression: 'childExpression'},
1403-
{scope: isolateScope, expression: 'isolateExpression'}
1401+
{scope: $rootScope, expression: $parse('rootExpression')},
1402+
{scope: childScope, expression: $parse('childExpression')},
1403+
{scope: isolateScope, expression: $parse('isolateExpression')}
14041404
]);
14051405
}));
14061406

0 commit comments

Comments
 (0)