Fix clone() for redefined properties. Closes #278

Author: kanongil

lib/index.js

@@ -33,8 +33,9 @@ exports.clone = function (obj, seen) {
 
     let newObj;
     let cloneDeep = false;
+    const isArray = Array.isArray(obj);
 
-    if (!Array.isArray(obj)) {
+    if (!isArray) {
         if (Buffer.isBuffer(obj)) {
             newObj = Buffer.from(obj);
         }
@@ -68,6 +69,11 @@ exports.clone = function (obj, seen) {
         const keys = Object.getOwnPropertyNames(obj);
         for (let i = 0; i < keys.length; ++i) {
             const key = keys[i];
+
+            if (isArray && key === 'length') {
+                continue;
+            }
+
             const descriptor = Object.getOwnPropertyDescriptor(obj, key);
             if (descriptor &&
                 (descriptor.get ||
@@ -76,9 +82,18 @@ exports.clone = function (obj, seen) {
                 Object.defineProperty(newObj, key, descriptor);
             }
             else {
-                newObj[key] = exports.clone(obj[key], seen);
+                Object.defineProperty(newObj, key, {
+                    enumerable: descriptor ? descriptor.enumerable : true,
+                    writable: true,
+                    configurable: true,
+                    value: exports.clone(obj[key], seen)
+                });
             }
         }
+
+        if (isArray) {
+            newObj.length = obj.length;
+        }
     }
 
     return newObj;

test/index.js

@@ -371,6 +371,26 @@ describe('clone()', () => {
         Object.getOwnPropertyDescriptor = oldGetOwnPropertyDescriptor;
         expect(copy).to.equal(obj);
     });
+
+    it('clones own property when class property is not writable', () => {
+
+        const Cl = class {
+
+            get x() {
+
+                return 'hi';
+            }
+        };
+
+        const obj = new Cl();
+
+        Object.defineProperty(obj, 'x', {
+            value: 0, writable: true
+        });
+
+        const copy = Hoek.clone(obj);
+        expect(copy).to.equal(obj);
+    });
 });
 
 describe('merge()', () => {
@@ -436,7 +456,7 @@ describe('merge()', () => {
     it('merges from null prototype objects', () => {
 
         const a = {};
-        
+
         const b = Object.create(null);
         b.x = true;