fix(xfa): bound the name and path bytes data nodes and queued instances copy

This commit is contained in:
Dylan M. Taylor committed 2026-11-03 20:18:35 +00:00
1 parent ecdd1e658c
commit 50fcdf52f9
4 files changed
+39 -3

No files matched your search

+4
View File
@@ -112,6 +112,10 @@ impl DataNode {
return None;
}
let have = node.children.iter().filter(|c| &c.name == name).count();
// Every skipped sibling owns a copy of the name: bounded as `Build::at` bounds a write.
if (*idx + 1).saturating_sub(have).saturating_mul(name.len()) > build::MAX_NEW_NAME_BYTES {
return None;
}
for _ in have..=*idx {
node.children.push(DataNode { name: name.clone(), ..Default::default() });
}
+1 -1
View File
@@ -38,7 +38,7 @@ pub(super) const MAX_INDEX: usize = 10_000;
/// Most bytes of names the nodes one write creates hold together: every sibling an index
/// skips over gets its own copy of the name.
const MAX_NEW_NAME_BYTES: usize = 4 << 20;
pub(super) const MAX_NEW_NAME_BYTES: usize = 4 << 20;
/// What one write may still create: nodes and bytes of their names.
pub(super) struct NewNodes {
+9 -2
View File
@@ -82,6 +82,11 @@ pub fn remove_data_instance(doc: &mut Document, path: &DataPath) -> Result<Datas
/// pass, which asks for the instances again).
const MAX_ENSURED: usize = 200_000;
/// Most bytes of path text those queued instances hold together: each one owns a copy of its
/// parent path and its name (twice, once as the path and once as its label), so a long ancestor
/// would otherwise be multiplied by the instance count. Ordinary paths are tens of bytes.
const MAX_ENSURED_BYTES: usize = 128 << 20;
// Instance nodes write_data_ops has queued on this thread, for tests that bound its work.
#[cfg(test)]
thread_local! {
@@ -99,7 +104,7 @@ pub fn write_data_ops(doc: &mut Document, ops: &[DataOp], reuse: Option<ObjRef>)
let mut run: Vec<Want> = Vec::new();
// Instances queued in this run, by parent and name: asking for more only queues the rest.
let mut queued: HashMap<(&DataPath, &str), usize> = HashMap::new();
let (mut ensured, mut capped) = (0usize, false);
let (mut ensured, mut ensured_bytes, mut capped) = (0usize, 0usize, false);
let flush = |run: &mut Vec<Want>, steps: &mut Vec<Vec<Want>>, removes: &mut Vec<Option<DataPath>>| {
if !run.is_empty() {
let mut seen = std::collections::HashSet::new();
@@ -117,12 +122,14 @@ pub fn write_data_ops(doc: &mut Document, ops: &[DataOp], reuse: Option<ObjRef>)
}
DataOp::Instances { parent, name, count } => {
let done = queued.entry((parent, name.as_str())).or_insert(0);
let per_instance = parent.iter().fold(name.len().saturating_mul(2), |n, (p, _)| n.saturating_add(p.len()).saturating_add(16));
while *done < (*count).min(MAX_INDEX) {
if ensured >= MAX_ENSURED {
if ensured >= MAX_ENSURED || ensured_bytes.saturating_add(per_instance) > MAX_ENSURED_BYTES {
capped = true;
break;
}
ensured += 1;
ensured_bytes += per_instance;
#[cfg(test)]
ENSURED.with(|n| n.set(n.get() + 1));
let k = *done;
+25
View File
@@ -4531,3 +4531,28 @@ fn a_container_is_as_wide_as_a_turned_draw_stands() {
assert!((got - right).abs() < 0.01, "{rotate}: the neighbour stands {got} right of the anchor, not {right}");
}
}
/// Every sibling a SOM index skips owns a copy of the element name: the in-memory builder refuses
/// a path whose copies would pass the name-byte bound the writer holds itself to, and creates
/// nothing for it.
#[test]
fn the_in_memory_data_builder_bounds_copied_name_bytes() {
let mut root = DataNode::default();
// 1 KiB x 9,999 skipped siblings is about 10 MB, over the 4 MiB bound.
let long = "n".repeat(1 << 10);
root.apply(&DataOp::Value { path: vec![(long.clone(), 9_999)], text: "x".into() });
assert!(root.children.is_empty());
// Within the bound, the same shape still builds (and keeps indexes).
root.apply(&DataOp::Value { path: vec![(long, 100)], text: "x".into() });
assert_eq!(root.children.len(), 101);
}
/// Each queued instance owns a copy of its parent path: a long ancestor stops the queue at the
/// byte allowance, reported as capped, instead of being multiplied by the instance count.
#[test]
fn queued_instances_are_bounded_by_the_path_bytes_they_copy() {
let mut doc = Document::new_empty();
let parent: DataPath = vec![("a".repeat(16 << 20), 0), ("b".repeat(16 << 20), 0)];
let written = crate::data::write_data_ops(&mut doc, &[DataOp::Instances { parent, name: "row".into(), count: 100 }], None).unwrap();
assert!(written.warnings.iter().any(|w| w.contains("not written past")), "{:?}", written.warnings);
}