diff --git a/docs/content/error/ng/badname.ngdoc b/docs/content/error/ng/badname.ngdoc new file mode 100644 index 000000000000..5d3a6f616981 --- /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 000000000000..99f73bdc77cd --- /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 b7d77437c070..879efb3560cc 100644 --- a/src/Angular.js +++ b/src/Angular.js @@ -2,16 +2,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 @@ -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]; } @@ -1187,6 +1179,17 @@ function assertArgFn(arg, name, acceptArrayAnnotation) { return arg; } +/** + * 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 diff --git a/src/auto/injector.js b/src/auto/injector.js index 03e8025fa7de..37ba59780063 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 0edb7b87551e..15dab8f6eb05 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 fce6f34eb4b7..55de18eca3ab 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 2203c69858de..dc291c8cb0ff 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 ed9d7052c41d..455ad15fcb36 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 16a810ef1133..78b054ff995a 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 ae1a94eb0724..fb03e0ca3714 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 @@ -148,10 +149,11 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) { ngModelCtrl = ngModelCtrl_; nullOption = nullOption_; unknownOption = unknownOption_; - } + }; self.addOption = function(value) { + assertNotHasOwnProperty(value, '"option value"'); optionsMap[value] = true; if (ngModelCtrl.$viewValue == value) { @@ -177,12 +179,12 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) { $element.prepend(unknownOption); $element.val(unknownVal); unknownOption.prop('selected', true); // needed for IE - } + }; self.hasOption = function(value) { return optionsMap.hasOwnProperty(value); - } + }; $scope.$on('$destroy', function() { // disable unknown option so that we don't do work when the whole select is being destroyed @@ -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)); } @@ -339,7 +341,7 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) { var optionGroup, collection = valuesFn(scope) || [], locals = {}, - key, value, optionElement, index, groupIndex, length, groupLength; + key, value, optionElement, index, groupIndex, length, groupLength, trackIndex; if (multiple) { value = []; @@ -354,7 +356,7 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) { key = optionElement.val(); if (keyName) locals[keyName] = key; if (trackFn) { - for (var trackIndex = 0; trackIndex < collection.length; trackIndex++) { + for (trackIndex = 0; trackIndex < collection.length; trackIndex++) { locals[valueName] = collection[trackIndex]; if (trackFn(scope, locals) == key) break; } @@ -373,7 +375,7 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) { value = null; } else { if (trackFn) { - for (var trackIndex = 0; trackIndex < collection.length; trackIndex++) { + for (trackIndex = 0; trackIndex < collection.length; trackIndex++) { locals[valueName] = collection[trackIndex]; if (trackFn(scope, locals) == key) { value = valueFn(scope, locals); @@ -446,7 +448,7 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) { optionGroupNames.push(optionGroupName); } if (multiple) { - selected = selectedSet.remove(trackFn ? trackFn(scope, locals) : valueFn(scope, locals)) != undefined; + selected = selectedSet.remove(trackFn ? trackFn(scope, locals) : valueFn(scope, locals)) !== undefined; } else { if (trackFn) { var modelCast = {}; @@ -564,7 +566,7 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) { } } } - } + }; }]; var optionDirective = ['$interpolate', function($interpolate) { @@ -613,5 +615,5 @@ var optionDirective = ['$interpolate', function($interpolate) { }); }; } - } + }; }]; diff --git a/src/ng/parse.js b/src/ng/parse.js index ae22f0e8f502..965e555eb59d 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; } /////////////////////////////////// @@ -1034,19 +1043,28 @@ function $ParseProvider() { var cache = {}; this.$get = ['$filter', '$sniffer', function($filter, $sniffer) { return function(exp) { + var lexer, parser, parsedExpression; switch (typeof exp) { case 'string': if (cache.hasOwnProperty(exp)) { return cache[exp]; } - var lexer = new Lexer($sniffer.csp); - var parser = new Parser(lexer, $filter, $sniffer.csp); - return cache[exp] = parser.parse(exp, false); + lexer = new Lexer($sniffer.csp); + parser = new Parser(lexer, $filter, $sniffer.csp); + 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 parser.parse(exp, false); + case 'function': return exp; - + default: return noop; } diff --git a/src/ngMock/angular-mocks.js b/src/ngMock/angular-mocks.js index 1ba642f00c51..9bc80ff31cce 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 d330f737e4c6..5cdfa823f61d 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 62ea31d9f1ab..c1914947415e 100644 --- a/test/AngularSpec.js +++ b/test/AngularSpec.js @@ -417,11 +417,25 @@ 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("

s1s2

").find("span"), log = []; - forEach(jqObject, function(value, key) { log.push(key + ':' + value.innerHTML)}); + forEach(jqObject, function(value, key) { log.push(key + ':' + value.innerHTML); }); expect(log).toEqual(['0:s1', '1:s2']); }); diff --git a/test/auto/injectorSpec.js b/test/auto/injectorSpec.js index efed2aa40f9c..5c186cf14ed6 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 = []; @@ -599,8 +617,8 @@ describe('injector', function() { expect(function() { createInjector([function($provide){ $provide.factory('service', function(service){}); - return function(service) {} - }]) + return function(service) {}; + }]); }).toThrowMinErr('$injector', 'cdep', 'Circular dependency found: service'); }); @@ -610,37 +628,51 @@ describe('injector', function() { createInjector([function($provide){ $provide.factory('a', function(b){}); $provide.factory('b', function(a){}); - return function(a) {} - }]) + return function(a) {}; + }]); }).toThrowMinErr('$injector', 'cdep', 'Circular dependency found: b <- a'); }); + }); }); describe('retrieval', function() { - var instance, - $injector, - $provide; + var instance = {name:'angular'}; + var Instance = function() { this.name = 'angular'; }; - beforeEach(function() { - $injector = createInjector([ ['$provide', function(provide) { - ($provide = provide).value('instance', instance = {name:'angular'}); + function createInjectorWithValue(instanceName, instance) { + return createInjector([ ['$provide', function(provide) { + provide.value(instanceName, instance); }]]); + } + function createInjectorWithFactory(serviceName, serviceDef) { + return createInjector([ ['$provide', function(provide) { + provide.factory(serviceName, serviceDef); + }]]); + } + + + it('should retrieve by name', function() { + var $injector = createInjectorWithValue('instance', instance); + var retrievedInstance = $injector.get('instance'); + expect(retrievedInstance).toBe(instance); }); - it('should retrieve by name and cache instance', function() { - expect(instance).toEqual({name: 'angular'}); + it('should cache instance', function() { + var $injector = createInjectorWithFactory('instance', function() { return new Instance(); }); + var instance = $injector.get('instance'); expect($injector.get('instance')).toBe(instance); expect($injector.get('instance')).toBe(instance); }); it('should call functions and infer arguments', function() { - expect($injector.invoke(function(instance) { return instance; })).toBe(instance); + var $injector = createInjectorWithValue('instance', instance); expect($injector.invoke(function(instance) { return instance; })).toBe(instance); }); + }); diff --git a/test/loaderSpec.js b/test/loaderSpec.js index 302852cb6db2..2a564115be7c 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 5e28c62bf0ce..1e6f6e26b592 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 2a9922c6eeba..4f94402fe0e9 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 fb64fdb33275..53fd3d907432 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( + '
'+ + ''+ + ''+ + '
'); + 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 720355666458..4cf79dbf326d 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('
')(scope); + scope.$digest(); + expect($exceptionHandler.errors.shift().message).toMatch(/ng:badname/); + }); + + it('should track using build in $id function', function() { element = $compile( '