feat(xml): preserve strict end-tag parsing as an option

crates/dav/src/lib.rs

@@ -74,7 +74,14 @@ pub fn parse_multistatus(bytes: &[u8]) -> Result<El, Error> {
     if !text.chars().all(craft_xml::is_xml_char) {
         return Err(Error::Characters);
     }
-    let limits = Limits { max_input: MAX_BYTES, max_depth: MAX_DEPTH, max_nodes: MAX_NODES, max_attrs: MAX_ATTRS, max_attrs_total: MAX_ATTRS_TOTAL };
+    let limits = Limits {
+        max_input: MAX_BYTES,
+        max_depth: MAX_DEPTH,
+        max_nodes: MAX_NODES,
+        max_elements: MAX_NODES,
+        max_attrs: MAX_ATTRS,
+        max_attrs_total: MAX_ATTRS_TOTAL,
+    };
     let options = Options { doctype: Doctype::Reject, ..Options::canonical(&CANONICAL).strict().with_limits(limits) };
     let root = options.parse(bytes).map_err(Error::Xml)?;
     if root.name != "d:multistatus" {

crates/docx/src/xml.rs

@@ -373,6 +373,7 @@ fn parse_with(bytes: &[u8], extra: &[(&'static str, &str)], strict: bool) -> Res
         max_depth: MAX_DEPTH + 1,
         // Text stretches count as nodes too: two for each element.
         max_nodes: 2 * MAX_ELEMENTS,
+        max_elements: MAX_ELEMENTS,
         max_attrs: MAX_ATTRS,
         max_attrs_total: 2 * MAX_ELEMENTS,
     };

crates/idml/src/part.rs

@@ -4,8 +4,14 @@ use craft_xml::{DECL_STANDALONE, El, Local, Node, Options};
 
 use crate::{DOM_VERSION, PACKAGING_NS};
 
-pub(crate) const XML_LIMITS: craft_xml::Limits =
-    craft_xml::Limits { max_input: 512 << 20, max_depth: 256, max_nodes: 8_000_000, max_attrs: 1024, max_attrs_total: 32_000_000 };
+pub(crate) const XML_LIMITS: craft_xml::Limits = craft_xml::Limits {
+    max_input: 512 << 20,
+    max_depth: 256,
+    max_nodes: 8_000_000,
+    max_elements: 8_000_000,
+    max_attrs: 1024,
+    max_attrs_total: 32_000_000,
+};
 
 /// The XML limits a part is read with: 512 MiB of input, 256 levels of nesting (InDesign's
 /// packages stay under a few dozen), 8,000,000 nodes and 32,000,000 attributes in one part.

crates/materialx/src/lib.rs

@@ -183,7 +183,14 @@ const OPENPBR_WRITABLE: &[(&str, InputKind)] = &[
 impl Document {
     /// Parses a MaterialX document while enforcing size, nesting and node-count limits.
     pub fn parse(bytes: &[u8]) -> Result<Self, Error> {
-        let limits = Limits { max_input: MAX_BYTES, max_depth: MAX_DEPTH, max_nodes: MAX_NODES, max_attrs: 256, max_attrs_total: 400_000 };
+        let limits = Limits {
+            max_input: MAX_BYTES,
+            max_depth: MAX_DEPTH,
+            max_nodes: MAX_NODES,
+            max_elements: MAX_NODES,
+            max_attrs: 256,
+            max_attrs_total: 400_000,
+        };
         let options = Options {
             doctype: Doctype::Reject,
             unknown_entity: UnknownEntity::Error,

crates/odf/src/text/tests_more/lossless.rs

@@ -345,8 +345,14 @@ const SAMPLES: [Sample; 18] = [
 
 /// Whether `xml` is well-formed: every element closed, in order, once.
 fn well_formed(xml: &[u8]) -> bool {
-    let limits =
-        craft_xml::Limits { max_input: usize::MAX, max_depth: usize::MAX, max_nodes: usize::MAX, max_attrs: usize::MAX, max_attrs_total: usize::MAX };
+    let limits = craft_xml::Limits {
+        max_input: usize::MAX,
+        max_depth: usize::MAX,
+        max_nodes: usize::MAX,
+        max_elements: usize::MAX,
+        max_attrs: usize::MAX,
+        max_attrs_total: usize::MAX,
+    };
     craft_xml::Options::default().strict().with_limits(limits).parse(xml).is_ok()
 }
 

crates/odf/src/text/xml.rs

@@ -45,8 +45,14 @@ pub(super) fn events(xml: &[u8], mut f: impl FnMut(Ev)) {
 pub(super) fn events_at(xml: &[u8], mut f: impl FnMut(Ev, (usize, usize))) {
     // Names and attributes as written; no limit but the input's own size (each part is at most a
     // package entry), and at most 128 attributes kept per element.
-    let limits =
-        craft_xml::Limits { max_input: usize::MAX, max_depth: usize::MAX, max_nodes: usize::MAX, max_attrs: usize::MAX, max_attrs_total: usize::MAX };
+    let limits = craft_xml::Limits {
+        max_input: usize::MAX,
+        max_depth: usize::MAX,
+        max_nodes: usize::MAX,
+        max_elements: usize::MAX,
+        max_attrs: usize::MAX,
+        max_attrs_total: usize::MAX,
+    };
     let opts = craft_xml::Options::default().with_limits(limits);
     let mut events = opts.events(xml);
     // Malformed XML ends the stream.

crates/xml/src/lib.rs

@@ -49,6 +49,9 @@ pub enum Error {
     /// The tree would hold more than [`Limits::max_nodes`] nodes.
     #[error("the XML holds more than {0} nodes")]
     TooManyNodes(usize),
+    /// The tree would hold more than [`Limits::max_elements`] elements.
+    #[error("the XML holds more than {0} elements")]
+    TooManyElements(usize),
     /// One start tag has more than [`Limits::max_attrs`] attributes.
     #[error("an XML element has more than {0} attributes")]
     TooManyAttrs(usize),
@@ -76,6 +79,14 @@ pub enum Error {
 impl Error {
     /// Was a limit exceeded (as opposed to the markup being wrong)?
     pub fn is_limit(&self) -> bool {
-        matches!(self, Error::TooLarge(_) | Error::TooDeep(_) | Error::TooManyNodes(_) | Error::TooManyAttrs(_) | Error::TooManyAttrsTotal(_))
+        matches!(
+            self,
+            Error::TooLarge(_)
+                | Error::TooDeep(_)
+                | Error::TooManyNodes(_)
+                | Error::TooManyElements(_)
+                | Error::TooManyAttrs(_)
+                | Error::TooManyAttrsTotal(_)
+        )
     }
 }

crates/xml/src/read.rs

@@ -25,6 +25,10 @@ pub struct Limits {
     /// 2,000,000: a tree node costs about 72 bytes before its strings, and text lies between
     /// elements at most, so this is about a million elements of an indented document.
     pub max_nodes: usize,
+    /// Most elements kept in a tree at once, independent of text nodes. Elements handed to a
+    /// streaming callback leave the count with everything inside them. Default unlimited; the
+    /// node limit already bounds element count for callers using default limits.
+    pub max_elements: usize,
     /// Most attributes on one start tag, namespace declarations and malformed ones included.
     /// With [`Names::Canonical`] it is also the most namespace declarations in scope at once.
     /// Default 1024.
@@ -38,8 +42,14 @@ pub struct Limits {
 }
 
 impl Limits {
-    pub const DEFAULT: Limits =
-        Limits { max_input: 256 * 1024 * 1024, max_depth: 256, max_nodes: 2_000_000, max_attrs: 1024, max_attrs_total: 4_000_000 };
+    pub const DEFAULT: Limits = Limits {
+        max_input: 256 * 1024 * 1024,
+        max_depth: 256,
+        max_nodes: 2_000_000,
+        max_elements: usize::MAX,
+        max_attrs: 1024,
+        max_attrs_total: 4_000_000,
+    };
 }
 
 impl Default for Limits {
@@ -174,6 +184,11 @@ pub struct Options<'a> {
     /// malformed attribute is skipped, and bytes that are not UTF-8 become U+FFFD. Strict: each
     /// of those is an error, as are a second root element and text outside the root.
     pub strict: bool,
+    /// Require closing tags to match their opening tags, and reject unclosed elements at EOF,
+    /// while keeping the other lenient behaviors (including stopping after the first root).
+    /// This is useful when a format historically accepted a single-root prefix but rejected
+    /// malformed nesting.
+    pub strict_end_tags: bool,
     pub unknown_entity: UnknownEntity,
     pub doctype: Doctype,
     /// Keep CDATA sections as [`Node::CData`] instead of joining them to the text around them.
@@ -197,6 +212,7 @@ impl Default for Options<'_> {
             names: Names::AsWritten,
             ns_decls: true,
             strict: false,
+            strict_end_tags: false,
             unknown_entity: UnknownEntity::Drop,
             doctype: Doctype::Ignore,
             cdata: false,
@@ -271,7 +287,7 @@ impl<'a> Options<'a> {
         }
         let text = to_utf8(bytes);
         let mut events = Events::new(*self, &text, false);
-        let mut tree = Builder { opts: self, split, stack: Vec::new(), marks: Vec::new(), kept: 0, attrs: 0, root: None, at: 0 };
+        let mut tree = Builder { opts: self, split, stack: Vec::new(), marks: Vec::new(), kept: 0, elements: 0, attrs: 0, root: None, at: 0 };
         let mut err = tree.run(&mut events, &mut each).err();
         // What was open when the read stopped goes into its parent as it is.
         while let Some(el) = tree.stack.pop() {
@@ -443,9 +459,9 @@ impl<'o, 'i> Events<'o, 'i> {
             }
             Q::End(e) => {
                 let Some(top) = self.open.last() else {
-                    return if self.opts.strict { self.syntax("an end tag has no start tag") } else { Ok(None) };
+                    return if self.opts.strict || self.opts.strict_end_tags { self.syntax("an end tag has no start tag") } else { Ok(None) };
                 };
-                if self.opts.strict && top.raw != e.name().as_ref() {
+                if (self.opts.strict || self.opts.strict_end_tags) && top.raw != e.name().as_ref() {
                     return self.syntax(format!("the end tag `{}` does not match its start tag", String::from_utf8_lossy(e.name().as_ref())));
                 }
                 self.end()
@@ -485,7 +501,11 @@ impl<'o, 'i> Events<'o, 'i> {
             }
             Q::Decl(_) => return Ok(None),
             Q::Eof => {
-                return if self.opts.strict && !self.open.is_empty() { self.syntax("an element is not closed") } else { Ok(Some(None)) };
+                return if (self.opts.strict || self.opts.strict_end_tags) && !self.open.is_empty() {
+                    self.syntax("an element is not closed")
+                } else {
+                    Ok(Some(None))
+                };
             }
         };
         Ok(Some(Some(event)))
@@ -614,7 +634,7 @@ impl<'o, 'i> Events<'o, 'i> {
         let keep_name = self.end_names || canonical.is_some_and(|c| c.element.is_some());
         self.open.push(Open {
             scope,
-            raw: if self.opts.strict { raw.as_ref().to_vec() } else { Vec::new() },
+            raw: if self.opts.strict || self.opts.strict_end_tags { raw.as_ref().to_vec() } else { Vec::new() },
             name: if keep_name { name.clone() } else { String::new() },
         });
         Ok(Event::Start { name, attrs: out })
@@ -626,8 +646,9 @@ struct Builder<'b, 'o> {
     split: &'b [&'b str],
     stack: Vec<El>,
     /// The node and attribute counts when each open element started.
-    marks: Vec<(usize, usize)>,
+    marks: Vec<(usize, usize, usize)>,
     kept: usize,
+    elements: usize,
     /// Attributes in the elements built so far and not handed out.
     attrs: usize,
     root: Option<El>,
@@ -686,7 +707,11 @@ impl Builder<'_, '_> {
                 if self.root.is_some() {
                     return Err(Error::Syntax { at: self.at, msg: "more than one root element".into() }.into());
                 }
-                self.marks.push((self.kept, self.attrs));
+                self.marks.push((self.kept, self.attrs, self.elements));
+                self.elements = self.elements.saturating_add(1);
+                if self.elements > self.opts.limits.max_elements {
+                    return Err(Error::TooManyElements(self.opts.limits.max_elements).into());
+                }
                 self.attrs = self.attrs.saturating_add(attrs.len());
                 if self.attrs > self.opts.limits.max_attrs_total {
                     return Err(Error::TooManyAttrsTotal(self.opts.limits.max_attrs_total).into());
@@ -710,7 +735,7 @@ impl Builder<'_, '_> {
 
     /// Attaches a complete element to its parent, hands it out, or makes it the root.
     fn finish<E: From<Error>>(&mut self, mut el: El, each: &mut impl FnMut(El) -> Result<(), E>) -> Result<(), E> {
-        let (mark, attrs_mark) = self.marks.pop().unwrap_or((0, 0));
+        let (mark, attrs_mark, elements_mark) = self.marks.pop().unwrap_or((0, 0, 0));
         if self.opts.drop_blank_text && el.children.iter().any(|n| matches!(n, Node::El(_))) {
             let is_text = |n: &Node| matches!(n, Node::Text(_));
             if el.children.iter().all(|n| !is_text(n) || matches!(n, Node::Text(t) if t.trim().is_empty())) {
@@ -727,6 +752,7 @@ impl Builder<'_, '_> {
             each(el)?;
             // The element and everything inside it have left the tree.
             self.kept = mark;
+            self.elements = elements_mark;
             self.attrs = attrs_mark;
         } else {
             self.push(Node::El(el))?;

crates/xml/tests/hostile.rs

@@ -22,14 +22,40 @@ fn text_len(e: &El) -> usize {
 fn the_defaults_are_the_documented_ones() {
     assert_eq!(
         Limits::default(),
-        Limits { max_input: 256 * 1024 * 1024, max_depth: 256, max_nodes: 2_000_000, max_attrs: 1024, max_attrs_total: 4_000_000 }
+        Limits {
+            max_input: 256 * 1024 * 1024,
+            max_depth: 256,
+            max_nodes: 2_000_000,
+            max_elements: usize::MAX,
+            max_attrs: 1024,
+            max_attrs_total: 4_000_000
+        }
     );
-    for e in [Error::TooLarge(1), Error::TooDeep(1), Error::TooManyNodes(1), Error::TooManyAttrs(1), Error::TooManyAttrsTotal(1)] {
+    for e in [
+        Error::TooLarge(1),
+        Error::TooDeep(1),
+        Error::TooManyNodes(1),
+        Error::TooManyElements(1),
+        Error::TooManyAttrs(1),
+        Error::TooManyAttrsTotal(1),
+    ] {
         assert!(e.is_limit(), "{e}");
     }
     assert!(!Error::NoRoot.is_limit() && !Error::Doctype.is_limit());
 }
 
+#[test]
+fn strict_end_tags_keep_first_root_and_entity_compatibility() {
+    let opts = Options { strict_end_tags: true, unknown_entity: UnknownEntity::Literal, doctype: Doctype::Ignore, ..Options::default() };
+    let root = opts.parse(b"<!DOCTYPE a [<!ENTITY ignored 'x'>]><a>&unknown;<b/></a><tail/>").unwrap();
+    assert_eq!(root.name, "a");
+    assert_eq!(root.text().as_ref(), "&unknown;");
+
+    for malformed in [b"<a><b></a>".as_slice(), b"<a><b/>".as_slice(), b"</a><b/>".as_slice()] {
+        assert!(opts.parse(malformed).is_err(), "{}", String::from_utf8_lossy(malformed));
+    }
+}
+
 #[test]
 fn a_depth_bomb_is_refused_at_the_limit() {
     let limits = Limits { max_depth: 8, ..Limits::DEFAULT };
@@ -412,6 +438,23 @@ fn a_node_count_charges_a_budget_shared_by_several_parts() {
     assert_eq!(short.parse(part.as_bytes()), Err(Error::TooManyNodes(200)));
 }
 
+#[test]
+fn element_limits_are_independent_and_streamed_rows_release_them() {
+    let limited = Options::default().with_limits(Limits { max_elements: 2, ..Limits::DEFAULT });
+    assert_eq!(limited.parse(b"<r><a/><b/></r>"), Err(Error::TooManyElements(2)));
+
+    let streaming = Options::default().with_limits(Limits { max_elements: 3, ..Limits::DEFAULT });
+    let mut rows = 0;
+    let root = streaming
+        .parse_streaming(b"<s><row><a/></row><row><b/></row></s>", &["row"], |_| {
+            rows += 1;
+            Ok::<(), Error>(())
+        })
+        .unwrap();
+    assert_eq!(rows, 2);
+    assert!(root.children.is_empty());
+}
+
 /// A part that cannot be read to its end still gives what came before the failure.
 #[test]
 fn a_failed_streaming_read_returns_what_it_had() {