fix(): nested duplicated clipPath causes infinite recursion

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 ## [next]
 
+- fix(): nested duplicated clipPath causes infinite recursion [#10667](https://github.com/fabricjs/fabric.js/pull/10667)
 - chore(): update playwright [#10657](https://github.com/fabricjs/fabric.js/pull/10657)
 - refactor(): swap lodash with es-toolkit [#10651](https://github.com/fabricjs/fabric.js/pull/10651)
 - chore(): update vitest [#10648](https://github.com/fabricjs/fabric.js/pull/10648)

src/parser/elements_parser.spec.ts

@@ -0,0 +1,70 @@
+import { Gradient, loadSVGFromString } from '../../fabric';
+import { describe, it, expect } from 'vitest';
+
+describe('Parser', () => {
+  it('should not enter an infinite loop with nested duplicated clipPath', async () => {
+    const svg = `
+<svg version="1.1" width="100px" height="100px" viewBox="0 0 100 100"
+    xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
+    <defs>
+        <clipPath id="i0">
+            <circle cx="0" cy="0" r="50" />
+        </clipPath>
+    </defs>
+
+    <g clip-path="url(#i0)">
+        <g clip-path="url(#i0)">
+            <rect
+                width="50"
+                height="50"
+                fill="red"
+            />
+        </g>
+    </g>
+</svg>`;
+    const { objects } = await loadSVGFromString(svg);
+    expect(objects).toHaveLength(1);
+  });
+
+  it('should not throw when gradient is missing', async () => {
+    const svg = `
+<svg version="1.1" width="100px" height="100px" viewBox="0 0 100 100"
+    xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
+    <g fill="url(#non-existent-gradient)">
+        <rect
+            width="50"
+            height="50"
+            fill="red"
+        />
+    </g>
+</svg>`;
+    const { objects } = await loadSVGFromString(svg);
+    expect(objects).toHaveLength(1);
+    expect(typeof objects[0]!.fill).toBe('string');
+    expect(objects[0]!.fill).toBe('red');
+  });
+
+  it('should correctly parse a valid gradient', async () => {
+    const svg = `
+<svg version="1.1" width="100px" height="100px" viewBox="0 0 100 100"
+    xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">
+    <defs>
+        <linearGradient id="myGradient">
+            <stop offset="0%" stop-color="red" />
+            <stop offset="100%" stop-color="blue" />
+        </linearGradient>
+    </defs>
+    <rect x="10" y="10" width="80" height="80" fill="url(#myGradient)" />
+</svg>`;
+    const { objects } = await loadSVGFromString(svg);
+    expect(objects).toHaveLength(1);
+    const rect = objects[0];
+    expect(rect!.fill).toBeInstanceOf(Gradient);
+    const gradient = rect!.fill as Gradient<'linear'>;
+    expect(gradient.type).toBe('linear');
+    expect(gradient.colorStops).toHaveLength(2);
+    const colors = gradient.colorStops.map((cs) => cs.color);
+    expect(colors).toContain('rgba(255,0,0,1)');
+    expect(colors).toContain('rgba(0,0,255,1)');
+  });
+});

src/parser/elements_parser.ts

@@ -94,11 +94,21 @@ export class ElementsParser {
     return null;
   }
 
+  extractPropertyDefinition(
+    obj: NotParsedFabricObject,
+    property: 'fill' | 'stroke',
+    storage: Record<string, SVGGradientElement>,
+  ): { def: SVGGradientElement; id: string } | undefined;
+  extractPropertyDefinition(
+    obj: NotParsedFabricObject,
+    property: 'clipPath',
+    storage: Record<string, Element[]>,
+  ): { def: Element[]; id: string } | undefined;
   extractPropertyDefinition(
     obj: NotParsedFabricObject,
     property: 'fill' | 'stroke' | 'clipPath',
     storage: Record<string, StorageType[typeof property]>,
-  ): StorageType[typeof property] | undefined {
+  ): { def: StorageType[typeof property]; id: string } | undefined {
     const value = obj[property]!,
       regex = this.regexUrl;
     if (!regex.test(value)) {
@@ -110,7 +120,7 @@ export class ElementsParser {
     const id = regex.exec(value)![1];
     regex.lastIndex = 0;
     // @todo fix this
-    return storage[id];
+    return { def: storage[id], id };
   }
 
   resolveGradient(
@@ -122,10 +132,10 @@ export class ElementsParser {
       obj,
       property,
       this.gradientDefs,
-    ) as SVGGradientElement;
-    if (gradientDef) {
+    );
+    if (gradientDef && gradientDef.def) {
       const opacityAttr = el.getAttribute(property + '-opacity');
-      const gradient = Gradient.fromElement(gradientDef, obj, {
+      const gradient = Gradient.fromElement(gradientDef.def, obj, {
         ...this.options,
         opacity: opacityAttr,
       } as SVGOptions);
@@ -139,13 +149,19 @@ export class ElementsParser {
     obj: NotParsedFabricObject,
     usingElement: Element,
     exactOwner?: Element,
+    processedClipPaths: string[] = [],
   ) {
-    const clipPathElements = this.extractPropertyDefinition(
+    const clipPathDefinition = this.extractPropertyDefinition(
       obj,
       'clipPath',
       this.clipPaths,
-    ) as Element[];
-    if (clipPathElements) {
+    );
+    if (
+      clipPathDefinition &&
+      clipPathDefinition.def &&
+      !processedClipPaths.includes(clipPathDefinition.id)
+    ) {
+      const clipPathElements = clipPathDefinition.def;
       const objTransformInv = invertTransform(obj.calcTransformMatrix());
       const clipPathTag = clipPathElements[0].parentElement!;
       let clipPathOwner = usingElement;
@@ -199,7 +215,8 @@ export class ElementsParser {
           // this is tricky.
           // it tries to differentiate from when clipPaths are inherited by outside groups
           // or when are really clipPaths referencing other clipPaths
-          clipPathTag.getAttribute('clip-path') ? clipPathOwner : undefined,
+          exactOwner || usingElement,
+          [...processedClipPaths, clipPathDefinition.id],
         );
       }
       const { scaleX, scaleY, angle, skewX, translateX, translateY } =