Browse Source

fix(core): Fix circular merge-deep stack overflow issue (#3549)

Twilight 8 months ago
parent
commit
bdc3438bbd

+ 29 - 0
packages/core/src/service/helpers/entity-hydrator/merge-deep.spec.ts

@@ -42,4 +42,33 @@ describe('mergeDeep()', () => {
         expect(line2?.sales[0].id).toBe('sale-of-line-2');
         expect(line2?.productVariant?.id).toBe('variant-of-line-2');
     });
+
+    it('should handle circular objects', () => {
+        const first = {
+            name: 'John',
+            age: 30,
+            address: {
+              city: 'New York',
+              zip: '10001',
+            },
+        };
+        
+        const second = {
+            name: 'Jane',
+            age: 25,
+            address: {
+              city: 'Los Angeles',
+              zip: '90001',
+            },
+        };
+
+        // @ts-ignore
+        first.entity = first;
+        // @ts-ignore
+        second.entity = second;
+
+        const merged = mergeDeep(first, second);
+
+        expect(merged.name).toBe('Jane');
+    });
 });

+ 23 - 4
packages/core/src/service/helpers/entity-hydrator/merge-deep.ts

@@ -6,17 +6,30 @@ import { isObject } from '@vendure/common/lib/shared-utils';
  * property with a different set of properties. This prevents the original target
  * entity from having data overwritten.
  */
-export function mergeDeep<T extends { [key: string]: any }>(a: T | undefined, b: T): T {
+export function mergeDeep<T extends { [key: string]: any }>(
+    a: T | undefined,
+    b: T,
+    visited: WeakSet<object> = new WeakSet(),
+): T {
     if (!a) {
         return b;
     }
+
+    // Prevent circular references
+    if (isObject(b)) {
+        if (visited.has(b)) {
+            return a;
+        }
+        visited.add(b);
+    }
+
     if (Array.isArray(a) && Array.isArray(b) && a.length === b.length && a.length > 1) {
         if (a[0].hasOwnProperty('id')) {
             // If the array contains entities, we can use the id to match them up
             // so that we ensure that we don't merge properties from different entities
             // with the same index.
-            const aIds = a.map(e => e.id);
-            const bIds = b.map(e => e.id);
+            const aIds = a.map((e) => e.id);
+            const bIds = b.map((e) => e.id);
             if (JSON.stringify(aIds) !== JSON.stringify(bIds)) {
                 // The entities in the arrays are not in the same order, so we can't
                 // safely merge them. We need to sort the `b` array so that the entities
@@ -31,14 +44,20 @@ export function mergeDeep<T extends { [key: string]: any }>(a: T | undefined, b:
             }
         }
     }
+
     for (const [key, value] of Object.entries(b)) {
         if (Object.getOwnPropertyDescriptor(b, key)?.writable) {
             if (Array.isArray(value) || isObject(value)) {
-                (a as any)[key] = mergeDeep(a?.[key], b[key]);
+                // Skip if we detect a circular reference
+                if (isObject(value) && visited.has(value)) {
+                    continue;
+                }
+                (a as any)[key] = mergeDeep(a?.[key], b[key], visited);
             } else {
                 (a as any)[key] = b[key];
             }
         }
     }
+
     return a ?? b;
 }