diff --git a/crates/xfdf/src/common.rs b/crates/xfdf/src/common.rs index 488277dc19..1968cd0e85 100644 --- a/crates/xfdf/src/common.rs +++ b/crates/xfdf/src/common.rs @@ -8,6 +8,9 @@ use super::{DataError, Dict, Document, Object}; const MAX_NODES: u32 = 2_000_000; pub(crate) const MAX_DEPTH: usize = 64; +/// Most bytes of full field names and values one XFDF import expands to: a long parent name is +/// written into every descendant's full name, so a small file can name far more than it holds. +pub(crate) const MAX_XFDF_TEXT: usize = 64 << 20; /// The deepest element nesting accepted before the text reaches the XML parser. /// @@ -240,7 +243,13 @@ pub(crate) fn xml_name(name: &str) -> String { s } -pub(crate) fn walk_xfdf(node: roxmltree::Node, prefix: &str, out: &mut Vec<(String, Vec)>, depth: usize) -> Result<(), DataError> { +pub(crate) fn walk_xfdf( + node: roxmltree::Node, + prefix: &str, + out: &mut Vec<(String, Vec)>, + depth: usize, + bytes: &mut usize, +) -> Result<(), DataError> { if depth > MAX_DEPTH { return Err(DataError::Malformed(format!("XFDF fields nest more than {MAX_DEPTH} deep"))); } @@ -258,9 +267,13 @@ pub(crate) fn walk_xfdf(node: roxmltree::Node, prefix: &str, out: &mut Vec<(Stri // leaf holding only `` is not read yet, so it leaves the field alone. let names_kids_or_rich = f.children().any(|c| c.has_tag_name("field") || c.has_tag_name("value-richtext")); if !vals.is_empty() || !names_kids_or_rich { + *bytes = bytes.saturating_add(full.len()).saturating_add(vals.iter().map(String::len).fold(0, usize::saturating_add)); + if *bytes > MAX_XFDF_TEXT { + return Err(DataError::Malformed(format!("the file expands to more than {} MiB of field names and values", MAX_XFDF_TEXT >> 20))); + } out.push((full.clone(), vals)); } - walk_xfdf(f, &full, out, depth + 1)?; + walk_xfdf(f, &full, out, depth + 1, bytes)?; } Ok(()) } diff --git a/crates/xfdf/src/fdf.rs b/crates/xfdf/src/fdf.rs index ceb4ad99eb..3fe90f32f3 100644 --- a/crates/xfdf/src/fdf.rs +++ b/crates/xfdf/src/fdf.rs @@ -419,16 +419,27 @@ pub(crate) fn walk_fdf( let full = if prefix.is_empty() { t } else { format!("{prefix}.{t}") }; budget.spend(0, full.len())?; if let Some(v) = d.get(b"V").map(|v| fdf.resolve(v)) { - let vals: Vec = match &*v { - Object::String(s) => vec![s.to_text()], - Object::Name(n) => vec![pdfcraft_forms::name_text(n)], + // Each value is charged as it is decoded, before the next: an array of references to + // one long string is not decoded whole and then refused. + let mut vals: Vec = Vec::new(); + let keep = |vals: &mut Vec, s: String, budget: &mut ImportBudget| -> Result<(), DataError> { + budget.spend(1, s.len())?; + vals.push(s); + Ok(()) + }; + match &*v { + Object::String(s) => keep(&mut vals, s.to_text(), budget)?, + Object::Name(n) => keep(&mut vals, pdfcraft_forms::name_text(n), budget)?, Object::Array(a) => { budget.spend(a.len(), 0)?; - a.iter().filter_map(|x| fdf.resolve(x).as_string().map(|s| s.to_text())).collect() + for x in a { + if let Some(s) = fdf.resolve(x).as_string() { + keep(&mut vals, s.to_text(), budget)?; + } + } } - _ => Vec::new(), - }; - budget.spend(vals.len(), vals.iter().map(String::len).fold(0, usize::saturating_add))?; + _ => {} + } out.push((full.clone(), vals)); } if let Some(kids) = d.get(b"Kids").map(|k| fdf.resolve(k)) diff --git a/crates/xfdf/src/lib.rs b/crates/xfdf/src/lib.rs index 16c554d72f..a5f8f7f12c 100644 --- a/crates/xfdf/src/lib.rs +++ b/crates/xfdf/src/lib.rs @@ -124,7 +124,11 @@ pub fn export_data(doc: &Document, format: Format) -> String { ) } _ => { - let clean = |s: &str| s.replace(['\t', '\n', '\r'], " "); + // A formula-like cell gets a leading `'`, as in CSV; `tab_delimited` takes it off. + let clean = |s: &str| { + let s = if csv_guarded(s) { format!("'{s}") } else { s.to_string() }; + s.replace(['\t', '\n', '\r'], " ") + }; format!( "{}\n{}\n", fields.iter().map(|f| clean(&f.0)).collect::>().join("\t"), @@ -171,7 +175,7 @@ pub fn data_values(bytes: &[u8]) -> Result)>, DataError } let mut values = Vec::new(); if let Some(fields) = root.children().find(|c| c.has_tag_name("fields")) { - walk_xfdf(fields, "", &mut values, 0)?; + walk_xfdf(fields, "", &mut values, 0, &mut 0)?; } Ok(values) } @@ -184,9 +188,10 @@ pub fn data_values(bytes: &[u8]) -> Result)>, DataError /// is written with a leading `'`, which keeps it text; a plain number such as `-12.5` is not. pub fn merge_csv(files: &[Vec<(String, Vec)>]) -> String { let mut columns: Vec<&str> = Vec::new(); + let mut known: HashSet<&str> = HashSet::new(); for f in files { for (k, _) in f { - if !columns.contains(&k.as_str()) { + if known.insert(k.as_str()) { columns.push(k); } } @@ -199,7 +204,12 @@ pub fn merge_csv(files: &[Vec<(String, Vec)>]) -> String { let mut out = columns.iter().map(|c| q(c)).collect::>().join(","); out.push('\n'); for f in files { - let row: Vec = columns.iter().map(|c| f.iter().find(|(k, _)| k == c).map_or(String::new(), |(_, v)| q(&v.join(", ")))).collect(); + // The first value of a name repeated in one file wins, as the scan it replaces found it. + let mut by_name: HashMap<&str, &Vec> = HashMap::new(); + for (k, v) in f { + by_name.entry(k.as_str()).or_insert(v); + } + let row: Vec = columns.iter().map(|c| by_name.get(c).map_or(String::new(), |v| q(&v.join(", ")))).collect(); out.push_str(&row.join(",")); out.push('\n'); } @@ -326,7 +336,7 @@ fn import_table(doc: &mut Document, text: &str) -> Result { fn tab_delimited(t: &str) -> Result)>, DataError> { let mut lines = t.lines(); let (Some(head), Some(row)) = (lines.next(), lines.next()) else { return Err(DataError::UnknownFormat) }; - Ok(head.split('\t').map(str::to_string).zip(row.split('\t')).map(|(k, v)| (k, vec![v.to_string()])).collect()) + Ok(head.split('\t').map(|k| csv_text(k.to_string())).zip(row.split('\t')).map(|(k, v)| (k, vec![csv_text(v.to_string())])).collect()) } /// Import comments and/or field values from XFDF, FDF, XML, CSV or tab-delimited text diff --git a/crates/xfdf/src/tests.rs b/crates/xfdf/src/tests.rs index 0da17c1e67..341de91c1c 100644 --- a/crates/xfdf/src/tests.rs +++ b/crates/xfdf/src/tests.rs @@ -2099,3 +2099,80 @@ fn an_fdf_comment_popup_without_a_rect_is_dropped() { let popups = page.iter().filter(|a| dst.resolve(a).as_dict().is_some_and(|d| d.name(b"Subtype") == Some(b"Popup".as_slice()))).count(); assert_eq!(popups, 0); } + +/// Tab-delimited export guards a formula-like cell as CSV does, and the import takes the guard +/// off, so a spreadsheet never runs form data and the value survives the round trip. +#[test] +fn tab_delimited_export_guards_formula_cells_and_round_trips() { + let mut doc = blank(); + set_value(&mut doc, "Name", &FieldValue::Text("=SUM(A1:A9)".into())).unwrap(); + let text = export_data(&doc, Format::Txt); + let row = text.lines().nth(1).unwrap(); + assert!(row.starts_with("'=SUM(A1:A9)"), "{row:?}"); + let mut again = blank(); + import(&mut again, text.as_bytes()).unwrap(); + assert_eq!(values(&again).iter().find(|(k, _)| k == "Name").map(|(_, v)| v.clone()), Some(vec!["=SUM(A1:A9)".to_string()])); +} + +/// Merging many files with many distinct names stays linear: every column once, in first-seen +/// order, and a row looks its names up rather than scanning them. +#[test] +fn merge_csv_lists_each_column_once_in_order() { + let files: Vec)>> = + (0..3).map(|f| (0..4).map(|i| (format!("c{}", (i + f) % 5), vec![format!("{f}-{i}")])).collect()).collect(); + let csv = merge_csv(&files); + let mut lines = csv.lines(); + assert_eq!(lines.next(), Some("c0,c1,c2,c3,c4")); + assert_eq!(lines.next(), Some("0-0,0-1,0-2,0-3,")); + assert_eq!(lines.next(), Some(",1-0,1-1,1-2,1-3")); + assert_eq!(lines.next(), Some("2-3,,2-0,2-1,2-2")); +} + +/// A long parent name is written into every descendant's full name: past the import's text +/// bound the file is refused rather than expanded. +#[test] +fn xfdf_field_names_past_the_text_bound_are_refused() { + let parent = "p".repeat(1 << 20); + let kids: String = (0..100).map(|i| format!("v")).collect(); + let xfdf = + format!("{kids}"); + let mut doc = blank(); + match import(&mut doc, xfdf.as_bytes()) { + Err(DataError::Malformed(m)) => assert!(m.contains("64 MiB"), "{m}"), + other => panic!("{other:?}"), + } +} + +/// A same-name import never replaces a widget whose field type is inherited, or whose subtype is +/// written as a reference: the field keeps its dictionary and the import goes beside it. +#[test] +fn same_name_import_keeps_a_widget_with_an_inherited_field_type() { + let mut doc = blank(); + let page = pdfcraft_model::pages(&doc)[0].obj; + // A field with one widget kid: /FT on the field, the widget names it as /Parent. + let mut field = Dict::new(); + field.set(b"FT".to_vec(), Object::name("Tx")); + field.set(b"T".to_vec(), Object::String(PdfString::text("inherited"))); + let field = doc.add(field); + let subtype = doc.add(Object::name("Widget")); + let mut widget = Dict::new(); + widget.set(b"Type".to_vec(), Object::name("Annot")); + widget.set(b"Subtype".to_vec(), Object::Ref(subtype)); + widget.set(b"Rect".to_vec(), arr(&[10.0, 10.0, 60.0, 30.0])); + widget.set(b"NM".to_vec(), Object::String(PdfString::text("w1"))); + widget.set(b"Parent".to_vec(), Object::Ref(field)); + let widget = doc.add(widget); + doc.update_dict(field, |d| d.set(b"Kids".to_vec(), Object::Array(vec![Object::Ref(widget)]))).unwrap(); + doc.update_dict(page, |p| p.set(b"Annots".to_vec(), Object::Array(vec![Object::Ref(widget)]))).unwrap(); + let mut note = Dict::new(); + note.set(b"Type".to_vec(), Object::name("Annot")); + note.set(b"Subtype".to_vec(), Object::name("Text")); + note.set(b"Rect".to_vec(), arr(&[10.0, 10.0, 30.0, 30.0])); + note.set(b"NM".to_vec(), Object::String(PdfString::text("w1"))); + let added = xfdf::place(&mut doc, page, note).unwrap(); + assert_ne!(added, widget); + assert_eq!(doc.get(widget).as_dict().unwrap().name(b"Type"), Some(b"Annot".as_slice())); + assert!(doc.get(widget).as_dict().unwrap().reference(b"Parent").is_some(), "the widget was replaced"); + let annots = doc.get(page).as_dict().unwrap().get(b"Annots").unwrap().as_array().unwrap().len(); + assert_eq!(annots, 2); +} diff --git a/crates/xfdf/src/xfdf.rs b/crates/xfdf/src/xfdf.rs index 06ad9f34ec..a9831ca331 100644 --- a/crates/xfdf/src/xfdf.rs +++ b/crates/xfdf/src/xfdf.rs @@ -148,6 +148,8 @@ fn xfdf_fields(doc: &Document, out: &mut String) { struct Node { values: Vec, kids: Vec<(String, Node)>, + /// Each child's place in `kids`, by name: a wide level is not scanned per field. + index: std::collections::HashMap, } let mut root = Node::default(); for f in pdfcraft_forms::fields(doc) { @@ -162,10 +164,11 @@ fn xfdf_fields(doc: &Document, out: &mut String) { // the last level stay together in the deepest element's name, and the import joins them // back with periods into the same full name. for part in f.name.splitn(MAX_DEPTH, '.') { - let i = match node.kids.iter().position(|(k, _)| k == part) { - Some(i) => i, + let i = match node.index.get(part) { + Some(&i) => i, None => { node.kids.push((part.to_string(), Node::default())); + node.index.insert(part.to_string(), node.kids.len() - 1); node.kids.len() - 1 } }; @@ -214,8 +217,13 @@ pub fn export_xfdf(doc: &Document, comments: bool, fields: bool, file: &str) -> /// Set the values of fields named in `values` (full names), counting what took. pub(crate) fn apply_values(doc: &mut Document, values: &[(String, Vec)], report: &mut Report) { let all = pdfcraft_forms::fields(doc); + // The first field of a name, as the scan it replaces found it. + let mut by_name: std::collections::HashMap<&str, usize> = std::collections::HashMap::new(); + for (i, f) in all.iter().enumerate() { + by_name.entry(f.name.as_str()).or_insert(i); + } for (name, vals) in values { - let Some(f) = all.iter().find(|f| &f.name == name) else { continue }; + let Some(f) = by_name.get(name.as_str()).and_then(|&i| all.get(i)) else { continue }; let first = vals.first().cloned().unwrap_or_default(); let v = match f.kind { FieldKind::Text => FieldValue::Text(first), @@ -369,7 +377,27 @@ pub(crate) const LOCKED: i64 = 128; /// widget or field (replacing it would set the field's value and policy around the form setter). fn import_protected(doc: &Document, x: &Dict) -> bool { let flags = x.get(b"F").and_then(|f| doc.resolve(f).as_int()).unwrap_or(0); - flags & LOCKED != 0 || matches!(x.name(b"Subtype"), Some(b"Redact" | b"Widget")) || x.contains(b"FT") + let subtype = x.get(b"Subtype").map(|s| doc.resolve(s)); + flags & LOCKED != 0 || matches!(subtype.as_deref().and_then(Object::as_name), Some(b"Redact" | b"Widget")) || is_field(doc, x) +} + +/// Whether `x` is a form field or a widget of one: it has `/FT`, or a `/Parent` up the field +/// tree does (a widget inherits its field's type). +fn is_field(doc: &Document, x: &Dict) -> bool { + if x.contains(b"FT") { + return true; + } + let mut parent = x.reference(b"Parent"); + let mut seen = std::collections::HashSet::new(); + while let Some(r) = parent.filter(|r| seen.len() < 32 && seen.insert(*r)) { + let o = doc.get(r); + let Some(d) = o.as_dict() else { return false }; + if d.contains(b"FT") { + return true; + } + parent = d.reference(b"Parent"); + } + false } pub(crate) fn place(doc: &mut Document, page_ref: ObjRef, mut d: Dict) -> Result { @@ -389,7 +417,19 @@ pub(crate) fn place(doc: &mut Document, page_ref: ObjRef, mut d: Dict) -> Result let (quads, rest) = v.as_chunks::<8>(); v.len() == q.len() && rest.is_empty() && !quads.is_empty() && quads.iter().all(quad_has_area) }); - if !quads_drawable { + if quads_drawable { + // The rectangle holds all the mark removes: its review outline is clipped to the + // rectangle, so a quad outside it would be removed unseen. + let v: Vec = d.get(b"QuadPoints").and_then(Object::as_array).into_iter().flatten().filter_map(Object::as_f64).collect(); + let mut r = rect_strict(doc, d.get(b"Rect")) + .map_or([f64::INFINITY, f64::INFINITY, f64::NEG_INFINITY, f64::NEG_INFINITY], |[x0, y0, x1, y1]| { + [x0.min(x1), y0.min(y1), x0.max(x1), y0.max(y1)] + }); + for [x, y] in v.as_chunks::<2>().0 { + r = [r[0].min(*x), r[1].min(*y), r[2].max(*x), r[3].max(*y)]; + } + d.set(b"Rect".to_vec(), arr(&r)); + } else { // Quads the outline can't draw, or any quad that encloses no area, are replaced by the // /Rect's, or dropped when there is no usable /Rect either; such a mark then removes // nothing. @@ -418,9 +458,11 @@ pub(crate) fn place(doc: &mut Document, page_ref: ObjRef, mut d: Dict) -> Result // and add the imported dict beside it under no name of its own. let mut clash = false; for a in annots { - let existing = a.as_ref().and_then(|r| doc.get(r).as_dict().cloned()); - let same = nm.is_some() && existing.as_ref().and_then(|x| x.get(b"NM").and_then(|n| doc.resolve(n).as_string().map(|s| s.to_text()))) == nm; - if same && existing.as_ref().is_some_and(|x| import_protected(doc, x)) { + // Borrowed: the page's annotations are read once per imported one, never copied. + let held = a.as_ref().map(|r| doc.get(r)); + let existing = held.as_deref().and_then(Object::as_dict); + let same = nm.is_some() && existing.and_then(|x| x.get(b"NM").and_then(|n| doc.resolve(n).as_string().map(|s| s.to_text()))) == nm; + if same && existing.is_some_and(|x| import_protected(doc, x)) { clash = true; kept.push(a); } else if same { @@ -434,7 +476,8 @@ pub(crate) fn place(doc: &mut Document, page_ref: ObjRef, mut d: Dict) -> Result } // The replaced note's pop-up goes with it; otherwise it stays on the page pointing at a note that is no longer there. kept.retain(|a| { - let Some(x) = a.as_ref().map(|r| doc.get(r)).and_then(|o| o.as_dict().cloned()) else { return true }; + let Some(held) = a.as_ref().map(|r| doc.get(r)) else { return true }; + let Some(x) = held.as_dict() else { return true }; x.name(b"Subtype") != Some(b"Popup".as_slice()) || x.reference(b"Parent").is_none_or(|p| !removed.contains(&p)) }); // Keep the replaced comment's identity: replies and review statuses not included in a @@ -516,7 +559,7 @@ pub(crate) fn import_xfdf(doc: &mut Document, text: &str) -> Result