Skip to content

Commit

Permalink
fix($resource): don't use $parse for @dotted.member
Browse files Browse the repository at this point in the history
params and paramDefaults support looking up the parameter value from the
data object.  The syntax for that is `@nested.property.name`.
Currently, $resource uses $parse to do this.  This is too liberal
(you can use values like `@a=b` or `@a | filter` and have it work -
which doesn't really make sense).  It also puts up a dependency on
$parse which is has restrictions to secure expressions used in
templates.  The value here, though a string, is specified in Javascript
code and shouldn't have those restrictions.
  • Loading branch information
chirayuk authored and jamesdaily committed Jan 27, 2014
1 parent 203c9ae commit f04f84f
Show file tree
Hide file tree
Showing 3 changed files with 108 additions and 6 deletions.
27 changes: 27 additions & 0 deletions docs/content/error/resource/badmember.ngdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
@ngdoc error
@name $resource:badmember
@fullName Syntax error in param value using @member lookup
@description

Occurs when there is a syntax error when attempting to extract a param
value from the data object.

Here's an example of valid syntax for `params` or `paramsDefault`:

````javascript
{
bar: '@foo.bar'
}
````

The part following the `@`, `foo.bar` in this case, should be a simple
dotted member lookup using only ASCII identifiers. This error occurs
when there is an error in that expression. The following are all syntax
errors

| Value | Error |
|---------|----------------|
| `@` | Empty expression following `@`. |
| `@1.a` | `1` is an invalid javascript identifier. |
| `@.a` | Leading `.` is invalid. |
| `@a[1]` | Only dotted lookups are supported (no index operator) |
32 changes: 26 additions & 6 deletions src/ngResource/resource.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,28 @@

var $resourceMinErr = angular.$$minErr('$resource');

// Helper functions and regex to lookup a dotted path on an object
// stopping at undefined/null. The path must be composed of ASCII
// identifiers (just like $parse)
var MEMBER_NAME_REGEX = /^(\.[a-zA-Z_$][0-9a-zA-Z_$]*)+$/;

function isValidDottedPath(path) {
return (path != null && path !== '' && path !== 'hasOwnProperty' &&
MEMBER_NAME_REGEX.test('.' + path));
}

function lookupDottedPath(obj, path) {
if (!isValidDottedPath(path)) {
throw $resourceMinErr('badmember', 'Dotted member path "@{0}" is invalid.', path);
}
var keys = path.split('.');
for (var i = 0, ii = keys.length; i < ii && obj !== undefined; i++) {
var key = keys[i];
obj = (obj !== null) ? obj[key] : undefined;
}
return obj;
}

/**
* @ngdoc overview
* @name ngResource
Expand Down Expand Up @@ -285,7 +307,8 @@ var $resourceMinErr = angular.$$minErr('$resource');
</doc:example>
*/
angular.module('ngResource', ['ng']).
factory('$resource', ['$http', '$parse', '$q', function($http, $parse, $q) {
factory('$resource', ['$http', '$q', function($http, $q) {

var DEFAULT_ACTIONS = {
'get': {method:'GET'},
'save': {method:'POST'},
Expand All @@ -297,10 +320,7 @@ angular.module('ngResource', ['ng']).
forEach = angular.forEach,
extend = angular.extend,
copy = angular.copy,
isFunction = angular.isFunction,
getter = function(obj, path) {
return $parse(path)(obj);
};
isFunction = angular.isFunction;

/**
* We need our custom method because encodeURIComponent is too aggressive and doesn't follow
Expand Down Expand Up @@ -415,7 +435,7 @@ angular.module('ngResource', ['ng']).
forEach(actionParams, function(value, key){
if (isFunction(value)) { value = value(); }
ids[key] = value && value.charAt && value.charAt(0) == '@' ?
getter(data, value.substr(1)) : value;
lookupDottedPath(data, value.substr(1)) : value;
});
return ids;
}
Expand Down
55 changes: 55 additions & 0 deletions test/ngResource/resourceSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,48 @@ describe("resource", function() {
$httpBackend.verifyNoOutstandingExpectation();
});

describe('isValidDottedPath', function() {
it('should support arbitrary dotted names', function() {
expect(isValidDottedPath('')).toBe(false);
expect(isValidDottedPath('1')).toBe(false);
expect(isValidDottedPath('1abc')).toBe(false);
expect(isValidDottedPath('.')).toBe(false);
expect(isValidDottedPath('$')).toBe(true);
expect(isValidDottedPath('a')).toBe(true);
expect(isValidDottedPath('A')).toBe(true);
expect(isValidDottedPath('a1')).toBe(true);
expect(isValidDottedPath('$a')).toBe(true);
expect(isValidDottedPath('$1')).toBe(true);
expect(isValidDottedPath('$$')).toBe(true);
expect(isValidDottedPath('$.$')).toBe(true);
expect(isValidDottedPath('.$')).toBe(false);
expect(isValidDottedPath('$.')).toBe(false);
});
});

describe('lookupDottedPath', function() {
var data = {a: {b: 'foo', c: null}};

it('should throw for invalid path', function() {
expect(function() {
lookupDottedPath(data, '.ckck')
}).toThrowMinErr('$resource', 'badmember',
'Dotted member path "@.ckck" is invalid.');
});

it('should get dotted paths', function() {
expect(lookupDottedPath(data, 'a')).toEqual({b: 'foo', c: null});
expect(lookupDottedPath(data, 'a.b')).toBe('foo');
expect(lookupDottedPath(data, 'a.c')).toBeNull();
});

it('should skip over null/undefined members', function() {
expect(lookupDottedPath(data, 'a.b.c')).toBe(undefined);
expect(lookupDottedPath(data, 'a.c.c')).toBe(undefined);
expect(lookupDottedPath(data, 'a.b.c.d')).toBe(undefined);
expect(lookupDottedPath(data, 'NOT_EXIST')).toBe(undefined);
});
});

it('should not include a request body when calling $delete', function() {
$httpBackend.expect('DELETE', '/fooresource', null).respond({});
Expand Down Expand Up @@ -189,6 +231,19 @@ describe("resource", function() {
});


it('should support @_property lookups with underscores', function() {
$httpBackend.expect('GET', '/Order/123').respond({_id: {_key:'123'}, count: 0});
var LineItem = $resource('/Order/:_id', {_id: '@_id._key'});
var item = LineItem.get({_id: 123});
$httpBackend.flush();
expect(item).toEqualData({_id: {_key: '123'}, count: 0});
$httpBackend.expect('POST', '/Order/123').respond({_id: {_key:'123'}, count: 1});
item.$save();
$httpBackend.flush();
expect(item).toEqualData({_id: {_key: '123'}, count: 1});
});


it('should not pass default params between actions', function() {
var R = $resource('/Path', {}, {get: {method: 'GET', params: {objId: '1'}}, perform: {method: 'GET'}});

Expand Down

0 comments on commit f04f84f

Please sign in to comment.