Bug 208279

Summary:

Hit test with clipPath referencing parent element causes infinite recursion

Product:

WebKit

Reporter:

Doug Kelly <dougk>

Component:

SVG

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

commit-queue, dino, esprehn+autocc, ews-watchlist, fmalita, glenn, gyuyoung.kim, kondapallykalyan, pdr, rniwa, sabouhallawa, schenney, sergio, webkit-bug-importer, zimmermann

Priority:

Keywords:

InRadar

Version:

WebKit Local Build

Hardware:

Mac

OS:

macOS 10.15

See Also:

https://bugs.webkit.org/show_bug.cgi?id=209773

Attachments:

Description	Flags
Patch	none
test case (will crash)	none

Description Doug Kelly 2020-02-26 17:22:11 PST

When using a hit test that checks an element nested inside a clipPath (but the child element refers to the clipPath), this can result in an infinite recursion.  An extra check is needed to break the cycle when performing the hit test.

<rdar://problem/58381090>

Comment 1 Doug Kelly 2020-02-26 17:28:30 PST

Created attachment 391817 [details]
Patch

Comment 2 Ryosuke Niwa 2020-02-27 19:53:55 PST

Comment on attachment 391817 [details]
Patch

Seems ok.

Comment 3 WebKit Commit Bot 2020-02-27 20:15:43 PST

Comment on attachment 391817 [details]
Patch

Clearing flags on attachment: 391817

Committed r257616: <https://trac.webkit.org/changeset/257616>

Comment 4 WebKit Commit Bot 2020-02-27 20:15:45 PST

All reviewed patches have been landed.  Closing bug.

Comment 5 Said Abou-Hallawa 2020-03-02 12:37:41 PST

Comment on attachment 391817 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=391817&action=review

> Source/WebCore/rendering/svg/RenderSVGResourceClipper.cpp:290
> +        const RenderStyle& style = renderer->style();
> +        if (is<ReferenceClipPathOperation>(style.clipPath())) {
> +            auto& clipPath = downcast<ReferenceClipPathOperation>(*style.clipPath());
> +            AtomString id(clipPath.fragment());
> +            RenderSVGResourceClipper* clipper = getRenderSVGResourceById<RenderSVGResourceClipper>(document(), id);
> +            if (clipper == this)
> +                continue;
> +        }

I do not think this is the right solution. Detecting SVG resources cyclic referencing is more complicated than going one level up in the resources tree. For example if you delete the last two lines of your test case below:

    <clipPath id="clippath" clipPathUnits="objectBoundingBox">
    <text clip-path="url(#clippath)" to="currentColor">Text</text>

And you add these lines instead:

    <g id="group">
        <text clip-path="url(#clippath)" to="currentColor">Text</text>
    </g>
    <clipPath id="clippath" clipPathUnits="objectBoundingBox">
        <use href="#group"/>
    </clipPath>

The new test case will hit the following infinite recursive call stack:

#1	0x000000011c854e44 in WebCore::RenderBlock::nodeAtPoint(WebCore::HitTestRequest const&, WebCore::HitTestResult&, WebCore::HitTestLocation const&, WebCore::LayoutPoint const&, WebCore::HitTestAction) at /Volumes/Data/WebKit/OpenSource/Source/WebCore/rendering/RenderBlock.cpp:1994
#2	0x000000011cc241e1 in WebCore::RenderSVGText::nodeAtFloatPoint(WebCore::HitTestRequest const&, WebCore::HitTestResult&, WebCore::FloatPoint const&, WebCore::HitTestAction) at /Volumes/Data/WebKit/OpenSource/Source/WebCore/rendering/svg/RenderSVGText.cpp:444
#3	0x000000011cbcc501 in WebCore::RenderSVGContainer::nodeAtFloatPoint(WebCore::HitTestRequest const&, WebCore::HitTestResult&, WebCore::FloatPoint const&, WebCore::HitTestAction) at /Volumes/Data/WebKit/OpenSource/Source/WebCore/rendering/svg/RenderSVGContainer.cpp:170
#4	0x000000011cbcc501 in WebCore::RenderSVGContainer::nodeAtFloatPoint(WebCore::HitTestRequest const&, WebCore::HitTestResult&, WebCore::FloatPoint const&, WebCore::HitTestAction) at /Volumes/Data/WebKit/OpenSource/Source/WebCore/rendering/svg/RenderSVGContainer.cpp:170
#5	0x000000011cbe0606 in WebCore::RenderSVGResourceClipper::hitTestClipContent(WebCore::FloatRect const&, WebCore::FloatPoint const&) at /Volumes/Data/WebKit/OpenSource/Source/WebCore/rendering/svg/RenderSVGResourceClipper.cpp:293
#6	0x000000011c8553c8 in WebCore::RenderBlock::nodeAtPoint(WebCore::HitTestRequest const&, WebCore::HitTestResult&, WebCore::HitTestLocation const&, WebCore::LayoutPoint const&, WebCore::HitTestAction) at /Volumes/Data/WebKit/OpenSource/Source/WebCore/rendering/RenderBlock.cpp:2048
#7	0x000000011cc241e1 in WebCore::RenderSVGText::nodeAtFloatPoint(WebCore::HitTestRequest const&, WebCore::HitTestResult&, WebCore::FloatPoint const&, WebCore::HitTestAction) at /Volumes/Data/WebKit/OpenSource/Source/WebCore/rendering/svg/RenderSVGText.cpp:444

Please see SVGResourcesCycleSolver::resolveCycles().

Comment 6 Said Abou-Hallawa 2020-03-02 12:41:19 PST

Created attachment 392173 [details]
test case (will crash)

Comment 7 Ryosuke Niwa 2020-03-02 22:49:01 PST

Let's track that in a separate bug.