[librsvg/librsvg-2.52: 1/2] (#721): Catch circular references when drawing patterns




commit 348f1837b11c173bbad662c0dbd2752d9288019a
Author: Federico Mena Quintero <federico gnome org>
Date:   Mon Mar 14 11:38:47 2022 -0600

    (#721): Catch circular references when drawing patterns
    
    It seems I broke this when splitting patterns into the resolve stage
    and the drawing stage.
    
    Fixes https://gitlab.gnome.org/GNOME/librsvg/-/issues/721
    
    Part-of: <https://gitlab.gnome.org/GNOME/librsvg/-/merge_requests/679>

 src/drawing_ctx.rs                                 | 27 ++++++++++++++--------
 src/pattern.rs                                     | 19 +++++++++++++--
 .../render-crash/721-pattern-cycle-from-child.svg  |  7 ++++++
 .../721-pattern-cycle-from-other-child.svg         | 10 ++++++++
 4 files changed, 52 insertions(+), 11 deletions(-)
---
diff --git a/src/drawing_ctx.rs b/src/drawing_ctx.rs
index ba2ed541e..b5d926c9d 100644
--- a/src/drawing_ctx.rs
+++ b/src/drawing_ctx.rs
@@ -1035,10 +1035,25 @@ impl DrawingCtx {
         pattern: &UserSpacePattern,
         acquired_nodes: &mut AcquiredNodes<'_>,
     ) -> Result<bool, RenderingError> {
+        // Bail out early if the pattern has zero size, per the spec
         if approx_eq!(f64, pattern.width, 0.0) || approx_eq!(f64, pattern.height, 0.0) {
             return Ok(false);
         }
 
+        // Bail out early if this pattern has a circular reference
+        let pattern_node_acquired = match pattern.acquire_pattern_node(acquired_nodes) {
+            Ok(n) => n,
+
+            Err(AcquireError::CircularReference(ref node)) => {
+                rsvg_log!("circular reference in element {}", node);
+                return Ok(false);
+            }
+
+            _ => unreachable!(),
+        };
+
+        let pattern_node = pattern_node_acquired.get();
+
         let taffine = self.get_transform().pre_transform(&pattern.transform);
 
         let mut scwscale = (taffine.xx.powi(2) + taffine.xy.powi(2)).sqrt();
@@ -1084,11 +1099,10 @@ impl DrawingCtx {
 
             pattern_draw_ctx
                 .with_alpha(pattern.opacity, &mut |dc| {
-                    let pattern_cascaded =
-                        CascadedValues::new_from_node(&pattern.node_with_children);
+                    let pattern_cascaded = CascadedValues::new_from_node(pattern_node);
                     let pattern_values = pattern_cascaded.get();
 
-                    let elt = pattern.node_with_children.borrow_element();
+                    let elt = pattern_node.borrow_element();
 
                     let stacking_ctx = StackingContext::new(
                         acquired_nodes,
@@ -1104,12 +1118,7 @@ impl DrawingCtx {
                         false,
                         None,
                         &mut |an, dc, _transform| {
-                            pattern.node_with_children.draw_children(
-                                an,
-                                &pattern_cascaded,
-                                dc,
-                                false,
-                            )
+                            pattern_node.draw_children(an, &pattern_cascaded, dc, false)
                         },
                     )
                 })
diff --git a/src/pattern.rs b/src/pattern.rs
index 5d887266d..66bb66acb 100644
--- a/src/pattern.rs
+++ b/src/pattern.rs
@@ -5,7 +5,7 @@ use markup5ever::{expanded_name, local_name, namespace_url, ns};
 use crate::aspect_ratio::*;
 use crate::bbox::BoundingBox;
 use crate::coord_units::CoordUnits;
-use crate::document::{AcquiredNodes, NodeId, NodeStack};
+use crate::document::{AcquiredNode, AcquiredNodes, NodeId, NodeStack};
 use crate::drawing_ctx::ViewParams;
 use crate::element::{Draw, Element, ElementResult, SetAttributes};
 use crate::error::*;
@@ -112,7 +112,9 @@ pub struct UserSpacePattern {
     pub coord_transform: Transform,
     pub content_transform: Transform,
     pub opacity: UnitInterval,
-    pub node_with_children: Node,
+
+    // This one is private so the caller has to go through fn acquire_pattern_node()
+    node_with_children: Node,
 }
 
 #[derive(Default)]
@@ -399,6 +401,19 @@ impl ResolvedPattern {
     }
 }
 
+impl UserSpacePattern {
+    /// Gets the `<pattern>` node that contains the children to be drawn for the pattern's contents.
+    ///
+    /// This has to go through [AcquiredNodes] to catch circular references among
+    /// patterns and their children.
+    pub fn acquire_pattern_node(
+        &self,
+        acquired_nodes: &mut AcquiredNodes<'_>,
+    ) -> Result<AcquiredNode, AcquireError> {
+        acquired_nodes.acquire_ref(&self.node_with_children)
+    }
+}
+
 impl Pattern {
     fn get_unresolved(&self, node: &Node) -> Unresolved {
         let pattern = UnresolvedPattern {
diff --git a/tests/fixtures/render-crash/721-pattern-cycle-from-child.svg 
b/tests/fixtures/render-crash/721-pattern-cycle-from-child.svg
new file mode 100644
index 000000000..512051488
--- /dev/null
+++ b/tests/fixtures/render-crash/721-pattern-cycle-from-child.svg
@@ -0,0 +1,7 @@
+<svg xmlns="http://www.w3.org/2000/svg"; width="100" height="100">
+  <pattern id="p1" width="100%" height="100%">
+    <rect width="100" height="100" fill="url(#p1)"/>
+  </pattern>
+
+  <rect width="100" height="100" fill="url(#p1)"/>
+</svg>
diff --git a/tests/fixtures/render-crash/721-pattern-cycle-from-other-child.svg 
b/tests/fixtures/render-crash/721-pattern-cycle-from-other-child.svg
new file mode 100644
index 000000000..668c3d35d
--- /dev/null
+++ b/tests/fixtures/render-crash/721-pattern-cycle-from-other-child.svg
@@ -0,0 +1,10 @@
+<svg xmlns="http://www.w3.org/2000/svg"; width="100" height="100">
+  <pattern id="p1" width="100%" height="100%">
+    <rect width="100" height="100" fill="url(#p2)"/>
+  </pattern>
+  <pattern id="p2" width="100%" height="100%">
+    <rect width="100" height="100" fill="url(#p1)"/>
+  </pattern>
+
+  <rect width="100" height="100" fill="url(#p1)"/>
+</svg>


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]