From 26c26235033f369ee3803147baca9255b6d642b0 Mon Sep 17 00:00:00 2001 From: Said Abou-Hallawa Date: Mon, 16 Oct 2017 12:56:26 +0000 Subject: [PATCH] Merge r222304 - 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. 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 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. --- LayoutTests/ChangeLog | 10 +++++ ...n-invalid-content-inheritance-expected.svg | 3 ++ .../pattern-invalid-content-inheritance.svg | 9 ++++ Source/WebCore/ChangeLog | 45 +++++++++++++++++++ .../svg/RenderSVGResourcePattern.cpp | 1 + Source/WebCore/rendering/svg/SVGResources.cpp | 25 +++++++++-- Source/WebCore/rendering/svg/SVGResources.h | 2 +- 7 files changed, 90 insertions(+), 5 deletions(-) create mode 100644 LayoutTests/svg/custom/pattern-invalid-content-inheritance-expected.svg create mode 100644 LayoutTests/svg/custom/pattern-invalid-content-inheritance.svg diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog index 9b4b0e3537a7..4830ea52a101 100644 --- a/LayoutTests/ChangeLog +++ b/LayoutTests/ChangeLog @@ -1,3 +1,13 @@ +2017-09-20 Said Abou-Hallawa + + 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 [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 new file mode 100644 index 000000000000..aa22ba00cbb5 --- /dev/null +++ b/LayoutTests/svg/custom/pattern-invalid-content-inheritance-expected.svg @@ -0,0 +1,3 @@ + + + diff --git a/LayoutTests/svg/custom/pattern-invalid-content-inheritance.svg b/LayoutTests/svg/custom/pattern-invalid-content-inheritance.svg new file mode 100644 index 000000000000..63a7a1bd0019 --- /dev/null +++ b/LayoutTests/svg/custom/pattern-invalid-content-inheritance.svg @@ -0,0 +1,9 @@ + + + + + + + + + diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog index 71e0c4b80d99..bb8555d512a9 100644 --- a/Source/WebCore/ChangeLog +++ b/Source/WebCore/ChangeLog @@ -1,3 +1,48 @@ +2017-09-20 Said Abou-Hallawa + + 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 + 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 [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 index d3a1e3c90914..ec73a0f06e26 100644 --- 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(resources->linkedResource())); current = resources ? downcast(resources->linkedResource()) : nullptr; } } diff --git a/Source/WebCore/rendering/svg/SVGResources.cpp b/Source/WebCore/rendering/svg/SVGResources.cpp index ea9b45211573..f0237e37c00b 100644 --- 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(element)) + return is(linkedResource); + + if (is(element)) + return is(linkedResource); + + if (is(element)) + return is(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 index e92b76199f20..365db1b66b5e 100644 --- a/Source/WebCore/rendering/svg/SVGResources.h +++ b/Source/WebCore/rendering/svg/SVGResources.h @@ -155,7 +155,7 @@ class SVGResources { std::unique_ptr m_clipperFilterMaskerData; std::unique_ptr m_markerData; std::unique_ptr m_fillStrokeData; - RenderSVGResourceContainer* m_linkedResource; + RenderSVGResourceContainer* m_linkedResource { nullptr }; }; } // namespace WebCore