Skip to content

Commit 939ab51

Browse files
federicomenaquinteroMarge Bot
authored andcommitted
Try to propagate an error earlier when there are circular references
The problematic file has circular references, and they are detected, but the <use> element tries to just return an empty bbox rather than propagate the error upstream. The problem with this particular file is not that it enters infinite recursion, but that the <g> are nested so deep that we run out of stack. So, propagate errors about circular references upstream as much as possible, and stop propagating when we reach Node::draw(). The effect now is that rendering proceeds until the maximum number of referenced elements is reached, and then we get a hard error that makes the rendering terminate. I'm not 100% satisfied with this approach; it fixes the stack exhaustion, but we could try other things: - Mark <use> elements as "has a circular reference" so they are not re-rendered if referenced again. - Mark all the elements involved in a circular reference chain as tainted, so they can't get referenced again. Part-of: <https://gitlab.gnome.org/GNOME/librsvg/-/merge_requests/988>
1 parent f536198 commit 939ab51

File tree

5 files changed

+26
-5
lines changed

5 files changed

+26
-5
lines changed

rsvg/src/api.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,9 @@ impl From<InternalRenderingError> for RenderingError {
8484
InternalRenderingError::InvalidTransform => {
8585
RenderingError::Rendering("invalid transform".to_string())
8686
}
87+
InternalRenderingError::CircularReference(c) => {
88+
RenderingError::Rendering(format!("circular reference in node {c}"))
89+
}
8790
InternalRenderingError::IdNotFound => RenderingError::IdNotFound,
8891
InternalRenderingError::InvalidId(s) => RenderingError::InvalidId(s),
8992
InternalRenderingError::OutOfMemory(s) => RenderingError::OutOfMemory(s),

rsvg/src/drawing_ctx.rs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1755,7 +1755,7 @@ impl DrawingCtx {
17551755

17561756
Err(AcquireError::CircularReference(circular)) => {
17571757
rsvg_log!(self.session, "circular reference in element {}", circular);
1758-
return Ok(self.empty_bbox());
1758+
return Err(InternalRenderingError::CircularReference(circular));
17591759
}
17601760

17611761
_ => unreachable!(),
@@ -1765,8 +1765,13 @@ impl DrawingCtx {
17651765
Ok(acquired) => acquired,
17661766

17671767
Err(AcquireError::CircularReference(circular)) => {
1768-
rsvg_log!(self.session, "circular reference from {} to element {}", node, circular);
1769-
return Ok(self.empty_bbox());
1768+
rsvg_log!(
1769+
self.session,
1770+
"circular reference from {} to element {}",
1771+
node,
1772+
circular
1773+
);
1774+
return Err(InternalRenderingError::CircularReference(circular));
17701775
}
17711776

17721777
Err(AcquireError::MaxReferencesExceeded) => {

rsvg/src/error.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ impl fmt::Display for DefsLookupErrorKind {
121121
/// "If a transform function causes the current transformation matrix of an
122122
/// object to be non-invertible, the object and its content do not get
123123
/// displayed."
124-
#[derive(Debug, Clone)]
124+
#[derive(Clone)]
125125
pub enum InternalRenderingError {
126126
/// An error from the rendering backend.
127127
Rendering(String),
@@ -135,6 +135,8 @@ pub enum InternalRenderingError {
135135
/// the problematic element.
136136
InvalidTransform,
137137

138+
CircularReference(Node),
139+
138140
/// Tried to reference an SVG element that does not exist.
139141
IdNotFound,
140142

@@ -169,6 +171,9 @@ impl fmt::Display for InternalRenderingError {
169171
InternalRenderingError::Rendering(ref s) => write!(f, "rendering error: {s}"),
170172
InternalRenderingError::LimitExceeded(ref l) => write!(f, "{l}"),
171173
InternalRenderingError::InvalidTransform => write!(f, "invalid transform"),
174+
InternalRenderingError::CircularReference(ref c) => {
175+
write!(f, "circular reference in element {c}")
176+
}
172177
InternalRenderingError::IdNotFound => write!(f, "element id not found"),
173178
InternalRenderingError::InvalidId(ref s) => write!(f, "invalid id: {s:?}"),
174179
InternalRenderingError::OutOfMemory(ref s) => write!(f, "out of memory: {s}"),

rsvg/src/filters/error.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ use std::fmt;
33
use crate::error::InternalRenderingError;
44

55
/// An enumeration of errors that can occur during filter primitive rendering.
6-
#[derive(Debug, Clone)]
6+
#[derive(Clone)]
77
pub enum FilterError {
88
/// The filter was passed invalid input (the `in` attribute).
99
InvalidInput,

rsvg/src/node.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -348,6 +348,14 @@ impl NodeDraw for Node {
348348
// displayed."
349349
Err(InternalRenderingError::InvalidTransform) => Ok(draw_ctx.empty_bbox()),
350350

351+
Err(InternalRenderingError::CircularReference(node)) => {
352+
if node != *self {
353+
return Ok(draw_ctx.empty_bbox());
354+
} else {
355+
return Err(InternalRenderingError::CircularReference(node));
356+
}
357+
}
358+
351359
Err(e) => Err(e),
352360
};
353361

0 commit comments

Comments
 (0)