Title: [285769] trunk
Revision
285769
Author
s...@apple.com
Date
2021-11-12 18:12:05 -0800 (Fri, 12 Nov 2021)

Log Message

REGRESSION(r285481): Infinite recursion with cyclic filter reference
https://bugs.webkit.org/show_bug.cgi?id=232972
rdar://85264240

Reviewed by Wenson Hsieh.

Source/WebCore:

Before r285481, we were creating the ImageBuffer of the referenced SVGElement
for the FEImage through RenderSVGResourceFilter::postApplyResource(). Now
we create this ImageBuffer through RenderSVGResourceFilter::applyResource().
The difference is at the end of RenderSVGResourceFilter::applyResource()
we add an entry to m_rendererFilterDataMap. This entry was preventing
trying to rebuild the SVGFilter for the same renderer if there is a
cyclic reference.

The fix is to add the entry in m_rendererFilterDataMap before creating the
SVGFilter. If an error happens, this entry will be removed before returning.

Test: svg/filters/feImage-cyclic-reference.svg

* rendering/svg/RenderSVGResourceFilter.cpp:
(WebCore::RenderSVGResourceFilter::applyResource):

LayoutTests:

* svg/filters/feImage-cyclic-reference-expected.txt: Added.
* svg/filters/feImage-cyclic-reference.svg: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (285768 => 285769)


--- trunk/LayoutTests/ChangeLog	2021-11-13 01:23:27 UTC (rev 285768)
+++ trunk/LayoutTests/ChangeLog	2021-11-13 02:12:05 UTC (rev 285769)
@@ -1,3 +1,14 @@
+2021-11-12  Said Abou-Hallawa  <s...@apple.com>
+
+        REGRESSION(r285481): Infinite recursion with cyclic filter reference
+        https://bugs.webkit.org/show_bug.cgi?id=232972
+        rdar://85264240
+
+        Reviewed by Wenson Hsieh.
+
+        * svg/filters/feImage-cyclic-reference-expected.txt: Added.
+        * svg/filters/feImage-cyclic-reference.svg: Added.
+
 2021-11-12  Rob Buis  <rb...@igalia.com>
 
         Null check m_spanElement

Added: trunk/LayoutTests/svg/filters/feImage-cyclic-reference-expected.txt (0 => 285769)


--- trunk/LayoutTests/svg/filters/feImage-cyclic-reference-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/svg/filters/feImage-cyclic-reference-expected.txt	2021-11-13 02:12:05 UTC (rev 285769)
@@ -0,0 +1 @@
+This test passes if it does not crash.

Added: trunk/LayoutTests/svg/filters/feImage-cyclic-reference.svg (0 => 285769)


--- trunk/LayoutTests/svg/filters/feImage-cyclic-reference.svg	                        (rev 0)
+++ trunk/LayoutTests/svg/filters/feImage-cyclic-reference.svg	2021-11-13 02:12:05 UTC (rev 285769)
@@ -0,0 +1,11 @@
+<svg xmlns="http://www.w3.org/2000/svg">
+    <filter id="filter">
+        <feImage href=""
+    </filter>
+    <rect x="10" y="10" width="100" height="100" filter="url(#filter)"/>
+    <text x="10" y="120">This test passes if it does not crash.</text>
+    <script>
+        if (window.testRunner)
+            testRunner.dumpAsText(true);
+    </script>
+</svg>

Modified: trunk/Source/WebCore/ChangeLog (285768 => 285769)


--- trunk/Source/WebCore/ChangeLog	2021-11-13 01:23:27 UTC (rev 285768)
+++ trunk/Source/WebCore/ChangeLog	2021-11-13 02:12:05 UTC (rev 285769)
@@ -1,3 +1,27 @@
+2021-11-12  Said Abou-Hallawa  <s...@apple.com>
+
+        REGRESSION(r285481): Infinite recursion with cyclic filter reference
+        https://bugs.webkit.org/show_bug.cgi?id=232972
+        rdar://85264240
+
+        Reviewed by Wenson Hsieh.
+
+        Before r285481, we were creating the ImageBuffer of the referenced SVGElement
+        for the FEImage through RenderSVGResourceFilter::postApplyResource(). Now
+        we create this ImageBuffer through RenderSVGResourceFilter::applyResource(). 
+        The difference is at the end of RenderSVGResourceFilter::applyResource()
+        we add an entry to m_rendererFilterDataMap. This entry was preventing 
+        trying to rebuild the SVGFilter for the same renderer if there is a
+        cyclic reference.
+
+        The fix is to add the entry in m_rendererFilterDataMap before creating the
+        SVGFilter. If an error happens, this entry will be removed before returning.
+
+        Test: svg/filters/feImage-cyclic-reference.svg
+
+        * rendering/svg/RenderSVGResourceFilter.cpp:
+        (WebCore::RenderSVGResourceFilter::applyResource):
+
 2021-11-12  Rob Buis  <rb...@igalia.com>
 
         Null check m_spanElement

