From a90502f2c901ddccf85c5f5207961bbb1c14537b Mon Sep 17 00:00:00 2001
From: mtlcl410
Date: Sun, 8 Nov 2026 17:43:00 +0800
Subject: [PATCH 1/6] M5: comments keep their formatting and appearance through
XFDF and FDF (comment.import-fdf-xfdf, comment.export-fdf-xfdf)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
XFDF export and import carry each comment's formatting as XFDF 3.0
(ISO 19444-1) names it: contents-richtext (/RC), defaultstyle (/DS),
width/style/dashes (/BS, or an older /Border), cloudy style and
intensity (/BE), rotation, justification (/Q), fringe (/RD) on every
type, leaderLength/leaderExtend/leader-offset and the caption
attributes of lines, the caret symbol and redaction overlay text.
A stamp's appearance travels as : base 64 of the
DICT/STREAM/ARRAY/NAME/INT/FIXED/BOOL/STRING/NULL XML other tools
write, with stream data in DATA as XFDF 3.0 "Stream encoding" gives it.
Copy and paste carries every comment's appearance. Import keeps an
appearance it can read (never on a redaction mark), records an
unreadable one in Report::skipped and draws the comment instead; it is
bounded at 16 MiB per appearance, 64 MiB per import, 64 levels and
100,000 values.
FDF export keeps /AP with the forms and images it draws (ISO 32000-2
§12.7.8.3.4); FDF import copies those streams into the document, once
when shared, only for comments that land on a page, and no longer
nulls values past 16 levels (a gradient stamp's shading functions sit
deeper).
Co-Authored-By: Claude Opus 5.5
Claude-Session: https://claude.ai/code/session_01Pq7HyCHD1R3rywQBPCHXZA
---
Cargo.lock | 2 +
ROADMAP.md | 2 +
crates/xfdf/Cargo.toml | 4 +
crates/xfdf/README.md | 26 +-
crates/xfdf/src/appearance.rs | 341 +++++++++++++++
crates/xfdf/src/fdf.rs | 68 ++-
crates/xfdf/src/lib.rs | 12 +-
crates/xfdf/src/tests.rs | 6 +-
crates/xfdf/src/tests_format.rs | 729 ++++++++++++++++++++++++++++++++
crates/xfdf/src/xfdf.rs | 339 +++++++++++++--
parity/acrobat-features.toml | 16 +-
11 files changed, 1478 insertions(+), 67 deletions(-)
create mode 100644 crates/xfdf/src/appearance.rs
create mode 100644 crates/xfdf/src/tests_format.rs
diff --git a/Cargo.lock b/Cargo.lock
index 6710dddeea..8bd476ad54 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -4796,10 +4796,12 @@ dependencies = [
name = "pdfcraft-xfdf"
version = "0.18.1"
dependencies = [
+ "base64 0.23.1",
"pdfcraft-annot",
"pdfcraft-cos",
"pdfcraft-forms",
"pdfcraft-model",
+ "proptest",
"roxmltree 0.21.2",
"thiserror 2.0.21",
]
diff --git a/ROADMAP.md b/ROADMAP.md
index 1de96a7cda..3270d42bdb 100644
--- a/ROADMAP.md
+++ b/ROADMAP.md
@@ -230,6 +230,8 @@ Ranked; detail and the full milestone table in [docs/roadmap.md](docs/roadmap.md
## Progress log
+- **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 and import keep each comment's /AP with its forms and images (ISO 32000-2 §12.7.8.3.4). 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-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/xfdf/Cargo.toml b/crates/xfdf/Cargo.toml
index f868c95719..d82e75394e 100644
--- a/crates/xfdf/Cargo.toml
+++ b/crates/xfdf/Cargo.toml
@@ -14,4 +14,8 @@ pdfcraft-annot = { workspace = true }
pdfcraft-forms = { workspace = true }
pdfcraft-model = { workspace = true }
roxmltree = "0.21"
+base64 = "0.23"
thiserror = { workspace = true }
+
+[dev-dependencies]
+proptest = { workspace = true }
diff --git a/crates/xfdf/README.md b/crates/xfdf/README.md
index d286a457c3..b5a6d8d65e 100644
--- a/crates/xfdf/README.md
+++ b/crates/xfdf/README.md
@@ -14,9 +14,25 @@ let report = import(&mut doc, &bytes)?; // XFDF, FDF, XML, CSV o
name, author, subject, dates, flags, colours, opacity, width, icon, quads, line ends, ink,
default appearance, callouts (`callout`, `fringe`, `intent`, `head`), pop-ups and reply threads
(`inreplyto`); field values nested by name, with text preserved across XML comments and processing instructions.
+ Formatting travels both ways as XFDF 3.0 names it: rich text (``, the `/RC`
+ XHTML as markup; plain text in it is the comment's text), default style (``),
+ border width, style and dash pattern (`width`, `style`, `dashes`; also from an older `/Border`
+ array), cloudy borders (`style="cloudy"`, `intensity`), `rotation`, `justification` (free text
+ `left`/`centered`/`right`, redaction `0`–`2`), `fringe` (`/RD`) on every type, line leaders and
+ captions (`leaderLength`, `leaderExtend`, `leader-offset`, `caption`, `caption-style`,
+ `caption-offset-h`/`-v`), the caret `symbol` and redaction `overlay-text`/`overlay-text-repeat`.
+- **Appearances** (``, `appearance.rs`): a stamp's `/AP`, as XFDF 3.0 defines it for
+ stamps, base 64 of the dictionary written as `DICT`/`STREAM`/`ARRAY`/`NAME`/`INT`/`FIXED`/`BOOL`/
+ `STRING`/`NULL` elements with stream data in `DATA` (XFDF 3.0 "Stream encoding": `filtered`/`ascii`
+ 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.
- **FDF**: fields (with `/Kids` hierarchies) and indirect comments. Nested annotation values
- are copied under FDF object numbers; shared references and cycles remain shared. Document
- links and appearances are excluded. Source `/Page` and `/Pages` dictionaries become null
+ are copied under FDF object numbers; shared references and cycles remain shared. Appearances
+ (`/AP`, with the forms and images they draw) are copied and come back on import as objects of
+ the document (a stream shared by several comments once); document links are excluded. Source `/Page` and `/Pages` dictionaries become null
at every copy depth, so nested destination actions cannot carry the source page tree or content.
Type names may be indirect; lookup checks each reference generation, is iterative, stops at
cycles and refuses chains beyond
@@ -28,7 +44,7 @@ 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 semantics are unchanged.
+ encoded bytes. FDF import copies a comment's streams only when the comment lands on a page.
- **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).
@@ -44,7 +60,7 @@ tables remains heuristic. Both write a cell that a spreadsheet would run as a fo
or `+` or `-` when it isn't a plain number) with a leading `'`; CSV import takes off the `'` that
export added, so exported data imports back unchanged.
-Import replaces comments with the same `/NM` (and drops the replaced comment's pop-up), draws
-appearances PdfCraft can draw, and sets values through the form's own checks (formats,
+Import replaces comments with the same `/NM` (and drops the replaced comment's pop-up), keeps the
+appearance the file carries or draws one where PdfCraft can, and sets values through the form's own checks (formats,
validation, recalculation); read-only fields take imported values, as in Acrobat. XML is read
with `roxmltree`.
diff --git a/crates/xfdf/src/appearance.rs b/crates/xfdf/src/appearance.rs
new file mode 100644
index 0000000000..d8cbddb4ad
--- /dev/null
+++ b/crates/xfdf/src/appearance.rs
@@ -0,0 +1,341 @@
+//! A comment's appearance dictionary (`/AP`) in XFDF.
+//!
+//! ISO 19444-1 (XFDF 3.0, "appearance") carries a stamp's `/AP` as a base 64 encoded string. What
+//! the string holds is the dictionary written as XML, one element per PDF object: `DICT`, `STREAM`,
+//! `ARRAY`, `NAME`, `INT`, `FIXED` (a real number), `BOOL`, `STRING` and `NULL`, each with its
+//! dictionary key in `KEY` and a simple value in `VAL`; a stream's data is a `DATA` element whose
+//! `MODE` and `ENCODING` follow XFDF 3.0 "Stream encoding" (`filtered`/`ascii` for short printable
+//! data, `raw`/`hex` otherwise). That is what the XFDF files in the wild hold (for example the
+//! stamps attached to PDFBOX-4437 and PDFBOX-4478). Indirect objects are written in place, so the
+//! XML is self-contained.
+//!
+//! Both directions are bounded: the decoded XML, the stream data, the nesting and the number of
+//! values. Anything unreadable makes the whole appearance unusable; the caller then records it and
+//! draws the comment from its other entries.
+
+use base64::Engine as _;
+
+use super::common::{check_form_xml_depth, esc, parse_form_xml};
+use super::{Dict, Document, ObjRef, Object, PdfString};
+
+/// At most this much decoded XML in one appearance (and at most this much XML is written for one).
+pub(crate) const MAX_APPEARANCE_BYTES: usize = 16 << 20;
+/// At most this much appearance data (decoded XML) in one import, over all of its comments.
+pub(crate) const MAX_IMPORT_APPEARANCE_BYTES: usize = 64 << 20;
+/// How deeply objects may nest inside one appearance.
+const MAX_NESTING: usize = 64;
+/// How many PDF values one appearance may hold.
+const MAX_VALUES: usize = 100_000;
+/// XFDF 3.0 "Stream encoding": data shorter than this that is all printable ASCII is written as it
+/// is (`filtered`/`ascii`); everything else as hexadecimal (`raw`/`hex`).
+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).
+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 writer = Writer { doc, out: String::new(), values: MAX_VALUES, path: Vec::new() };
+ writer.dict(Some(b"AP"), ap.as_dict()?, 0)?;
+ 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);
+ for (i, c) in b64.chars().enumerate() {
+ if i > 0 && i % 76 == 0 {
+ wrapped.push('\n');
+ }
+ wrapped.push(c);
+ }
+ Some(wrapped)
+}
+
+struct Writer<'a> {
+ doc: &'a Document,
+ out: String,
+ values: usize,
+ /// The references being written, so a reference back to one of them ends the cycle.
+ path: Vec,
+}
+
+impl Writer<'_> {
+ fn key_attr(key: Option<&[u8]>) -> String {
+ key.map(|k| format!(" KEY=\"{}\"", esc(&pdfcraft_forms::name_text(k)))).unwrap_or_default()
+ }
+
+ fn spend(&mut self, depth: usize) -> Option<()> {
+ if depth > MAX_NESTING || self.out.len() > MAX_APPEARANCE_BYTES {
+ return None;
+ }
+ self.values = self.values.checked_sub(1)?;
+ Some(())
+ }
+
+ fn dict(&mut self, key: Option<&[u8]>, d: &Dict, depth: usize) -> Option<()> {
+ self.spend(depth)?;
+ self.out.push_str(&format!("\n", Self::key_attr(key)));
+ for (k, v) in d.iter() {
+ self.value(Some(k), v, depth + 1)?;
+ }
+ self.out.push_str("\n");
+ Some(())
+ }
+
+ fn value(&mut self, key: Option<&[u8]>, o: &Object, depth: usize) -> Option<()> {
+ self.spend(depth)?;
+ let k = Self::key_attr(key);
+ match o {
+ Object::Ref(r) => {
+ if self.path.contains(r) {
+ // A cycle: the object is already being written further up.
+ self.out.push_str(&format!("\n"));
+ return Some(());
+ }
+ self.path.push(*r);
+ let target = self.doc.get(*r);
+ let written = self.value(key, &target, depth + 1);
+ self.path.pop();
+ written?;
+ }
+ Object::Null => self.out.push_str(&format!("\n")),
+ Object::Bool(b) => self.out.push_str(&format!("\n")),
+ Object::Int(i) => self.out.push_str(&format!("\n")),
+ // Rust prints the shortest decimal that reads back as the same number, never with an
+ // exponent; a non-finite number (which PDF can't hold) is written as 0.
+ Object::Real(r) => self.out.push_str(&format!("\n", if r.is_finite() { *r } else { 0.0 })),
+ Object::Name(n) => self.out.push_str(&format!("\n", esc(&pdfcraft_forms::name_text(n)))),
+ Object::String(s) => match std::str::from_utf8(&s.bytes) {
+ // Text XML can hold as it is (`esc` would drop other control characters).
+ Ok(t) if t.chars().all(|c| !(c < ' ' && !matches!(c, '\t' | '\n' | '\r')) && c != '\u{FFFE}' && c != '\u{FFFF}') => {
+ self.out.push_str(&format!("\n", esc(t)));
+ }
+ _ => self.out.push_str(&format!("\n", hex_of(&s.bytes, false))),
+ },
+ Object::Array(a) => {
+ self.out.push_str(&format!("\n"));
+ for v in a {
+ self.value(None, v, depth + 1)?;
+ }
+ self.out.push_str("\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.
+ if s.raw.len().saturating_mul(2).saturating_add(self.out.len()) > MAX_APPEARANCE_BYTES {
+ return None;
+ }
+ self.out.push_str(&format!("\n"));
+ // `Length` follows from the data.
+ 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');
+ 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);
+ self.out.push_str(&format!("{}\n", esc_data(&text)));
+ } else {
+ self.out.push_str(&format!("{}\n", hex_of(&s.raw, true)));
+ }
+ self.out.push_str("\n");
+ }
+ }
+ Some(())
+ }
+}
+
+/// XFDF 3.0 "Stream encoding", method 1: the XML control characters as entities; line breaks stay.
+fn esc_data(s: &str) -> String {
+ s.replace('&', "&").replace('<', "<").replace('>', ">").replace('"', """)
+}
+
+/// Bytes as upper-case hexadecimal, with a line feed after every 80 characters when `wrap`.
+fn hex_of(bytes: &[u8], wrap: bool) -> String {
+ const DIGITS: &[u8; 16] = b"0123456789ABCDEF";
+ let mut out = String::with_capacity(bytes.len().saturating_mul(2).saturating_add(bytes.len() / 40));
+ for (i, b) in bytes.iter().enumerate() {
+ if wrap && i > 0 && i % 40 == 0 {
+ out.push('\n');
+ }
+ out.push(char::from(DIGITS[usize::from(b >> 4)]));
+ out.push(char::from(DIGITS[usize::from(b & 15)]));
+ }
+ out
+}
+
+/// Hexadecimal digits back to bytes, ignoring white space; `None` for any other character or an
+/// odd number of digits.
+fn bytes_of_hex(text: &str) -> Option> {
+ let digits: Vec = text.bytes().filter(|b| !b.is_ascii_whitespace()).collect();
+ if !digits.len().is_multiple_of(2) {
+ return None;
+ }
+ let value = |c: u8| char::from(c).to_digit(16).and_then(|d| u8::try_from(d).ok());
+ digits.as_chunks::<2>().0.iter().map(|[h, l]| Some(value(*h)? << 4 | value(*l)?)).collect()
+}
+
+/// An appearance read from XFDF, before it is put into a document.
+#[derive(Debug, PartialEq)]
+pub(crate) enum Node {
+ Value(Object),
+ Array(Vec),
+ Dict(Vec<(Vec, Node)>),
+ Stream(Vec<(Vec, Node)>, Vec),
+}
+
+/// Read the text of an `` element: base 64, then the XML it holds. `budget` is what
+/// the import may still read (decoded bytes) and is charged. The result is the `/AP` dictionary,
+/// with a normal appearance (`/N`) that is a stream or a dictionary of streams.
+pub(crate) fn decode(text: &str, budget: &mut usize) -> Result {
+ let limit = MAX_APPEARANCE_BYTES.min(*budget);
+ // Base 64 is 4 characters for 3 bytes: refuse a text that decodes past the limit before
+ // decoding it.
+ let mut compact = String::new();
+ for c in text.chars().filter(|c| !c.is_whitespace()) {
+ if compact.len() / 4 * 3 > limit {
+ return Err(format!("the appearance is larger than {} MiB", limit >> 20));
+ }
+ compact.push(c);
+ }
+ let engine = base64::engine::GeneralPurpose::new(
+ &base64::alphabet::STANDARD,
+ base64::engine::GeneralPurposeConfig::new().with_decode_padding_mode(base64::engine::DecodePaddingMode::Indifferent),
+ );
+ let bytes = engine.decode(compact.as_bytes()).map_err(|e| format!("the appearance is not base 64: {e}"))?;
+ if bytes.len() > limit {
+ return Err(format!("the appearance is larger than {} MiB", limit >> 20));
+ }
+ *budget = budget.saturating_sub(bytes.len());
+ let xml = std::str::from_utf8(&bytes).map_err(|_| "the appearance is not UTF-8 text".to_string())?;
+ let xml = xml.trim_start_matches('\u{feff}');
+ check_form_xml_depth(xml).map_err(|e| e.to_string())?;
+ let parsed = parse_form_xml(xml).map_err(|e| format!("the appearance is not XML: {e}"))?;
+ let root = parsed.root_element();
+ if !root.tag_name().name().eq_ignore_ascii_case("DICT") {
+ return Err("the appearance is not a dictionary".into());
+ }
+ let mut values = MAX_VALUES;
+ let node = read(root, 0, &mut values)?;
+ let usable = match &node {
+ Node::Dict(entries) => entries.iter().any(|(k, v)| {
+ k.as_slice() == b"N"
+ && match v {
+ Node::Stream(..) => true,
+ Node::Dict(states) => !states.is_empty() && states.iter().all(|(_, s)| matches!(s, Node::Stream(..))),
+ _ => false,
+ }
+ }),
+ _ => false,
+ };
+ if !usable {
+ return Err("the appearance has no normal appearance stream".into());
+ }
+ Ok(node)
+}
+
+fn attr<'a>(node: roxmltree::Node<'a, '_>, name: &str) -> Option<&'a str> {
+ node.attributes().find(|a| a.name().eq_ignore_ascii_case(name)).map(|a| a.value())
+}
+
+fn read(node: roxmltree::Node, depth: usize, values: &mut usize) -> Result {
+ if depth > MAX_NESTING {
+ return Err(format!("the appearance nests objects more than {MAX_NESTING} deep"));
+ }
+ *values = values.checked_sub(1).ok_or_else(|| format!("the appearance holds more than {MAX_VALUES} values"))?;
+ let tag = node.tag_name().name().to_ascii_uppercase();
+ let val = || attr(node, "VAL").ok_or_else(|| format!("{tag} has no VAL"));
+ let entries = |values: &mut usize| -> Result, Node)>, String> {
+ let mut out = Vec::new();
+ for child in node.children().filter(|c| c.is_element() && !c.tag_name().name().eq_ignore_ascii_case("DATA")) {
+ let key = attr(child, "KEY").ok_or_else(|| format!("an entry of {tag} has no KEY"))?;
+ out.push((pdfcraft_forms::name_bytes(key), read(child, depth + 1, values)?));
+ }
+ Ok(out)
+ };
+ Ok(match tag.as_str() {
+ "DICT" => Node::Dict(entries(values)?),
+ "ARRAY" => {
+ let mut items = Vec::new();
+ for child in node.children().filter(|c| c.is_element()) {
+ items.push(read(child, depth + 1, values)?);
+ }
+ Node::Array(items)
+ }
+ "STREAM" => {
+ let mut dict = entries(values)?;
+ let data = node.children().find(|c| c.is_element() && c.tag_name().name().eq_ignore_ascii_case("DATA")).ok_or("a stream has no DATA")?;
+ let text: String = data.children().filter(|c| c.is_text()).filter_map(|c| c.text()).collect();
+ let bytes = match attr(data, "ENCODING").map(str::to_ascii_lowercase).as_deref() {
+ Some("hex") => bytes_of_hex(&text).ok_or("stream data is not hexadecimal")?,
+ Some("ascii") | None => text.into_bytes(),
+ Some(other) => return Err(format!("unknown stream encoding {other}")),
+ };
+ // `filtered` data is already decoded: the filters no longer apply to it.
+ if attr(data, "MODE").is_some_and(|m| m.eq_ignore_ascii_case("filtered")) {
+ dict.retain(|(k, _)| !matches!(k.as_slice(), b"Filter" | b"DecodeParms" | b"FFilter" | b"FDecodeParms"));
+ }
+ dict.retain(|(k, _)| k.as_slice() != b"Length");
+ Node::Stream(dict, bytes)
+ }
+ "NAME" => Node::Value(Object::Name(pdfcraft_forms::name_bytes(val()?))),
+ "INT" => Node::Value(Object::Int(val()?.trim().parse::().map_err(|_| "INT is not an integer")?)),
+ "FIXED" => {
+ let v = val()?.trim().parse::().ok().filter(|v| v.is_finite()).ok_or("FIXED is not a number")?;
+ Node::Value(Object::Real(v))
+ }
+ "BOOL" => Node::Value(Object::Bool(match val()?.trim() {
+ "true" => true,
+ "false" => false,
+ _ => return Err("BOOL is neither true nor false".into()),
+ })),
+ "STRING" => {
+ let v = val()?;
+ let bytes = if attr(node, "ENCODING").is_some_and(|e| e.eq_ignore_ascii_case("hex")) {
+ bytes_of_hex(v).ok_or("STRING is not hexadecimal")?
+ } else {
+ v.as_bytes().to_vec()
+ };
+ Node::Value(Object::String(PdfString::literal(bytes)))
+ }
+ "NULL" => Node::Value(Object::Null),
+ other => return Err(format!("unknown appearance element {other}")),
+ })
+}
+
+/// Put a read appearance into `doc`: each stream becomes an object of its own. Returns the `/AP`
+/// value. Null dictionary entries are left out (ISO 32000-2 §7.3.7: they mean the entry is absent).
+pub(crate) fn materialize(doc: &mut Document, node: Node) -> Object {
+ let dict = |doc: &mut Document, entries: Vec<(Vec, Node)>| {
+ let mut d = Dict::new();
+ for (k, v) in entries {
+ let v = materialize(doc, v);
+ if !matches!(v, Object::Null) {
+ d.set(k, v);
+ }
+ }
+ d
+ };
+ match node {
+ Node::Value(o) => o,
+ Node::Array(items) => Object::Array(items.into_iter().map(|n| materialize(doc, n)).collect()),
+ Node::Dict(entries) => Object::Dict(dict(doc, entries)),
+ Node::Stream(entries, raw) => {
+ let d = dict(doc, entries);
+ Object::Ref(doc.add(Object::Stream(pdfcraft_cos::Stream { dict: d, raw: raw.into() })))
+ }
+ }
+}
+
+/// Whether an annotation dictionary carries an appearance the document can draw: `/AP` with a
+/// normal appearance that is a stream, or a dictionary of streams (one per state).
+pub(crate) fn has_appearance(doc: &Document, d: &Dict) -> bool {
+ let Some(ap) = d.get(b"AP").map(|a| doc.resolve(a)) else { return false };
+ let Some(normal) = ap.as_dict().and_then(|ap| ap.get(b"N")).map(|n| doc.resolve(n)) else { return false };
+ match normal.as_ref() {
+ Object::Stream(_) => true,
+ Object::Dict(states) => !states.is_empty() && states.iter().all(|(_, s)| matches!(doc.resolve(s).as_ref(), Object::Stream(_))),
+ _ => false,
+ }
+}
diff --git a/crates/xfdf/src/fdf.rs b/crates/xfdf/src/fdf.rs
index ceb4ad99eb..42d0d28ecc 100644
--- a/crates/xfdf/src/fdf.rs
+++ b/crates/xfdf/src/fdf.rs
@@ -86,13 +86,14 @@ impl<'a> FdfCopy<'a> {
Ok(())
}
- /// Skip source-document links and appearances without cloning their values. The
- /// dictionary and every retained entry are charged before any owned copy is made.
+ /// Skip source-document links without cloning their values. The appearance (`/AP`) is
+ /// copied with its streams and resources, so the comment looks the same where it is imported.
+ /// The dictionary and every retained entry are charged before any owned copy is made.
fn annotation_dict(&mut self, dict: &Dict) -> Result {
self.charge_value(0)?;
let mut out = Dict::new();
for (key, value) in dict.iter() {
- if matches!(key.as_slice(), b"P" | b"AP" | b"Popup" | b"IRT" | b"Parent" | b"FS" | b"Sound") {
+ if matches!(key.as_slice(), b"P" | b"Popup" | b"IRT" | b"Parent" | b"FS" | b"Sound") {
continue;
}
self.dict_entry(&mut out, key, value, 1)?;
@@ -202,7 +203,7 @@ impl<'a> FdfCopy<'a> {
}
}
-/// FDF with field values and/or comments (appearances are not carried). Each comment is an
+/// FDF with field values and/or comments, each with its appearance. 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). Nested values are copied and renumbered, including shared or cyclic graphs.
/// Source `/Page` and `/Pages` dictionaries become null at every copy depth.
@@ -356,11 +357,27 @@ impl ImportBudget {
}
}
+/// Where [`deep`] copies to: the document, and the streams already copied into it (by their
+/// reference in the FDF), so a stream two comments share is copied once.
+struct DeepTarget<'a> {
+ doc: &'a mut Document,
+ streams: HashMap,
+}
+
/// Copy an object out of another document, resolving references. A reference back to an object
-/// being copied becomes null.
-fn deep(src: &Document, o: &Object, depth: usize, path: &mut Vec, budget: &mut ImportBudget) -> Result
two
"#,
+ ] {
+ assert!(xml.contains(want), "missing {want} in\n{xml}");
+ }
+ // What it writes is XML the reader accepts.
+ assert!(roxmltree::Document::parse(&xml).is_ok());
+}
+
+#[test]
+fn free_text_formatting_comes_back_from_xfdf() {
+ let mut src = pages(1);
+ put(&mut src, formatted_freetext());
+ let (_, dst) = round_trip_xfdf(&src);
+ let d = named(&dst, "ft");
+ assert_eq!(text(&dst, &d, b"RC").as_deref(), Some(RICH));
+ assert_eq!(text(&dst, &d, b"DS").as_deref(), Some("font: Helvetica,sans-serif 14.0pt; text-align:right; color:#0000FF"));
+ assert_eq!(text(&dst, &d, b"Contents").as_deref(), Some("Bold & plain\rtwo"));
+ assert_eq!(d.int(b"Q"), Some(2));
+ assert_eq!(d.int(b"Rotate"), Some(90));
+ assert_eq!(f64s(&dst, d.get(b"RD")), vec![1.5, 2.0, 3.0, 4.0]);
+ let bs = sub_dict(&dst, &d, b"BS");
+ assert_eq!(bs.name(b"S"), Some(b"D".as_slice()), "a dashed border comes back dashed, not solid");
+ assert_eq!(f64s(&dst, bs.get(b"D")), vec![3.0, 2.0]);
+ assert_eq!(bs.get(b"W").and_then(Object::as_f64), Some(2.0));
+}
+
+#[test]
+fn line_leaders_and_caption_round_trip() {
+ let mut src = pages(1);
+ put(&mut src, formatted_line());
+ let (xml, dst) = round_trip_xfdf(&src);
+ for want in [
+ r#"leaderLength="-12""#,
+ r#"leaderExtend="3""#,
+ r#"leader-offset="1.5""#,
+ r#"caption="yes""#,
+ r#"caption-style="Top""#,
+ r#"caption-offset-h="4" caption-offset-v="-2""#,
+ r#"dashes="4,3,2,3""#,
+ ] {
+ assert!(xml.contains(want), "missing {want} in\n{xml}");
+ }
+ let d = named(&dst, "ln");
+ assert_eq!(d.get(b"LL").and_then(Object::as_f64), Some(-12.0));
+ assert_eq!(d.get(b"LLE").and_then(Object::as_f64), Some(3.0));
+ assert_eq!(d.get(b"LLO").and_then(Object::as_f64), Some(1.5));
+ assert_eq!(d.get(b"Cap"), Some(&Object::Bool(true)));
+ assert_eq!(d.name(b"CP"), Some(b"Top".as_slice()));
+ assert_eq!(f64s(&dst, d.get(b"CO")), vec![4.0, -2.0]);
+ assert_eq!(f64s(&dst, sub_dict(&dst, &d, b"BS").get(b"D")), vec![4.0, 3.0, 2.0, 3.0]);
+}
+
+#[test]
+fn square_circle_caret_and_redaction_attributes_round_trip() {
+ let mut src = pages(1);
+ let mut sq = annot("Square", "sq", [10.0, 10.0, 110.0, 110.0]);
+ sq.set(b"RD".to_vec(), nums(&[5.0, 5.0, 5.0, 5.0]));
+ let mut be = Dict::new();
+ be.set(b"S".to_vec(), Object::name("C"));
+ be.set(b"I".to_vec(), Object::Real(2.0));
+ sq.set(b"BE".to_vec(), Object::Dict(be));
+ put(&mut src, sq);
+ let mut ci = annot("Circle", "ci", [120.0, 10.0, 220.0, 110.0]);
+ let mut bs = Dict::new();
+ bs.set(b"W".to_vec(), Object::Real(3.0));
+ bs.set(b"S".to_vec(), Object::name("B"));
+ ci.set(b"BS".to_vec(), Object::Dict(bs));
+ put(&mut src, ci);
+ let mut ca = annot("Caret", "ca", [230.0, 10.0, 250.0, 30.0]);
+ ca.set(b"Sy".to_vec(), Object::name("P"));
+ ca.set(b"RD".to_vec(), nums(&[1.0, 1.0, 1.0, 1.0]));
+ put(&mut src, ca);
+ let mut rd = annot("Redact", "rd", [300.0, 10.0, 400.0, 30.0]);
+ rd.set(b"OverlayText".to_vec(), PdfString::text("WITHHELD"));
+ rd.set(b"Repeat".to_vec(), Object::Bool(true));
+ rd.set(b"Q".to_vec(), Object::Int(1));
+ put(&mut src, rd);
+ // A solid border that still holds a dash array keeps it, and stays solid.
+ let mut solid = annot("Square", "solid", [120.0, 200.0, 220.0, 300.0]);
+ let mut bs = Dict::new();
+ bs.set(b"W".to_vec(), Object::Real(1.0));
+ bs.set(b"S".to_vec(), Object::name("S"));
+ bs.set(b"D".to_vec(), nums(&[2.0]));
+ solid.set(b"BS".to_vec(), Object::Dict(bs));
+ put(&mut src, solid);
+ // An older /Border array: width 2, dashes 4 on 1.
+ let mut old = annot("Square", "old", [10.0, 200.0, 110.0, 300.0]);
+ old.set(b"Border".to_vec(), Object::Array(vec![0.into(), 0.into(), 2.into(), nums(&[4.0, 1.0])]));
+ put(&mut src, old);
+ let (xml, dst) = round_trip_xfdf(&src);
+ for want in [
+ r#"style="cloudy" intensity="2""#,
+ r#"fringe="5,5,5,5""#,
+ r#"style="bevelled""#,
+ r#"symbol="paragraph""#,
+ r#"overlay-text="WITHHELD""#,
+ r#"overlay-text-repeat="true""#,
+ r#"justification="1""#,
+ ] {
+ assert!(xml.contains(want), "missing {want} in\n{xml}");
+ }
+ let sq = named(&dst, "sq");
+ assert_eq!(f64s(&dst, sq.get(b"RD")), vec![5.0; 4]);
+ assert_eq!(sub_dict(&dst, &sq, b"BE").get(b"I").and_then(Object::as_f64), Some(2.0));
+ assert_eq!(sub_dict(&dst, &named(&dst, "ci"), b"BS").name(b"S"), Some(b"B".as_slice()));
+ let ca = named(&dst, "ca");
+ assert_eq!(ca.name(b"Sy"), Some(b"P".as_slice()));
+ assert_eq!(f64s(&dst, ca.get(b"RD")), vec![1.0; 4]);
+ let rd = named(&dst, "rd");
+ assert_eq!(text(&dst, &rd, b"OverlayText").as_deref(), Some("WITHHELD"));
+ assert_eq!(rd.get(b"Repeat"), Some(&Object::Bool(true)));
+ assert_eq!(rd.int(b"Q"), Some(1));
+ let solid = sub_dict(&dst, &named(&dst, "solid"), b"BS");
+ assert_eq!((solid.name(b"S"), f64s(&dst, solid.get(b"D"))), (Some(b"S".as_slice()), vec![2.0]));
+ let old = sub_dict(&dst, &named(&dst, "old"), b"BS");
+ assert_eq!((old.get(b"W").and_then(Object::as_f64), old.name(b"S")), (Some(2.0), Some(b"D".as_slice())));
+ assert_eq!(f64s(&dst, old.get(b"D")), vec![4.0, 1.0]);
+}
+
+/// The XFDF the audit imported: what another tool writes for formatted comments.
+#[test]
+fn formatting_written_by_another_tool_is_imported() {
+ let xml = r#"
+
+
+
+"#;
+ 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"));
+}
+
+/// 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"
From 9bd278a9b3ed9a5eb210a3f9d3a69fde6163020f Mon Sep 17 00:00:00 2001
From: mtlcl410
Date: Mon, 9 Nov 2026 02:12:23 +0800
Subject: [PATCH 5/6] M5: comment imports are all or nothing and appearance
export stops where import does (comment.import-fdf-xfdf,
comment.export-fdf-xfdf)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
- import and import_comments work on a copy, kept only on success.
- XFDF export shares the 64 MiB appearance budget import reads over all
comments; import refuses a normal appearance of several states (XFDF has
no /AS to select one, ISO 32000-2 §12.5.5).
- FDF import nulls a page or page tree an appearance reaches (/Type resolved).
- Rich text: an inherited default namespace other than XFDF's is declared,
and a namespace declaration with spaces around `=` is recognised.
- A redaction mark's /Rect grows to hold its quads, so the outline shows.
- The repair log gets one line per import (first five, then a count), and
no longer claims the comment is drawn.
- Tests: the sixty-form chain, and the redaction outline is stroked.
Co-Authored-By: Claude Opus 5.5
Claude-Session: https://claude.ai/code/session_01Pq7HyCHD1R3rywQBPCHXZA
---
crates/engine/src/edit/run.rs | 18 +++++--
crates/engine/src/tests.rs | 7 ++-
crates/xfdf/README.md | 5 +-
crates/xfdf/src/appearance.rs | 30 +++++------
crates/xfdf/src/fdf.rs | 4 ++
crates/xfdf/src/lib.rs | 9 +++-
crates/xfdf/src/tests_format.rs | 88 +++++++++++++++++++++++++++++++--
crates/xfdf/src/xfdf.rs | 83 ++++++++++++++++++++++++++-----
parity/acrobat-features.toml | 2 +-
9 files changed, 206 insertions(+), 40 deletions(-)
diff --git a/crates/engine/src/edit/run.rs b/crates/engine/src/edit/run.rs
index 25b55de22a..7413ab1241 100644
--- a/crates/engine/src/edit/run.rs
+++ b/crates/engine/src/edit/run.rs
@@ -162,12 +162,22 @@ 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.
+/// What a comment import left out of comments it did import (an appearance it could not read),
+/// in the repair log as one line: 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"));
+ if report.skipped.is_empty() {
+ return;
}
+ let shown: Vec<&str> = report.skipped.iter().take(5).map(String::as_str).collect();
+ let more = report.skipped.len().saturating_sub(shown.len());
+ let more = if more > 0 { format!("; and {more} more") } else { String::new() };
+ doc.note_repair(format!(
+ "imported {} comment{} without the appearance the file gave {} (PdfCraft draws it where it can; it may look different or not show): {}{more}",
+ report.skipped.len(),
+ if report.skipped.len() == 1 { "" } else { "s" },
+ if report.skipped.len() == 1 { "it" } else { "them" },
+ shown.join("; ")
+ ));
}
/// Dates and unique ids stamped onto what an edit creates.
diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs
index 6e225bdc6e..38bf0a002e 100644
--- a/crates/engine/src/tests.rs
+++ b/crates/engine/src/tests.rs
@@ -11287,7 +11287,12 @@ fn imported_bookmarks_undo_in_one_step() {
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()
+ s.get(id)
+ .unwrap()
+ .repair_log()
+ .iter()
+ .filter(|l| l.contains("imported 1 comment without the appearance") && l.contains("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();
diff --git a/crates/xfdf/README.md b/crates/xfdf/README.md
index 4417a15b70..8e1604d6bc 100644
--- a/crates/xfdf/README.md
+++ b/crates/xfdf/README.md
@@ -30,7 +30,10 @@ let report = import(&mut doc, &bytes)?; // XFDF, FDF, XML, CSV o
PdfCraft always draws) and otherwise draws the comment and records the reason in
`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.
+ rect has no area, and stops at the 64 MiB an import reads; where `/AS` selects one of several
+ states, the selected stream is written (and import, with no `/AS` in XFDF, refuses a normal
+ appearance of several states). Import and FDF import are all or nothing: on an error the
+ document is left as it was.
- **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
diff --git a/crates/xfdf/src/appearance.rs b/crates/xfdf/src/appearance.rs
index 55acba0e8f..17d4ee790f 100644
--- a/crates/xfdf/src/appearance.rs
+++ b/crates/xfdf/src/appearance.rs
@@ -37,7 +37,11 @@ const ASCII_DATA_LIMIT: usize = 4096;
/// 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 {
+///
+/// `budget` is what the export may still write (decoded XML), shared by all of its comments, as
+/// import reads at most [`MAX_IMPORT_APPEARANCE_BYTES`]: an appearance past it is left out rather
+/// than written for import to skip. It is charged for what is written.
+pub(crate) fn encode(doc: &Document, d: &Dict, budget: &mut usize) -> Option {
if !has_appearance(doc, d) {
return None;
}
@@ -55,13 +59,15 @@ pub(crate) fn encode(doc: &Document, d: &Dict) -> Option {
}
}
}
- let mut writer = Writer { doc, out: String::new(), values: MAX_VALUES, path: Vec::new() };
+ let limit = MAX_APPEARANCE_BYTES.min(*budget);
+ let mut writer = Writer { doc, out: String::new(), values: MAX_VALUES, path: Vec::new(), limit };
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 {
+ if writer.out.len() > limit {
return None;
}
+ *budget = budget.saturating_sub(writer.out.len());
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);
@@ -80,6 +86,8 @@ struct Writer<'a> {
values: usize,
/// The references being written, so a reference back to one of them ends the cycle.
path: Vec,
+ /// At most this much XML.
+ limit: usize,
}
impl Writer<'_> {
@@ -88,7 +96,7 @@ impl Writer<'_> {
}
fn spend(&mut self, depth: usize) -> Option<()> {
- if depth > MAX_NESTING || self.out.len() > MAX_APPEARANCE_BYTES {
+ if depth > MAX_NESTING || self.out.len() > self.limit {
return None;
}
self.values = self.values.checked_sub(1)?;
@@ -151,7 +159,7 @@ impl Writer<'_> {
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.
- if s.raw.len().saturating_mul(2).saturating_add(self.out.len()) > MAX_APPEARANCE_BYTES {
+ if s.raw.len().saturating_mul(2).saturating_add(self.out.len()) > self.limit {
return None;
}
self.out.push_str(&format!("\n"));
@@ -218,7 +226,8 @@ pub(crate) enum Node {
/// Read the text of an `` element: base 64, then the XML it holds. `budget` is what
/// the import may still read (decoded bytes) and is charged. The result is the `/AP` dictionary,
-/// with a normal appearance (`/N`) that is a stream or a dictionary of streams.
+/// with a normal appearance (`/N`) that is one stream: XFDF has no appearance state (`/AS`), so a
+/// dictionary of states would leave nothing selected to draw (ISO 32000-2 §12.5.5).
pub(crate) fn decode(text: &str, budget: &mut usize) -> Result {
let limit = MAX_APPEARANCE_BYTES.min(*budget);
// Base 64 is 4 characters for 3 bytes: refuse a text that decodes past the limit before
@@ -250,14 +259,7 @@ pub(crate) fn decode(text: &str, budget: &mut usize) -> Result {
let mut values = MAX_VALUES;
let node = read(root, 0, &mut values)?;
let usable = match &node {
- Node::Dict(entries) => entries.iter().any(|(k, v)| {
- k.as_slice() == b"N"
- && match v {
- Node::Stream(..) => true,
- Node::Dict(states) => !states.is_empty() && states.iter().all(|(_, s)| matches!(s, Node::Stream(..))),
- _ => false,
- }
- }),
+ Node::Dict(entries) => entries.iter().any(|(k, v)| k.as_slice() == b"N" && matches!(v, Node::Stream(..))),
_ => false,
};
if !usable {
diff --git a/crates/xfdf/src/fdf.rs b/crates/xfdf/src/fdf.rs
index 22c2f197b8..dffd3f9098 100644
--- a/crates/xfdf/src/fdf.rs
+++ b/crates/xfdf/src/fdf.rs
@@ -413,6 +413,10 @@ fn deep(
copy?
}
Object::Array(a) => Object::Array(a.iter().map(|x| deep(src, dst, x, depth + 1, path, budget)).collect::>()?),
+ // A page or page tree the comment reaches is the file's, not the comment's: copied in, it
+ // would bring its contents along (as export, which nulls them; `resolve` bounds the hops
+ // of an indirect /Type).
+ Object::Dict(d) if d.get(b"Type").is_some_and(|t| matches!(src.resolve(t).as_name(), Some(b"Page" | b"Pages"))) => Object::Null,
Object::Dict(d) => {
Object::Dict(d.iter().map(|(k, v)| Ok((k.clone(), deep(src, dst, v, depth + 1, path, budget)?))).collect::>()?)
}
diff --git a/crates/xfdf/src/lib.rs b/crates/xfdf/src/lib.rs
index f29789ac0b..984d1cdbe5 100644
--- a/crates/xfdf/src/lib.rs
+++ b/crates/xfdf/src/lib.rs
@@ -338,8 +338,15 @@ fn tab_delimited(t: &str) -> Result)>, DataError> {
}
/// Import comments and/or field values from XFDF, FDF, XML, CSV or tab-delimited text
-/// (detected from the content).
+/// (detected from the content). All or nothing: on an error `doc` is left as it was.
pub fn import(doc: &mut Document, bytes: &[u8]) -> Result {
+ let mut work = doc.clone();
+ let report = import_into(&mut work, bytes)?;
+ *doc = work;
+ Ok(report)
+}
+
+fn import_into(doc: &mut Document, bytes: &[u8]) -> Result {
let report = if bytes.starts_with(b"%FDF") {
import_fdf(doc, bytes)?
} else {
diff --git a/crates/xfdf/src/tests_format.rs b/crates/xfdf/src/tests_format.rs
index a48122df34..510e3fb40d 100644
--- a/crates/xfdf/src/tests_format.rs
+++ b/crates/xfdf/src/tests_format.rs
@@ -535,6 +535,10 @@ fn unreadable_appearances_are_skipped_recorded_and_drawn_again() {
("a bad number", b64(r#"q Q"#)),
("a DTD", b64(r#"]>&a;"#)),
("too deep", b64(&deep)),
+ (
+ "states with none selected",
+ b64(r#"q Q"#),
+ ),
] {
let mut dst = pages(1);
let report = import(&mut dst, appearance_xfdf(&inner, "square").as_bytes()).unwrap_or_else(|e| panic!("{why}: {e}"));
@@ -817,7 +821,7 @@ 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) {
+ if let Some(text) = appearance::encode(&src, &d, &mut appearance::MAX_IMPORT_APPEARANCE_BYTES.clone()) {
let mut budget = appearance::MAX_IMPORT_APPEARANCE_BYTES;
appearance::decode(&text, &mut budget).expect("what export writes, import reads");
}
@@ -891,6 +895,7 @@ fn the_selected_appearance_state_is_what_xfdf_carries() {
fn a_chain_of_forms_comes_back_whole_from_fdf_or_is_refused() {
let chain = |forms: usize| {
let mut src = pages(1);
+ put(&mut src, annot("Text", "first", [10.0, 10.0, 30.0, 30.0]));
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();
@@ -910,18 +915,23 @@ fn a_chain_of_forms_comes_back_whole_from_fdf_or_is_refused() {
export_fdf(&src, true, false, "src.pdf").unwrap()
};
let mut dst = pages(1);
- import(&mut dst, &chain(20)).unwrap();
+ import(&mut dst, &chain(60)).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 {
+ for level in 0..60 {
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();
+ let mut dst = pages(1);
+ let before = dst.add(Object::Null);
+ let err = import(&mut dst, &chain(80)).unwrap_err();
assert!(err.to_string().contains("nest more than"), "{err}");
+ // All or nothing: the note before the stamp, and the forms copied so far, are not left behind.
+ assert_eq!(dst.add(Object::Null).num, before.num + 1);
+ assert!(xfdf::find_by_name(&dst, "first").is_none());
}
/// A `/P` an FDF comment holds points into the FDF; it is replaced, and nothing it reaches is
@@ -1005,7 +1015,9 @@ fn an_imported_redaction_mark_is_drawn_even_when_cloudy_or_undrawable() {
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}");
+ let drawn = n.decoded().unwrap();
+ assert_ne!(drawn, b"%nothing", "{extra}");
+ assert!(String::from_utf8_lossy(&drawn).contains("h S"), "{extra}: the outline is stroked: {}", String::from_utf8_lossy(&drawn));
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 {
@@ -1048,3 +1060,69 @@ fn a_long_dash_array_comes_back() {
let (_, dst) = round_trip_xfdf(&src);
assert_eq!(f64s(&dst, sub_dict(&dst, &named(&dst, "sq"), b"BS").get(b"D")), dashes);
}
+
+/// Export stops writing appearances where import would stop reading them, so every appearance
+/// written comes back.
+#[test]
+fn export_writes_no_more_appearance_than_import_reads() {
+ let mut src = pages(1);
+ content_stamp(&mut src, "a", b"0 0 m 1 1 l S".to_vec());
+ let d = named(&src, "a");
+ let mut probe = usize::MAX;
+ let one = appearance::encode(&src, &d, &mut probe).unwrap();
+ let size = usize::MAX - probe;
+ let mut budget = size * 3 / 2;
+ assert_eq!(appearance::encode(&src, &d, &mut budget), Some(one));
+ assert_eq!(appearance::encode(&src, &d, &mut budget), None, "past what import reads");
+}
+
+/// A page an FDF stamp's appearance reaches (its /Type a reference here) is not copied in with
+/// its contents.
+#[test]
+fn an_fdf_appearance_that_reaches_a_page_copies_no_page_in() {
+ let fdf = b"%FDF-1.2\n1 0 obj\n<< /FDF << /Annots [2 0 R] >> >>\nendobj\n\
+2 0 obj\n<< /Type /Annot /Subtype /Stamp /Rect [10 10 60 60] /NM (s) /Page 0 /AP << /N 3 0 R >> >>\nendobj\n\
+3 0 obj\n<< /Subtype /Form /BBox [0 0 1 1] /PieceInfo 4 0 R /Length 3 >>\nstream\nq Q\nendstream\nendobj\n\
+4 0 obj\n<< /Type 6 0 R /Contents 5 0 R >>\nendobj\n\
+5 0 obj\n<< /Length 11 >>\nstream\npagecontent\nendstream\nendobj\n\
+6 0 obj\n/Page\nendobj\ntrailer\n<< /Root 1 0 R >>\n%%EOF\n";
+ let mut dst = pages(1);
+ let before = dst.add(Object::Null);
+ import(&mut dst, fdf).unwrap();
+ 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"pagecontent", "the page's contents were copied in");
+ }
+ }
+ let n = dst.resolve(sub_dict(&dst, &named(&dst, "s"), b"AP").get(b"N").unwrap());
+ let Object::Stream(n) = n.as_ref() else { panic!("no appearance") };
+ assert!(matches!(n.dict.get(b"PieceInfo"), None | Some(Object::Null)), "{:?}", n.dict.get(b"PieceInfo"));
+}
+
+/// Rich text keeps the default (XHTML) namespace it inherits from ``, and a
+/// prefix it redeclares with spaces around `=` is not declared twice.
+#[test]
+fn rich_text_keeps_an_inherited_default_namespace() {
+ let xml = r#"
+hi
hi
+hi
hi
+"#;
+ let mut dst = pages(1);
+ import(&mut dst, xml.as_bytes()).unwrap();
+ let rc = text(&dst, &named(&dst, "d"), b"RC").unwrap();
+ let parsed = common::parse_form_xml(&rc).unwrap();
+ assert_eq!(parsed.root_element().tag_name().namespace(), Some("http://www.w3.org/1999/xhtml"), "{rc}");
+ let rc = text(&dst, &named(&dst, "w"), b"RC").unwrap();
+ assert!(common::parse_form_xml(&rc).is_ok(), "{rc}");
+ assert!(!rc.contains(r#"xmlns:a="urn:a""#), "{rc}");
+}
+
+/// A redaction mark whose /Rect has no area shows its quads all the same: the /Rect grows to hold
+/// them.
+#[test]
+fn a_redaction_marks_rect_holds_its_quads() {
+ let xml = r#""#;
+ let mut dst = pages(1);
+ import(&mut dst, xml.as_bytes()).unwrap();
+ assert_eq!(f64s(&dst, named(&dst, "r").get(b"Rect")), vec![10.0, 20.0, 90.0, 50.0]);
+}
diff --git a/crates/xfdf/src/xfdf.rs b/crates/xfdf/src/xfdf.rs
index d1dc2f10e0..1f8418e09c 100644
--- a/crates/xfdf/src/xfdf.rs
+++ b/crates/xfdf/src/xfdf.rs
@@ -357,7 +357,7 @@ 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) {
+fn xfdf_annot_children(doc: &Document, out: &mut String, d: &Dict, pi: usize, ap: Option<&mut usize>) {
let rc = text_of(doc, d, b"RC");
match text_of(doc, d, b"Contents") {
Some(c) => {
@@ -386,7 +386,9 @@ fn xfdf_annot_children(doc: &Document, out: &mut String, d: &Dict, pi: usize, ap
if let Some(ds) = text_of(doc, d, b"DS") {
let _ = write!(out, "{}", esc(&ds));
}
- if ap && let Some(b64) = super::appearance::encode(doc, d) {
+ if let Some(budget) = ap
+ && let Some(b64) = super::appearance::encode(doc, d, budget)
+ {
let _ = write!(out, "{b64}");
}
let ink = d.get(b"InkList").map(|o| doc.resolve(o)).and_then(|o| o.as_array().cloned()).unwrap_or_default();
@@ -428,6 +430,8 @@ fn xfdf_annots(doc: &Document, out: &mut String, only: Option<&HashSet<(usize, u
// NM of every annotation, for inreplyto. A selected annotation without one (other tools
// leave /NM out) gets a name for this export, so a reply copied with it still finds it.
let names = annotation_names(doc, &pages, only);
+ // What the import reads of appearances, over all of the comments.
+ let mut appearance_budget = super::appearance::MAX_IMPORT_APPEARANCE_BYTES;
for (pi, p) in pages.iter().enumerate() {
for (ai, a) in p.dict.get(b"Annots").map(|a| doc.resolve(a)).and_then(|a| a.as_array().cloned()).unwrap_or_default().into_iter().enumerate() {
if only.is_some_and(|only| !only.contains(&(pi, ai))) {
@@ -516,7 +520,7 @@ fn xfdf_annots(doc: &Document, out: &mut String, only: Option<&HashSet<(usize, u
// XFDF 3.0 carries a stamp's appearance, and none for a rect with no area; a copy and
// paste (`own`) carries every comment's, so the copy looks the same.
let has_area = rect.len() == 4 && rect[0] != rect[2] && rect[1] != rect[3];
- xfdf_annot_children(doc, out, &d, pi, has_area && (own || sub == b"Stamp"));
+ xfdf_annot_children(doc, out, &d, pi, (has_area && (own || sub == b"Stamp")).then_some(&mut appearance_budget));
let _ = writeln!(out, "{tag}>");
}
}
@@ -980,7 +984,10 @@ 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() };
+ let markup = match elements.as_slice() {
+ [only] => standalone(rich, *only, markup),
+ _ => 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 {
@@ -988,31 +995,52 @@ 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();
- }
+/// Whether start tag `tag` declares attribute `name` (white space may come before the `=`).
+fn declares(tag: &str, name: &str) -> bool {
+ tag.match_indices(name).any(|(i, _)| {
+ let before = tag.get(..i).and_then(|b| b.chars().next_back());
+ let after = tag.get(i + name.len()..).map(str::trim_start);
+ before.is_some_and(char::is_whitespace) && after.is_some_and(|a| a.starts_with('='))
+ })
+}
+
+/// The one element `markup` (`element`, cut out of `rich`, a ``) as an XML
+/// document of its own: the namespaces it inherits from the XFDF around it are declared on it,
+/// the default one (XHTML declared on ``, say; not XFDF's own, which a
+/// standalone document has no use for) and the prefixes it uses. Unchanged when nothing is
+/// inherited, or when the declarations would not make it parse.
+fn standalone(rich: roxmltree::Node, element: roxmltree::Node, markup: &str) -> 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();
+ if let Some(uri) = element.tag_name().namespace()
+ && !markup.get(1..name_end).is_some_and(|n| n.contains(':'))
+ && uri != XFDF_NS
+ && !declares(start_tag, "xmlns")
+ {
+ let _ = write!(declared, " xmlns=\"{}\"", esc(uri));
+ }
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}=")) {
+ if prefix == "xml" || !markup.contains(&format!("{prefix}:")) || declares(start_tag, &format!("xmlns:{prefix}")) {
continue;
}
let _ = write!(declared, " xmlns:{prefix}=\"{}\"", esc(ns.uri()));
}
+ if declared.is_empty() {
+ return markup.to_string();
+ }
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() }
}
+/// XFDF's own namespace (ISO 19444-1).
+const XFDF_NS: &str = "http://ns.adobe.com/xfdf/";
+
/// 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
@@ -1066,6 +1094,24 @@ pub(crate) fn place(doc: &mut Document, page_ref: ObjRef, mut d: Dict) -> Result
}
}
}
+ // The outline is drawn inside /Rect: it is grown to hold every quad, so the whole area
+ // the mark removes shows (a /Rect with no area would show none of it).
+ let quads: Vec =
+ d.get(b"QuadPoints").and_then(Object::as_array).map(|q| q.iter().filter_map(Object::as_f64).collect()).unwrap_or_default();
+ if !quads.is_empty() {
+ let rect = rect_strict(doc, d.get(b"Rect"));
+ let start = rect.map(|[x0, y0, x1, y1]| [x0.min(x1), y0.min(y1), x0.max(x1), y0.max(y1)]);
+ let bounds = quads
+ .as_chunks::<2>()
+ .0
+ .iter()
+ .fold(start, |b, [x, y]| Some(b.map_or([*x, *y, *x, *y], |[l, bo, r, t]| [l.min(*x), bo.min(*y), r.max(*x), t.max(*y)])));
+ if let Some(b) = bounds
+ && Some(b) != rect
+ {
+ d.set(b"Rect".to_vec(), arr(&b));
+ }
+ }
}
// 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
@@ -1163,7 +1209,18 @@ 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())), super::appearance::MAX_IMPORT_APPEARANCE_BYTES)
+ // Into a copy, kept only when the whole import succeeds.
+ let mut work = doc.clone();
+ let report = import_xfdf_with(
+ &mut work,
+ text,
+ page,
+ fresh,
+ offset.filter(|o| o.iter().all(|v| v.is_finite())),
+ super::appearance::MAX_IMPORT_APPEARANCE_BYTES,
+ )?;
+ *doc = work;
+ Ok(report)
}
pub(crate) fn import_xfdf(doc: &mut Document, text: &str) -> Result {
diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml
index 9e72710c60..d933f02742 100644
--- a/parity/acrobat-features.toml
+++ b/parity/acrobat-features.toml
@@ -4335,7 +4335,7 @@ 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", "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."
+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) (or one whose normal appearance is several states, which XFDF cannot select) 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 (an import is all or nothing); a page the file's appearance reaches is not copied in. 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"
From 79b7a79d97b20e69774d6f351212c7f51a980b08 Mon Sep 17 00:00:00 2001
From: mtlcl410
Date: Mon, 9 Nov 2026 02:59:47 +0800
Subject: [PATCH 6/6] M5: comment imports leave field values alone and paste
keeps the appearance budget for what import reads (comment.import-fdf-xfdf,
comment.export-fdf-xfdf)
- Appearance export checks a stream's size in the encoding it is written in
(printable data as text, not as hexadecimal).
- Copy no longer writes a redaction mark's appearance, which import never
reads, so it does not use up the budget the other comments share.
- Rich text: a namespace declaration inside another attribute's value is not
a declaration, a '>' in a value does not end the start tag, and a prefixed
root keeps the default namespace its unprefixed descendants inherit.
- import_comments never applies the file's field values, whatever the
placement arguments (a non-finite offset used to fall through to them).
- The repair-log line for skipped appearances escapes control characters and
cuts each description to 200 characters.
- Tests for the shared budget through the export loop, a near-limit appearance
that round-trips, and the six-comment summary.
Co-Authored-By: Claude Opus 5.5
Claude-Session: https://claude.ai/code/session_01Pq7HyCHD1R3rywQBPCHXZA
---
crates/engine/src/edit/run.rs | 27 +++++++++-
crates/engine/src/tests.rs | 25 +++++++++
crates/xfdf/README.md | 4 +-
crates/xfdf/src/appearance.rs | 17 ++++---
crates/xfdf/src/tests.rs | 13 +++++
crates/xfdf/src/tests_format.rs | 90 ++++++++++++++++++++++++++++++++-
crates/xfdf/src/xfdf.rs | 72 +++++++++++++++++++-------
7 files changed, 219 insertions(+), 29 deletions(-)
diff --git a/crates/engine/src/edit/run.rs b/crates/engine/src/edit/run.rs
index 443358bd52..ebe4f938b3 100644
--- a/crates/engine/src/edit/run.rs
+++ b/crates/engine/src/edit/run.rs
@@ -164,12 +164,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),
-/// in the repair log as one line: said, not slipped in.
+/// in the repair log as one line: said, not slipped in. The first five are named, each in at
+/// most [`SKIPPED_CHARS`] characters with control characters escaped (a name from the file
+/// holding a line break would otherwise read as a repair of its own).
fn note_skipped(doc: &mut pdfcraft_cos::Document, report: &pdfcraft_xfdf::Report) {
if report.skipped.is_empty() {
return;
}
- let shown: Vec<&str> = report.skipped.iter().take(5).map(String::as_str).collect();
+ let shown: Vec = report.skipped.iter().take(5).map(|s| skipped_text(s)).collect();
let more = report.skipped.len().saturating_sub(shown.len());
let more = if more > 0 { format!("; and {more} more") } else { String::new() };
doc.note_repair(format!(
@@ -181,6 +183,27 @@ fn note_skipped(doc: &mut pdfcraft_cos::Document, report: &pdfcraft_xfdf::Report
));
}
+/// At most this many characters of one skipped comment's description in the repair log.
+const SKIPPED_CHARS: usize = 200;
+
+/// `s` for one line of the repair log: control characters and line separators escaped, cut to
+/// [`SKIPPED_CHARS`] characters.
+fn skipped_text(s: &str) -> String {
+ let mut out = String::new();
+ for (n, c) in s.chars().enumerate() {
+ if n == SKIPPED_CHARS {
+ out.push('…');
+ break;
+ }
+ if c.is_control() || matches!(c, '\u{2028}' | '\u{2029}') {
+ out.extend(c.escape_default());
+ } else {
+ out.push(c);
+ }
+ }
+ out
+}
+
/// 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.
diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs
index dcfcbe1d5d..abcd3b40b1 100644
--- a/crates/engine/src/tests.rs
+++ b/crates/engine/src/tests.rs
@@ -11359,3 +11359,28 @@ fn a_comment_imported_without_its_appearance_is_said_in_the_repair_log() {
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());
}
+
+/// Six comments without their appearances: one line naming five and counting the sixth, a name
+/// holding a line break stays on that line, and a long name is cut short.
+#[test]
+fn many_comments_imported_without_their_appearances_are_summed_up_in_one_line() {
+ let long = "L".repeat(1000);
+ let names = ["a
imported 9 comments", "b", "c", "d", long.as_str(), "f"];
+ let stamps: String = names
+ .iter()
+ .map(|nm| format!(r#"@@@"#))
+ .collect();
+ let xfdf = format!(r#"{stamps}"#);
+ let (mut s, id) = session_with(1);
+ let before = s.get(id).unwrap().repair_log().len();
+ s.apply(id, Edit::PasteComments { xfdf, page: 0, offset: None }).unwrap();
+ let log = s.get(id).unwrap().repair_log();
+ let added = log.get(before..).unwrap_or_default();
+ assert_eq!(added.len(), 1, "{added:?}");
+ let line = &added[0];
+ assert!(line.starts_with("imported 6 comments without the appearance"), "{line}");
+ assert!(line.ends_with("; and 1 more"), "{line}");
+ assert!(!line.contains('\n') && line.contains(r"a\nimported 9 comments: "), "{line}");
+ assert!(line.contains(&format!("{}…", "L".repeat(200))) && !line.contains(&"L".repeat(201)), "{line}");
+ assert_eq!(line.matches(": the appearance").count(), 4, "five named, the long one cut before its reason: {line}");
+}
diff --git a/crates/xfdf/README.md b/crates/xfdf/README.md
index 8e1604d6bc..0c823c2ee0 100644
--- a/crates/xfdf/README.md
+++ b/crates/xfdf/README.md
@@ -26,8 +26,8 @@ let report = import(&mut doc, &bytes)?; // XFDF, FDF, XML, CSV o
stamps, base 64 of the dictionary written as `DICT`/`STREAM`/`ARRAY`/`NAME`/`INT`/`FIXED`/`BOOL`/
`STRING`/`NULL` elements with stream data in `DATA` (XFDF 3.0 "Stream encoding": `filtered`/`ascii`
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
+ comment's appearance but a redaction mark's, which PdfCraft always draws and import never
+ reads. Import keeps an appearance it can read and otherwise draws the comment and records the reason in
`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, and stops at the 64 MiB an import reads; where `/AS` selects one of several
diff --git a/crates/xfdf/src/appearance.rs b/crates/xfdf/src/appearance.rs
index 17d4ee790f..3690a19f42 100644
--- a/crates/xfdf/src/appearance.rs
+++ b/crates/xfdf/src/appearance.rs
@@ -158,8 +158,15 @@ impl Writer<'_> {
}
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.
- if s.raw.len().saturating_mul(2).saturating_add(self.out.len()) > self.limit {
+ // 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');
+ let ascii = s.raw.len() < ASCII_DATA_LIMIT && printable && !s.dict.contains(b"Filter");
+ // The data, in the encoding it is written in, must fit what is left of the budget
+ // (as text it is at least its own length; the finished XML is checked again).
+ let size = if ascii { s.raw.len() } else { s.raw.len().saturating_mul(2) };
+ if size.saturating_add(self.out.len()) > self.limit {
return None;
}
self.out.push_str(&format!("\n"));
@@ -167,11 +174,7 @@ impl Writer<'_> {
for (dk, dv) in s.dict.iter().filter(|(dk, _)| dk.as_slice() != b"Length") {
self.value(Some(dk), dv, depth + 1)?;
}
- // 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") {
+ if ascii {
// Printable ASCII is valid UTF-8.
let text = String::from_utf8_lossy(&s.raw);
self.out.push_str(&format!("{}\n", esc_data(&text)));
diff --git a/crates/xfdf/src/tests.rs b/crates/xfdf/src/tests.rs
index f645d7bbb6..53faf661ba 100644
--- a/crates/xfdf/src/tests.rs
+++ b/crates/xfdf/src/tests.rs
@@ -2157,6 +2157,19 @@ fn importing_comments_as_copies_adds_them_on_the_page_with_fresh_names() {
assert_eq!(summaries(&other).len(), 4, "the named import replaced its own earlier copy");
}
+/// Comment import never takes the file's field values, whatever the placement (a non-finite
+/// offset, which is ignored, and no page or prefix made it the full import).
+#[test]
+fn comment_import_leaves_field_values_alone() {
+ let xfdf = r#"from the file"#;
+ for offset in [Some([f64::NAN, 0.0]), None] {
+ let mut doc = one_field_named("Name", "mine");
+ let r = import_comments(&mut doc, xfdf, None, None, offset).unwrap();
+ assert_eq!((r.comments, r.fields), (1, 0), "{offset:?}: {r:?}");
+ assert_eq!(values(&doc), [("Name".to_string(), vec!["mine".to_string()])], "{offset:?}");
+ }
+}
+
/// Other tools write comments straight into the page's `/Annots` array, without an object of
/// their own: they copy like any other, and two source comments sharing a name give two copies.
#[test]
diff --git a/crates/xfdf/src/tests_format.rs b/crates/xfdf/src/tests_format.rs
index 510e3fb40d..35a9735ede 100644
--- a/crates/xfdf/src/tests_format.rs
+++ b/crates/xfdf/src/tests_format.rs
@@ -825,6 +825,14 @@ fn an_exported_appearance_always_fits_what_import_reads() {
let mut budget = appearance::MAX_IMPORT_APPEARANCE_BYTES;
appearance::decode(&text, &mut budget).expect("what export writes, import reads");
}
+ // Just under the limit once the line breaks are counted (one per 80 hexadecimal digits): it
+ // is written, and read back.
+ let mut src = pages(1);
+ content_stamp(&mut src, "near", vec![0u8; appearance::MAX_APPEARANCE_BYTES / 2 * 80 / 81 - 4096]);
+ let d = named(&src, "near");
+ let text = appearance::encode(&src, &d, &mut appearance::MAX_IMPORT_APPEARANCE_BYTES.clone()).expect("under the limit");
+ 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
@@ -1039,7 +1047,7 @@ fn the_import_budget_is_shared_by_every_comment() {
.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();
+ let report = xfdf::import_xfdf_with(&mut dst, &xml, None, None, None, size * 5 / 2, false).unwrap();
assert_eq!(report.comments, 3);
assert_eq!(report.skipped.len(), 1, "{:?}", report.skipped);
assert!(report.skipped[0].starts_with("c: "), "{:?}", report.skipped);
@@ -1076,6 +1084,62 @@ fn export_writes_no_more_appearance_than_import_reads() {
assert_eq!(appearance::encode(&src, &d, &mut budget), None, "past what import reads");
}
+/// The export loop shares one budget over all of its comments: of five stamps whose appearances
+/// together are past what import reads, the fifth goes out without one, and import reads every
+/// appearance that was written.
+#[test]
+fn the_export_loop_writes_no_more_appearance_than_import_reads() {
+ let mut src = pages(1);
+ // About 14 MiB of XML each (hexadecimal): four fit in 64 MiB, five do not.
+ let form = content_stamp(&mut src, "s1", vec![0u8; 7 << 20]);
+ for nm in ["s2", "s3", "s4", "s5"] {
+ let mut ap = Dict::new();
+ ap.set(b"N".to_vec(), Object::Ref(form));
+ let mut d = annot("Stamp", nm, [100.0, 500.0, 200.0, 550.0]);
+ d.set(b"AP".to_vec(), Object::Dict(ap));
+ put(&mut src, d);
+ }
+ let xml = export_xfdf(&src, true, false, "src.pdf");
+ assert_eq!(xml.matches("").count(), 4);
+ let mut dst = pages(1);
+ let report = xfdf::import_xfdf(&mut dst, &xml).unwrap();
+ assert_eq!(report.comments, 5);
+ assert!(report.skipped.is_empty(), "{:?}", report.skipped);
+}
+
+/// Printable data is written as it is, so a budget it fits as text takes it (the size check
+/// counted it as hexadecimal, twice as long, and left the appearance out).
+#[test]
+fn printable_appearance_data_that_fits_the_budget_as_text_is_written() {
+ let mut src = pages(1);
+ // Printable as XFDF 3.0 counts it: no spaces.
+ content_stamp(&mut src, "a", "q\nQ\n".repeat(500).into_bytes());
+ let d = named(&src, "a");
+ let mut probe = usize::MAX;
+ let one = appearance::encode(&src, &d, &mut probe).unwrap();
+ let mut budget = usize::MAX - probe;
+ assert!(budget < 3000, "written as text, not hexadecimal: {budget}");
+ assert_eq!(appearance::encode(&src, &d, &mut budget), Some(one), "fits exactly");
+ assert_eq!(budget, 0);
+}
+
+/// A copied redaction mark goes without its appearance (import draws the mark itself and never
+/// reads one), so it leaves the shared budget to the comments whose appearances are read.
+#[test]
+fn a_copied_redaction_mark_spends_no_appearance_budget() {
+ let mut src = pages(1);
+ let form = content_stamp(&mut src, "st", b"0 0 m 1 1 l S".to_vec());
+ let mut ap = Dict::new();
+ ap.set(b"N".to_vec(), Object::Ref(form));
+ let mut rd = annot("Redact", "rd", [300.0, 10.0, 400.0, 30.0]);
+ rd.set(b"AP".to_vec(), Object::Dict(ap));
+ put(&mut src, rd);
+ let copy = export_xfdf_comments(&src, &[(0, 0), (0, 1)], "src.pdf");
+ assert_eq!(copy.matches("").count(), 1, "{copy}");
+ let redact = copy.split("").next().unwrap();
+ assert!(!redact.contains(""), "{copy}");
+}
+
/// A page an FDF stamp's appearance reaches (its /Type a reference here) is not copied in with
/// its contents.
#[test]
@@ -1117,6 +1181,30 @@ fn rich_text_keeps_an_inherited_default_namespace() {
assert!(!rc.contains(r#"xmlns:a="urn:a""#), "{rc}");
}
+/// A declaration written inside another attribute's value declares nothing, so the prefix the
+/// rich text inherits is still declared on it; and a prefixed root keeps the default (XHTML)
+/// namespace its unprefixed descendants inherit. A `>` in an attribute value is not the end of
+/// the start tag.
+#[test]
+fn rich_text_namespaces_come_from_attributes_and_the_scope() {
+ let xml = r#"
+hi
hi
+hi
hi
+hi
hi
+"#;
+ let mut dst = pages(1);
+ import(&mut dst, xml.as_bytes()).unwrap();
+ let rc = text(&dst, &named(&dst, "v"), b"RC").unwrap();
+ let parsed = common::parse_form_xml(&rc).unwrap_or_else(|e| panic!("{e}: {rc}"));
+ assert!(parsed.root_element().attributes().any(|a| a.namespace() == Some("urn:b")), "{rc}");
+ let rc = text(&dst, &named(&dst, "g"), b"RC").unwrap();
+ assert!(common::parse_form_xml(&rc).is_ok(), "a `>` in a value does not end the start tag: {rc}");
+ let rc = text(&dst, &named(&dst, "p"), b"RC").unwrap();
+ let parsed = common::parse_form_xml(&rc).unwrap();
+ let p = parsed.descendants().find(|n| n.has_tag_name("p")).unwrap();
+ assert_eq!(p.tag_name().namespace(), Some("http://www.w3.org/1999/xhtml"), "{rc}");
+}
+
/// A redaction mark whose /Rect has no area shows its quads all the same: the /Rect grows to hold
/// them.
#[test]
diff --git a/crates/xfdf/src/xfdf.rs b/crates/xfdf/src/xfdf.rs
index 1f8418e09c..3ff8853470 100644
--- a/crates/xfdf/src/xfdf.rs
+++ b/crates/xfdf/src/xfdf.rs
@@ -518,9 +518,12 @@ fn xfdf_annots(doc: &Document, out: &mut String, only: Option<&HashSet<(usize, u
}
let _ = write!(out, "<{tag}{attrs}>");
// XFDF 3.0 carries a stamp's appearance, and none for a rect with no area; a copy and
- // paste (`own`) carries every comment's, so the copy looks the same.
+ // paste (`own`) carries every comment's, so the copy looks the same, except a
+ // redaction mark's, which import never reads (PdfCraft draws the mark) and which would
+ // only use up the budget the others share.
let has_area = rect.len() == 4 && rect[0] != rect[2] && rect[1] != rect[3];
- xfdf_annot_children(doc, out, &d, pi, (has_area && (own || sub == b"Stamp")).then_some(&mut appearance_budget));
+ let carried = has_area && sub != b"Redact" && (own || sub == b"Stamp");
+ xfdf_annot_children(doc, out, &d, pi, carried.then_some(&mut appearance_budget));
let _ = writeln!(out, "{tag}>");
}
}
@@ -985,7 +988,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 = match elements.as_slice() {
- [only] => standalone(rich, *only, markup),
+ [_] => standalone(rich, markup),
_ => markup.to_string(),
};
// Rich text is an XML document of its own; export left its declaration out.
@@ -995,29 +998,61 @@ fn read_rich_text(rich: roxmltree::Node, has_contents: bool, d: &mut Dict) {
}
}
-/// Whether start tag `tag` declares attribute `name` (white space may come before the `=`).
+/// Whether start tag `tag` (from `<` up to its closing `>`) declares attribute `name` (white
+/// space may come before the `=`). Attribute values are skipped: `title=" xmlns:b='x'"` declares
+/// nothing.
fn declares(tag: &str, name: &str) -> bool {
- tag.match_indices(name).any(|(i, _)| {
- let before = tag.get(..i).and_then(|b| b.chars().next_back());
- let after = tag.get(i + name.len()..).map(str::trim_start);
- before.is_some_and(char::is_whitespace) && after.is_some_and(|a| a.starts_with('='))
- })
+ let mut quote = None;
+ let mut after_space = false;
+ for (i, c) in tag.char_indices() {
+ if let Some(q) = quote {
+ if c == q {
+ quote = None;
+ }
+ continue;
+ }
+ if c == '"' || c == '\'' {
+ quote = Some(c);
+ } else if after_space && tag.get(i..).and_then(|rest| rest.strip_prefix(name)).is_some_and(|a| a.trim_start().starts_with('=')) {
+ return true;
+ }
+ after_space = c.is_whitespace();
+ }
+ false
}
-/// The one element `markup` (`element`, cut out of `rich`, a ``) as an XML
+/// Where the start tag at the beginning of `markup` ends: its first `>` outside an attribute
+/// value (the end of `markup` when there is none).
+fn start_tag_end(markup: &str) -> usize {
+ let mut quote = None;
+ for (i, c) in markup.char_indices() {
+ match quote {
+ Some(q) if c == q => quote = None,
+ Some(_) => {}
+ None if c == '"' || c == '\'' => quote = Some(c),
+ None if c == '>' => return i,
+ None => {}
+ }
+ }
+ markup.len()
+}
+
+/// The one element `markup` (cut out of `rich`, a ``) as an XML
/// document of its own: the namespaces it inherits from the XFDF around it are declared on it,
/// the default one (XHTML declared on ``, say; not XFDF's own, which a
/// standalone document has no use for) and the prefixes it uses. Unchanged when nothing is
/// inherited, or when the declarations would not make it parse.
-fn standalone(rich: roxmltree::Node, element: roxmltree::Node, markup: &str) -> String {
+fn standalone(rich: roxmltree::Node, markup: &str) -> 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 start_tag = markup.get(..start_tag_end(markup)).unwrap_or(markup);
let mut declared = String::new();
- if let Some(uri) = element.tag_name().namespace()
- && !markup.get(1..name_end).is_some_and(|n| n.contains(':'))
+ // The default namespace in scope where the element sits: its own when it has no prefix, and
+ // its unprefixed descendants' when it has one.
+ if let Some(uri) = rich.namespaces().find(|ns| ns.name().is_none()).map(|ns| ns.uri())
+ && !uri.is_empty()
&& uri != XFDF_NS
&& !declares(start_tag, "xmlns")
{
@@ -1218,16 +1253,18 @@ pub fn import_comments(
fresh,
offset.filter(|o| o.iter().all(|v| v.is_finite())),
super::appearance::MAX_IMPORT_APPEARANCE_BYTES,
+ true,
)?;
*doc = work;
Ok(report)
}
pub(crate) fn import_xfdf(doc: &mut Document, text: &str) -> Result {
- import_xfdf_with(doc, text, None, None, None, super::appearance::MAX_IMPORT_APPEARANCE_BYTES)
+ import_xfdf_with(doc, text, None, None, None, super::appearance::MAX_IMPORT_APPEARANCE_BYTES, false)
}
-/// The import, with `appearance_budget` decoded bytes of appearance for all of its comments.
+/// The import, with `appearance_budget` decoded bytes of appearance for all of its comments;
+/// `comments_only` leaves the file's field values out (comments placed as copies).
pub(crate) fn import_xfdf_with(
doc: &mut Document,
text: &str,
@@ -1235,6 +1272,7 @@ pub(crate) fn import_xfdf_with(
fresh: Option<&str>,
offset: Option<[f64; 2]>,
mut appearance_budget: usize,
+ comments_only: bool,
) -> Result {
check_form_xml_depth(text)?;
let xml = parse_form_xml(text).map_err(|e| DataError::Malformed(e.to_string()))?;
@@ -1326,7 +1364,7 @@ pub(crate) fn import_xfdf_with(
}
}
// Comments placed as copies: the file's field values are not part of the paste.
- if fresh.is_some() || target_page.is_some() || offset.is_some() {
+ if comments_only {
return Ok(report);
}
// Fields.