angular · caitp · May 5, 2015 · gkalpak · May 6, 2015 · caitp
diff --git a/src/Angular.js b/src/Angular.js
@@ -36,6 +36,7 @@
   isUndefined: true,
   isDefined: true,
   isObject: true,
+  isBlankObject: true,
   isString: true,
   isNumber: true,
   isDate: true,
@@ -175,6 +176,7 @@ var
     splice            = [].splice,
     push              = [].push,
     toString          = Object.prototype.toString,
+    getPrototypeOf    = Object.getPrototypeOf,
     ngMinErr          = minErr('ng'),
 
     /** @name angular */
@@ -267,12 +269,25 @@ function forEach(obj, iterator, context) {
       }
     } else if (obj.forEach && obj.forEach !== forEach) {
         obj.forEach(iterator, context, obj);
-    } else {
+    } else if (isBlankObject(obj)) {
+      // createMap() fast path --- Safe to avoid hasOwnProperty check because prototype chain is empty
+      for (key in obj) {
+        iterator.call(context, obj[key], key, obj);
+      }
+    } else if (typeof obj.hasOwnProperty === 'function') {
+      // Slow path for objects inheriting Object.prototype, hasOwnProperty check needed
       for (key in obj) {
         if (obj.hasOwnProperty(key)) {
           iterator.call(context, obj[key], key, obj);
         }
       }
+    } else {
+      // Slow path for objects which do not have a method `hasOwnProperty`
+      for (key in obj) {
+        if (hasOwnProperty.call(obj, key)) {
+          iterator.call(context, obj[key], key, obj);
+        }
+      }
     }
   }
   return obj;
@@ -498,6 +513,16 @@ function isObject(value) {
 }
 
 
+/**
+ * Determine if a value is an object with a null prototype
+ *
+ * @returns {boolean} True if `value` is an `Object` with a null prototype
+ */
+function isBlankObject(value) {
+  return value !== null && typeof value === 'object' && !getPrototypeOf(value);
+}
+
+
 /**
  * @ngdoc function
  * @name angular.isString
@@ -781,7 +806,7 @@ function copy(source, destination, stackSource, stackDest) {
         destination = new RegExp(source.source, source.toString().match(/[^\/]*$/)[0]);
         destination.lastIndex = source.lastIndex;
       } else if (isObject(source)) {
-        var emptyObject = Object.create(Object.getPrototypeOf(source));
+        var emptyObject = Object.create(getPrototypeOf(source));
         destination = copy(source, emptyObject, stackSource, stackDest);
       }
     }
@@ -800,7 +825,7 @@ function copy(source, destination, stackSource, stackDest) {
      stackDest.push(destination);
     }
 
-    var result;
+    var result, key;
     if (isArray(source)) {
       destination.length = 0;
       for (var i = 0; i < source.length; i++) {
@@ -820,21 +845,40 @@ function copy(source, destination, stackSource, stackDest) {
           delete destination[key];
         });
       }
-      for (var key in source) {
-        if (source.hasOwnProperty(key)) {
-          result = copy(source[key], null, stackSource, stackDest);
-          if (isObject(source[key])) {
-            stackSource.push(source[key]);
-            stackDest.push(result);
+      if (isBlankObject(source)) {
+        // createMap() fast path --- Safe to avoid hasOwnProperty check because prototype chain is empty
+        for (key in source) {
+          putValue(key, source[key], destination, stackSource, stackDest);
+        }
+      } else if (source && typeof source.hasOwnProperty === 'function') {
+        // Slow path, which must rely on hasOwnProperty
+        for (key in source) {
+          if (source.hasOwnProperty(key)) {
+            putValue(key, source[key], destination, stackSource, stackDest);
+          }
+        }
+      } else {
+        // Slowest path --- hasOwnProperty can't be called as a method
+        for (key in source) {
+          if (hasOwnProperty.call(source, key)) {
+            putValue(key, source[key], destination, stackSource, stackDest);
           }
-          destination[key] = result;
         }
       }
       setHashKey(destination,h);
     }
-
   }
   return destination;
+
+  function putValue(key, val, destination, stackSource, stackDest) {
+    // No context allocation, trivial outer scope, easily inlined
+    var result = copy(val, null, stackSource, stackDest);
+    if (isObject(val)) {
+      stackSource.push(val);
+      stackDest.push(result);
+    }
+    destination[key] = result;
+  }
 }
 
 /**
@@ -915,14 +959,14 @@ function equals(o1, o2) {
       } else {
         if (isScope(o1) || isScope(o2) || isWindow(o1) || isWindow(o2) ||
           isArray(o2) || isDate(o2) || isRegExp(o2)) return false;
-        keySet = {};
+        keySet = createMap();
         for (key in o1) {
           if (key.charAt(0) === '$' || isFunction(o1[key])) continue;
           if (!equals(o1[key], o2[key])) return false;
           keySet[key] = true;
         }
         for (key in o2) {
-          if (!keySet.hasOwnProperty(key) &&
+          if (!(key in keySet) &&
               key.charAt(0) !== '$' &&
               o2[key] !== undefined &&
               !isFunction(o2[key])) return false;

diff --git a/src/ng/animate.js b/src/ng/animate.js
@@ -26,7 +26,9 @@ function splitClasses(classes) {
     classes = classes.split(' ');
   }
 
-  var obj = {};
+  // Use createMap() to prevent class assumptions involving property names in
+  // Object.prototype
+  var obj = createMap();
   forEach(classes, function(klass) {
     // sometimes the split leaves empty string values
     // incase extra spaces were applied to the options

diff --git a/src/ng/directive/ngClass.js b/src/ng/directive/ngClass.js
@@ -39,7 +39,9 @@ function classDirective(name, selector) {
         }
 
         function digestClassCounts(classes, count) {
-          var classCounts = element.data('$classCounts') || {};
+          // Use createMap() to prevent class assumptions involving property
+          // names in Object.prototype
+          var classCounts = element.data('$classCounts') || createMap();
           var classesToUpdate = [];
           forEach(classes, function(className) {
             if (count > 0 || classCounts[className]) {

diff --git a/test/AngularSpec.js b/test/AngularSpec.js
@@ -370,6 +370,21 @@ describe('angular', function() {
       expect(copy(undefined, [1,2,3])).toEqual([]);
       expect(copy({0: 1, 1: 2}, [1,2,3])).toEqual([1,2]);
     });
+
+    it('should copy objects with no prototype parent', function() {
+      var obj = extend(Object.create(null), {
+        a: 1,
+        b: 2,
+        c: 3
+      });
+      var dest = copy(obj);
+
+      expect(Object.getPrototypeOf(dest)).toBe(null);
+      expect(dest.a).toBe(1);
+      expect(dest.b).toBe(2);
+      expect(dest.c).toBe(3);
+      expect(Object.keys(dest)).toEqual(['a', 'b', 'c']);
+    });
   });
 
   describe("extend", function() {
@@ -651,6 +666,38 @@ describe('angular', function() {
     it('should return false when comparing an object and a Date', function() {
       expect(equals({}, new Date())).toBe(false);
     });
+
+    it('should safely compare objects with no prototype parent', function() {
+      var o1 = extend(Object.create(null), {
+        a: 1, b: 2, c: 3
+      });
+      var o2 = extend(Object.create(null), {
+        a: 1, b: 2, c: 3
+      });
+      expect(equals(o1, o2)).toBe(true);
+      o2.c = 2;
+      expect(equals(o1, o2)).toBe(false);
+    });
+
+
+    it('should safely compare objects which shadow Object.prototype.hasOwnProperty', function() {
+      /* jshint -W001 */
+      var o1 = {
+        hasOwnProperty: true,
+        a: 1,
+        b: 2,
+        c: 3
+      };
+      var o2 = {
+        hasOwnProperty: true,
+        a: 1,
+        b: 2,
+        c: 3
+      };
+      expect(equals(o1, o2)).toBe(true);
+      o1.hasOwnProperty = function() {};
+      expect(equals(o1, o2)).toBe(false);
+    });
   });
 
 
