diff --git a/src/Angular.js b/src/Angular.js index 5ba8b6c88eb2..824c0a9d70c8 100644 --- 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 index e105de1ab28f..7905f062ce05 100644 --- 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 index cd1f6241efd3..dd06bd0c4635 100644 --- 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 index e70e49b49d3b..36101bbe707b 100644 --- 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 index f9f8f044ad89..e230c1a6042d 100644 --- 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('
')($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('')($rootScope); $rootScope.$digest();