Merge pull request #17374 from jacobq/test-for-prototype-extension-re…

…gression

[BUGFIX release] Fix cyclic references on Array.prototype

packages/@ember/-internals/runtime/lib/mixins/array.js

@@ -171,6 +171,18 @@ export function isArray(_obj) {
   return false;
 }
 
+/*
+  This allows us to define computed properties that are not enumerable.
+  The primary reason this is important is that when `NativeArray` is
+  applied to `Array.prototype` we need to ensure that we do not add _any_
+  new enumerable properties.
+*/
+function nonEnumerableComputed() {
+  let property = computed(...arguments);
+  property.enumerable = false;
+  return property;
+}
+
 // ..........................................................
 // ARRAY
 //
@@ -273,7 +285,7 @@ const ArrayMixin = Mixin.create(Enumerable, {
     @return this
     @public
   */
-  '[]': computed({
+  '[]': nonEnumerableComputed({
     get() {
       return this;
     },
@@ -290,7 +302,7 @@ const ArrayMixin = Mixin.create(Enumerable, {
     @return {Object | undefined} The first object in the array
     @public
   */
-  firstObject: computed(function() {
+  firstObject: nonEnumerableComputed(function() {
     return objectAt(this, 0);
   }).readOnly(),
 
@@ -301,7 +313,7 @@ const ArrayMixin = Mixin.create(Enumerable, {
     @return {Object | undefined} The last object in the array
     @public
   */
-  lastObject: computed(function() {
+  lastObject: nonEnumerableComputed(function() {
     return objectAt(this, this.length - 1);
   }).readOnly(),
 
@@ -474,7 +486,7 @@ const ArrayMixin = Mixin.create(Enumerable, {
     @property {Boolean} hasArrayObservers
     @public
   */
-  hasArrayObservers: computed(function() {
+  hasArrayObservers: nonEnumerableComputed(function() {
     return hasListeners(this, '@array:change') || hasListeners(this, '@array:before');
   }),
 
@@ -1154,7 +1166,7 @@ const ArrayMixin = Mixin.create(Enumerable, {
     @public
   */
   '@each': ARRAY_AT_EACH
-    ? computed(function() {
+    ? nonEnumerableComputed(function() {
         deprecate(`Getting the '@each' property on object ${toString(this)} is deprecated`, false, {
           id: 'ember-metal.getting-each',
           until: '3.5.0',

packages/@ember/-internals/runtime/tests/array/apply-test.js

@@ -0,0 +1,31 @@
+import { NativeArray } from '../../lib/mixins/array';
+import { AbstractTestCase, moduleFor } from 'internal-test-helpers';
+
+class ArrayPrototypeExtensionSelfReferenceTests extends AbstractTestCase {
+  '@test should not create non-Symbol, enumerable properties that refer to itself'() {
+    // Don't want to pollute Array.prototype so we make a fake / simple prototype
+    function ThrowAwayArray() {}
+
+    // Extend our throw-away prototype (like EXTEND_PROTOTYPES.Array would)
+    NativeArray.apply(ThrowAwayArray.prototype);
+
+    // Create an instance to test
+    let obj = new ThrowAwayArray();
+
+    // Make sure that no enumerable properties refer back to the object (creating a cyclic structure)
+    for (let p in obj) {
+      this.assert.notStrictEqual(
+        obj[p],
+        obj,
+        `Property "${p}" is an enumerable part of the prototype
+        so must not refer back to the original array.
+        Otherwise code that explores all properties,
+        such as jQuery.extend and other "deep cloning" functions,
+        will get stuck in an infinite loop.
+        `.replace(/\s+/g, ' ')
+      );
+    }
+  }
+}
+
+moduleFor(`NativeArray: apply`, ArrayPrototypeExtensionSelfReferenceTests);