Skip to content

Commit

Permalink
fix($parse): return 'undefined' if a middle key's value is null
Browse files Browse the repository at this point in the history
Prior to this fix, $parse/$eval would return 'null' if a middle key in
an expression's value is null, when it should be expected to be undefined.

This patch tries to remedy this by returning undefined for middle values in
expressions, when fetching a child of that null value.

For example:

```js
// Given the following object:
$scope.a = {
  b: null
};

// $scope.$eval('a.b.c') returns undefined, whereas previously it would return null
```

Closes angular#5480
  • Loading branch information
Caitlin Potter authored and jamesdaily committed Jan 27, 2014
1 parent 28f3e34 commit 794724d
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 13 deletions.
25 changes: 12 additions & 13 deletions src/ng/parse.js
Original file line number Diff line number Diff line change
Expand Up @@ -894,16 +894,16 @@ function cspSafeGetterFn(key0, key1, key2, key3, key4, fullExp, options) {
if (pathVal == null) return pathVal;
pathVal = pathVal[key0];

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

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

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

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

return pathVal;
Expand All @@ -924,7 +924,7 @@ function cspSafeGetterFn(key0, key1, key2, key3, key4, fullExp, options) {
}
pathVal = pathVal.$$v;
}
if (!key1 || pathVal == null) return pathVal;
if (pathVal == null) return key1 ? undefined : pathVal;

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

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

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

pathVal = pathVal[key4];
if (pathVal && pathVal.then) {
Expand All @@ -980,7 +980,7 @@ function simpleGetterFn1(key0, fullExp) {
ensureSafeMemberName(key0, fullExp);

return function simpleGetterFn1(scope, locals) {
if (scope == null) return scope;
if (scope == null) return undefined;
return ((locals && locals.hasOwnProperty(key0)) ? locals : scope)[key0];
};
}
Expand All @@ -990,10 +990,9 @@ function simpleGetterFn2(key0, key1, fullExp) {
ensureSafeMemberName(key1, fullExp);

return function simpleGetterFn2(scope, locals) {
if (scope == null) return scope;
if (scope == null) return undefined;
scope = ((locals && locals.hasOwnProperty(key0)) ? locals : scope)[key0];

return scope == null ? scope : scope[key1];
return scope == null ? undefined : scope[key1];
};
}

Expand Down Expand Up @@ -1036,7 +1035,7 @@ function getterFn(path, options, fullExp) {
var code = 'var p;\n';
forEach(pathKeys, function(key, index) {
ensureSafeMemberName(key, fullExp);
code += 'if(s == null) return s;\n' +
code += 'if(s == null) return undefined;\n' +
's='+ (index
// we simply dereference 's' on any .dot notation
? 's'
Expand Down
49 changes: 49 additions & 0 deletions test/ng/parseSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -977,6 +977,55 @@ describe('parser', function() {
expect($parse('"name" + id').constant).toBe(false);
}));
});

describe('nulls in expressions', function() {
// simpleGetterFn1
it('should return null for `a` where `a` is null', inject(function($rootScope) {
$rootScope.a = null;
expect($rootScope.$eval('a')).toBe(null);
}));

it('should return undefined for `a` where `a` is undefined', inject(function($rootScope) {
expect($rootScope.$eval('a')).toBeUndefined();
}));

// simpleGetterFn2
it('should return undefined for properties of `null` constant', inject(function($rootScope) {
expect($rootScope.$eval('null.a')).toBeUndefined();
}));

it('should return undefined for properties of `null` values', inject(function($rootScope) {
$rootScope.a = null;
expect($rootScope.$eval('a.b')).toBeUndefined();
}));

it('should return null for `a.b` where `b` is null', inject(function($rootScope) {
$rootScope.a = { b: null };
expect($rootScope.$eval('a.b')).toBe(null);
}));

// cspSafeGetter && pathKeys.length < 6 || pathKeys.length > 2
it('should return null for `a.b.c.d.e` where `e` is null', inject(function($rootScope) {
$rootScope.a = { b: { c: { d: { e: null } } } };
expect($rootScope.$eval('a.b.c.d.e')).toBe(null);
}));

it('should return undefined for `a.b.c.d.e` where `d` is null', inject(function($rootScope) {
$rootScope.a = { b: { c: { d: null } } };
expect($rootScope.$eval('a.b.c.d.e')).toBeUndefined();
}));

// cspSafeGetter || pathKeys.length > 6
it('should return null for `a.b.c.d.e.f.g` where `g` is null', inject(function($rootScope) {
$rootScope.a = { b: { c: { d: { e: { f: { g: null } } } } } };
expect($rootScope.$eval('a.b.c.d.e.f.g')).toBe(null);
}));

it('should return undefined for `a.b.c.d.e.f.g` where `f` is null', inject(function($rootScope) {
$rootScope.a = { b: { c: { d: { e: { f: null } } } } };
expect($rootScope.$eval('a.b.c.d.e.f.g')).toBeUndefined();
}));
});
});
});
});
Expand Down

0 comments on commit 794724d

Please sign in to comment.