[librsvg: 1/5] Write internal docs for AcquiredNodes
- From: Marge Bot <marge-bot src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [librsvg: 1/5] Write internal docs for AcquiredNodes
- Date: Wed, 16 Mar 2022 01:43:46 +0000 (UTC)
commit ccb60160b1534783b89784e7c4f7f20600091a9e
Author: Federico Mena Quintero <federico gnome org>
Date: Mon Mar 14 17:31:26 2022 -0600
Write internal docs for AcquiredNodes
Part-of: <https://gitlab.gnome.org/GNOME/librsvg/-/merge_requests/681>
src/document.rs | 54 +++++++++++++++++++++++++++++++++++++++++++++---------
src/error.rs | 20 ++++++++++++++++++++
2 files changed, 65 insertions(+), 9 deletions(-)
---
diff --git a/src/document.rs b/src/document.rs
index 406a6d79a..f682f7460 100644
--- a/src/document.rs
+++ b/src/document.rs
@@ -339,15 +339,37 @@ impl AcquiredNode {
}
}
-/// This helper struct is used when looking up urls to other nodes.
-/// Its methods do recursion checking and thereby avoid infinite loops.
+/// Detects circular references between nodes, and enforces referencing limits.
///
-/// Malformed SVGs, for example, may reference a marker by its IRI, but
-/// the object referenced by the IRI is not a marker.
+/// Consider this fragment of SVG:
///
-/// Note that if you acquire a node, you have to release it before trying to
-/// acquire it again. If you acquire a node "#foo" and don't release it before
-/// trying to acquire "foo" again, you will obtain a None the second time.
+/// ```xml
+/// <pattern id="foo">
+/// <rect width="1" height="1" fill="url(#foo)"/>
+/// </pattern>
+/// ```
+///
+/// The pattern has a child element that references the pattern itself. This kind of circular
+/// reference is invalid. The `AcquiredNodes` struct is passed around
+/// wherever it may be necessary to resolve references to nodes, or to access nodes
+/// "elsewhere" in the DOM that is not the current subtree.
+///
+/// Also, such constructs that reference other elements can be maliciously arranged like
+/// in the [billion laughs attack][lol], to cause huge amounts of CPU to be consumed through
+/// creating an exponential number of references. `AcquiredNodes` imposes a hard limit on
+/// the number of references that can be resolved for typical, well-behaved SVG documents.
+///
+/// The [`Self::acquire()`] and [`Self::acquire_ref()`] methods return an [`AcquiredNode`], which
+/// acts like a smart pointer for a [`Node`]. Once a node has been acquired, it cannot be
+/// acquired again until its [`AcquiredNode`] is dropped. In the example above, a graphic element
+/// would acquire the `pattern`, which would then acquire its `rect` child, which then would fail
+/// to re-acquired the `pattern` — thus signaling a circular reference.
+///
+/// Those methods return an [`AcquireError`] to signal circular references. Also, they
+/// can return [`AcquireError::MaxReferencesExceeded`] if the aforementioned referencing limit
+/// is exceeded.
+///
+/// [lol]: https://bitbucket.org/tiran/defusedxml
pub struct AcquiredNodes<'i> {
document: &'i Document,
num_elements_acquired: usize,
@@ -367,8 +389,10 @@ impl<'i> AcquiredNodes<'i> {
self.document.lookup_image(href)
}
- /// Acquires a node.
- /// Nodes acquired by this function must be released in reverse acquiring order.
+ /// Acquires a node by its id.
+ ///
+ /// This is typically used during an "early resolution" stage, when XML `id`s are being
+ /// resolved to node references.
pub fn acquire(&mut self, node_id: &NodeId) -> Result<AcquiredNode, AcquireError> {
self.num_elements_acquired += 1;
@@ -402,6 +426,18 @@ impl<'i> AcquiredNodes<'i> {
}
}
+ /// Acquires a node whose reference is already known.
+ ///
+ /// This is useful for cases where a node is initially referenced by its id with
+ /// [`Self::acquire`] and kept around for later use. During the later use, the node
+ /// needs to be re-acquired with this method. For example:
+ ///
+ /// * At an "early resolution" stage, `acquire()` a pattern by its id, and keep around its
+ /// [`Node`] reference.
+ ///
+ /// * At the drawing stage, `acquire_ref()` the pattern node that we already had, so that
+ /// its child elements that reference other paint servers will be able to detect circular
+ /// references to the pattern.
pub fn acquire_ref(&self, node: &Node) -> Result<AcquiredNode, AcquireError> {
if self.node_stack.borrow().contains(node) {
Err(AcquireError::CircularReference(node.clone()))
diff --git a/src/error.rs b/src/error.rs
index afe422a20..0cf70177b 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -167,10 +167,30 @@ impl From<cairo::Error> for RenderingError {
}
}
+/// Errors from [`crate::document::AcquiredNodes`].
pub enum AcquireError {
+ /// An element with the specified id was not found.
LinkNotFound(NodeId),
+
InvalidLinkType(NodeId),
+
+ /// A circular reference was detected; non-fatal error.
+ ///
+ /// Callers are expected to treat the offending element as invalid, for example
+ /// if a graphic element uses a pattern fill, but the pattern in turn includes
+ /// another graphic element that references the same pattern.
+ ///
+ /// ```xml
+ /// <pattern id="foo">
+ /// <rect width="1" height="1" fill="url(#foo)"/>
+ /// </pattern>
+ /// ```
CircularReference(Node),
+
+ /// Too many referenced objects were resolved; fatal error.
+ ///
+ /// Callers are expected to exit as early as possible and return an error to
+ /// the public API. See [`ImplementationLimit::TooManyReferencedElements`] for details.
MaxReferencesExceeded,
}
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]