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

11 files changed, 58 insertions, 17 deletions

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;
           }