[Webkit-unassigned] [Bug 63290] Stack overflow with enormous SVG filter

[bugzilla-daemon at webkit.org]

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


Dean Jackson <dino at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #238693|review?                     |review-
               Flag|                            |




--- Comment #24 from Dean Jackson <dino at apple.com>  2014-09-26 13:37:03 PST ---
(From update of attachment 238693)
View in context: https://bugs.webkit.org/attachment.cgi?id=238693&action=review

> Source/WebCore/ChangeLog:20
> +        -- Do not build a filter if its has more than 200 FilterEffect's in its map.

Typo: its -> it
Typo: FilterEffects

> Source/WebCore/ChangeLog:22
> +        -- Discard a filter after it has more than 100 FilterEffect's in its tree.

Typo: FilterEffects

> Source/WebCore/platform/graphics/filters/FilterEffect.cpp:116
> +unsigned FilterEffect::numberOfEffectInputsRecursive() const
> +{
> +    unsigned size = m_inputEffects.size();
> +    unsigned count = 0;
> +    for (unsigned i = 0; i < size; ++i) {
> +        FilterEffect* in = m_inputEffects.at(i).get();
> +        count += in->numberOfEffectInputsRecursive();
> +    }
> +    return count + 1;
> +}

This isn't quite right.  e.g. feFlood and feTurbulence don't have any input effects, so we need to return 0 if the m_inputEffects.size() is 0.

But also, this could count the same effect multiple times - it could be the input to more than one later step. Doing this properly is a bit tricky :( I'll make another comment down where we might be able to count properly (in RenderSVGResourceFilter.cpp).

> Source/WebCore/rendering/svg/RenderSVGResourceFilter.cpp:181
> -    if (!lastEffect)
> +    if (!lastEffect || lastEffect->numberOfEffectInputsRecursive() > maxNumberOfEffectInputsRecursive)

OK, so we have to recursively count the input effects without duplicates. Luckily the builder has a map of all the named inputs and effect references, so it already has this information, but we just need to get it to expose it.

I think you should add this to SVGFilterBuilder. You'd keep a HashSet<RefPtr<FilterEffect>> that is added to in appendEffectToEffectReferences, and then a method to expose totalNumberOfInputs() or something like that. It would be the size of that set, plus the named inputs (size of m_namedEffects), plus the built-in effects (m_builtInEffects). 

You should double check that appendEffectToEffectReferences is not called for the named or built-in effects though. If it is, then we just need the size of our new set.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.