Merge r222304 - REGRESSION(r191731): SVGPatternElement can only refer…

…ence another SVGPatternElement in the same SVG document https://bugs.webkit.org/show_bug.cgi?id=176221 Reviewed by Tim Horton. Source/WebCore: According to the specs: https://www.w3.org/TR/SVG11/filters.html#FilterElementHrefAttribute https://www.w3.org/TR/SVG11/pservers.html#LinearGradientElementHrefAttribute https://www.w3.org/TR/SVG11/pservers.html#RadialGradientElementHrefAttribute https://www.w3.org/TR/SVG11/pservers.html#PatternElementHrefAttribute The xlink:href attribute of the SVG filter, gradient and pattern elements must reference another element within the current SVG of the same type. In r191731, the code of SVGPatternElement::collectPatternAttributes() was removed and replaced by RenderSVGResourcePattern::collectPatternAttributes() to avoid cyclic reference in the pattern element. The problem is the old code used to check whether the referenced element is<SVGPatternElement> before casting it. This code was not copied to the new function. So we now allow the SVGPatternElement to reference any SVG resource element. To fix this issue, we need to prevent SVGResources from chaining an incorrect type of element to the SVG filter, gradient and pattern elements. We also need to use the SVGResources for getting the referenced element when collecting the attributes for the gradient elements. SVGResources solves the cyclic referencing issue so there is no need to repeat the same code in many places. Also, from now on the SVGResources will have valid linked resource only. So casting the referenced element should always be valid. Tests: svg/custom/pattern-invalid-content-inheritance.svg * rendering/svg/RenderSVGResourcePattern.cpp: (WebCore::RenderSVGResourcePattern::collectPatternAttributes const): Asserts the linkedResource is of type RenderSVGResourcePattern. * rendering/svg/SVGResources.cpp: (WebCore::SVGResources::SVGResources): (WebCore::isChainableResource): Ensure that an SVG resource can reference only an SVG resource with the valid type. (WebCore::SVGResources::buildCachedResources): * rendering/svg/SVGResources.h: LayoutTests: * svg/custom/pattern-invalid-content-inheritance-expected.svg: Added. * svg/custom/pattern-invalid-content-inheritance.svg: Added.
WebKit · Oct 16, 2017 · 26c2623 · 26c2623
1 parent 1c9a997
commit 26c2623
Show file tree

Hide file tree

Showing 7 changed files with 90 additions and 5 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,13 @@
+2017-09-20  Said Abou-Hallawa  <sabouhallawa@apple.com>
+
+        REGRESSION(r191731): SVGPatternElement can only reference another SVGPatternElement in the same SVG document
+        https://bugs.webkit.org/show_bug.cgi?id=176221
+
+        Reviewed by Tim Horton.
+
+        * svg/custom/pattern-invalid-content-inheritance-expected.svg: Added.
+        * svg/custom/pattern-invalid-content-inheritance.svg: Added.
+
 2017-09-20  Joanmarie Diggs  <jdiggs@igalia.com>
 
         [ATK] atk_table_get_n_rows() and atk_table_get_n_columns() should return values of aria-rowcount and aria-colcount, if present

diff --git a/LayoutTests/svg/custom/pattern-invalid-content-inheritance-expected.svg b/LayoutTests/svg/custom/pattern-invalid-content-inheritance-expected.svg
diff --git a/LayoutTests/svg/custom/pattern-invalid-content-inheritance.svg b/LayoutTests/svg/custom/pattern-invalid-content-inheritance.svg
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,48 @@
+2017-09-20  Said Abou-Hallawa  <sabouhallawa@apple.com>
+
+        REGRESSION(r191731): SVGPatternElement can only reference another SVGPatternElement in the same SVG document
+        https://bugs.webkit.org/show_bug.cgi?id=176221
+
+        Reviewed by Tim Horton.
+
+        According to the specs:
+
+        https://www.w3.org/TR/SVG11/filters.html#FilterElementHrefAttribute
+        https://www.w3.org/TR/SVG11/pservers.html#LinearGradientElementHrefAttribute
+        https://www.w3.org/TR/SVG11/pservers.html#RadialGradientElementHrefAttribute
+        https://www.w3.org/TR/SVG11/pservers.html#PatternElementHrefAttribute
+
+        The xlink:href attribute of the SVG filter, gradient and pattern elements
+        must reference another element within the current SVG of the same type.
+
+        In r191731, the code of SVGPatternElement::collectPatternAttributes() was
+        removed and replaced by RenderSVGResourcePattern::collectPatternAttributes()
+        to avoid cyclic reference in the pattern element. The problem is the old
+        code used to check whether the referenced element is<SVGPatternElement>
+        before casting it. This code was not copied to the new function. So we
+        now allow the SVGPatternElement to reference any SVG resource element.
+
+        To fix this issue, we need to prevent SVGResources from chaining an incorrect
+        type of element to the SVG filter, gradient and pattern elements.
+
+        We also need to use the SVGResources for getting the referenced element
+        when collecting the attributes for the gradient elements. SVGResources solves
+        the cyclic referencing issue so there is no need to repeat the same code
+        in many places. Also, from now on the SVGResources will have valid linked
+        resource only. So casting the referenced element should always be valid.
+
+        Tests: svg/custom/pattern-invalid-content-inheritance.svg
+
+        * rendering/svg/RenderSVGResourcePattern.cpp:
+        (WebCore::RenderSVGResourcePattern::collectPatternAttributes const): Asserts
+        the linkedResource is of type RenderSVGResourcePattern.
+        * rendering/svg/SVGResources.cpp:
+        (WebCore::SVGResources::SVGResources):
+        (WebCore::isChainableResource): Ensure that an SVG resource can reference
+        only an SVG resource with the valid type.
+        (WebCore::SVGResources::buildCachedResources):
+        * rendering/svg/SVGResources.h:
+
 2017-09-20  Joanmarie Diggs  <jdiggs@igalia.com>
 
         [ATK] atk_table_get_n_rows() and atk_table_get_n_columns() should return values of aria-rowcount and aria-colcount, if present

