Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

fix($parse): fix CSP nested property evaluation, and issue that prevente... #5592

Closed
wants to merge 7 commits into from
29 changes: 19 additions & 10 deletions src/ng/parse.js
Original file line number Diff line number Diff line change
@@ -894,16 +894,20 @@ function cspSafeGetterFn(key0, key1, key2, key3, key4, fullExp, options) {
if (pathVal == null) return pathVal;
pathVal = pathVal[key0];

if (pathVal == null) return key1 ? undefined : pathVal;
if (!key1) return pathVal;
if (pathVal == null) return undefined;
pathVal = pathVal[key1];

if (pathVal == null) return key2 ? undefined : pathVal;
if (!key2) return pathVal;
if (pathVal == null) return undefined;
pathVal = pathVal[key2];

if (pathVal == null) return key3 ? undefined : pathVal;
if (!key3) return pathVal;
if (pathVal == null) return undefined;
pathVal = pathVal[key3];

if (pathVal == null) return key4 ? undefined : pathVal;
if (!key4) return pathVal;
if (pathVal == null) return undefined;
pathVal = pathVal[key4];

return pathVal;
@@ -924,8 +928,9 @@ function cspSafeGetterFn(key0, key1, key2, key3, key4, fullExp, options) {
}
pathVal = pathVal.$$v;
}
if (pathVal == null) return key1 ? undefined : pathVal;

if (!key1) return pathVal;
if (pathVal == null) return undefined;
pathVal = pathVal[key1];
if (pathVal && pathVal.then) {
promiseWarning(fullExp);
@@ -936,8 +941,9 @@ function cspSafeGetterFn(key0, key1, key2, key3, key4, fullExp, options) {
}
pathVal = pathVal.$$v;
}
if (pathVal == null) return key2 ? undefined : pathVal;

if (!key2) return pathVal;
if (pathVal == null) return undefined;
pathVal = pathVal[key2];
if (pathVal && pathVal.then) {
promiseWarning(fullExp);
@@ -948,8 +954,9 @@ function cspSafeGetterFn(key0, key1, key2, key3, key4, fullExp, options) {
}
pathVal = pathVal.$$v;
}
if (pathVal == null) return key3 ? undefined : pathVal;

if (!key3) return pathVal;
if (pathVal == null) return undefined;
pathVal = pathVal[key3];
if (pathVal && pathVal.then) {
promiseWarning(fullExp);
@@ -960,8 +967,9 @@ function cspSafeGetterFn(key0, key1, key2, key3, key4, fullExp, options) {
}
pathVal = pathVal.$$v;
}
if (pathVal == null) return key4 ? undefined : pathVal;

if (!key4) return pathVal;
if (pathVal == null) return undefined;
pathVal = pathVal[key4];
if (pathVal && pathVal.then) {
promiseWarning(fullExp);
@@ -1218,8 +1226,6 @@ function $ParseProvider() {


this.$get = ['$filter', '$sniffer', '$log', function($filter, $sniffer, $log) {
$parseOptions.csp = $sniffer.csp;

promiseWarning = function promiseWarningFn(fullExp) {
if (!$parseOptions.logPromiseWarnings || promiseWarningCache.hasOwnProperty(fullExp)) return;
promiseWarningCache[fullExp] = true;
@@ -1237,6 +1243,9 @@ function $ParseProvider() {
return cache[exp];
}

// The csp option has to be set here because in tests the $sniffer service sets its csp
// property after $get has executed.
$parseOptions.csp = $sniffer.csp;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm still not sure about this though, it seems weird to read this every time $parse is called. If it's the best we can do then that's one thing, but is there not a better solution?

var lexer = new Lexer($parseOptions);
var parser = new Parser(lexer, $filter, $parseOptions);
parsedExpression = parser.parse(exp, false);
19 changes: 19 additions & 0 deletions test/ng/parseSpec.js
Original file line number Diff line number Diff line change
@@ -344,6 +344,25 @@ describe('parser', function() {
expect(scope.$eval("a.b.c.d.e.f.g.h.i.j.k.l.m.n", scope)).toBe('nooo!');
});

forEach([2, 3, 4, 5, 6, 7, 8, 9, 10, 20, 42, 99], function(pathLength) {
it('should resolve nested paths of length ' + pathLength, function() {
// Create a nested object {x2: {x3: {x4: ... {x[n]: 42} ... }}}.
var obj = 42;
for (var i = pathLength; i >= 2; i--) {
var newObj = {};
newObj['x' + i] = obj;
obj = newObj;
}
// Assign to x1 and build path 'x1.x2.x3. ... .x[n]' to access the final value.
scope.x1 = obj;
var path = 'x1';
for (var i = 2; i <= pathLength; i++) {
path += '.x' + i;
}
expect(scope.$eval(path)).toBe(42);
});
});

it('should be forgiving', function() {
scope.a = {b: 23};
expect(scope.$eval('b')).toBeUndefined();