Modified: trunk/Source/WebCore/rendering/svg/RenderSVGResourceFilter.cpp (285768 => 285769)


--- trunk/Source/WebCore/rendering/svg/RenderSVGResourceFilter.cpp	2021-11-13 01:23:27 UTC (rev 285768)
+++ trunk/Source/WebCore/rendering/svg/RenderSVGResourceFilter.cpp	2021-11-13 02:12:05 UTC (rev 285769)
@@ -96,17 +96,23 @@
         return false; // Already built, or we're in a cycle, or we're marked for removal. Regardless, just do nothing more now.
     }
 
-    auto filterData = makeUnique<FilterData>();
+    auto addResult = m_rendererFilterDataMap.set(&renderer, makeUnique<FilterData>());
+    auto filterData = addResult.iterator->value.get();
+    
     FloatRect targetBoundingBox = renderer.objectBoundingBox();
 
     filterData->boundaries = SVGLengthContext::resolveRectangle<SVGFilterElement>(&filterElement(), filterElement().filterUnits(), targetBoundingBox);
-    if (filterData->boundaries.isEmpty())
+    if (filterData->boundaries.isEmpty()) {
+        m_rendererFilterDataMap.remove(&renderer);
         return false;
+    }
 
     // Determine absolute transformation matrix for filter. 
     AffineTransform absoluteTransform = SVGRenderingContext::calculateTransformationToOutermostCoordinateSystem(renderer);
-    if (!absoluteTransform.isInvertible())
+    if (!absoluteTransform.isInvertible()) {
+        m_rendererFilterDataMap.remove(&renderer);
         return false;
+    }
 
     // Eliminate shear of the absolute transformation matrix, to be able to produce unsheared tile images for feTile.
     FloatSize filterScale(absoluteTransform.xScale(), absoluteTransform.yScale());
@@ -123,8 +129,10 @@
     // Create the SVGFilter object.
     filterData->builder = makeUnique<SVGFilterBuilder>();
     filterData->filter = SVGFilter::create(filterElement(), *filterData->builder, filterScale, absoluteDrawingRegion, filterData->boundaries, targetBoundingBox);
-    if (!filterData->filter)
+    if (!filterData->filter) {
+        m_rendererFilterDataMap.remove(&renderer);
         return false;
+    }
 
     auto lastEffect = filterData->builder->lastEffect();
     ASSERT(lastEffect);
@@ -144,9 +152,8 @@
     // If the drawingRegion is empty, we have something like <g filter=".."/>.
     // Even if the target objectBoundingBox() is empty, we still have to draw the last effect result image in postApplyResource.
     if (filterData->drawingRegion.isEmpty()) {
-        ASSERT(!m_rendererFilterDataMap.contains(&renderer));
+        ASSERT(m_rendererFilterDataMap.contains(&renderer));
         filterData->savedContext = context;
-        m_rendererFilterDataMap.set(&renderer, WTFMove(filterData));
         return false;
     }
 
@@ -161,9 +168,8 @@
 #endif
     auto sourceGraphic = SVGRenderingContext::createImageBuffer(filterData->drawingRegion, effectiveTransform, colorSpace, renderingMode, context);
     if (!sourceGraphic) {
-        ASSERT(!m_rendererFilterDataMap.contains(&renderer));
+        ASSERT(m_rendererFilterDataMap.contains(&renderer));
         filterData->savedContext = context;
-        m_rendererFilterDataMap.set(&renderer, WTFMove(filterData));
         return false;
     }
     
@@ -177,9 +183,7 @@
 
     context = &sourceGraphicContext;
 
-    ASSERT(!m_rendererFilterDataMap.contains(&renderer));
-    m_rendererFilterDataMap.set(&renderer, WTFMove(filterData));
-
+    ASSERT(m_rendererFilterDataMap.contains(&renderer));
     return true;
 }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to