From f04f84fecd6a40b2b87393a583af9d06e4136662 Mon Sep 17 00:00:00 2001 From: Chirayu Krishnappa Date: Fri, 8 Nov 2013 20:44:32 -0800 Subject: [PATCH] fix($resource): don't use $parse for @dotted.member 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. --- docs/content/error/resource/badmember.ngdoc | 27 ++++++++++ src/ngResource/resource.js | 32 +++++++++--- test/ngResource/resourceSpec.js | 55 +++++++++++++++++++++ 3 files changed, 108 insertions(+), 6 deletions(-) create mode 100644 docs/content/error/resource/badmember.ngdoc diff --git a/docs/content/error/resource/badmember.ngdoc b/docs/content/error/resource/badmember.ngdoc new file mode 100644 index 000000000000..4a61eb9b41fb --- /dev/null +++ b/docs/content/error/resource/badmember.ngdoc @@ -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) | diff --git a/src/ngResource/resource.js b/src/ngResource/resource.js index 56d32f5e1548..f2e7ff624f8f 100644 --- a/src/ngResource/resource.js +++ b/src/ngResource/resource.js @@ -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 @@ -285,7 +307,8 @@ var $resourceMinErr = angular.$$minErr('$resource'); */ 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'}, @@ -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 @@ -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; } diff --git a/test/ngResource/resourceSpec.js b/test/ngResource/resourceSpec.js index d13156b3fcbc..5b75c8cfdca6 100644 --- a/test/ngResource/resourceSpec.js +++ b/test/ngResource/resourceSpec.js @@ -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({}); @@ -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'}});