From 2e0297d7035a807d4a18c89c8b310ec06bef005d Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sun, 10 Mar 2024 12:18:09 +0100 Subject: [PATCH] LibWeb: Handle reference cycles in SVG gradient linking Since SVG gradients can reference each other, we have to keep track of visited gradients when traversing the link chain, or we will recurse infinitely when there's a reference cycle. --- .../SVG/gradient-with-reference-cycle.txt | 1 + .../SVG/gradient-with-reference-cycle.html | 13 ++++ .../LibWeb/SVG/SVGGradientElement.cpp | 36 ++++++++--- .../Libraries/LibWeb/SVG/SVGGradientElement.h | 21 +++++-- .../LibWeb/SVG/SVGLinearGradientElement.cpp | 40 ++++++++++--- .../LibWeb/SVG/SVGLinearGradientElement.h | 9 ++- .../LibWeb/SVG/SVGRadialGradientElement.cpp | 60 +++++++++++++++---- .../LibWeb/SVG/SVGRadialGradientElement.h | 11 +++- 8 files changed, 155 insertions(+), 36 deletions(-) create mode 100644 Tests/LibWeb/Text/expected/SVG/gradient-with-reference-cycle.txt create mode 100644 Tests/LibWeb/Text/input/SVG/gradient-with-reference-cycle.html diff --git a/Tests/LibWeb/Text/expected/SVG/gradient-with-reference-cycle.txt b/Tests/LibWeb/Text/expected/SVG/gradient-with-reference-cycle.txt new file mode 100644 index 0000000000..b7a061f853 --- /dev/null +++ b/Tests/LibWeb/Text/expected/SVG/gradient-with-reference-cycle.txt @@ -0,0 +1 @@ + PASS (didn't hang or crash) diff --git a/Tests/LibWeb/Text/input/SVG/gradient-with-reference-cycle.html b/Tests/LibWeb/Text/input/SVG/gradient-with-reference-cycle.html new file mode 100644 index 0000000000..d9f61d2ef6 --- /dev/null +++ b/Tests/LibWeb/Text/input/SVG/gradient-with-reference-cycle.html @@ -0,0 +1,13 @@ + + + + + + + + + diff --git a/Userland/Libraries/LibWeb/SVG/SVGGradientElement.cpp b/Userland/Libraries/LibWeb/SVG/SVGGradientElement.cpp index 71bb28ed1b..c1a724d5e6 100644 --- a/Userland/Libraries/LibWeb/SVG/SVGGradientElement.cpp +++ b/Userland/Libraries/LibWeb/SVG/SVGGradientElement.cpp @@ -34,29 +34,47 @@ void SVGGradientElement::attribute_changed(FlyString const& name, Optional seen_gradients; + return gradient_units_impl(seen_gradients); +} + +GradientUnits SVGGradientElement::gradient_units_impl(HashTable& seen_gradients) const { if (m_gradient_units.has_value()) return *m_gradient_units; - if (auto gradient = linked_gradient()) - return gradient->gradient_units(); + if (auto gradient = linked_gradient(seen_gradients)) + return gradient->gradient_units_impl(seen_gradients); return GradientUnits::ObjectBoundingBox; } SpreadMethod SVGGradientElement::spread_method() const +{ + HashTable seen_gradients; + return spread_method_impl(seen_gradients); +} + +SpreadMethod SVGGradientElement::spread_method_impl(HashTable& seen_gradients) const { if (m_spread_method.has_value()) return *m_spread_method; - if (auto gradient = linked_gradient()) - return gradient->spread_method(); + if (auto gradient = linked_gradient(seen_gradients)) + return gradient->spread_method_impl(seen_gradients); return SpreadMethod::Pad; } Optional SVGGradientElement::gradient_transform() const +{ + HashTable seen_gradients; + return gradient_transform_impl(seen_gradients); +} + +Optional SVGGradientElement::gradient_transform_impl(HashTable& seen_gradients) const { if (m_gradient_transform.has_value()) return m_gradient_transform; - if (auto gradient = linked_gradient()) - return gradient->gradient_transform(); + if (auto gradient = linked_gradient(seen_gradients)) + return gradient->gradient_transform_impl(seen_gradients); return {}; } @@ -89,7 +107,7 @@ void SVGGradientElement::add_color_stops(Gfx::SVGGradientPaintStyle& paint_style }); } -JS::GCPtr SVGGradientElement::linked_gradient() const +JS::GCPtr SVGGradientElement::linked_gradient(HashTable& seen_gradients) const { // FIXME: This entire function is an ad-hoc hack! // It can only resolve # in the same document. @@ -103,8 +121,12 @@ JS::GCPtr SVGGradientElement::linked_gradient() const auto element = document().get_element_by_id(id.value()); if (!element) return {}; + if (element == this) + return {}; if (!is(*element)) return {}; + if (seen_gradients.set(&verify_cast(*element)) != AK::HashSetResult::InsertedNewEntry) + return {}; return &verify_cast(*element); } return {}; diff --git a/Userland/Libraries/LibWeb/SVG/SVGGradientElement.h b/Userland/Libraries/LibWeb/SVG/SVGGradientElement.h index ae9ac23e37..096d9b6705 100644 --- a/Userland/Libraries/LibWeb/SVG/SVGGradientElement.h +++ b/Userland/Libraries/LibWeb/SVG/SVGGradientElement.h @@ -55,12 +55,22 @@ protected: virtual void initialize(JS::Realm&) override; - JS::GCPtr linked_gradient() const; + JS::GCPtr linked_gradient(HashTable& seen_gradients) const; Gfx::AffineTransform gradient_paint_transform(SVGPaintContext const&) const; template Callback> void for_each_color_stop(Callback const& callback) const + { + HashTable seen_gradients; + return for_each_color_stop_impl(callback, seen_gradients); + } + + void add_color_stops(Gfx::SVGGradientPaintStyle&) const; + +private: + template Callback> + void for_each_color_stop_impl(Callback const& callback, HashTable& seen_gradients) const { bool color_stops_found = false; for_each_child_of_type([&](auto& stop) { @@ -68,14 +78,15 @@ protected: callback(stop); }); if (!color_stops_found) { - if (auto gradient = linked_gradient()) - gradient->for_each_color_stop(callback); + if (auto gradient = linked_gradient(seen_gradients)) + gradient->for_each_color_stop_impl(callback, seen_gradients); } } - void add_color_stops(Gfx::SVGGradientPaintStyle&) const; + GradientUnits gradient_units_impl(HashTable& seen_gradients) const; + SpreadMethod spread_method_impl(HashTable& seen_gradients) const; + Optional gradient_transform_impl(HashTable& seen_gradients) const; -private: Optional m_gradient_units = {}; Optional m_spread_method = {}; Optional m_gradient_transform = {}; diff --git a/Userland/Libraries/LibWeb/SVG/SVGLinearGradientElement.cpp b/Userland/Libraries/LibWeb/SVG/SVGLinearGradientElement.cpp index aa610176a2..84d5a11e77 100644 --- a/Userland/Libraries/LibWeb/SVG/SVGLinearGradientElement.cpp +++ b/Userland/Libraries/LibWeb/SVG/SVGLinearGradientElement.cpp @@ -48,44 +48,68 @@ void SVGLinearGradientElement::attribute_changed(FlyString const& name, Optional // https://www.w3.org/TR/SVG11/pservers.html#LinearGradientElementX1Attribute NumberPercentage SVGLinearGradientElement::start_x() const +{ + HashTable seen_gradients; + return start_x_impl(seen_gradients); +} + +NumberPercentage SVGLinearGradientElement::start_x_impl(HashTable& seen_gradients) const { if (m_x1.has_value()) return *m_x1; - if (auto gradient = linked_linear_gradient()) - return gradient->start_x(); + if (auto gradient = linked_linear_gradient(seen_gradients)) + return gradient->start_x_impl(seen_gradients); // If the attribute is not specified, the effect is as if a value of '0%' were specified. return NumberPercentage::create_percentage(0); } // https://www.w3.org/TR/SVG11/pservers.html#LinearGradientElementY1Attribute NumberPercentage SVGLinearGradientElement::start_y() const +{ + HashTable seen_gradients; + return start_y_impl(seen_gradients); +} + +NumberPercentage SVGLinearGradientElement::start_y_impl(HashTable& seen_gradients) const { if (m_y1.has_value()) return *m_y1; - if (auto gradient = linked_linear_gradient()) - return gradient->start_x(); + if (auto gradient = linked_linear_gradient(seen_gradients)) + return gradient->start_y_impl(seen_gradients); // If the attribute is not specified, the effect is as if a value of '0%' were specified. return NumberPercentage::create_percentage(0); } // https://www.w3.org/TR/SVG11/pservers.html#LinearGradientElementX2Attribute NumberPercentage SVGLinearGradientElement::end_x() const +{ + HashTable seen_gradients; + return end_x_impl(seen_gradients); +} + +NumberPercentage SVGLinearGradientElement::end_x_impl(HashTable& seen_gradients) const { if (m_x2.has_value()) return *m_x2; - if (auto gradient = linked_linear_gradient()) - return gradient->start_x(); + if (auto gradient = linked_linear_gradient(seen_gradients)) + return gradient->end_x_impl(seen_gradients); // If the attribute is not specified, the effect is as if a value of '100%' were specified. return NumberPercentage::create_percentage(100); } // https://www.w3.org/TR/SVG11/pservers.html#LinearGradientElementY2Attribute NumberPercentage SVGLinearGradientElement::end_y() const +{ + HashTable seen_gradients; + return end_y_impl(seen_gradients); +} + +NumberPercentage SVGLinearGradientElement::end_y_impl(HashTable& seen_gradients) const { if (m_y2.has_value()) return *m_y2; - if (auto gradient = linked_linear_gradient()) - return gradient->start_x(); + if (auto gradient = linked_linear_gradient(seen_gradients)) + return gradient->end_y_impl(seen_gradients); // If the attribute is not specified, the effect is as if a value of '0%' were specified. return NumberPercentage::create_percentage(0); } diff --git a/Userland/Libraries/LibWeb/SVG/SVGLinearGradientElement.h b/Userland/Libraries/LibWeb/SVG/SVGLinearGradientElement.h index 88840c64b5..4b60c4e50f 100644 --- a/Userland/Libraries/LibWeb/SVG/SVGLinearGradientElement.h +++ b/Userland/Libraries/LibWeb/SVG/SVGLinearGradientElement.h @@ -34,9 +34,9 @@ protected: virtual void initialize(JS::Realm&) override; private: - JS::GCPtr linked_linear_gradient() const + JS::GCPtr linked_linear_gradient(HashTable& seen_gradients) const { - if (auto gradient = linked_gradient(); gradient && is(*gradient)) + if (auto gradient = linked_gradient(seen_gradients); gradient && is(*gradient)) return &verify_cast(*gradient); return {}; } @@ -46,6 +46,11 @@ private: NumberPercentage end_x() const; NumberPercentage end_y() const; + NumberPercentage start_x_impl(HashTable& seen_gradients) const; + NumberPercentage start_y_impl(HashTable& seen_gradients) const; + NumberPercentage end_x_impl(HashTable& seen_gradients) const; + NumberPercentage end_y_impl(HashTable& seen_gradients) const; + Optional m_x1; Optional m_y1; Optional m_x2; diff --git a/Userland/Libraries/LibWeb/SVG/SVGRadialGradientElement.cpp b/Userland/Libraries/LibWeb/SVG/SVGRadialGradientElement.cpp index 98d1c4f3e8..6909559164 100644 --- a/Userland/Libraries/LibWeb/SVG/SVGRadialGradientElement.cpp +++ b/Userland/Libraries/LibWeb/SVG/SVGRadialGradientElement.cpp @@ -52,13 +52,19 @@ void SVGRadialGradientElement::attribute_changed(FlyString const& name, Optional // https://svgwg.org/svg2-draft/pservers.html#RadialGradientElementFXAttribute NumberPercentage SVGRadialGradientElement::start_circle_x() const +{ + HashTable seen_gradients; + return start_circle_x_impl(seen_gradients); +} + +NumberPercentage SVGRadialGradientElement::start_circle_x_impl(HashTable& seen_gradients) const { if (m_fx.has_value()) return *m_fx; // If the element references an element that specifies a value for 'fx', then the value of 'fx' is // inherited from the referenced element. - if (auto gradient = linked_radial_gradient()) - return gradient->start_circle_x(); + if (auto gradient = linked_radial_gradient(seen_gradients)) + return gradient->start_circle_x_impl(seen_gradients); // If attribute ‘fx’ is not specified, ‘fx’ will coincide with the presentational value of ‘cx’ for // the element whether the value for 'cx' was inherited or not. return end_circle_x(); @@ -66,13 +72,19 @@ NumberPercentage SVGRadialGradientElement::start_circle_x() const // https://svgwg.org/svg2-draft/pservers.html#RadialGradientElementFYAttribute NumberPercentage SVGRadialGradientElement::start_circle_y() const +{ + HashTable seen_gradients; + return start_circle_y_impl(seen_gradients); +} + +NumberPercentage SVGRadialGradientElement::start_circle_y_impl(HashTable& seen_gradients) const { if (m_fy.has_value()) return *m_fy; // If the element references an element that specifies a value for 'fy', then the value of 'fy' is // inherited from the referenced element. - if (auto gradient = linked_radial_gradient()) - return gradient->start_circle_y(); + if (auto gradient = linked_radial_gradient(seen_gradients)) + return gradient->start_circle_y_impl(seen_gradients); // If attribute ‘fy’ is not specified, ‘fy’ will coincide with the presentational value of ‘cy’ for // the element whether the value for 'cy' was inherited or not. return end_circle_y(); @@ -80,46 +92,70 @@ NumberPercentage SVGRadialGradientElement::start_circle_y() const // https://svgwg.org/svg2-draft/pservers.html#RadialGradientElementFRAttribute NumberPercentage SVGRadialGradientElement::start_circle_radius() const +{ + HashTable seen_gradients; + return start_circle_radius_impl(seen_gradients); +} + +NumberPercentage SVGRadialGradientElement::start_circle_radius_impl(HashTable& seen_gradients) const { // Note: A negative value is an error. if (m_fr.has_value() && m_fr->value() >= 0) return *m_fr; // if the element references an element that specifies a value for 'fr', then the value of // 'fr' is inherited from the referenced element. - if (auto gradient = linked_radial_gradient()) - return gradient->start_circle_radius(); + if (auto gradient = linked_radial_gradient(seen_gradients)) + return gradient->start_circle_radius_impl(seen_gradients); // If the attribute is not specified, the effect is as if a value of '0%' were specified. return NumberPercentage::create_percentage(0); } // https://svgwg.org/svg2-draft/pservers.html#RadialGradientElementCXAttribute NumberPercentage SVGRadialGradientElement::end_circle_x() const +{ + HashTable seen_gradients; + return end_circle_x_impl(seen_gradients); +} + +NumberPercentage SVGRadialGradientElement::end_circle_x_impl(HashTable& seen_gradients) const { if (m_cx.has_value()) return *m_cx; - if (auto gradient = linked_radial_gradient()) - return gradient->end_circle_x(); + if (auto gradient = linked_radial_gradient(seen_gradients)) + return gradient->end_circle_x_impl(seen_gradients); return NumberPercentage::create_percentage(50); } // https://svgwg.org/svg2-draft/pservers.html#RadialGradientElementCYAttribute NumberPercentage SVGRadialGradientElement::end_circle_y() const +{ + HashTable seen_gradients; + return end_circle_y_impl(seen_gradients); +} + +NumberPercentage SVGRadialGradientElement::end_circle_y_impl(HashTable& seen_gradients) const { if (m_cy.has_value()) return *m_cy; - if (auto gradient = linked_radial_gradient()) - return gradient->end_circle_y(); + if (auto gradient = linked_radial_gradient(seen_gradients)) + return gradient->end_circle_y_impl(seen_gradients); return NumberPercentage::create_percentage(50); } // https://svgwg.org/svg2-draft/pservers.html#RadialGradientElementRAttribute NumberPercentage SVGRadialGradientElement::end_circle_radius() const +{ + HashTable seen_gradients; + return end_circle_radius_impl(seen_gradients); +} + +NumberPercentage SVGRadialGradientElement::end_circle_radius_impl(HashTable& seen_gradients) const { // Note: A negative value is an error. if (m_r.has_value() && m_r->value() >= 0) return *m_r; - if (auto gradient = linked_radial_gradient()) - return gradient->end_circle_radius(); + if (auto gradient = linked_radial_gradient(seen_gradients)) + return gradient->end_circle_radius_impl(seen_gradients); return NumberPercentage::create_percentage(50); } diff --git a/Userland/Libraries/LibWeb/SVG/SVGRadialGradientElement.h b/Userland/Libraries/LibWeb/SVG/SVGRadialGradientElement.h index 31222582ae..8fa5c62f14 100644 --- a/Userland/Libraries/LibWeb/SVG/SVGRadialGradientElement.h +++ b/Userland/Libraries/LibWeb/SVG/SVGRadialGradientElement.h @@ -36,9 +36,9 @@ protected: virtual void initialize(JS::Realm&) override; private: - JS::GCPtr linked_radial_gradient() const + JS::GCPtr linked_radial_gradient(HashTable& seen_gradients) const { - if (auto gradient = linked_gradient(); gradient && is(*gradient)) + if (auto gradient = linked_gradient(seen_gradients); gradient && is(*gradient)) return &verify_cast(*gradient); return {}; } @@ -50,6 +50,13 @@ private: NumberPercentage end_circle_y() const; NumberPercentage end_circle_radius() const; + NumberPercentage start_circle_x_impl(HashTable& seen_gradients) const; + NumberPercentage start_circle_y_impl(HashTable& seen_gradients) const; + NumberPercentage start_circle_radius_impl(HashTable& seen_gradients) const; + NumberPercentage end_circle_x_impl(HashTable& seen_gradients) const; + NumberPercentage end_circle_y_impl(HashTable& seen_gradients) const; + NumberPercentage end_circle_radius_impl(HashTable& seen_gradients) const; + Optional m_cx; Optional m_cy; Optional m_fx;