diff --git a/Source/WebCore/rendering/svg/RenderSVGResourcePattern.cpp b/Source/WebCore/rendering/svg/RenderSVGResourcePattern.cpp
@@ -64,6 +64,7 @@ void RenderSVGResourcePattern::collectPatternAttributes(PatternAttributes& attri
         pattern.collectPatternAttributes(attributes);
 
         auto* resources = SVGResourcesCache::cachedResourcesForRenderer(*current);
+        ASSERT_IMPLIES(resources && resources->linkedResource(), is<RenderSVGResourcePattern>(resources->linkedResource()));
         current = resources ? downcast<RenderSVGResourcePattern>(resources->linkedResource()) : nullptr;
     }
 }

diff --git a/Source/WebCore/rendering/svg/SVGResources.cpp b/Source/WebCore/rendering/svg/SVGResources.cpp
@@ -39,7 +39,6 @@
 namespace WebCore {
 
 SVGResources::SVGResources()
-    : m_linkedResource(0)
 {
 }
 
@@ -154,6 +153,21 @@ static inline String targetReferenceFromResource(SVGElement& element)
     return SVGURIReference::fragmentIdentifierFromIRIString(target, element.document());
 }
 
+static inline bool isChainableResource(const SVGElement& element, const SVGElement& linkedResource)
+{
+    if (is<SVGPatternElement>(element))
+        return is<SVGPatternElement>(linkedResource);
+
+    if (is<SVGGradientElement>(element))
+        return is<SVGGradientElement>(linkedResource);
+
+    if (is<SVGFilterElement>(element))
+        return is<SVGFilterElement>(linkedResource);
+
+    ASSERT_NOT_REACHED();
+    return false;
+}
+
 static inline RenderSVGResourceContainer* paintingResourceFromSVGPaint(Document& document, const SVGPaintType& paintType, const String& paintUri, AtomicString& id, bool& hasPendingResource)
 {
     if (paintType != SVG_PAINTTYPE_URI && paintType != SVG_PAINTTYPE_URI_RGBCOLOR && paintType != SVG_PAINTTYPE_URI_CURRENTCOLOR)
@@ -274,10 +288,13 @@ bool SVGResources::buildCachedResources(const RenderElement& renderer, const Ren
 
     if (chainableResourceTags().contains(tagName)) {
         AtomicString id(targetReferenceFromResource(element));
-        if (setLinkedResource(getRenderSVGResourceContainerById(document, id)))
-            foundResources = true;
-        else
+        auto* linkedResource = getRenderSVGResourceContainerById(document, id);
+        if (!linkedResource)
             registerPendingResource(extensions, id, element);
+        else if (isChainableResource(element, linkedResource->element())) {
+            setLinkedResource(linkedResource);
+            foundResources = true;
+        }
     }
 
     return foundResources;

diff --git a/Source/WebCore/rendering/svg/SVGResources.h b/Source/WebCore/rendering/svg/SVGResources.h
@@ -155,7 +155,7 @@ class SVGResources {
     std::unique_ptr<ClipperFilterMaskerData> m_clipperFilterMaskerData;
     std::unique_ptr<MarkerData> m_markerData;
     std::unique_ptr<FillStrokeData> m_fillStrokeData;
-    RenderSVGResourceContainer* m_linkedResource;
+    RenderSVGResourceContainer* m_linkedResource { nullptr };
 };
 
 } // namespace WebCore