Skip to content

Commit

Permalink
fix($parse): fix CSP nested property evaluation, and issue that preve…
Browse files Browse the repository at this point in the history 
…nted its tests from failing

cspSafeGetterFn incorrectly returned undefined if any of its key parameters were undefined. This
wasn't caught by the $parse unit tests because of a timing problem where $ParseProvider was reading
the CSP flag before the tests manually set it, so the CSP property evaluation tests never ran. Add
test that verifies evaluation of nested properties of multiple lengths.

Closes angular#5591
Closes angular#5592
  • Loading branch information
@dozingcat @jamesdaily
dozingcat authored and jamesdaily committed Jan 27, 2014
1 parent 264a644 commit 7c67041
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 13 deletions.
24 changes: 16 additions & 8 deletions src/ng/parse.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down
63 changes: 58 additions & 5 deletions test/ng/parseSpec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -204,13 +204,24 @@ describe('parser', function() {

describe('csp: ' + cspEnabled + ", unwrapPromises: " + unwrapPromisesEnabled, function() {

beforeEach(module(function ($parseProvider) {
var originalSecurityPolicy;


beforeEach(function() {
originalSecurityPolicy = window.document.securityPolicy;
window.document.securityPolicy = {isActive : cspEnabled};
});

afterEach(function() {
window.document.securityPolicy = originalSecurityPolicy;
});

beforeEach(module(function ($parseProvider, $provide) {
$parseProvider.unwrapPromises(unwrapPromisesEnabled);
}));

beforeEach(inject(function ($rootScope, $sniffer) {
beforeEach(inject(function ($rootScope) {
scope = $rootScope;
$sniffer.csp = cspEnabled;
}));

it('should parse expressions', function() {
Expand Down Expand Up @@ -344,6 +355,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();
Expand Down Expand Up @@ -1069,6 +1099,17 @@ describe('parser', function() {

var $log;
var PROMISE_WARNING_REGEXP = /\[\$parse\] Promise found in the expression `[^`]+`. Automatic unwrapping of promises in Angular expressions is deprecated\./;
var originalSecurityPolicy;


beforeEach(function() {
originalSecurityPolicy = window.document.securityPolicy;
window.document.securityPolicy = {isActive : cspEnabled};
});

afterEach(function() {
window.document.securityPolicy = originalSecurityPolicy;
});

beforeEach(module(function($parseProvider) {
$parseProvider.unwrapPromises(true);
Expand Down Expand Up @@ -1142,15 +1183,27 @@ describe('parser', function() {

describe('csp ' + cspEnabled, function() {

var originalSecurityPolicy;


beforeEach(function() {
originalSecurityPolicy = window.document.securityPolicy;
window.document.securityPolicy = {isActive : cspEnabled};
});

afterEach(function() {
window.document.securityPolicy = originalSecurityPolicy;
});


beforeEach(module(function($parseProvider) {
$parseProvider.unwrapPromises(true);
$parseProvider.logPromiseWarnings(false);
}));


beforeEach(inject(function($rootScope, $sniffer, $q) {
beforeEach(inject(function($rootScope, $q) {
scope = $rootScope;
$sniffer.csp = cspEnabled;

q = $q;
deferred = q.defer();
Expand Down

0 comments on commit 7c67041

Please sign in to comment.