diff --git a/crates/obscura-dom/src/serialize.rs b/crates/obscura-dom/src/serialize.rs index 608b5b4..b7e19ee 100644 --- a/crates/obscura-dom/src/serialize.rs +++ b/crates/obscura-dom/src/serialize.rs @@ -1,103 +1,151 @@ use crate::tree::{DomTree, NodeData, NodeId}; +// A unit of pending serialization work. Held on an explicit heap stack instead +// of the call stack so a deeply nested tree cannot overflow the thread stack and +// abort the process (a stack overflow is a hard abort that `op_dom`'s +// catch_unwind cannot recover). `descendants()` is iterative + capped for the +// same reason; the serializer must be too. +enum SerializeWork { + // Serialize this node; `include_self` mirrors the old recursive param. + Node(NodeId, bool), + // Emit a previously-opened element's closing tag, after its children. + CloseTag(String), +} + impl DomTree { pub fn outer_html(&self, node_id: NodeId) -> String { let mut buf = String::new(); - self.serialize_node(node_id, true, &mut buf); + self.serialize_worklist(vec![SerializeWork::Node(node_id, true)], &mut buf); buf } pub fn inner_html(&self, node_id: NodeId) -> String { let mut buf = String::new(); - self.serialize_children(node_id, &mut buf); + self.serialize_worklist(self.child_work(node_id), &mut buf); buf } - fn serialize_node(&self, node_id: NodeId, include_self: bool, buf: &mut String) { - let node = match self.get_node(node_id) { - Some(n) => n, - None => return, - }; - - match &node.data { - NodeData::Document => { - self.serialize_children(node_id, buf); - } - NodeData::Doctype { name, .. } => { - buf.push_str("'); - } - NodeData::Element { name, attrs, .. } => { - let tag = name.local.as_ref(); - if include_self { - buf.push('<'); - buf.push_str(tag); - for attr in attrs { - buf.push(' '); - let attr_name = attr.name.local.as_ref(); - buf.push_str(attr_name); - buf.push_str("=\""); - escape_attr(&attr.value, buf); - buf.push('"'); - } - buf.push('>'); - } - - if !is_void_element(tag) { - self.serialize_children(node_id, buf); - if include_self { - buf.push_str("'); - } - } - } - NodeData::Text { contents } => { - let parent_is_raw = node.parent - .and_then(|pid| { - self.with_node(pid, |p| { - p.as_element() - .map(|name| is_raw_text_element(name.local.as_ref())) - .unwrap_or(false) - }) - }) - .unwrap_or(false); - - if parent_is_raw { - buf.push_str(contents); - } else { - escape_text(contents, buf); - } - } - NodeData::Comment { contents } => { - buf.push_str("" (that ends the comment), but script can via - // document.createComment("a-->b"). Emitting it verbatim would close - // the comment early and let the trailing text escape into markup. - // Neutralize the terminator so the serialized output round-trips as - // a single comment. - if contents.contains("-->") { - buf.push_str(&contents.replace("-->", "-->")); - } else { - buf.push_str(contents); - } - buf.push_str("-->"); - } - NodeData::ProcessingInstruction { target, data } => { - buf.push_str("'); - } - } + // Children as work items in document order (top of the LIFO stack first). + fn child_work(&self, node_id: NodeId) -> Vec { + self.children(node_id) + .into_iter() + .rev() + .map(|c| SerializeWork::Node(c, true)) + .collect() } - fn serialize_children(&self, node_id: NodeId, buf: &mut String) { - for child_id in self.children(node_id) { - self.serialize_node(child_id, true, buf); + fn serialize_worklist(&self, mut stack: Vec, buf: &mut String) { + // Defense in depth, mirroring descendants(): a well-formed subtree emits + // at most one Node plus one CloseTag per node, so 2*nodes.len() work + // items bound a valid walk. Exceeding it means the graph is cyclic (the + // append_child / insert_before guards prevent that); stop rather than + // spin forever. On a valid tree this bound is never reached. + let max_steps = self + .node_slot_count() + .saturating_mul(2) + .saturating_add(16); + let mut steps = 0usize; + + while let Some(work) = stack.pop() { + steps += 1; + if steps > max_steps { + eprintln!("obscura: serialize worklist cap hit - tree has a cycle"); + break; + } + + let (node_id, include_self) = match work { + SerializeWork::CloseTag(tag) => { + buf.push_str("'); + continue; + } + SerializeWork::Node(node_id, include_self) => (node_id, include_self), + }; + + let node = match self.get_node(node_id) { + Some(n) => n, + None => continue, + }; + + match &node.data { + NodeData::Document => { + for w in self.child_work(node_id) { + stack.push(w); + } + } + NodeData::Doctype { name, .. } => { + buf.push_str("'); + } + NodeData::Element { name, attrs, .. } => { + let tag = name.local.as_ref(); + if include_self { + buf.push('<'); + buf.push_str(tag); + for attr in attrs { + buf.push(' '); + let attr_name = attr.name.local.as_ref(); + buf.push_str(attr_name); + buf.push_str("=\""); + escape_attr(&attr.value, buf); + buf.push('"'); + } + buf.push('>'); + } + + if !is_void_element(tag) { + // Push the closing tag first so it pops after all the + // children we push next. + if include_self { + stack.push(SerializeWork::CloseTag(tag.to_string())); + } + for w in self.child_work(node_id) { + stack.push(w); + } + } + } + NodeData::Text { contents } => { + let parent_is_raw = node.parent + .and_then(|pid| { + self.with_node(pid, |p| { + p.as_element() + .map(|name| is_raw_text_element(name.local.as_ref())) + .unwrap_or(false) + }) + }) + .unwrap_or(false); + + if parent_is_raw { + buf.push_str(contents); + } else { + escape_text(contents, buf); + } + } + NodeData::Comment { contents } => { + buf.push_str("" (that ends the comment), but script can via + // document.createComment("a-->b"). Emitting it verbatim would close + // the comment early and let the trailing text escape into markup. + // Neutralize the terminator so the serialized output round-trips as + // a single comment. + if contents.contains("-->") { + buf.push_str(&contents.replace("-->", "-->")); + } else { + buf.push_str(contents); + } + buf.push_str("-->"); + } + NodeData::ProcessingInstruction { target, data } => { + buf.push_str("'); + } + } } } } diff --git a/crates/obscura-dom/src/tree.rs b/crates/obscura-dom/src/tree.rs index 1b2e471..4b7369d 100644 --- a/crates/obscura-dom/src/tree.rs +++ b/crates/obscura-dom/src/tree.rs @@ -659,20 +659,27 @@ impl DomTree { } fn import_node_from(&self, parent_id: NodeId, source: &DomTree, source_node_id: NodeId) { - let node_data = { - let source_inner = source.inner.borrow(); - match source_inner.nodes.get(source_node_id.index()) { - Some(Some(node)) => node.data.clone(), - _ => return, + // Iterative DFS with an explicit (dest_parent, source_node) stack so a + // deeply nested source tree cannot overflow the thread stack and abort + // the process. Children are pushed in reverse so they are appended in + // document order (append_child always appends to the end, so each level + // keeps the source ordering). + let mut stack = vec![(parent_id, source_node_id)]; + while let Some((dest_parent, src_id)) = stack.pop() { + let node_data = { + let source_inner = source.inner.borrow(); + match source_inner.nodes.get(src_id.index()) { + Some(Some(node)) => node.data.clone(), + _ => continue, + } + }; + + let new_id = self.new_node(node_data); + self.append_child(dest_parent, new_id); + + for child_id in source.children(src_id).into_iter().rev() { + stack.push((new_id, child_id)); } - }; - - let new_id = self.new_node(node_data); - self.append_child(parent_id, new_id); - - let children = source.children(source_node_id); - for child_id in children { - self.import_node_from(new_id, source, child_id); } } @@ -680,6 +687,14 @@ impl DomTree { self.inner.borrow().nodes.iter().filter(|n| n.is_some()).count() } + // Number of node slots (live plus freed), i.e. the same upper bound + // descendants() uses to cap a tree walk. A well-formed subtree has at most + // this many nodes, so it is a safe ceiling for iterative walkers that need a + // cycle backstop. + pub(crate) fn node_slot_count(&self) -> usize { + self.inner.borrow().nodes.len() + } + pub fn is_empty(&self) -> bool { self.len() <= 1 } @@ -696,7 +711,28 @@ impl DomTree { } fn collect_text_inner(inner: &DomTreeInner, node_id: NodeId, buf: &mut String) { - if let Some(Some(node)) = inner.nodes.get(node_id.index()) { + // Iterative pre-order walk on an explicit heap stack. The recursive form + // overflowed the thread stack and aborted the process on deeply nested + // trees; descendants() is iterative + capped for the same reason. A valid + // subtree visits at most nodes.len() nodes, so exceeding that means the + // graph is cyclic (prevented by the append_child / insert_before guards); + // stop rather than spin forever. + let max_steps = inner.nodes.len().saturating_add(16); + let mut steps = 0usize; + let mut stack = vec![node_id]; + + while let Some(id) = stack.pop() { + steps += 1; + if steps > max_steps { + eprintln!("obscura: collect_text_inner cap hit - tree has a cycle"); + break; + } + + let node = match inner.nodes.get(id.index()) { + Some(Some(n)) => n, + _ => continue, + }; + match &node.data { NodeData::Text { contents } => buf.push_str(contents), // Comment and ProcessingInstruction are intentionally NOT @@ -704,13 +740,23 @@ fn collect_text_inner(inner: &DomTreeInner, node_id: NodeId, buf: &mut String) { // on an Element only includes Text descendants. Direct // textContent on a Comment/PI is handled by the caller. _ => { + // Collect children, then push them in reverse so they pop in + // document order. + let mut kids = Vec::new(); let mut child = node.first_child; while let Some(child_id) = child { - collect_text_inner(inner, child_id, buf); + kids.push(child_id); + if kids.len() > inner.nodes.len() { + eprintln!("obscura: collect_text_inner sibling cap hit - cycle"); + break; + } child = inner.nodes.get(child_id.index()) .and_then(|n| n.as_ref()) .and_then(|n| n.next_sibling); } + for child_id in kids.into_iter().rev() { + stack.push(child_id); + } } } } @@ -933,4 +979,67 @@ mod tests { tree.remove(div); assert_eq!(tree.len(), 1); } + + // Builds a chain of `depth` nested
elements under the document and + // returns (outermost_div, innermost_div). Depth this large overflows a + // recursive tree walk and aborts the process, so it guards the iterative + // serialize / text_content / import paths against regressing to recursion. + fn build_deep_chain(tree: &DomTree, depth: usize) -> (NodeId, NodeId) { + let mk_div = || { + tree.new_node(NodeData::Element { + name: QualName::new(None, ns!(html), local_name!("div")), + attrs: vec![], + template_contents: None, + mathml_annotation_xml_integration_point: false, + }) + }; + let root = mk_div(); + tree.append_child(tree.document(), root); + let mut cur = root; + for _ in 1..depth { + let next = mk_div(); + tree.append_child(cur, next); + cur = next; + } + (root, cur) + } + + #[test] + fn test_outer_html_deeply_nested_does_not_overflow() { + let tree = DomTree::new(); + let (root, leaf) = build_deep_chain(&tree, 100_000); + let marker = tree.new_node(NodeData::Text { + contents: "leaf".into(), + }); + tree.append_child(leaf, marker); + + let html = tree.outer_html(root); + assert!(html.starts_with("
")); + assert!(html.contains("leaf")); + assert!(html.ends_with("
")); + } + + #[test] + fn test_text_content_deeply_nested_does_not_overflow() { + let tree = DomTree::new(); + let (root, leaf) = build_deep_chain(&tree, 100_000); + let marker = tree.new_node(NodeData::Text { + contents: "deep".into(), + }); + tree.append_child(leaf, marker); + + assert_eq!(tree.text_content(root), "deep"); + } + + #[test] + fn test_import_deeply_nested_does_not_overflow() { + let source = DomTree::new(); + build_deep_chain(&source, 100_000); + + let dest = DomTree::new(); + let dest_doc = dest.document(); + dest.import_children_from(dest_doc, &source, source.document()); + + assert!(dest.len() >= 100_000); + } }