@@ -980,6 +1027,42 @@ describe('angular', function() {
     });
 
 
+    it('should safely iterate through objects with no prototype parent', function() {
+      var obj = extend(Object.create(null), {
+        a: 1, b: 2, c: 3
+      });
+      var log = [];
+      var self = {};
+      forEach(obj, function(val, key, collection) {
+        expect(this).toBe(self);
+        expect(collection).toBe(obj);
+        log.push(key + '=' + val);
+      }, self);
+      expect(log.length).toBe(3);
+      expect(log).toEqual(['a=1', 'b=2', 'c=3']);
+    });
+
+
+    it('should safely iterate through objects which shadow Object.prototype.hasOwnProperty', function() {
+      /* jshint -W001 */
+      var obj = {
+        hasOwnProperty: true,
+        a: 1,
+        b: 2,
+        c: 3
+      };
+      var log = [];
+      var self = {};
+      forEach(obj, function(val, key, collection) {
+        expect(this).toBe(self);
+        expect(collection).toBe(obj);
+        log.push(key + '=' + val);
+      }, self);
+      expect(log.length).toBe(4);
+      expect(log).toEqual(['hasOwnProperty=true', 'a=1', 'b=2', 'c=3']);
+    });
+
+
     describe('ES spec api compliance', function() {
 
       function testForEachSpec(expectedSize, collection) {

diff --git a/test/ng/directive/ngClassSpec.js b/test/ng/directive/ngClassSpec.js
@@ -33,6 +33,32 @@ describe('ngClass', function() {
   }));
 
 
+  it('should add new and remove old classes with same names as Object.prototype properties dynamically', inject(function($rootScope, $compile) {
+    /* jshint -W001 */
+    element = $compile('<div class="existing" ng-class="dynClass"></div>')($rootScope);
+    $rootScope.dynClass = { watch: true, hasOwnProperty: true, isPrototypeOf: true };
+    $rootScope.$digest();
+    expect(element.hasClass('existing')).toBe(true);
+    expect(element.hasClass('watch')).toBe(true);
+    expect(element.hasClass('hasOwnProperty')).toBe(true);
+    expect(element.hasClass('isPrototypeOf')).toBe(true);
+
+    $rootScope.dynClass.watch = false;
+    $rootScope.$digest();
+    expect(element.hasClass('existing')).toBe(true);
+    expect(element.hasClass('watch')).toBe(false);
+    expect(element.hasClass('hasOwnProperty')).toBe(true);
+    expect(element.hasClass('isPrototypeOf')).toBe(true);
+
+    delete $rootScope.dynClass;
+    $rootScope.$digest();
+    expect(element.hasClass('existing')).toBe(true);
+    expect(element.hasClass('watch')).toBe(false);
+    expect(element.hasClass('hasOwnProperty')).toBe(false);
+    expect(element.hasClass('isPrototypeOf')).toBe(false);
+  }));
+
+
   it('should support adding multiple classes via an array', inject(function($rootScope, $compile) {
     element = $compile('<div class="existing" ng-class="[\'A\', \'B\']"></div>')($rootScope);
     $rootScope.$digest();