From 72bd60bbdda7016cbc3c3a9d478eb6800e4a3133 Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Sat, 5 Oct 2013 10:32:12 +0100 Subject: [PATCH 1/4] style(angularSpec): add missing semicolon --- test/AngularSpec.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/AngularSpec.js b/test/AngularSpec.js index 62ea31d9f1ab..a97c75911f46 100644 --- a/test/AngularSpec.js +++ b/test/AngularSpec.js @@ -421,7 +421,7 @@ describe('angular', 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']); }); From e9618a7890e8ac1f9ddb3023a642a73ea6b0ee92 Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Sat, 5 Oct 2013 10:36:28 +0100 Subject: [PATCH 2/4] style(injectorSpec): add semicolons & test helpers --- test/auto/injectorSpec.js | 40 ++++++++++++++++++++++++++------------- 1 file changed, 27 insertions(+), 13 deletions(-) diff --git a/test/auto/injectorSpec.js b/test/auto/injectorSpec.js index efed2aa40f9c..f010fc910ba3 100644 --- a/test/auto/injectorSpec.js +++ b/test/auto/injectorSpec.js @@ -599,8 +599,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 +610,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); }); + }); From 6ff52a7e407a6293fa727f652f5399e8bbf5bada Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Sat, 5 Oct 2013 10:43:11 +0100 Subject: [PATCH 3/4] style(select): fix semicolons and vars --- src/ng/directive/select.js | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/ng/directive/select.js b/src/ng/directive/select.js index ae1a94eb0724..0b6288c10e1f 100644 --- a/src/ng/directive/select.js +++ b/src/ng/directive/select.js @@ -148,7 +148,7 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) { ngModelCtrl = ngModelCtrl_; nullOption = nullOption_; unknownOption = unknownOption_; - } + }; self.addOption = function(value) { @@ -177,12 +177,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 @@ -339,7 +339,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 +354,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 +373,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 +446,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 +564,7 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) { } } } - } + }; }]; var optionDirective = ['$interpolate', function($interpolate) { @@ -613,5 +613,5 @@ var optionDirective = ['$interpolate', function($interpolate) { }); }; } - } + }; }]; From 94246ba766d726c2b59c2eb4ee6dff9c2499935a Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Sat, 5 Oct 2013 10:49:09 +0100 Subject: [PATCH 4/4] fix(*): protect calls to hasOwnProperty in public API Objects received from outside AngularJS may have had their `hasOwnProperty` method overridden with something else. In cases where we can do this without incurring a performance penalty we call directly on Object.prototype.hasOwnProperty to ensure that we use the correct method. Also, we have some internal hash objects, where the keys for the map are provided from outside AngularJS. In such cases we either prevent `hasOwnProperty` from being used as a key or provide some other way of preventing our objects from having their `hasOwnProperty` overridden. BREAKING CHANGE: Inputs with name equal to "hasOwnProperty" are not allowed inside form or ngForm directives. Before, inputs whose name was "hasOwnProperty" were quietly ignored and not added to the scope. Now a badname exception is thrown. Using "hasOwnProperty" for an input name would be very unusual and bad practice. Either do not include such an input in a `form` or `ngForm` directive or change the name of the input. Closes #3331 --- docs/content/error/ng/badname.ngdoc | 8 +++++++ docs/content/error/resource/badname.ngdoc | 8 +++++++ src/Angular.js | 23 +++++++++++-------- src/auto/injector.js | 2 ++ src/loader.js | 5 +++- src/ng/compile.js | 4 ++++ src/ng/controller.js | 1 + src/ng/directive/form.js | 5 +++- src/ng/directive/ngRepeat.js | 2 ++ src/ng/directive/select.js | 4 +++- src/ng/parse.js | 28 +++++++++++++++++++---- src/ngMock/angular-mocks.js | 4 ++-- src/ngResource/resource.js | 3 +++ test/AngularSpec.js | 14 ++++++++++++ test/auto/injectorSpec.js | 18 +++++++++++++++ test/loaderSpec.js | 6 +++++ test/ng/compileSpec.js | 9 ++++++++ test/ng/controllerSpec.js | 7 ++++++ test/ng/directive/formSpec.js | 12 ++++++++++ test/ng/directive/ngRepeatSpec.js | 8 +++++++ test/ng/directive/selectSpec.js | 10 ++++++++ test/ng/parseSpec.js | 20 ++++++++++++++++ test/ngMock/angular-mocksSpec.js | 11 +++++++++ test/ngResource/resourceSpec.js | 7 ++++++ 24 files changed, 199 insertions(+), 20 deletions(-) create mode 100644 docs/content/error/ng/badname.ngdoc create mode 100644 docs/content/error/resource/badname.ngdoc 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 0b6288c10e1f..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 @@ -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 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 a97c75911f46..c1914947415e 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("

s1s2

").find("span"), log = []; diff --git a/test/auto/injectorSpec.js b/test/auto/injectorSpec.js index f010fc910ba3..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 = []; 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( '
    ' + diff --git a/test/ng/directive/selectSpec.js b/test/ng/directive/selectSpec.js index 584fe6140e59..85acba1938ec 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(''); + }).toThrowMinErr('ng','badname', 'hasOwnProperty is not a valid "option value" name'); + }); + }); }); diff --git a/test/ng/parseSpec.js b/test/ng/parseSpec.js index 87cc79af2c02..191823326b47 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 e506e60843ef..1a4290e006f1 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 550b44326ed9..d13156b3fcbc 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'});