diff --git a/crates/xfdf/src/lib.rs b/crates/xfdf/src/lib.rs index c41f8da2..e52e2318 100644 --- a/crates/xfdf/src/lib.rs +++ b/crates/xfdf/src/lib.rs @@ -13,6 +13,7 @@ use std::collections::HashMap; use std::fmt::Write as _; +use std::io::Write as _; use pdfcraft_cos::{Dict, Document, ObjRef, Object, PdfString}; use pdfcraft_forms::{FieldKind, FieldValue}; @@ -350,9 +351,13 @@ fn pdf_string(s: &str) -> Vec { v } -/// FDF with field values and/or comments (direct objects only; appearances are not carried). +/// FDF with field values and/or comments (appearances are not carried). Each comment is an +/// indirect object of the FDF, so a reply's `/IRT` can point at its parent's object there +/// (§12.7.8.3.1); nothing else refers back into the source document. pub fn export_fdf(doc: &Document, comments: bool, fields: bool, file: &str) -> Vec { let mut body: Vec = b"<< /FDF << ".to_vec(); + // Comment dictionaries, written after the root as objects 2, 3, … + let mut annots: Vec = Vec::new(); if fields { body.extend_from_slice(b"/Fields ["); for f in pdfcraft_forms::fields(doc) { @@ -377,44 +382,71 @@ pub fn export_fdf(doc: &Document, comments: bool, fields: bool, file: &str) -> V body.extend_from_slice(b"] "); } if comments { - body.extend_from_slice(b"/Annots ["); + // Every exported comment with its source reference, numbered in export order. + let mut picked: Vec<(Option, Dict, usize)> = Vec::new(); for (pi, p) in pdfcraft_model::pages(doc).iter().enumerate() { for a in p.dict.get(b"Annots").map(|a| doc.resolve(a)).and_then(|a| a.as_array().cloned()).unwrap_or_default() { - let Some(mut d) = doc.resolve(&a).as_dict().cloned() else { continue }; - let Some(sub) = d.name(b"Subtype") else { continue }; - if !SUBTYPES.iter().any(|(s, _)| s.as_bytes() == sub) { - continue; + let Some(d) = doc.resolve(&a).as_dict().cloned() else { continue }; + if d.name(b"Subtype").is_some_and(|sub| SUBTYPES.iter().any(|(s, _)| s.as_bytes() == sub)) { + picked.push((a.as_ref(), d, pi)); } - // Only direct values travel: references into this file would dangle. - for k in [&b"P"[..], b"AP", b"Popup", b"IRT", b"Parent", b"FS", b"Sound"] { - d.remove(k); - } - let keys: Vec> = d.iter().filter(|(_, v)| matches!(v, Object::Ref(_))).map(|(k, _)| k.clone()).collect(); - for k in keys { - let Some(o) = d.get(&k) else { continue }; - let v = (*doc.resolve(o)).clone(); - if matches!(v, Object::Stream(_)) { - d.remove(&k); - } else { - d.set(k, v); - } - } - d.set(b"Page".to_vec(), Object::Int(pi as i64)); - pdfcraft_cos::serialize(&Object::Dict(d), &mut body); - body.push(b' '); } } + let numbers: HashMap = + picked.iter().enumerate().filter_map(|(i, (r, _, _))| Some(((*r)?, u32::try_from(i).ok()?.checked_add(2)?))).collect(); + for (_, mut d, pi) in picked { + // A reply points at its parent's object in this FDF; a parent that isn't exported + // leaves the reply unthreaded. + let irt = d.get(b"IRT").and_then(Object::as_ref).and_then(|r| numbers.get(&r)).copied(); + // Only direct values travel: references into this file would dangle. + for k in [&b"P"[..], b"AP", b"Popup", b"IRT", b"Parent", b"FS", b"Sound"] { + d.remove(k); + } + let keys: Vec> = d.iter().filter(|(_, v)| matches!(v, Object::Ref(_))).map(|(k, _)| k.clone()).collect(); + for k in keys { + let Some(o) = d.get(&k) else { continue }; + let v = (*doc.resolve(o)).clone(); + if matches!(v, Object::Stream(_)) { + d.remove(&k); + } else { + d.set(k, v); + } + } + if let Some(num) = irt { + d.set(b"IRT".to_vec(), Object::Ref(ObjRef::new(num, 0))); + } + d.set(b"Page".to_vec(), Object::Int(pi as i64)); + annots.push(d); + } + body.extend_from_slice(b"/Annots ["); + for i in 0..annots.len() { + let _ = write!(body, "{} 0 R ", i + 2); + } body.extend_from_slice(b"] "); } body.extend_from_slice(b"/F "); body.extend(pdf_string(file)); body.extend_from_slice(b" >> >>"); - let mut out = b"%FDF-1.2\n%\xE2\xE3\xCF\xD3\n1 0 obj\n".to_vec(); + let mut out = b"%FDF-1.2\n%\xE2\xE3\xCF\xD3\n".to_vec(); + let mut offsets = Vec::with_capacity(annots.len() + 1); + offsets.push(out.len()); + out.extend_from_slice(b"1 0 obj\n"); out.extend(body); - out.extend_from_slice(b"\nendobj\ntrailer\n<< /Root 1 0 R >>\n%%EOF\n"); + out.extend_from_slice(b"\nendobj\n"); + for (i, d) in annots.into_iter().enumerate() { + offsets.push(out.len()); + let _ = writeln!(out, "{} 0 obj", i + 2); + pdfcraft_cos::serialize(&Object::Dict(d), &mut out); + out.extend_from_slice(b"\nendobj\n"); + } + let xref = out.len(); + let _ = write!(out, "xref\n0 {}\n0000000000 65535 f \n", offsets.len() + 1); + for o in offsets { + let _ = writeln!(out, "{o:010} 00000 n "); + } + let _ = write!(out, "trailer\n<< /Root 1 0 R >>\nstartxref\n{xref}\n%%EOF\n"); out } - /// The XML element name for a field name (letters, digits, `_`, `-`, `.`). fn xml_name(name: &str) -> String { let mut s: String = name.chars().map(|c| if c.is_alphanumeric() || matches!(c, '_' | '-' | '.') { c } else { '_' }).collect(); @@ -817,10 +849,16 @@ fn import_fdf(doc: &mut Document, bytes: &[u8]) -> Result { if let Some(list) = body.get(b"Fields").map(|f| fdf.resolve(f)).and_then(|f| f.as_array().cloned()) { walk_fdf(&fdf, &list, "", &mut values, 0); } - // Comments. + // Comments: parents first, then replies (whose /IRT needs the parent's new reference). let pages: Vec = pdfcraft_model::pages(doc).iter().map(|p| p.obj).collect(); + let mut placed: HashMap = HashMap::new(); + let mut later: Vec<(ObjRef, Option, Option)> = Vec::new(); for a in body.get(b"Annots").map(|a| fdf.resolve(a)).and_then(|a| a.as_array().cloned()).unwrap_or_default() { - let Object::Dict(mut d) = deep(&fdf, &a, 0) else { continue }; + let Some(mut src) = fdf.resolve(&a).as_dict().cloned() else { continue }; + // The parent is another comment: link it below rather than copying it in. + let irt = src.get(b"IRT").cloned(); + src.remove(b"IRT"); + let Object::Dict(mut d) = deep(&fdf, &Object::Dict(src), 0) else { continue }; let Some(page) = d.get(b"Page").and_then(Object::as_int).and_then(|p| usize::try_from(p).ok()).and_then(|p| pages.get(p).copied()) else { continue; }; @@ -829,9 +867,23 @@ fn import_fdf(doc: &mut Document, bytes: &[u8]) -> Result { } d.remove(b"Page"); d.set(b"P".to_vec(), Object::Ref(page)); - place(doc, page, d)?; + let r = place(doc, page, d)?; + if let Some(fr) = a.as_ref() { + placed.insert(fr, r); + } + if let Some(irt) = irt { + let parent_name = fdf.resolve(&irt).as_dict().and_then(|p| text_of(&fdf, p, b"NM")); + later.push((r, irt.as_ref(), parent_name)); + } report.comments += 1; } + for (r, parent_obj, parent_name) in later { + // The parent imported from this file, else one already in the document with its name. + let parent = parent_obj.and_then(|p| placed.get(&p).copied()).or_else(|| parent_name.and_then(|nm| find_by_name(doc, &nm))); + if let Some(parent) = parent.filter(|p| *p != r) { + doc.update_dict(r, |d| d.set(b"IRT".to_vec(), Object::Ref(parent)))?; + } + } apply_values(doc, &values, &mut report); Ok(report) } diff --git a/crates/xfdf/src/tests.rs b/crates/xfdf/src/tests.rs index 07d079a5..10b17981 100644 --- a/crates/xfdf/src/tests.rs +++ b/crates/xfdf/src/tests.rs @@ -129,6 +129,78 @@ fn fdf_round_trips_values_and_comments() { assert_eq!(values(&dst), values(&src)); assert!(r.comments >= 3, "{r:?}"); assert!(summaries(&dst).iter().any(|s| s.contents.as_deref() == Some("scribble"))); + assert_eq!(comment_view(&dst), comment_view(&src), "replies keep their thread"); +} + +/// Who each comment replies to, by name, with its review state. +fn threads(doc: &Document) -> Vec<(Option, Option, Option)> { + summaries(doc).into_iter().map(|s| (s.name, s.in_reply_to, s.state)).collect() +} + +/// #333: a note with two replies and an Accepted review status keeps its thread through FDF. +#[test] +fn fdf_keeps_replies_and_review_status_on_their_note() { + let mut src = blank(); + let meta = |id: &str| Meta { date: Some("D:20261002120000Z".into()), id: id.into() }; + let note = Shape::Note { at: [100.0, 100.0], icon: pdfcraft_annot::NoteIcon::Comment }; + add_annotation( + &mut src, + &NewAnnotation { page: 1, style: Style::default_for(¬e), shape: note, contents: "Literal Note".into(), author: "Alpha".into() }, + &meta("note"), + ) + .unwrap(); + let index = summaries(&src).iter().find(|s| s.subtype == "Text").map(|s| s.index).unwrap(); + add_reply(&mut src, 1, index, "Reply One", "Beta", &meta("one")).unwrap(); + add_reply(&mut src, 1, index, "Reply Two", "Gamma", &meta("two")).unwrap(); + pdfcraft_annot::set_review_state(&mut src, 1, index, pdfcraft_annot::ReviewState::Accepted, "Review", &meta("status")).unwrap(); + let want = threads(&src); + assert_eq!(want.iter().filter(|t| t.1.as_deref() == Some("note")).count(), 3, "{want:?}"); + + let fdf = export_fdf(&src, true, false, "notes.pdf"); + // Every cross-reference entry points at its object (other readers trust the table). + let tail = std::str::from_utf8(&fdf[fdf.windows(5).position(|w| w == b"xref\n").unwrap()..]).unwrap(); + let xref: usize = tail.lines().skip_while(|l| *l != "startxref").nth(1).unwrap().parse().unwrap(); + assert!(fdf[xref..].starts_with(b"xref\n0 6\n"), "{tail}"); + for (num, line) in tail.lines().skip(3).take(5).enumerate() { + let at: usize = line[..10].parse().unwrap(); + assert!(fdf[at..].starts_with(format!("{} 0 obj\n", num + 1).as_bytes()), "object {}: {line}", num + 1); + } + let mut dst = blank(); + assert_eq!(import(&mut dst, &fdf).unwrap().comments, 4); + assert_eq!(threads(&dst), want); + assert_eq!(comment_view(&dst), comment_view(&src)); + // Importing again replaces the thread instead of duplicating or detaching it. + import(&mut dst, &fdf).unwrap(); + assert_eq!(threads(&dst), want); +} + +/// FDFs written before #333 (direct comment dictionaries, no /IRT) still import. +#[test] +fn fdf_with_direct_comments_still_imports() { + let fdf = b"%FDF-1.2\n1 0 obj\n<< /FDF << /Annots [<< /Type /Annot /Subtype /Text /Rect [10 10 30 30] /Contents (old) /NM (a) /Page 0 >>] >> >>\nendobj\ntrailer\n<< /Root 1 0 R >>\n%%EOF\n"; + let mut dst = blank(); + assert_eq!(import(&mut dst, fdf).unwrap().comments, 1); + assert_eq!(threads(&dst), vec![(Some("a".into()), None, None)]); +} + +/// /IRT pointing at itself, at a missing object, at a non-comment or in a cycle neither panics +/// nor loops; only a real parent is linked. +#[test] +fn hostile_fdf_reply_parents_are_ignored() { + let fdf = b"%FDF-1.2\n1 0 obj\n<< /FDF << /Annots [2 0 R 3 0 R 4 0 R 5 0 R 6 0 R] >> >>\nendobj\n\ +2 0 obj\n<< /Subtype /Text /Rect [0 0 9 9] /NM (self) /IRT 2 0 R /Page 0 >>\nendobj\n\ +3 0 obj\n<< /Subtype /Text /Rect [0 0 9 9] /NM (missing) /IRT 99 0 R /Page 0 >>\nendobj\n\ +4 0 obj\n<< /Subtype /Text /Rect [0 0 9 9] /NM (cycle-a) /IRT 5 0 R /Page 0 >>\nendobj\n\ +5 0 obj\n<< /Subtype /Text /Rect [0 0 9 9] /NM (cycle-b) /IRT 4 0 R /Page 0 >>\nendobj\n\ +6 0 obj\n<< /Subtype /Text /Rect [0 0 9 9] /NM (number) /IRT 42 /Page 99 >>\nendobj\n\ +trailer\n<< /Root 1 0 R >>\n%%EOF\n"; + let mut dst = blank(); + assert_eq!(import(&mut dst, fdf).unwrap().comments, 4); + let t = threads(&dst); + let parent = |nm: &str| t.iter().find(|x| x.0.as_deref() == Some(nm)).and_then(|x| x.1.clone()); + assert_eq!(parent("self"), None); + assert_eq!(parent("missing"), None); + assert_eq!((parent("cycle-a").as_deref(), parent("cycle-b").as_deref()), (Some("cycle-b"), Some("cycle-a"))); } #[test]