diff options
| -rw-r--r-- | docs/content/error/ng/badname.ngdoc | 8 | ||||
| -rw-r--r-- | docs/content/error/resource/badname.ngdoc | 8 | ||||
| -rw-r--r-- | src/Angular.js | 23 | ||||
| -rw-r--r-- | src/auto/injector.js | 2 | ||||
| -rw-r--r-- | src/loader.js | 5 | ||||
| -rw-r--r-- | src/ng/compile.js | 4 | ||||
| -rw-r--r-- | src/ng/controller.js | 1 | ||||
| -rw-r--r-- | src/ng/directive/form.js | 5 | ||||
| -rw-r--r-- | src/ng/directive/ngRepeat.js | 2 | ||||
| -rw-r--r-- | src/ng/directive/select.js | 4 | ||||
| -rw-r--r-- | src/ng/parse.js | 22 | ||||
| -rw-r--r-- | src/ngMock/angular-mocks.js | 4 | ||||
| -rw-r--r-- | src/ngResource/resource.js | 3 | ||||
| -rw-r--r-- | test/AngularSpec.js | 14 | ||||
| -rw-r--r-- | test/auto/injectorSpec.js | 18 | ||||
| -rw-r--r-- | test/loaderSpec.js | 6 | ||||
| -rwxr-xr-x | test/ng/compileSpec.js | 9 | ||||
| -rw-r--r-- | test/ng/controllerSpec.js | 7 | ||||
| -rw-r--r-- | test/ng/directive/formSpec.js | 12 | ||||
| -rw-r--r-- | test/ng/directive/ngRepeatSpec.js | 8 | ||||
| -rw-r--r-- | test/ng/directive/selectSpec.js | 10 | ||||
| -rw-r--r-- | test/ng/parseSpec.js | 20 | ||||
| -rw-r--r-- | test/ngMock/angular-mocksSpec.js | 11 | ||||
| -rw-r--r-- | test/ngResource/resourceSpec.js | 7 |
24 files changed, 196 insertions, 17 deletions
diff --git a/docs/content/error/ng/badname.ngdoc b/docs/content/error/ng/badname.ngdoc new file mode 100644 index 00000000..5d3a6f61 --- /dev/null +++ b/docs/content/error/ng/badname.ngdoc @@ -0,0 +1,8 @@ +@ngdoc error +@name ng:badname +@fullName Bad `hasOwnProperty` Name +@description + +Occurs when you try to use the name `hasOwnProperty` in a context where it is not allow. +Generally, a name cannot be `hasOwnProperty` because it is used, internally, on a object +and allowing such a name would break lookups on this object.
\ No newline at end of file diff --git a/docs/content/error/resource/badname.ngdoc b/docs/content/error/resource/badname.ngdoc new file mode 100644 index 00000000..99f73bdc --- /dev/null +++ b/docs/content/error/resource/badname.ngdoc @@ -0,0 +1,8 @@ +@ngdoc error +@name $resource:badname +@fullName Cannot use hasOwnProperty as a parameter name +@description + +Occurs when you try to use the name `hasOwnProperty` as a name of a parameter. +Generally, a name cannot be `hasOwnProperty` because it is used, internally, on a object +and allowing such a name would break lookups on this object.
\ No newline at end of file diff --git a/src/Angular.js b/src/Angular.js index b7d77437..879efb35 100644 --- a/src/Angular.js +++ b/src/Angular.js @@ -3,16 +3,6 @@ //////////////////////////////////// /** - * hasOwnProperty may be overwritten by a property of the same name, or entirely - * absent from an object that does not inherit Object.prototype; this copy is - * used instead - */ -var hasOwnPropertyFn = Object.prototype.hasOwnProperty; -var hasOwnPropertyLocal = function(obj, key) { - return hasOwnPropertyFn.call(obj, key); -}; - -/** * @ngdoc function * @name angular.lowercase * @function @@ -691,6 +681,8 @@ function shallowCopy(src, dst) { dst = dst || {}; for(var key in src) { + // shallowCopy is only ever called by $compile nodeLinkFn, which has control over src + // so we don't need to worry hasOwnProperty here if (src.hasOwnProperty(key) && key.substr(0, 2) !== '$$') { dst[key] = src[key]; } @@ -1188,6 +1180,17 @@ function assertArgFn(arg, name, acceptArrayAnnotation) { } /** + * throw error if the name given is hasOwnProperty + * @param {String} name the name to test + * @param {String} context the context in which the name is used, such as module or directive + */ +function assertNotHasOwnProperty(name, context) { + if (name === 'hasOwnProperty') { + throw ngMinErr('badname', "hasOwnProperty is not a valid {0} name", context); + } +} + +/** * Return the value accessible from the object by path. Any undefined traversals are ignored * @param {Object} obj starting object * @param {string} path path to traverse diff --git a/src/auto/injector.js b/src/auto/injector.js index 03e8025f..37ba5978 100644 --- a/src/auto/injector.js +++ b/src/auto/injector.js @@ -455,6 +455,7 @@ function createInjector(modulesToLoad) { } function provider(name, provider_) { + assertNotHasOwnProperty(name, 'service'); if (isFunction(provider_) || isArray(provider_)) { provider_ = providerInjector.instantiate(provider_); } @@ -475,6 +476,7 @@ function createInjector(modulesToLoad) { function value(name, value) { return factory(name, valueFn(value)); } function constant(name, value) { + assertNotHasOwnProperty(name, 'constant'); providerCache[name] = value; instanceCache[name] = value; } diff --git a/src/loader.js b/src/loader.js index 0edb7b87..15dab8f6 100644 --- a/src/loader.js +++ b/src/loader.js @@ -10,6 +10,8 @@ function setupModuleLoader(window) { + var $injectorMinErr = minErr('$injector'); + function ensure(obj, name, factory) { return obj[name] || (obj[name] = factory()); } @@ -68,12 +70,13 @@ function setupModuleLoader(window) { * @returns {module} new module with the {@link angular.Module} api. */ return function module(name, requires, configFn) { + assertNotHasOwnProperty(name, 'module'); if (requires && modules.hasOwnProperty(name)) { modules[name] = null; } return ensure(modules, name, function() { if (!requires) { - throw minErr('$injector')('nomod', "Module '{0}' is not available! You either misspelled the module name " + + throw $injectorMinErr('nomod', "Module '{0}' is not available! You either misspelled the module name " + "or forgot to load it. If registering a module ensure that you specify the dependencies as the second " + "argument.", name); } diff --git a/src/ng/compile.js b/src/ng/compile.js index fce6f34e..55de18ec 100644 --- a/src/ng/compile.js +++ b/src/ng/compile.js @@ -178,6 +178,7 @@ function $CompileProvider($provide) { * @returns {ng.$compileProvider} Self for chaining. */ this.directive = function registerDirective(name, directiveFactory) { + assertNotHasOwnProperty(name, 'directive'); if (isString(name)) { assertArg(directiveFactory, 'directiveFactory'); if (!hasDirectives.hasOwnProperty(name)) { @@ -1175,6 +1176,9 @@ function $CompileProvider($provide) { dst['class'] = (dst['class'] ? dst['class'] + ' ' : '') + value; } else if (key == 'style') { $element.attr('style', $element.attr('style') + ';' + value); + // `dst` will never contain hasOwnProperty as DOM parser won't let it. + // You will get an "InvalidCharacterError: DOM Exception 5" error if you + // have an attribute like "has-own-property" or "data-has-own-property", etc. } else if (key.charAt(0) != '$' && !dst.hasOwnProperty(key)) { dst[key] = value; dstAttr[key] = srcAttr[key]; diff --git a/src/ng/controller.js b/src/ng/controller.js index 2203c698..dc291c8c 100644 --- a/src/ng/controller.js +++ b/src/ng/controller.js @@ -25,6 +25,7 @@ function $ControllerProvider() { * annotations in the array notation). */ this.register = function(name, constructor) { + assertNotHasOwnProperty(name, 'controller'); if (isObject(name)) { extend(controllers, name) } else { diff --git a/src/ng/directive/form.js b/src/ng/directive/form.js index ed9d7052..455ad15f 100644 --- a/src/ng/directive/form.js +++ b/src/ng/directive/form.js @@ -73,9 +73,12 @@ function FormController(element, attrs) { * Input elements using ngModelController do this automatically when they are linked. */ form.$addControl = function(control) { + // Breaking change - before, inputs whose name was "hasOwnProperty" were quietly ignored + // and not added to the scope. Now we throw an error. + assertNotHasOwnProperty(control.$name, 'input'); controls.push(control); - if (control.$name && !form.hasOwnProperty(control.$name)) { + if (control.$name) { form[control.$name] = control; } }; diff --git a/src/ng/directive/ngRepeat.js b/src/ng/directive/ngRepeat.js index 16a810ef..78b054ff 100644 --- a/src/ng/directive/ngRepeat.js +++ b/src/ng/directive/ngRepeat.js @@ -305,6 +305,7 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) { key = (collection === collectionKeys) ? index : collectionKeys[index]; value = collection[key]; trackById = trackByIdFn(key, value, index); + assertNotHasOwnProperty(trackById, '`track by` id'); if(lastBlockMap.hasOwnProperty(trackById)) { block = lastBlockMap[trackById] delete lastBlockMap[trackById]; @@ -327,6 +328,7 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) { // remove existing items for (key in lastBlockMap) { + // lastBlockMap is our own object so we don't need to use special hasOwnPropertyFn if (lastBlockMap.hasOwnProperty(key)) { block = lastBlockMap[key]; $animate.leave(block.elements); diff --git a/src/ng/directive/select.js b/src/ng/directive/select.js index 0b6288c1..fb03e0ca 100644 --- a/src/ng/directive/select.js +++ b/src/ng/directive/select.js @@ -1,5 +1,6 @@ 'use strict'; +var ngOptionsMinErr = minErr('ngOptions'); /** * @ngdoc directive * @name ng.directive:select @@ -152,6 +153,7 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) { self.addOption = function(value) { + assertNotHasOwnProperty(value, '"option value"'); optionsMap[value] = true; if (ngModelCtrl.$viewValue == value) { @@ -300,7 +302,7 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) { var match; if (! (match = optionsExp.match(NG_OPTIONS_REGEXP))) { - throw minErr('ngOptions')('iexp', + throw ngOptionsMinErr('iexp', "Expected expression in form of '_select_ (as _label_)? for (_key_,)?_value_ in _collection_' but got '{0}'. Element: {1}", optionsExp, startingTag(selectElement)); } diff --git a/src/ng/parse.js b/src/ng/parse.js index 4a1921fc..682b497b 100644 --- a/src/ng/parse.js +++ b/src/ng/parse.js @@ -290,6 +290,7 @@ Lexer.prototype = { text: ident }; + // OPERATORS is our own object so we don't need to use special hasOwnPropertyFn if (OPERATORS.hasOwnProperty(ident)) { token.fn = OPERATORS[ident]; token.json = OPERATORS[ident]; @@ -938,6 +939,9 @@ function cspSafeGetterFn(key0, key1, key2, key3, key4, fullExp) { } function getterFn(path, csp, fullExp) { + // Check whether the cache has this getter already. + // We can use hasOwnProperty directly on the cache because we ensure, + // see below, that the cache never stores a path called 'hasOwnProperty' if (getterFnCache.hasOwnProperty(path)) { return getterFnCache[path]; } @@ -986,7 +990,12 @@ function getterFn(path, csp, fullExp) { fn.toString = function() { return code; }; } - return getterFnCache[path] = fn; + // Only cache the value if it's not going to mess up the cache object + // This is more performant that using Object.prototype.hasOwnProperty.call + if (path !== 'hasOwnProperty') { + getterFnCache[path] = fn; + } + return fn; } /////////////////////////////////// @@ -1036,6 +1045,7 @@ function $ParseProvider() { return function(exp) { var lexer = new Lexer($sniffer.csp); var parser = new Parser(lexer, $filter, $sniffer.csp); + var parsedExpression; switch (typeof exp) { case 'string': @@ -1043,7 +1053,15 @@ function $ParseProvider() { return cache[exp]; } - return cache[exp] = parser.parse(exp, false); + parsedExpression = parser.parse(exp, false); + + if (exp !== 'hasOwnProperty') { + // Only cache the value if it's not going to mess up the cache object + // This is more performant that using Object.prototype.hasOwnProperty.call + cache[exp] = parsedExpression; + } + + return parsedExpression; case 'function': return exp; diff --git a/src/ngMock/angular-mocks.js b/src/ngMock/angular-mocks.js index 1ba642f0..9bc80ff3 100644 --- a/src/ngMock/angular-mocks.js +++ b/src/ngMock/angular-mocks.js @@ -731,7 +731,7 @@ angular.mock.dump = function(object) { offset = offset || ' '; var log = [offset + 'Scope(' + scope.$id + '): {']; for ( var key in scope ) { - if (scope.hasOwnProperty(key) && !key.match(/^(\$|this)/)) { + if (Object.prototype.hasOwnProperty.call(scope, key) && !key.match(/^(\$|this)/)) { log.push(' ' + key + ': ' + angular.toJson(scope[key])); } } @@ -1773,7 +1773,7 @@ angular.mock.clearDataCache = function() { cache = angular.element.cache; for(key in cache) { - if (cache.hasOwnProperty(key)) { + if (Object.prototype.hasOwnProperty.call(cache,key)) { var handle = cache[key].handle; handle && angular.element(handle.elem).off(); diff --git a/src/ngResource/resource.js b/src/ngResource/resource.js index 2aa11f52..2498bdf3 100644 --- a/src/ngResource/resource.js +++ b/src/ngResource/resource.js @@ -347,6 +347,9 @@ angular.module('ngResource', ['ng']). var urlParams = self.urlParams = {}; forEach(url.split(/\W/), function(param){ + if (param === 'hasOwnProperty') { + throw $resourceMinErr('badname', "hasOwnProperty is not a valid parameter name."); + } if (!(new RegExp("^\\d+$").test(param)) && param && (new RegExp("(^|[^\\\\]):" + param + "(\\W|$)").test(url))) { urlParams[param] = true; } diff --git a/test/AngularSpec.js b/test/AngularSpec.js index a97c7591..c1914947 100644 --- a/test/AngularSpec.js +++ b/test/AngularSpec.js @@ -417,6 +417,20 @@ describe('angular', function() { }); + it('should not break if obj is an array we override hasOwnProperty', function() { + var obj = []; + obj[0] = 1; + obj[1] = 2; + obj.hasOwnProperty = null; + var log = []; + forEach(obj, function(value, key) { + log.push(key + ':' + value); + }); + expect(log).toEqual(['0:1', '1:2']); + }); + + + it('should handle JQLite and jQuery objects like arrays', function() { var jqObject = jqLite("<p><span>s1</span><span>s2</span></p>").find("span"), log = []; diff --git a/test/auto/injectorSpec.js b/test/auto/injectorSpec.js index f010fc91..5c186cf1 100644 --- a/test/auto/injectorSpec.js +++ b/test/auto/injectorSpec.js @@ -295,6 +295,24 @@ describe('injector', function() { }); describe('$provide', function() { + + it('should throw an exception if we try to register a service called "hasOwnProperty"', function() { + createInjector([function($provide) { + expect(function() { + $provide.provider('hasOwnProperty', function() { }); + }).toThrowMinErr('ng', 'badname'); + }]); + }); + + it('should throw an exception if we try to register a constant called "hasOwnProperty"', function() { + createInjector([function($provide) { + expect(function() { + $provide.constant('hasOwnProperty', {}); + }).toThrowMinErr('ng', 'badname'); + }]); + }); + + describe('constant', function() { it('should create configuration injectable constants', function() { var log = []; diff --git a/test/loaderSpec.js b/test/loaderSpec.js index 302852cb..2a564115 100644 --- a/test/loaderSpec.js +++ b/test/loaderSpec.js @@ -72,4 +72,10 @@ describe('module loader', function() { "or forgot to load it. If registering a module ensure that you specify the dependencies as the second " + "argument."); }); + + it('should complain if a module is called "hasOwnProperty', function() { + expect(function() { + window.angular.module('hasOwnProperty', []); + }).toThrowMinErr('ng','badname', "hasOwnProperty is not a valid module name"); + }); }); diff --git a/test/ng/compileSpec.js b/test/ng/compileSpec.js index 5e28c62b..1e6f6e26 100755 --- a/test/ng/compileSpec.js +++ b/test/ng/compileSpec.js @@ -117,6 +117,15 @@ describe('$compile', function() { expect(log).toEqual('pre1; pre2; post2; post1'); }); }); + + it('should throw an exception if a directive is called "hasOwnProperty"', function() { + module(function() { + expect(function() { + directive('hasOwnProperty', function() { }); + }).toThrowMinErr('ng','badname', "hasOwnProperty is not a valid directive name"); + }); + inject(function($compile) {}); + }); }); diff --git a/test/ng/controllerSpec.js b/test/ng/controllerSpec.js index 2a9922c6..4f94402f 100644 --- a/test/ng/controllerSpec.js +++ b/test/ng/controllerSpec.js @@ -57,6 +57,13 @@ describe('$controller', function() { expect(scope.foo).toBe('bar'); expect(ctrl instanceof FooCtrl).toBe(true); }); + + + it('should throw an exception if a controller is called "hasOwnProperty"', function () { + expect(function() { + $controllerProvider.register('hasOwnProperty', function($scope) {}); + }).toThrowMinErr('ng', 'badname', "hasOwnProperty is not a valid controller name"); + }); }); diff --git a/test/ng/directive/formSpec.js b/test/ng/directive/formSpec.js index fb64fdb3..53fd3d90 100644 --- a/test/ng/directive/formSpec.js +++ b/test/ng/directive/formSpec.js @@ -137,6 +137,18 @@ describe('form', function() { }); + it('should throw an exception if an input has name="hasOwnProperty"', function() { + doc = jqLite( + '<form name="form">'+ + '<input name="hasOwnProperty" ng-model="some" />'+ + '<input name="other" ng-model="someOther" />'+ + '</form>'); + expect(function() { + $compile(doc)(scope); + }).toThrowMinErr('ng', 'badname'); + }); + + describe('preventing default submission', function() { it('should prevent form submission', function() { diff --git a/test/ng/directive/ngRepeatSpec.js b/test/ng/directive/ngRepeatSpec.js index 72035566..4cf79dbf 100644 --- a/test/ng/directive/ngRepeatSpec.js +++ b/test/ng/directive/ngRepeatSpec.js @@ -137,6 +137,14 @@ describe('ngRepeat', function() { }); + it("should throw an exception if 'track by' evaluates to 'hasOwnProperty'", function() { + scope.items = {age:20}; + $compile('<div ng-repeat="(key, value) in items track by \'hasOwnProperty\'"></div>')(scope); + scope.$digest(); + expect($exceptionHandler.errors.shift().message).toMatch(/ng:badname/); + }); + + it('should track using build in $id function', function() { element = $compile( '<ul>' + diff --git a/test/ng/directive/selectSpec.js b/test/ng/directive/selectSpec.js index 584fe614..85acba19 100644 --- a/test/ng/directive/selectSpec.js +++ b/test/ng/directive/selectSpec.js @@ -1247,5 +1247,15 @@ describe('select', function() { expect(element.find('span').text()).toBe('success'); dealoc(element); })); + + it('should throw an exception if an option value interpolates to "hasOwnProperty"', function() { + scope.hasOwnPropertyOption = "hasOwnProperty"; + expect(function() { + compile('<select ng-model="x">'+ + '<option>{{hasOwnPropertyOption}}</option>'+ + '</select>'); + }).toThrowMinErr('ng','badname', 'hasOwnProperty is not a valid "option value" name'); + }); + }); }); diff --git a/test/ng/parseSpec.js b/test/ng/parseSpec.js index 87cc79af..19182332 100644 --- a/test/ng/parseSpec.js +++ b/test/ng/parseSpec.js @@ -351,6 +351,26 @@ describe('parser', function() { expect(scope.$eval('toString()', scope)).toBe('custom toString'); }); + it('should not break if hasOwnProperty is referenced in an expression', function() { + scope.obj = { value: 1}; + // By evaluating an expression that calls hasOwnProperty, the getterFnCache + // will store a property called hasOwnProperty. This is effectively: + // getterFnCache['hasOwnProperty'] = null + scope.$eval('obj.hasOwnProperty("value")'); + // If we rely on this property then evaluating any expression will fail + // because it is not able to find out if obj.value is there in the cache + expect(scope.$eval('obj.value')).toBe(1); + }); + + it('should not break if the expression is "hasOwnProperty"', function() { + scope.fooExp = 'barVal'; + // By evaluating hasOwnProperty, the $parse cache will store a getter for + // the scope's own hasOwnProperty function, which will mess up future cache look ups. + // i.e. cache['hasOwnProperty'] = function(scope) { return scope.hasOwnProperty; } + scope.$eval('hasOwnProperty'); + expect(scope.$eval('fooExp')).toBe('barVal'); + }); + it('should evaluate grouped expressions', function() { expect(scope.$eval("(1+2)*3")).toEqual((1+2)*3); }); diff --git a/test/ngMock/angular-mocksSpec.js b/test/ngMock/angular-mocksSpec.js index e506e608..1a4290e0 100644 --- a/test/ngMock/angular-mocksSpec.js +++ b/test/ngMock/angular-mocksSpec.js @@ -1,5 +1,7 @@ 'use strict'; +var msie = +((/msie (\d+)/.exec(navigator.userAgent.toLowerCase()) || [])[1]); + describe('ngMock', function() { var noop = angular.noop; @@ -448,6 +450,15 @@ describe('ngMock', function() { expect(d($rootScope)).toMatch(/Scope\(.*\): \{/); expect(d($rootScope)).toMatch(/{"abc":"123"}/); })); + + it('should serialize scope that has overridden "hasOwnProperty"', inject(function($rootScope){ + // MS IE8 just doesn't work for this kind of thing, since "for ... in" doesn't return + // things like hasOwnProperty even if it is explicitly defined on the actual object! + if (msie<=8) return; + $rootScope.hasOwnProperty = 'X'; + expect(d($rootScope)).toMatch(/Scope\(.*\): \{/); + expect(d($rootScope)).toMatch(/hasOwnProperty: "X"/); + })); }); diff --git a/test/ngResource/resourceSpec.js b/test/ngResource/resourceSpec.js index 550b4432..d13156b3 100644 --- a/test/ngResource/resourceSpec.js +++ b/test/ngResource/resourceSpec.js @@ -242,6 +242,13 @@ describe("resource", function() { }); + it('should throw an exception if a param is called "hasOwnProperty"', function() { + expect(function() { + $resource('/:hasOwnProperty').get(); + }).toThrowMinErr('$resource','badname', "hasOwnProperty is not a valid parameter name"); + }); + + it("should create resource", function() { $httpBackend.expect('POST', '/CreditCard', '{"name":"misko"}').respond({id: 123, name: 'misko'}); |