diff --git a/ROADMAP.md b/ROADMAP.md index b2b1c1303e..2c81b6e039 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -233,7 +233,7 @@ Ranked; detail and the full milestone table in [docs/roadmap.md](docs/roadmap.md ## Progress log - **2026-10-13 (M8, redact by regex):** `redact_mark` takes `regex` (caller expression, linear-time engine, length/size/nesting caps, empty-matching refused) with optional `validate` (luhn, mod97, iban). Headless only; issue #1886. -- **2026-11-08 (M5, comment exchange):** Comments keep their formatting and appearance through XFDF and FDF. XFDF export and import carry rich text, default style, border width, style and dash pattern, rotation, justification, fringe, line leaders and captions, the caret symbol and redaction overlay text as XFDF 3.0 (ISO 19444-1) names them, and a stamp's appearance as `` (the DICT/STREAM/DATA encoding, bounded); copy and paste carries every comment's appearance. FDF export keeps a stamp's /AP with its forms and images, as XFDF does, and FDF import keeps any /AP the file carries (ISO 32000-2 §12.7.8.3.4). The intent goes out as both `intent` and `IT`. An unreadable appearance is recorded in the import report and the comment is drawn by PdfCraft; a redaction mark is always drawn by PdfCraft. +- **2026-11-08 (M5, comment exchange):** Comments keep their formatting and appearance through XFDF and FDF. XFDF export and import carry rich text, default style, border width, style and dash pattern, rotation, justification, fringe, line leaders and captions, the caret symbol and redaction overlay text as XFDF 3.0 (ISO 19444-1) names them, and a stamp's appearance as `` (the DICT/STREAM/DATA encoding, bounded); copy and paste carries every comment's appearance. FDF export keeps a stamp's /AP with its forms and images, as XFDF does, and FDF import keeps any /AP the file carries (ISO 32000-2 §12.7.8.3.4). The intent goes out as both `intent` and `IT`. An unreadable XFDF appearance is left out, said in the import report and the repair log, and the comment is drawn by PdfCraft; an FDF past the import budget is refused whole. A redaction mark is always drawn by PdfCraft. - **2026-10-28 (M10, #4119 partial):** Scan & OCR ▸ Enhance scanned file cleans scanned pages in place: background removal (level 0–100, as the camera clean-up), edge shadow removal (book gutters, copier borders) and despeckle. Each scan image is decoded, cleaned and written back into its own object at the same size and colour space (JPEG stays JPEG, others lossless, 1-bit stays 1-bit), so the OCR text layer, comments and links stay; one undo step, shared images once, unsupported images (CMYK, indexed, JPEG 2000, JBIG2, CCITT, broken data) reported. `page_enhance_scan` drives it headlessly. Not yet an option of Recognize text. Estimates unchanged. - **2026-11-03 (M11, shared JBIG2 dictionary):** With Monochrome Images set to JBIG2, Optimize has up to 16 JBIG2 images share one symbol dictionary (`/JBIG2Globals`), the letters found on more than one page stored once, when that is smaller (`pdfcraft_filters::encode_jbig2_shared`). Eight pages of 10 pt text rendered at 300 dpi take 41% less in Helvetica and 42% in Times than coded alone, 10% in Courier; groups of 16 save 45%, of 32 only 47%. Only images redaction can store again without the dictionary share one (image masks and DeviceGray named directly, at most 64 Mi pixels), and redacting one stores the rest of its group so, which the group bounds to 15. diff --git a/crates/engine/src/edit/run.rs b/crates/engine/src/edit/run.rs index 61e48a7d04..25b55de22a 100644 --- a/crates/engine/src/edit/run.rs +++ b/crates/engine/src/edit/run.rs @@ -162,6 +162,14 @@ pub(crate) fn check_permission(edit: &Edit, p: &pdfcraft_cos::Permissions) -> Re } } +/// What a comment import left out of comments it did import (an appearance it could not read, so +/// the comment is drawn from its other entries), in the repair log: said, not slipped in. +fn note_skipped(doc: &mut pdfcraft_cos::Document, report: &pdfcraft_xfdf::Report) { + for why in &report.skipped { + doc.note_repair(format!("imported comment {why}; its appearance was left out and it is drawn from its other entries")); + } +} + /// Dates and unique ids stamped onto what an edit creates. pub(crate) struct EditCtx { /// Field scripts run (`None`: JavaScript is off) and what they produced. @@ -507,6 +515,7 @@ pub(crate) fn run_edit(doc: &mut pdfcraft_cos::Document, edit: &Edit, cx: &mut E if report.comments == 0 { return Err(EditError::Invalid("the clipboard holds no comments".into())); } + note_skipped(doc, &report); } Edit::AddCustomStamp { page, rect, name, file, author } => { let src = mark_source(doc, file)?; @@ -856,7 +865,10 @@ pub(crate) fn run_edit(doc: &mut pdfcraft_cos::Document, edit: &Edit, cx: &mut E // the data importer's error. let mut data = doc.clone(); match pdfcraft_xfdf::import(&mut data, bytes) { - Ok(_) => *doc = data, + Ok(report) => { + note_skipped(&mut data, &report); + *doc = data; + } Err(e) if bytes[..bytes.len().min(1024)].windows(5).any(|w| w == b"%PDF-") => match comment_source(bytes) { Ok(src) => import_pdf_comments(doc, &src)?, Err(SourceProblem::Unreadable(_)) => return Err(e.into()), diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index cf01e9a5a2..6e225bdc6e 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -11280,3 +11280,19 @@ fn imported_bookmarks_undo_in_one_step() { assert!(err.to_string().contains("no bookmark"), "{err}"); assert!(s.get(id).unwrap().info.outline.is_empty()); } + +/// A comment imported or pasted without the appearance its file carried (unreadable there) is +/// said in the repair log, which the interface shows and the tools return. +#[test] +fn a_comment_imported_without_its_appearance_is_said_in_the_repair_log() { + let xfdf = r#"@@@not base 64@@@"#; + let said = |s: &Session, id: DocId| { + s.get(id).unwrap().repair_log().iter().filter(|l| l.contains("imported comment st: the appearance is not base 64")).count() + }; + let (mut s, id) = session_with(1); + s.apply(id, Edit::ImportData { name: "notes.xfdf".into(), bytes: Arc::new(xfdf.as_bytes().to_vec()) }).unwrap(); + assert_eq!(said(&s, id), 1, "{:?}", s.get(id).unwrap().repair_log()); + let (mut s, id) = session_with(1); + s.apply(id, Edit::PasteComments { xfdf: xfdf.into(), page: 0, offset: None }).unwrap(); + assert_eq!(said(&s, id), 1, "{:?}", s.get(id).unwrap().repair_log()); +} diff --git a/crates/xfdf/README.md b/crates/xfdf/README.md index 7ac67fd0f0..4417a15b70 100644 --- a/crates/xfdf/README.md +++ b/crates/xfdf/README.md @@ -28,8 +28,9 @@ let report = import(&mut doc, &bytes)?; // XFDF, FDF, XML, CSV o or `raw`/`hex`), the shape XFDF files from other tools carry. Copy and paste carries every comment's appearance. Import keeps an appearance it can read (except on a redaction mark, which PdfCraft always draws) and otherwise draws the comment and records the reason in - `Report::skipped`; it refuses one over 16 MiB, 64 MiB per import, 64 nesting levels or 100,000 - values. Export leaves out one past those limits or whose rect has no area. + `Report::skipped` (the engine puts it in the repair log); it refuses one over 16 MiB, 64 MiB per + import, 64 nesting levels or 100,000 values. Export leaves out one past those limits or whose + rect has no area; where `/AS` selects one of several states, the selected stream is written. - **FDF**: fields (with `/Kids` hierarchies) and indirect comments. Nested annotation values are copied under FDF object numbers; shared references and cycles remain shared. A stamp's appearance (`/AP`, with the forms and images it draws) is copied, as in XFDF, and an `/AP` in the file comes @@ -45,7 +46,9 @@ let report = import(&mut doc, &bytes)?; // XFDF, FDF, XML, CSV o levels or a conservative 64 MiB serialization budget instead of exporting partial data. These are copied-graph limits: source parsing and page discovery retain their existing COS/parser limits, with no total source-memory bound. Unknown indirect streams retain their - encoded bytes. FDF import copies a comment's streams only when the comment lands on a page. + encoded bytes. FDF import copies a comment's streams only when the comment lands on a page, and + refuses the file (nothing imported) when its comments expand past 1,000,000 values or 64 MiB, or + nest more than 256 levels, rather than import part of an appearance. - **Form data**: Acrobat's XML (`xfdf:original` keeps names that aren't XML names), CSV and tab-delimited text (a row of names, a row of values). diff --git a/crates/xfdf/src/appearance.rs b/crates/xfdf/src/appearance.rs index 80105c715c..55acba0e8f 100644 --- a/crates/xfdf/src/appearance.rs +++ b/crates/xfdf/src/appearance.rs @@ -33,13 +33,35 @@ const ASCII_DATA_LIMIT: usize = 4096; /// The base 64 `` text for annotation `d`'s appearance dictionary, or `None` when it /// has no normal appearance stream or is past the limits (the comment is then exported without /// one). +/// +/// XFDF has no appearance state (`/AS`), so where `/N`, `/R` or `/D` holds one stream per state +/// only the selected state's stream is written (ISO 32000-2 §12.5.5: `/AS` picks it); a normal +/// appearance with no selected stream has nothing to draw and nothing is written. pub(crate) fn encode(doc: &Document, d: &Dict) -> Option { if !has_appearance(doc, d) { return None; } let ap = d.get(b"AP").map(|a| doc.resolve(a))?; + let mut ap = ap.as_dict()?.clone(); + let state = d.get(b"AS").and_then(|s| doc.resolve(s).as_name().map(<[u8]>::to_vec)); + for key in [b"N".as_slice(), b"R", b"D"] { + let Some(entry) = ap.get(key).map(|e| doc.resolve(e)) else { continue }; + let Object::Dict(states) = entry.as_ref() else { continue }; + match state.as_deref().and_then(|s| states.get(s)).cloned() { + Some(selected) => ap.set(key.to_vec(), selected), + None if key == b"N" => return None, + None => { + ap.remove(key); + } + } + } let mut writer = Writer { doc, out: String::new(), values: MAX_VALUES, path: Vec::new() }; - writer.dict(Some(b"AP"), ap.as_dict()?, 0)?; + writer.dict(Some(b"AP"), &ap, 0)?; + // The checks while writing leave room for the closing tags and line breaks: the finished XML + // must fit what the reader accepts, or the comment would arrive without its appearance. + if writer.out.len() > MAX_APPEARANCE_BYTES { + return None; + } let b64 = base64::engine::general_purpose::STANDARD.encode(writer.out.as_bytes()); // Line breaks every 76 characters, as base 64 text usually has them; the reader skips them. let mut wrapped = String::with_capacity(b64.len() + b64.len() / 76 + 1); @@ -122,10 +144,10 @@ impl Writer<'_> { } // A page (or the page tree) reached from the appearance is the source document's, not // the comment's: written in place it would carry every page along. - Object::Dict(d) if matches!(d.name(b"Type"), Some(b"Page" | b"Pages")) => self.out.push_str(&format!( - " -" - )), + // `/Type` may itself be a reference (`resolve` follows a bounded number of them). + Object::Dict(d) if d.get(b"Type").is_some_and(|t| matches!(self.doc.resolve(t).as_name(), Some(b"Page" | b"Pages"))) => { + self.out.push_str(&format!("\n")); + } Object::Dict(d) => self.dict(key, d, depth)?, Object::Stream(s) => { // The data plus its hexadecimal form must fit what is left of the budget. @@ -137,7 +159,10 @@ impl Writer<'_> { for (dk, dv) in s.dict.iter().filter(|(dk, _)| dk.as_slice() != b"Length") { self.value(Some(dk), dv, depth + 1)?; } - let printable = s.raw.iter().all(|&b| (b > 32 && b < 127) || b == b'\r' || b == b'\n'); + // XFDF 3.0 counts a carriage return as printable, but XML reads one, alone or before + // a line feed, as a line feed (XML 1.0 §2.11): data holding one is written as + // hexadecimal so it comes back byte for byte (the 4 KiB rule "is not a rule"). + let printable = s.raw.iter().all(|&b| (b > 32 && b < 127) || b == b'\n'); if s.raw.len() < ASCII_DATA_LIMIT && printable && !s.dict.contains(b"Filter") { // Printable ASCII is valid UTF-8. let text = String::from_utf8_lossy(&s.raw); diff --git a/crates/xfdf/src/fdf.rs b/crates/xfdf/src/fdf.rs index 7f60907471..22c2f197b8 100644 --- a/crates/xfdf/src/fdf.rs +++ b/crates/xfdf/src/fdf.rs @@ -366,9 +366,15 @@ struct DeepTarget<'a> { streams: HashMap, } +/// How deeply [`deep`] follows a comment's objects, references included. A form that draws a form +/// costs four levels (the reference, the stream's dictionary, `/Resources`, `/XObject`), and +/// export writes a chain of them however long, so this leaves room for one sixty forms deep. +const MAX_COPY_DEPTH: usize = 4 * MAX_DEPTH; + /// Copy an object out of another document, resolving references. A reference back to an object /// being copied becomes null. A stream (an appearance and the images and forms it draws) becomes -/// an object of `dst`, its data charged to the budget. +/// an object of `dst`, its data charged to the budget. Objects nested past [`MAX_COPY_DEPTH`] are +/// an error, not a partial copy: the comment would arrive with part of its appearance missing. fn deep( src: &Document, dst: &mut DeepTarget, @@ -378,9 +384,8 @@ fn deep( budget: &mut ImportBudget, ) -> Result { budget.spend(1, 0)?; - // As deep as export writes (an appearance's shading functions sit a dozen levels down). - if depth > MAX_DEPTH { - return Ok(Object::Null); + if depth > MAX_COPY_DEPTH { + return Err(DataError::Malformed(format!("a comment's objects nest more than {MAX_COPY_DEPTH} levels deep"))); } Ok(match o { Object::Ref(r) => { @@ -524,6 +529,14 @@ pub(crate) fn import_fdf(doc: &mut Document, bytes: &[u8]) -> Result = a.as_ref().into_iter().collect(); let mut target = DeepTarget { doc: &mut *doc, streams: std::mem::take(&mut copied_streams) }; let copy = deep(&fdf, &mut target, &Object::Dict(src), 0, &mut path, &mut budget); diff --git a/crates/xfdf/src/tests_format.rs b/crates/xfdf/src/tests_format.rs index 28fbfbde06..a48122df34 100644 --- a/crates/xfdf/src/tests_format.rs +++ b/crates/xfdf/src/tests_format.rs @@ -788,3 +788,263 @@ proptest! { prop_assert_eq!(sub_dict(&dst, &n.dict, b"Extra"), extra); } } + +/// A stamp on page 0 whose normal appearance is one form stream holding `content`. +fn content_stamp(doc: &mut Document, nm: &str, content: Vec) -> ObjRef { + let mut form = Dict::new(); + form.set(b"Type".to_vec(), Object::name("XObject")); + form.set(b"Subtype".to_vec(), Object::name("Form")); + form.set(b"BBox".to_vec(), nums(&[0.0, 0.0, 100.0, 50.0])); + let n = doc.add(Object::Stream(Stream { dict: form, raw: content.into() })); + let mut ap = Dict::new(); + ap.set(b"N".to_vec(), Object::Ref(n)); + let mut d = annot("Stamp", nm, [100.0, 500.0, 200.0, 550.0]); + d.set(b"AP".to_vec(), Object::Dict(ap)); + put(doc, d); + n +} + +/// The `` XML export wrote for the first comment of `xml`. +fn exported_appearance(xml: &str) -> String { + let b64: String = xml.split("").nth(1).unwrap().split("").next().unwrap().split_whitespace().collect(); + String::from_utf8(base64::engine::general_purpose::STANDARD.decode(b64).unwrap()).unwrap() +} + +/// The write-time checks leave out the closing tags and the hexadecimal line breaks: an appearance +/// just under the limit there came out over it, and import then refused what export wrote. +#[test] +fn an_exported_appearance_always_fits_what_import_reads() { + let mut src = pages(1); + content_stamp(&mut src, "big", vec![0u8; (appearance::MAX_APPEARANCE_BYTES / 2) - 4096]); + let d = named(&src, "big"); + if let Some(text) = appearance::encode(&src, &d) { + let mut budget = appearance::MAX_IMPORT_APPEARANCE_BYTES; + appearance::decode(&text, &mut budget).expect("what export writes, import reads"); + } +} + +/// A page whose `/Type` is a reference is still the source page: written in place it would carry +/// the page's contents into a file meant to hold one comment. +#[test] +fn a_page_with_an_indirect_type_stays_out_of_an_exported_appearance() { + let mut src = pages(1); + let n = content_stamp(&mut src, "pg", b"q Q".to_vec()); + let page = page_ref(&src, 0); + let page_type = src.add(Object::name("Page")); + src.update_dict(page, |p| p.set(b"Type".to_vec(), Object::Ref(page_type))).unwrap(); + let Object::Stream(mut form) = (*src.get(n)).clone() else { panic!("no form") }; + form.dict.set(b"PieceInfo".to_vec(), Object::Ref(page)); + src.set(n, Object::Stream(form)); + let inner = exported_appearance(&export_xfdf(&src, true, false, "src.pdf")); + assert!(inner.contains(r#""#), "{inner}"); + assert!(!inner.contains("MediaBox"), "{inner}"); +} + +/// XML reads a carriage return as a line feed, so data holding one goes out as hexadecimal. +#[test] +fn stream_data_with_carriage_returns_comes_back_byte_for_byte() { + let mut src = pages(1); + let data = b"q\r/Im0\rDo\r\nQ\r".to_vec(); + content_stamp(&mut src, "cr", data.clone()); + let (_, dst) = round_trip_xfdf(&src); + let d = named(&dst, "cr"); + let n = dst.resolve(sub_dict(&dst, &d, b"AP").get(b"N").unwrap()); + let Object::Stream(n) = n.as_ref() else { panic!("no appearance") }; + assert_eq!(n.raw.as_ref(), data.as_slice()); +} + +/// XFDF has no appearance state: the stream `/AS` selects is what goes out. +#[test] +fn the_selected_appearance_state_is_what_xfdf_carries() { + let mut src = pages(1); + let on = content_stamp(&mut src, "st", b"0 0 1 rg 0 0 100 50 re f".to_vec()); + let off = src.add(Object::Stream(Stream { dict: Dict::new(), raw: b"%off".to_vec().into() })); + let mut states = Dict::new(); + states.set(b"On".to_vec(), Object::Ref(on)); + states.set(b"Off".to_vec(), Object::Ref(off)); + let mut ap = Dict::new(); + ap.set(b"N".to_vec(), Object::Dict(states.clone())); + ap.set(b"D".to_vec(), Object::Dict(states)); + let r = xfdf::find_by_name(&src, "st").unwrap(); + src.update_dict(r, |d| { + d.set(b"AP".to_vec(), Object::Dict(ap.clone())); + d.set(b"AS".to_vec(), Object::name("On")); + }) + .unwrap(); + let (_, dst) = round_trip_xfdf(&src); + let d = named(&dst, "st"); + let ap = sub_dict(&dst, &d, b"AP"); + for key in [b"N".as_slice(), b"D"] { + let s = dst.resolve(ap.get(key).unwrap()); + let Object::Stream(s) = s.as_ref() else { panic!("{key:?} is not the selected stream: {s:?}") }; + assert_eq!(s.raw.as_ref(), b"0 0 1 rg 0 0 100 50 re f"); + } + // A state no stream answers to has nothing to draw: no appearance goes out. + src.update_dict(r, |d| d.set(b"AS".to_vec(), Object::name("Gone"))).unwrap(); + assert!(!export_xfdf(&src, true, false, "src.pdf").contains("")); +} + +/// Forms that draw forms: FDF export writes the chain however long, and import copied only the +/// first dozen and nulled the rest without a word. A chain sixty forms deep comes back whole; one +/// deeper than import follows is refused rather than imported in part. +#[test] +fn a_chain_of_forms_comes_back_whole_from_fdf_or_is_refused() { + let chain = |forms: usize| { + let mut src = pages(1); + let mut inner = src.add(Object::Stream(Stream { dict: Dict::new(), raw: b"%innermost".to_vec().into() })); + for _ in 0..forms { + let mut xo = Dict::new(); + xo.set(b"Fm".to_vec(), Object::Ref(inner)); + let mut res = Dict::new(); + res.set(b"XObject".to_vec(), Object::Dict(xo)); + let mut form = Dict::new(); + form.set(b"Subtype".to_vec(), Object::name("Form")); + form.set(b"Resources".to_vec(), Object::Dict(res)); + inner = src.add(Object::Stream(Stream { dict: form, raw: b"/Fm Do".to_vec().into() })); + } + let mut ap = Dict::new(); + ap.set(b"N".to_vec(), Object::Ref(inner)); + let mut d = annot("Stamp", "chain", [100.0, 500.0, 200.0, 550.0]); + d.set(b"AP".to_vec(), Object::Dict(ap)); + put(&mut src, d); + export_fdf(&src, true, false, "src.pdf").unwrap() + }; + let mut dst = pages(1); + import(&mut dst, &chain(20)).unwrap(); + let d = named(&dst, "chain"); + let mut at = dst.resolve(sub_dict(&dst, &d, b"AP").get(b"N").unwrap()); + for level in 0..20 { + let Object::Stream(s) = at.as_ref() else { panic!("level {level}: {at:?}") }; + let fm = sub_dict(&dst, &sub_dict(&dst, &s.dict, b"Resources"), b"XObject").get(b"Fm").cloned().unwrap_or(Object::Null); + at = dst.resolve(&fm); + } + let Object::Stream(innermost) = at.as_ref() else { panic!("the innermost form was lost: {at:?}") }; + assert_eq!(innermost.raw.as_ref(), b"%innermost"); + let err = import(&mut pages(1), &chain(80)).unwrap_err(); + assert!(err.to_string().contains("nest more than"), "{err}"); +} + +/// A `/P` an FDF comment holds points into the FDF; it is replaced, and nothing it reaches is +/// copied into the document. +#[test] +fn an_fdf_comments_page_link_copies_nothing_in() { + let fdf = |p: &str| { + format!( + "%FDF-1.2\n1 0 obj\n<< /FDF << /Annots [2 0 R] >> >>\nendobj\n2 0 obj\n<< /Type /Annot /Subtype /Text /Rect [10 10 30 30] /NM (t) /Page 0{p} >>\nendobj\n3 0 obj\n<< /Type /Page /Junk 4 0 R >>\nendobj\n4 0 obj\n<< /Length 5 >>\nstream\nhello\nendstream\nendobj\ntrailer\n<< /Root 1 0 R >>\n%%EOF\n" + ) + .into_bytes() + }; + let added = |bytes: &[u8]| { + let mut dst = pages(1); + let before = dst.add(Object::Null); + import(&mut dst, bytes).unwrap(); + let p = named(&dst, "t").reference(b"P"); + assert_eq!(p, Some(page_ref(&dst, 0))); + dst.add(Object::Null).num - before.num + }; + assert_eq!(added(&fdf(" /P 3 0 R")), added(&fdf("")), "the FDF's page and its stream were not copied in"); +} + +/// Two line breaks in a row leave a blank line in the comment's plain text. +#[test] +fn consecutive_rich_text_line_breaks_are_kept() { + let xml = r#" +

one

two


three

four

+
"#; + let mut dst = pages(1); + import(&mut dst, xml.as_bytes()).unwrap(); + assert_eq!(text(&dst, &named(&dst, "b"), b"Contents").as_deref(), Some("one\r\rtwo\r\rthree\rfour")); +} + +/// Rich text that is not XML, on a comment with no plain text, comes back as rich text, not as +/// the comment's text (a line would otherwise show the markup as its caption). +#[test] +fn rich_text_alone_that_is_not_xml_comes_back_as_rich_text() { + let mut src = pages(1); + let mut d = annot("Line", "ln", [10.0, 10.0, 90.0, 30.0]); + d.set(b"L".to_vec(), nums(&[10.0, 20.0, 90.0, 20.0])); + d.set(b"RC".to_vec(), PdfString::text("

unclosed")); + put(&mut src, d); + let (_, dst) = round_trip_xfdf(&src); + let d = named(&dst, "ln"); + assert_eq!(text(&dst, &d, b"RC").as_deref(), Some("

unclosed")); + assert_ne!(text(&dst, &d, b"Contents").as_deref(), Some("

unclosed")); +} + +/// A prefix the rich text inherits from the XFDF around it is declared on it, so the rich text +/// is XML of its own and goes out again as markup. +#[test] +fn rich_text_keeps_the_namespaces_it_inherits() { + let xml = r#" +hi

hi

+"#; + let mut dst = pages(1); + import(&mut dst, xml.as_bytes()).unwrap(); + let rc = text(&dst, &named(&dst, "ns"), b"RC").unwrap(); + assert!(common::parse_form_xml(&rc).is_ok(), "{rc}"); + let out = export_xfdf(&dst, true, false, "dst.pdf"); + assert!( + out.contains(r#"%nothing"#, + ); + for extra in [r#" style="cloudy""#, r#" overlay-text="日本語""#] { + let xml = appearance_xfdf(&inner, "redact").replace(r#"name="x""#, &format!(r#"name="x"{extra}"#)); + let mut dst = pages(1); + let before = dst.add(Object::Null); + import(&mut dst, xml.as_bytes()).unwrap(); + let d = named(&dst, "x"); + let n = dst.resolve(sub_dict(&dst, &d, b"AP").get(b"N").unwrap()); + let Object::Stream(n) = n.as_ref() else { panic!("{extra}: no appearance") }; + assert_ne!(n.decoded().unwrap(), b"%nothing", "{extra}"); + assert!(!d.contains(b"BE"), "{extra}"); + // The file's appearance was not read in at all. + for num in before.num..dst.add(Object::Null).num { + if let Object::Stream(s) = dst.get(ObjRef::new(num, 0)).as_ref() { + assert_ne!(s.raw.as_ref(), b"%nothing", "{extra}"); + } + } + } +} + +/// Appearances share one budget over the whole import: past it, the rest are skipped. +#[test] +fn the_import_budget_is_shared_by_every_comment() { + let one = b64(r#"0 0 m 1 1 l S"#); + let size = base64::engine::general_purpose::STANDARD.decode(&one).unwrap().len(); + let stamps: String = ["a", "b", "c"] + .iter() + .map(|nm| format!(r#"{one}"#)) + .collect(); + let xml = format!(r#"{stamps}"#); + let mut dst = pages(1); + let report = xfdf::import_xfdf_with(&mut dst, &xml, None, None, None, size * 5 / 2).unwrap(); + assert_eq!(report.comments, 3); + assert_eq!(report.skipped.len(), 1, "{:?}", report.skipped); + assert!(report.skipped[0].starts_with("c: "), "{:?}", report.skipped); +} + +/// A dash array of any length export writes comes back (65 numbers were written and dropped). +#[test] +fn a_long_dash_array_comes_back() { + let mut src = pages(1); + let dashes: Vec = (1..=65).map(f64::from).collect(); + let mut bs = Dict::new(); + bs.set(b"W".to_vec(), Object::Real(1.0)); + bs.set(b"S".to_vec(), Object::name("D")); + bs.set(b"D".to_vec(), nums(&dashes)); + let mut d = annot("Square", "sq", [10.0, 10.0, 90.0, 60.0]); + d.set(b"BS".to_vec(), Object::Dict(bs)); + put(&mut src, d); + let (_, dst) = round_trip_xfdf(&src); + assert_eq!(f64s(&dst, sub_dict(&dst, &named(&dst, "sq"), b"BS").get(b"D")), dashes); +} diff --git a/crates/xfdf/src/xfdf.rs b/crates/xfdf/src/xfdf.rs index 27d70d7d17..d1dc2f10e0 100644 --- a/crates/xfdf/src/xfdf.rs +++ b/crates/xfdf/src/xfdf.rs @@ -102,8 +102,6 @@ fn read_own_keys(node: roxmltree::Node, d: &mut Dict) { /// Rich text deeper than this is written as text rather than as markup, so the XFDF it goes into /// stays well inside the nesting the reader accepts (`MAX_XML_DEPTH`). const MAX_RICH_DEPTH: usize = 512; -/// At most this many numbers in a `dashes` list. -const MAX_DASHES: usize = 64; /// Rich text (`/RC`) as `` holds it: its root element alone (no XML /// declaration), when the text is well-formed XML; `None` otherwise. @@ -220,10 +218,7 @@ fn format_attrs(doc: &Document, d: &Dict, sub: &[u8], attrs: &mut String) { fn read_format(node: roxmltree::Node, d: &mut Dict) { let num = |a: &str| node.attribute(a).and_then(|v| v.trim().parse::().ok()).filter(|v| v.is_finite()); let style = node.attribute("style").map(str::trim); - let dashes = node - .attribute("dashes") - .map(csv) - .filter(|v| !v.is_empty() && v.len() <= MAX_DASHES && v.iter().all(|x| *x >= 0.0) && v.iter().any(|x| *x > 0.0)); + let dashes = node.attribute("dashes").map(csv).filter(|v| !v.is_empty() && v.iter().all(|x| *x >= 0.0) && v.iter().any(|x| *x > 0.0)); let bs_style = match style { Some("dash") => Some("D"), Some("bevelled" | "beveled") => Some("B"), @@ -306,7 +301,11 @@ fn read_format(node: roxmltree::Node, d: &mut Dict) { fn rich_plain(node: roxmltree::Node) -> String { let mut out = String::new(); for n in node.descendants() { - if n.is_element() && matches!(n.tag_name().name(), "p" | "div" | "li" | "br") { + // A line break is a line of its own, so two in a row leave a blank line; a paragraph + // starts on a new line. + if n.is_element() && n.tag_name().name() == "br" { + out.push('\r'); + } else if n.is_element() && matches!(n.tag_name().name(), "p" | "div" | "li") { if !out.is_empty() && !out.ends_with('\r') { out.push('\r'); } @@ -359,10 +358,17 @@ fn annotation_names(doc: &Document, pages: &[pdfcraft_model::Page], only: Option /// The elements inside a comment's XFDF element: its text (plain and rich), default appearance /// and style, ink, vertices, appearance (with `ap`) and pop-up (on page `pi`). fn xfdf_annot_children(doc: &Document, out: &mut String, d: &Dict, pi: usize, ap: bool) { - if let Some(c) = text_of(doc, d, b"Contents") { - let _ = write!(out, "{}", esc(&c)); + let rc = text_of(doc, d, b"RC"); + match text_of(doc, d, b"Contents") { + Some(c) => { + let _ = write!(out, "{}", esc(&c)); + } + // Rich text written as text (below) is read back as rich text only beside ``; + // alone it would become the comment's plain text. + None if rc.as_deref().is_some_and(|rc| rich_text_element(rc).is_none()) => out.push_str(""), + None => {} } - if let Some(rc) = text_of(doc, d, b"RC") { + if let Some(rc) = rc { match rich_text_element(&rc) { // Rich text is an XML (XHTML) document (ISO 32000-2 Table 172, RC): its element goes in as markup. Some(markup) => { @@ -974,6 +980,7 @@ fn read_rich_text(rich: roxmltree::Node, has_contents: bool, d: &mut Dict) { }; let source = rich.document().input_text(); let Some(markup) = source.get(first.range().start..last.range().end) else { return }; + let markup = if elements.len() == 1 { standalone(rich, markup) } else { markup.to_string() }; // Rich text is an XML document of its own; export left its declaration out. d.set(b"RC".to_vec(), PdfString::text(&format!("{markup}"))); if !has_contents { @@ -981,6 +988,31 @@ fn read_rich_text(rich: roxmltree::Node, has_contents: bool, d: &mut Dict) { } } +/// The one element `markup` (cut out of `rich`, a ``) as an XML document of +/// its own: the namespace prefixes it uses but inherits from the XFDF around it are declared on +/// it. Unchanged when it stands alone already, or when the declarations would not make it parse. +fn standalone(rich: roxmltree::Node, markup: &str) -> String { + if parse_form_xml(markup).is_ok() { + return markup.to_string(); + } + // The start tag's name ends at white space, `/` or `>`. + let Some(name_end) = markup.get(1..).and_then(|t| t.find(|c: char| c.is_whitespace() || c == '/' || c == '>')).map(|i| i + 1) else { + return markup.to_string(); + }; + let start_tag = markup.get(..markup.find('>').unwrap_or(markup.len())).unwrap_or(markup); + let mut declared = String::new(); + for ns in rich.namespaces() { + let Some(prefix) = ns.name() else { continue }; + if prefix == "xml" || !markup.contains(&format!("{prefix}:")) || start_tag.contains(&format!("xmlns:{prefix}=")) { + continue; + } + let _ = write!(declared, " xmlns:{prefix}=\"{}\"", esc(ns.uri())); + } + let (Some(head), Some(tail)) = (markup.get(..name_end), markup.get(name_end..)) else { return markup.to_string() }; + let fixed = format!("{head}{declared}{tail}"); + if parse_form_xml(&fixed).is_ok() { fixed } else { markup.to_string() } +} + /// Put an annotation dictionary on a page, replacing one with the same `/NM`. Returns its /// reference. /// `/F` bits an imported file may not set on a redaction mark: Invisible, Hidden, NoView — so a @@ -1010,6 +1042,11 @@ pub(crate) fn place(doc: &mut Document, page_ref: ObjRef, mut d: Dict) -> Result // without usable /QuadPoints removes its /Rect, which the outline is then drawn from. d.remove(b"CA"); d.remove(b"ca"); + // The outline is PdfCraft's own (an appearance the file carries could hide the mark), and + // a redaction has no border effect (ISO 32000-2 Table 195 lists none; §12.5.4 gives + // /BE to squares, circles, polygons and free text), which the outline could not draw. + d.remove(b"AP"); + d.remove(b"BE"); let quads_drawable = d.get(b"QuadPoints").and_then(Object::as_array).is_some_and(|q| { let v: Vec = q.iter().filter_map(Object::as_f64).filter(|v| v.is_finite()).collect(); let (quads, rest) = v.as_chunks::<8>(); @@ -1033,7 +1070,10 @@ pub(crate) fn place(doc: &mut Document, page_ref: ObjRef, mut d: Dict) -> Result // An appearance the file carries is how the comment looked where it came from, and is kept // (XFDF 3.0 "stamp": it takes precedence over the icon). A redaction mark is always drawn by // PdfCraft, so what it marks for removal is shown. - let keep_appearance = d.name(b"Subtype") != Some(b"Redact".as_slice()) && super::appearance::has_appearance(doc, &d); + let redact = d.name(b"Subtype") == Some(b"Redact".as_slice()); + // A redaction mark with an area removes something when applied. + let marks = redact && d.contains(b"QuadPoints"); + let keep_appearance = !redact && super::appearance::has_appearance(doc, &d); let nm = d.get(b"NM").and_then(|n| n.as_string().map(|s| s.to_text())); let annots: Vec = doc .get(page_ref) @@ -1079,7 +1119,14 @@ pub(crate) fn place(doc: &mut Document, page_ref: ObjRef, mut d: Dict) -> Result }; // Otherwise our own appearance where we can draw one; others keep none (viewers draw a default). if !keep_appearance { - let _ = pdfcraft_annot::set_appearance(doc, r); + let drawn = pdfcraft_annot::set_appearance(doc, r); + // A redaction mark that removes something but shows nothing is refused: applying it + // would remove content the reviewer never saw marked. + if let Err(why) = drawn + && marks + { + return Err(DataError::Malformed(format!("a redaction mark could not be drawn, so what it removes would not show ({why})"))); + } } kept.push(Object::Ref(r)); doc.update_dict(page_ref, |p| p.set(b"Annots".to_vec(), Object::Array(kept)))?; @@ -1116,19 +1163,21 @@ pub fn import_comments( fresh: Option<&str>, offset: Option<[f64; 2]>, ) -> Result { - import_xfdf_with(doc, text, page, fresh, offset.filter(|o| o.iter().all(|v| v.is_finite()))) + import_xfdf_with(doc, text, page, fresh, offset.filter(|o| o.iter().all(|v| v.is_finite())), super::appearance::MAX_IMPORT_APPEARANCE_BYTES) } pub(crate) fn import_xfdf(doc: &mut Document, text: &str) -> Result { - import_xfdf_with(doc, text, None, None, None) + import_xfdf_with(doc, text, None, None, None, super::appearance::MAX_IMPORT_APPEARANCE_BYTES) } -fn import_xfdf_with( +/// The import, with `appearance_budget` decoded bytes of appearance for all of its comments. +pub(crate) fn import_xfdf_with( doc: &mut Document, text: &str, target_page: Option, fresh: Option<&str>, offset: Option<[f64; 2]>, + mut appearance_budget: usize, ) -> Result { check_form_xml_depth(text)?; let xml = parse_form_xml(text).map_err(|e| DataError::Malformed(e.to_string()))?; @@ -1137,7 +1186,6 @@ fn import_xfdf_with( return Err(DataError::UnknownFormat); } let mut report = Report::default(); - let mut appearance_budget = super::appearance::MAX_IMPORT_APPEARANCE_BYTES; let pages: Vec = pdfcraft_model::pages(doc).iter().map(|p| p.obj).collect(); if let Some(p) = target_page && p >= pages.len() @@ -1166,7 +1214,8 @@ fn import_xfdf_with( let Some(page) = target_page.or(own_page).and_then(|p| pages.get(p).copied()) else { continue }; let Some(mut d) = annot_from_xml(node, subtype, page) else { continue }; // The appearance the file carries (XFDF 3.0 "appearance"), when it can be read. - if let Some(ap) = node.children().find(|c| c.has_tag_name("appearance")) { + // A redaction mark is always drawn by PdfCraft (see `place`): its appearance is not read. + if let Some(ap) = node.children().find(|c| c.has_tag_name("appearance")).filter(|_| *subtype != "Redact") { let text: String = ap.children().filter(|c| c.is_text()).filter_map(|c| c.text()).collect(); match super::appearance::decode(&text, &mut appearance_budget) { Ok(tree) => { diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 7f946dc5f7..9e72710c60 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -4334,8 +4334,8 @@ milestone = "M5" status = "shipped" commands = ["comment.import"] tools = ["doc_import_data"] -evidence = ["crates/xfdf/src/tests.rs::xfdf_round_trips_comments_and_values", "crates/xfdf/src/tests.rs::fdf_round_trips_values_and_comments", "crates/automation/tests/it/automation.rs::comments_and_form_data_travel_as_xfdf_fdf_and_text", "crates/xfdf/src/tests_format.rs::formatting_written_by_another_tool_is_imported", "crates/xfdf/src/tests_format.rs::an_appearance_written_by_another_tool_is_imported", "crates/xfdf/src/tests_format.rs::unreadable_appearances_are_skipped_recorded_and_drawn_again", "crates/xfdf/src/tests_format.rs::fdf_carries_appearances_with_their_resources", "crates/xfdf/src/tests_format.rs::the_intent_is_written_and_read_as_intent_or_it"] -notes = "replies, pop-ups, ink, quads, line ends; same /NM replaces. XFDF formatting comes in as XFDF 3.0 names it: contents-richtext (/RC; plain text in it, or markup without , also gives /Contents), defaultstyle, width/style/dashes (a dashed border stays dashed), cloudy style and intensity, rotation, justification, fringe, leaderLength/leaderExtend/leader-offset, caption attributes, caret symbol, redaction overlay text, the intent (`intent` or `IT`). A stamp's (base 64 of the DICT/STREAM/DATA appearance XML other tools write) is kept, as are FDF /AP streams with their resources; an unreadable or oversized one (16 MiB each, 64 MiB per import, 64 levels, 100,000 values) is recorded in the import report and the comment is drawn by PdfCraft. A redaction mark is always drawn by PdfCraft. Rich text is kept as data; When PdfCraft draws a text box itself (no appearance in the file), it draws the plain text with /DA, not the rich text's fonts." +evidence = ["crates/xfdf/src/tests.rs::xfdf_round_trips_comments_and_values", "crates/xfdf/src/tests.rs::fdf_round_trips_values_and_comments", "crates/automation/tests/it/automation.rs::comments_and_form_data_travel_as_xfdf_fdf_and_text", "crates/xfdf/src/tests_format.rs::formatting_written_by_another_tool_is_imported", "crates/xfdf/src/tests_format.rs::an_appearance_written_by_another_tool_is_imported", "crates/xfdf/src/tests_format.rs::unreadable_appearances_are_skipped_recorded_and_drawn_again", "crates/xfdf/src/tests_format.rs::fdf_carries_appearances_with_their_resources", "crates/xfdf/src/tests_format.rs::the_intent_is_written_and_read_as_intent_or_it", "crates/xfdf/src/tests_format.rs::a_chain_of_forms_comes_back_whole_from_fdf_or_is_refused", "crates/xfdf/src/tests_format.rs::an_imported_redaction_mark_is_drawn_even_when_cloudy_or_undrawable", "crates/xfdf/src/tests_format.rs::the_import_budget_is_shared_by_every_comment", "crates/engine/src/tests.rs::a_comment_imported_without_its_appearance_is_said_in_the_repair_log"] +notes = "replies, pop-ups, ink, quads, line ends; same /NM replaces. XFDF formatting comes in as XFDF 3.0 names it: contents-richtext (/RC; plain text in it, or markup without , also gives /Contents), defaultstyle, width/style/dashes (a dashed border stays dashed), cloudy style and intensity, rotation, justification, fringe, leaderLength/leaderExtend/leader-offset, caption attributes, caret symbol, redaction overlay text, the intent (`intent` or `IT`). A stamp's XFDF (base 64 of the DICT/STREAM/DATA appearance XML other tools write) is kept; an unreadable or oversized one (16 MiB each, 64 MiB per import, 64 levels, 100,000 values) is left out, said in the import report and the repair log, and the comment is drawn by PdfCraft. FDF /AP streams are kept with their resources; an FDF whose comments expand past the import budget (1,000,000 values or 64 MiB of data) or nest more than 256 levels is refused with an error and nothing is imported. A redaction mark is always drawn by PdfCraft, and one with an area it cannot draw is refused. Rich text is kept as data; when PdfCraft draws a text box itself (no appearance in the file), it draws the plain text with /DA, not the rich text's fonts, and does not turn it by /Rotate." [[feature]] id = "comment.export-fdf-xfdf"