From cc1086a86221dd18ec713b9800449747236383f4 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:46:28 +0000 Subject: [PATCH 01/30] fix(audit): keep the pending path across Q The cover walk cleared the current path when it restored the graphics state, but the current path is not part of the graphics state. A rectangle built before Q and filled after it paints a box over the text, and the audit missed it and reported the page clean. --- crates/audit/src/cover.rs | 3 ++- crates/audit/src/tests.rs | 10 ++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/crates/audit/src/cover.rs b/crates/audit/src/cover.rs index 3df666c190..20ed811d40 100644 --- a/crates/audit/src/cover.rs +++ b/crates/audit/src/cover.rs @@ -144,10 +144,11 @@ impl<'a> Walker<'a> { match op.op.as_slice() { b"q" => stack.push(gs.clone()), b"Q" => { + // The current path is not graphics state (ISO 32000-2 §8.4.1): `re Q f` still + // fills the rectangle, whose corners are already in user space. if let Some(g) = stack.pop() { *gs = g; } - path = Path::Empty; } b"cm" => { let before = gs.ctm; diff --git a/crates/audit/src/tests.rs b/crates/audit/src/tests.rs index 4ca3b0bf5b..ded3a865ca 100644 --- a/crates/audit/src/tests.rs +++ b/crates/audit/src/tests.rs @@ -592,3 +592,13 @@ fn reading_blocks_respect_stream_comments_and_split_operands() { assert_eq!(text, "Replacement text", "{}", String::from_utf8_lossy(s1)); } } + +/// The current path is not graphics state: a rectangle built before `Q` and filled after it +/// paints the box, so it is a cover. +#[test] +fn rectangle_filled_after_restore_is_a_cover() { + let content = b"BT /F1 24 Tf 100 700 Td (SECRET DATA 123) Tj ET\n0 g\nq 100 688 220 32 re Q f\n"; + let findings = audit_redactions(&open(one_page(content, ""))).expect("audit must run"); + assert_eq!(findings.len(), 1, "{findings:?}"); + assert!(findings[0].covered_text.contains("SECRET"), "{findings:?}"); +} From 4c7ba85bebefcc4be2bd457faf5d30d07ef52e38 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:46:34 +0000 Subject: [PATCH 02/30] fix(audit): resolve an indirect /Subtype before skipping an object The annotation scan and the form XObject walk read /Subtype only as a direct name. A /Subtype stored as a reference to the name read as none, so a /Redact annotation or a covering form named that way was skipped as an unrelated object and the page reported clean. Both now follow the reference, and a reference that cannot be loaded fails the audit. --- crates/audit/src/cover.rs | 2 +- crates/audit/src/lib.rs | 12 +++++++++++- crates/audit/src/tests.rs | 29 +++++++++++++++++++++++++++++ 3 files changed, 41 insertions(+), 2 deletions(-) diff --git a/crates/audit/src/cover.rs b/crates/audit/src/cover.rs index 20ed811d40..d308c75a23 100644 --- a/crates/audit/src/cover.rs +++ b/crates/audit/src/cover.rs @@ -259,7 +259,7 @@ impl<'a> Walker<'a> { let resolved = self.doc.resolve(entry); let Object::Stream(stream) = &*resolved else { return Ok(()) }; let dict = &stream.dict; - if dict.name(b"Subtype") != Some(b"Form") { + if crate::subtype(self.doc, dict)?.as_deref() != Some(b"Form") { return Ok(()); } let matrix = dict diff --git a/crates/audit/src/lib.rs b/crates/audit/src/lib.rs index 365889d61a..b79353e85d 100644 --- a/crates/audit/src/lib.rs +++ b/crates/audit/src/lib.rs @@ -229,7 +229,7 @@ fn annots(doc: &Document, page: &Dict) -> Result, AuditErro continue; } let rect = [v[0].min(v[2]), v[1].min(v[3]), v[0].max(v[2]), v[1].max(v[3])]; - match dict.name(b"Subtype") { + match subtype(doc, &dict)?.as_deref() { Some(b"Redact") => out.push(AnnotationCover::Redact(rect)), Some(b"Square") => { let flags = dict.get(b"F").and_then(|f| doc.resolve(f).as_int()).unwrap_or(0); @@ -249,6 +249,16 @@ fn annots(doc: &Document, page: &Dict) -> Result, AuditErro Ok(out) } +/// The `/Subtype` name of `dict`, following an indirect reference. A reference that cannot be +/// loaded fails the audit: the subtype decides whether the object is checked at all. +fn subtype(doc: &Document, dict: &Dict) -> Result>, AuditError> { + let name = match dict.get(b"Subtype") { + Some(pdfcraft_cos::Object::Ref(r)) => doc.try_get(r.num)?.as_name().map(<[u8]>::to_vec), + other => other.and_then(pdfcraft_cos::Object::as_name).map(<[u8]>::to_vec), + }; + Ok(name) +} + fn annotation_color(doc: &Document, dict: &Dict, key: &[u8]) -> Option<[f64; 3]> { let values: Vec = dict.get(key).map(|o| doc.resolve(o))?.as_array()?.iter().map(|o| doc.resolve(o).as_f64()).collect::>()?; if !values.iter().all(|v| v.is_finite()) { diff --git a/crates/audit/src/tests.rs b/crates/audit/src/tests.rs index ded3a865ca..9be067ae0d 100644 --- a/crates/audit/src/tests.rs +++ b/crates/audit/src/tests.rs @@ -602,3 +602,32 @@ fn rectangle_filled_after_restore_is_a_cover() { assert_eq!(findings.len(), 1, "{findings:?}"); assert!(findings[0].covered_text.contains("SECRET"), "{findings:?}"); } + +/// A `/Subtype` may be an indirect reference to the name, like any other value: a `/Redact` +/// annotation and a covering form XObject named that way are still checked. +#[test] +fn indirect_subtype_is_resolved() { + let mut b = Builder::new(); + b.add("<< /Type /Catalog /Pages 2 0 R >>"); + b.add("<< /Type /Pages /Kids [3 0 R] /Count 1 >>"); + b.add("<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] /Contents 4 0 R /Resources << /Font << /F1 5 0 R >> >> /Annots [6 0 R] >>"); + b.add_stream(b"BT /F1 24 Tf 100 700 Td (DO NOT SHARE) Tj ET\n"); + b.add(FONT); + b.add("<< /Type /Annot /Subtype 7 0 R /Rect [95 685 305 725] >>"); + b.add("/Redact"); + let findings = audit_redactions(&open(b.build())).expect("audit must run"); + assert_eq!(findings.len(), 1, "{findings:?}"); + assert_eq!(findings[0].kind, FindingKind::UnappliedMark); + + let mut b = Builder::new(); + b.add("<< /Type /Catalog /Pages 2 0 R >>"); + b.add("<< /Type /Pages /Kids [3 0 R] /Count 1 >>"); + b.add("<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] /Contents 4 0 R /Resources << /Font << /F1 6 0 R >> /XObject << /X 5 0 R >> >> >>"); + b.add_stream(b"BT /F1 24 Tf 100 700 Td (SECRET) Tj ET\n/X Do\n"); + b.add_stream_with(b"0 g\n100 688 220 32 re f\n", "/Type /XObject /Subtype 7 0 R /BBox [0 0 612 792]"); + b.add(FONT); + b.add("/Form"); + let findings = audit_redactions(&open(b.build())).expect("audit must run"); + assert_eq!(findings.len(), 1, "{findings:?}"); + assert_eq!(findings[0].kind, FindingKind::CoveredText); +} From 3e306c22ef1dd255911e5c5ae4c2b9753ecf8299 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:46:43 +0000 Subject: [PATCH 03/30] fix(audit): compare a cover with the start of a merged text line A text line joins compatible runs on one baseline, and the audit took the line's last painting operator as its paint position. Text drawn after a cover on the same baseline joined the covered words and moved the line past the cover, so the covered words went unreported. The audit now compares against the line's first painting operator: any run painted before the cover is checked, and runs painted after it in the same low-contrast color are unreadable on it too. --- crates/audit/src/lib.rs | 10 ++++++---- crates/audit/src/tests.rs | 13 +++++++++++++ crates/edit/src/text.rs | 9 +++++---- 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/crates/audit/src/lib.rs b/crates/audit/src/lib.rs index b79353e85d..6dc329d9ae 100644 --- a/crates/audit/src/lib.rs +++ b/crates/audit/src/lib.rs @@ -143,11 +143,13 @@ pub fn audit_redactions(doc: &Document) -> Result, AuditError> } /// A filled rectangle is a faux-redaction cover for a text line iff it was painted strictly -/// after the line, overlaps it substantially, and is dark enough against the line's color to -/// make the text unreadable. Light covers (table shading, highlights) over dark text fail the -/// contrast check and are not flagged. +/// after the start of the line, overlaps it substantially, and is dark enough against the line's +/// color to make the text unreadable. Light covers (table shading, highlights) over dark text fail +/// the contrast check and are not flagged. A line can join runs painted before and after the +/// cover; the later ones are drawn on it in the same low-contrast color, so they are unreadable +/// too. fn is_cover(rect: &cover::FilledRect, line: &TextLine) -> bool { - rect.pos > line.paint_pos() && covers(&rect.rect, line) && contrast(line.color, rect.rgb) < COVER_CONTRAST + rect.pos > line.first_paint_pos() && covers(&rect.rect, line) && contrast(line.color, rect.rgb) < COVER_CONTRAST } /// Does `cover` hide a substantial part of what `line` draws? Three ways, any is enough: diff --git a/crates/audit/src/tests.rs b/crates/audit/src/tests.rs index 9be067ae0d..9d5a1726d2 100644 --- a/crates/audit/src/tests.rs +++ b/crates/audit/src/tests.rs @@ -631,3 +631,16 @@ fn indirect_subtype_is_resolved() { assert_eq!(findings.len(), 1, "{findings:?}"); assert_eq!(findings[0].kind, FindingKind::CoveredText); } + +/// Text drawn after a cover on the same baseline joins the covered text in one line. The cover +/// still hides the words painted before it. +#[test] +fn later_text_on_the_line_does_not_clear_an_earlier_cover() { + let content = b"0 g\nBT /F1 24 Tf 100 700 Td (SECRET) Tj ET\n98 688 94 32 re f\nBT /F1 24 Tf 200 700 Td (tail) Tj ET\n"; + let doc = open(one_page(content, "")); + let lines = pdfcraft_edit::text_lines(&doc, 0).expect("text"); + assert_eq!(lines.len(), 1, "the runs join one line: {:?}", lines.iter().map(|l| &l.text).collect::>()); + let findings = audit_redactions(&doc).expect("audit must run"); + assert_eq!(findings.len(), 1, "{findings:?}"); + assert!(findings[0].covered_text.contains("SECRET"), "{findings:?}"); +} diff --git a/crates/edit/src/text.rs b/crates/edit/src/text.rs index 93a5eec99e..1861b7816e 100644 --- a/crates/edit/src/text.rs +++ b/crates/edit/src/text.rs @@ -528,12 +528,13 @@ impl TextLine { self.origin.baseline } - /// Paint position of the line's last text-showing operator: `(content-stream index, - /// operator index in the page's joined stream)`. Lines paint in order of this tuple. + /// Paint position of the line's first operator: `(content-stream index, operator index in + /// the page's joined stream)`. A line joins runs painted at different times, so anything + /// painted after this position may lie on top of part of it. /// (Lines from `text_lines` never come from form XObjects, so `stream` is always a /// page content-stream index and `ops` are indices into the joined operator list.) - pub fn paint_pos(&self) -> (usize, usize) { - (self.stream, self.ops.iter().max().copied().unwrap_or(0)) + pub fn first_paint_pos(&self) -> (usize, usize) { + (self.stream, self.ops.iter().min().copied().unwrap_or(0)) } fn rotation(&self) -> f64 { From 65ee18d470acf96dc25acef5f2f024312b03b0e3 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:46:49 +0000 Subject: [PATCH 04/30] fix(audit): bound cover-line comparisons and covered text Every cover was compared with every text line of its page, and each finding kept its own copy of the text it covered. Both grow with the square of the content: a few megabytes of covers over one long line meant hours of comparisons and gigabytes of report. The audit now counts comparisons per page and covered-text bytes across the document and fails with an error past either bound, never with a partial report. --- crates/audit/src/lib.rs | 86 ++++++++++++++++++++++++++++----------- crates/audit/src/tests.rs | 31 ++++++++++++++ 2 files changed, 94 insertions(+), 23 deletions(-) diff --git a/crates/audit/src/lib.rs b/crates/audit/src/lib.rs index 6dc329d9ae..d827159570 100644 --- a/crates/audit/src/lib.rs +++ b/crates/audit/src/lib.rs @@ -79,6 +79,10 @@ pub enum AuditError { TooDeep, #[error("page has a malformed /Annots entry")] BadAnnots, + #[error("page has more covers and text lines than the audit compares")] + TooManyComparisons, + #[error("covered text exceeds the audit's report budget")] + TooMuchOutput, #[error(transparent)] Cos(#[from] pdfcraft_cos::CosError), #[error(transparent)] @@ -95,41 +99,77 @@ const WORD_FRACTION: f64 = 0.5; const COVER_CONTRAST: f64 = 3.0; /// Audit every page for faux redactions and unapplied redaction marks. Fails closed: any -/// content that cannot be decoded is returned as an error, never as an empty finding list. +/// content that cannot be decoded, or that needs more work or report than the audit's bounds, is +/// returned as an error, never as an empty finding list. pub fn audit_redactions(doc: &Document) -> Result, AuditError> { + audit(doc, Limits::DEFAULT) +} + +/// Work and output bounds for one audit. +#[derive(Clone, Copy)] +struct Limits { + /// Cover-against-line comparisons per page. + comparisons: usize, + /// Bytes of covered text across all findings. + covered_text: usize, +} + +impl Limits { + /// Every cover is compared with every line of its page, and each finding repeats the text + /// it covers, so both grow with the square of the content: a few megabytes of covers over + /// one long line would otherwise take hours and gigabytes. + const DEFAULT: Limits = Limits { comparisons: 50_000_000, covered_text: 64 << 20 }; +} + +/// The work and report the audit has spent against its [`Limits`]. +struct Budget { + limits: Limits, + comparisons: usize, + covered_text: usize, +} + +impl Budget { + /// The text of the `lines` that `hides`, joined with single spaces. + fn hidden_text(&mut self, lines: &[TextLine], hides: impl Fn(&TextLine) -> bool) -> Result { + self.comparisons = self.comparisons.saturating_add(lines.len()); + if self.comparisons > self.limits.comparisons { + return Err(AuditError::TooManyComparisons); + } + let mut covered = String::new(); + for line in lines.iter().filter(|l| hides(l)) { + let text = line.text.trim(); + self.covered_text = self.covered_text.saturating_add(text.len() + 1); + if self.covered_text > self.limits.covered_text { + return Err(AuditError::TooMuchOutput); + } + if !covered.is_empty() { + covered.push(' '); + } + covered.push_str(text); + } + Ok(covered) + } +} + +fn audit(doc: &Document, limits: Limits) -> Result, AuditError> { + let mut budget = Budget { limits, comparisons: 0, covered_text: 0 }; let mut out = Vec::new(); for (n, page) in pdfcraft_model::pages(doc).iter().enumerate() { let lines = pdfcraft_edit::text_lines(doc, n).map_err(|e| AuditError::Text(n, e.to_string()))?; + budget.comparisons = 0; // 1. Opaque filled rectangles over extractable text. for r in cover::filled_rects(doc, &page.dict)? { - let mut covered = String::new(); - for line in &lines { - if is_cover(&r, line) { - if !covered.is_empty() { - covered.push(' '); - } - covered.push_str(line.text.trim()); - } - } + let covered = budget.hidden_text(&lines, |line| is_cover(&r, line))?; if !covered.trim().is_empty() { out.push(AuditFinding { page: n, kind: FindingKind::CoveredText, rect: r.rect, covered_text: covered }); } } // 2. Annotation covers over extractable text. for annot in annots(doc, &page.dict)? { - let mut covered = String::new(); - for line in &lines { - let hidden = match &annot { - AnnotationCover::Redact(rect) => covers(rect, line), - AnnotationCover::Square { rect, rgb } => covers(rect, line) && contrast(line.color, *rgb) < COVER_CONTRAST, - }; - if hidden { - if !covered.is_empty() { - covered.push(' '); - } - covered.push_str(line.text.trim()); - } - } + let covered = budget.hidden_text(&lines, |line| match &annot { + AnnotationCover::Redact(rect) => covers(rect, line), + AnnotationCover::Square { rect, rgb } => covers(rect, line) && contrast(line.color, *rgb) < COVER_CONTRAST, + })?; if !covered.trim().is_empty() { let (kind, rect) = match annot { AnnotationCover::Redact(rect) => (FindingKind::UnappliedMark, rect), diff --git a/crates/audit/src/tests.rs b/crates/audit/src/tests.rs index 9d5a1726d2..ed809bc633 100644 --- a/crates/audit/src/tests.rs +++ b/crates/audit/src/tests.rs @@ -644,3 +644,34 @@ fn later_text_on_the_line_does_not_clear_an_earlier_cover() { assert_eq!(findings.len(), 1, "{findings:?}"); assert!(findings[0].covered_text.contains("SECRET"), "{findings:?}"); } + +/// Many covers over the same long line repeat its text in every finding: a small file must not +/// expand into an unbounded report. Past the budget the audit fails rather than truncate. +#[test] +fn covered_text_across_findings_is_bounded() { + let text = "X".repeat(60_000); + let mut content = format!("BT /F1 1 Tf 0 700 Td ({text}) Tj ET\n0 g\n").into_bytes(); + for i in 0..1_200 { + content.extend_from_slice(format!("-{i} 690 50000 20 re f\n").as_bytes()); + } + let result = audit_redactions(&open(one_page(&content, ""))); + assert!(matches!(result, Err(crate::AuditError::TooMuchOutput)), "{:?}", result.map(|f| f.len())); +} + +/// Covers compared against lines are bounded per page, so content that is quadratic in its +/// covers and lines fails the audit instead of running for hours. +#[test] +fn cover_and_line_comparisons_are_bounded() { + let mut content = Vec::new(); + for i in 0..20 { + content.extend_from_slice(format!("BT /F1 12 Tf 72 {} Td (Line {i}) Tj ET\n", 700 - 14 * i).as_bytes()); + } + for _ in 0..20 { + content.extend_from_slice(b"1 g 0 0 10 10 re f\n"); + } + let doc = open(one_page(&content, "")); + let limits = crate::Limits { comparisons: 399, covered_text: usize::MAX }; + assert!(matches!(crate::audit(&doc, limits), Err(crate::AuditError::TooManyComparisons))); + let limits = crate::Limits { comparisons: 400, covered_text: usize::MAX }; + assert!(crate::audit(&doc, limits).expect("within the budget").is_empty()); +} From 3fce90e94a3ec5dd70f977814f75f80f9dd74bb6 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 14:47:03 +0000 Subject: [PATCH 05/30] fix(audit): withhold covered text when copying is not permitted The audit returned the text under each cover whatever the document's permissions, and the CLI and the automation tool printed it. A user whose password withholds copying could read through the audit the text that text extraction refuses them. The audit now reports every finding for such a document with empty covered text; the owner password lifts the restriction as it does elsewhere. --- crates/audit/README.md | 7 ++++++- crates/audit/src/lib.rs | 13 ++++++++++--- crates/audit/src/tests.rs | 26 ++++++++++++++++++++++++++ 3 files changed, 42 insertions(+), 4 deletions(-) diff --git a/crates/audit/README.md b/crates/audit/README.md index 37771a5350..ce2c47915f 100644 --- a/crates/audit/README.md +++ b/crates/audit/README.md @@ -33,7 +33,12 @@ Limits, by design and documented rather than hidden: less than 40% of its words (a sliver of a long value) is not flagged. It fails closed: an undecodable content stream, malformed `/Annots`, form XObjects nested deeper -than 32 or in a cycle, or a walk past the operator bound is an error, never "no findings". +than 32 or in a cycle, a walk past the operator bound, more than 50 million cover-against-line +comparisons on a page, or more than 64 MiB of covered text in the report is an error, never "no +findings". + +A document whose permissions withhold copying (Table 22, bit 5) still gets every finding, with +an empty `covered_text`: the audit hands out no text that copying would refuse. Surfaces: the `doc_audit_redactions` automation tool and `pdfcraft-cli audit`. Both report each rectangle in points from the top-left of the page as displayed (y down, after `/Rotate`), like diff --git a/crates/audit/src/lib.rs b/crates/audit/src/lib.rs index d827159570..4f1877ef67 100644 --- a/crates/audit/src/lib.rs +++ b/crates/audit/src/lib.rs @@ -65,7 +65,7 @@ pub struct AuditFinding { /// `rect[0] <= rect[2]` and `rect[1] <= rect[3]`. pub rect: [f64; 4], /// The extractable text the cover hides, lines joined with single spaces and trimmed. - /// Possibly empty when nothing decodable remains under the cover. + /// Empty when the document's permissions withhold copying (see [`audit_redactions`]). pub covered_text: String, } @@ -101,6 +101,10 @@ const COVER_CONTRAST: f64 = 3.0; /// Audit every page for faux redactions and unapplied redaction marks. Fails closed: any /// content that cannot be decoded, or that needs more work or report than the audit's bounds, is /// returned as an error, never as an empty finding list. +/// +/// When the document's permissions withhold copying (ISO 32000-2 Table 22, bit 5), findings are +/// still reported but their `covered_text` is empty: the audit hands out no text that copying +/// would refuse. pub fn audit_redactions(doc: &Document) -> Result, AuditError> { audit(doc, Limits::DEFAULT) } @@ -152,6 +156,7 @@ impl Budget { } fn audit(doc: &Document, limits: Limits) -> Result, AuditError> { + let copying = doc.permissions().is_none_or(|p| p.copy()); let mut budget = Budget { limits, comparisons: 0, covered_text: 0 }; let mut out = Vec::new(); for (n, page) in pdfcraft_model::pages(doc).iter().enumerate() { @@ -161,7 +166,8 @@ fn audit(doc: &Document, limits: Limits) -> Result, AuditError for r in cover::filled_rects(doc, &page.dict)? { let covered = budget.hidden_text(&lines, |line| is_cover(&r, line))?; if !covered.trim().is_empty() { - out.push(AuditFinding { page: n, kind: FindingKind::CoveredText, rect: r.rect, covered_text: covered }); + let covered_text = if copying { covered } else { String::new() }; + out.push(AuditFinding { page: n, kind: FindingKind::CoveredText, rect: r.rect, covered_text }); } } // 2. Annotation covers over extractable text. @@ -175,7 +181,8 @@ fn audit(doc: &Document, limits: Limits) -> Result, AuditError AnnotationCover::Redact(rect) => (FindingKind::UnappliedMark, rect), AnnotationCover::Square { rect, .. } => (FindingKind::CoveredText, rect), }; - out.push(AuditFinding { page: n, kind, rect, covered_text: covered }); + let covered_text = if copying { covered } else { String::new() }; + out.push(AuditFinding { page: n, kind, rect, covered_text }); } } } diff --git a/crates/audit/src/tests.rs b/crates/audit/src/tests.rs index ed809bc633..de501d01a7 100644 --- a/crates/audit/src/tests.rs +++ b/crates/audit/src/tests.rs @@ -675,3 +675,29 @@ fn cover_and_line_comparisons_are_bounded() { let limits = crate::Limits { comparisons: 400, covered_text: usize::MAX }; assert!(crate::audit(&doc, limits).expect("within the budget").is_empty()); } + +/// Covered text is document text: a document whose permissions withhold copying reports where +/// its covers are but not what they hide, unless it was opened with the owner password. +#[test] +fn copy_restricted_documents_withhold_covered_text() { + let content = b"BT /F1 24 Tf 100 700 Td (SECRET DATA 123) Tj ET\n0 g\n100 688 220 32 re f\n"; + let mut doc = open(one_page(content, "")); + doc.set_encryption(&pdfcraft_cos::NewEncryption { + algorithm: pdfcraft_cos::Algorithm::Aes256, + user_password: "", + owner_password: "owner", + permissions: -1 ^ 0b1_0000, + encrypt_metadata: true, + seed: [7; 32], + }) + .expect("encrypt"); + let bytes = Arc::new(pdfcraft_cos::write_full(&doc, &Default::default()).expect("save")); + let user = Document::open_with_password(bytes.clone(), None).expect("open"); + let findings = audit_redactions(&user).expect("audit must run"); + assert_eq!(findings.len(), 1, "the cover is still reported: {findings:?}"); + assert_eq!(findings[0].covered_text, ""); + let owner = Document::open_with_password(bytes, Some("owner")).expect("open"); + let findings = audit_redactions(&owner).expect("audit must run"); + assert_eq!(findings.len(), 1, "{findings:?}"); + assert_eq!(findings[0].covered_text, "SECRET DATA 123"); +} From 693eae1d56063ee2cf2b185cf9cc0486c6b3bae5 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 16:49:00 +0000 Subject: [PATCH 06/30] docs(parity): cite the audit's new tests and record its bounds and copy restriction --- parity/acrobat-features.toml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 362c2aa776..700b74b8da 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -5841,8 +5841,8 @@ milestone = "M8" status = "partial" commands = [] tools = ["doc_audit_redactions"] -evidence = ["crates/audit/src/tests.rs::faux_redaction_is_found", "crates/audit/src/tests.rs::background_before_text_is_not_flagged", "crates/audit/src/tests.rs::light_shading_after_text_is_not_flagged", "crates/audit/src/tests.rs::graphics_state_carries_across_streams", "crates/audit/src/tests.rs::audit_fails_closed_on_undecodable_stream", "crates/audit/src/tests.rs::unapplied_redact_mark_is_found", "crates/audit/src/tests.rs::applied_redaction_at_the_end_of_a_line_is_not_flagged", "crates/audit/src/tests.rs::faux_redaction_at_the_end_of_a_line_is_found", "crates/audit/src/tests.rs::faux_redaction_over_a_short_value_after_a_long_label_is_found", "crates/audit/src/tests.rs::faux_redaction_over_mirrored_text_is_found", "crates/audit/src/tests.rs::faux_redaction_over_a_partial_value_after_a_long_label_is_found", "crates/audit/src/tests.rs::faux_redaction_over_a_partial_value_after_a_long_label_on_a_rotated_page_is_found", "crates/audit/src/tests.rs::faux_redactions_with_multiple_covers_lines_and_spacing_are_found", "crates/audit/src/tests.rs::a_cover_over_one_word_of_a_long_line_is_found", "crates/audit/src/tests.rs::a_box_grazing_a_word_is_not_flagged", "crates/automation/tests/it/automation.rs::auditing_faux_redactions_through_tools", "crates/automation/tests/it/automation.rs::an_applied_redaction_at_the_end_of_a_line_audits_clean", "crates/audit/src/tests.rs::rectangle_drawn_as_a_path_covers_text_like_re", "crates/audit/src/tests.rs::non_rectangular_path_is_not_a_cover", "apps/pdfcraft-cli/tests/it/audit.rs::audit_rectangles_match_the_tool_in_top_left_points"] -notes = "New pdfcraft-audit crate. A filled rectangle (re + fill, or a single m/l subpath whose corners form an axis-aligned rectangle, #1510; opaque) counts as a cover only when painted after the text it overlaps and dark enough to hide it (WCAG contrast < 3.0); page backgrounds, table shading and highlights are not flagged. Coverage is measured on the words a line draws, not its box, so the gap an applied redaction leaves is not counted as hidden text; a cover over 40% of the line's box still counts when it has a positive-area intersection with one word, so a short value after a long label is found. A cover hiding half of any one word counts too (#2662), so a box over one number in a long line, or an unapplied mark over it, is found; a sliver under half of every word it touches and under 40% of the line is not flagged. Graphics state carries across a page's content streams. Fails closed: undecodable content is an error, never a clean bill. Also flags /Redact annotations never applied. CLI: pdfcraft-cli audit (fails when findings exist). Tool and CLI report rectangles in points from the top-left of the displayed page, after /Rotate and rounded to 0.01 pt, like the other tools." +evidence = ["crates/audit/src/tests.rs::faux_redaction_is_found", "crates/audit/src/tests.rs::background_before_text_is_not_flagged", "crates/audit/src/tests.rs::light_shading_after_text_is_not_flagged", "crates/audit/src/tests.rs::graphics_state_carries_across_streams", "crates/audit/src/tests.rs::audit_fails_closed_on_undecodable_stream", "crates/audit/src/tests.rs::unapplied_redact_mark_is_found", "crates/audit/src/tests.rs::applied_redaction_at_the_end_of_a_line_is_not_flagged", "crates/audit/src/tests.rs::faux_redaction_at_the_end_of_a_line_is_found", "crates/audit/src/tests.rs::faux_redaction_over_a_short_value_after_a_long_label_is_found", "crates/audit/src/tests.rs::faux_redaction_over_mirrored_text_is_found", "crates/audit/src/tests.rs::faux_redaction_over_a_partial_value_after_a_long_label_is_found", "crates/audit/src/tests.rs::faux_redaction_over_a_partial_value_after_a_long_label_on_a_rotated_page_is_found", "crates/audit/src/tests.rs::faux_redactions_with_multiple_covers_lines_and_spacing_are_found", "crates/audit/src/tests.rs::a_cover_over_one_word_of_a_long_line_is_found", "crates/audit/src/tests.rs::a_box_grazing_a_word_is_not_flagged", "crates/automation/tests/it/automation.rs::auditing_faux_redactions_through_tools", "crates/automation/tests/it/automation.rs::an_applied_redaction_at_the_end_of_a_line_audits_clean", "crates/audit/src/tests.rs::rectangle_drawn_as_a_path_covers_text_like_re", "crates/audit/src/tests.rs::non_rectangular_path_is_not_a_cover", "apps/pdfcraft-cli/tests/it/audit.rs::audit_rectangles_match_the_tool_in_top_left_points", "crates/audit/src/tests.rs::rectangle_filled_after_restore_is_a_cover", "crates/audit/src/tests.rs::later_text_on_the_line_does_not_clear_an_earlier_cover", "crates/audit/src/tests.rs::indirect_subtype_is_resolved", "crates/audit/src/tests.rs::copy_restricted_documents_withhold_covered_text", "crates/audit/src/tests.rs::cover_and_line_comparisons_are_bounded", "crates/audit/src/tests.rs::covered_text_across_findings_is_bounded"] +notes = "New pdfcraft-audit crate. A filled rectangle (re + fill, or a single m/l subpath whose corners form an axis-aligned rectangle, #1510; opaque) counts as a cover only when painted after the start of the text line it overlaps and dark enough to hide it (WCAG contrast < 3.0); page backgrounds, table shading and highlights are not flagged. Coverage is measured on the words a line draws, not its box, so the gap an applied redaction leaves is not counted as hidden text; a cover over 40% of the line's box still counts when it has a positive-area intersection with one word, so a short value after a long label is found. A cover hiding half of any one word counts too (#2662), so a box over one number in a long line, or an unapplied mark over it, is found; a sliver under half of every word it touches and under 40% of the line is not flagged. Graphics state carries across a page's content streams. Fails closed: undecodable content, or more than 50 million cover-against-line comparisons on a page or 64 MiB of covered text in the report, is an error, never a clean bill. A document that withholds copying gets every finding with the covered text left out. Also flags /Redact annotations never applied. CLI: pdfcraft-cli audit (fails when findings exist). Tool and CLI report rectangles in points from the top-left of the displayed page, after /Rotate and rounded to 0.01 pt, like the other tools." [[feature]] id = "protect.remove-hidden-on-close" From 93436d41b050f7a199efdea1f5d2b84f0348a7de Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 15:43:04 +0000 Subject: [PATCH 07/30] fix(xfdf): escape paired line-ending names in exported XFDF The head and tail attributes were written from the annotation's /LE names without XML escaping, so a name holding a quote or angle bracket could add attributes or elements, including field values, to the exported file. --- crates/xfdf/src/lib.rs | 2 +- crates/xfdf/src/tests.rs | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/crates/xfdf/src/lib.rs b/crates/xfdf/src/lib.rs index 219b0de81b..bc3ea01f40 100644 --- a/crates/xfdf/src/lib.rs +++ b/crates/xfdf/src/lib.rs @@ -354,7 +354,7 @@ fn xfdf_annots(doc: &Document, out: &mut String) { && le.len() == 2 { let name = |o: &Object| o.as_name().map(|x| String::from_utf8_lossy(x).into_owned()).unwrap_or_else(|| "None".into()); - let _ = write!(attrs, " head=\"{}\" tail=\"{}\"", name(&le[0]), name(&le[1])); + let _ = write!(attrs, " head=\"{}\" tail=\"{}\"", esc(&name(&le[0])), esc(&name(&le[1]))); } // A callout: its leader path, text-box inset, intent and (single-name) line ending. let cl = nums_of(doc, d.get(b"CL")); diff --git a/crates/xfdf/src/tests.rs b/crates/xfdf/src/tests.rs index c98fab06ac..a93f70c70e 100644 --- a/crates/xfdf/src/tests.rs +++ b/crates/xfdf/src/tests.rs @@ -1758,3 +1758,26 @@ fn an_imported_redaction_rect_with_a_non_number_is_not_used() { let mark = imported_redact_mark(fdf); assert!(mark.get(b"QuadPoints").is_none(), "quads were drawn from a malformed /Rect: {mark:?}"); } + +/// Line-ending names are attribute values: a name holding a quote or angle bracket cannot add +/// attributes or elements to the exported XFDF. +#[test] +fn xfdf_line_ending_names_are_escaped() { + let mut doc = blank(); + let page = pdfcraft_model::pages(&doc)[0].obj; + let hostile = b"a\" tail=\"x\">pwned Date: Thu, 22 Oct 2026 15:43:57 +0000 Subject: [PATCH 08/30] fix(xfdf): bound what an FDF import copies out of repeated references The FDF field walker and the comment copier followed every reference again each time it appeared, so a field tree or comment whose kids or properties repeat one reference level after level expanded a small file into millions of rows or values before the depth limit stopped it. Both now skip a reference back to an object they are inside of, and share one budget per import of values and text; past it the import is refused. --- crates/xfdf/src/lib.rs | 117 +++++++++++++++++++++++++++++++++------ crates/xfdf/src/tests.rs | 61 ++++++++++++++++++++ 2 files changed, 161 insertions(+), 17 deletions(-) diff --git a/crates/xfdf/src/lib.rs b/crates/xfdf/src/lib.rs index bc3ea01f40..99470b58a1 100644 --- a/crates/xfdf/src/lib.rs +++ b/crates/xfdf/src/lib.rs @@ -1198,18 +1198,66 @@ fn find_by_name(doc: &Document, nm: &str) -> Option { None } -/// Copy an object out of another document, resolving references (FDF objects are small). -fn deep(src: &Document, o: &Object, depth: usize) -> Object { +/// What one FDF import may copy out of the file. A field tree or comment that repeats a reference +/// level after level would otherwise expand a small file into millions of values. +const MAX_FDF_IMPORT_VALUES: usize = 1_000_000; + +struct ImportBudget { + values: usize, + bytes: usize, +} + +impl ImportBudget { + fn new() -> Self { + Self { values: MAX_FDF_IMPORT_VALUES, bytes: MAX_FDF_BYTES } + } + + fn spend(&mut self, values: usize, bytes: usize) -> Result<(), DataError> { + match (self.values.checked_sub(values), self.bytes.checked_sub(bytes)) { + (Some(v), Some(b)) => { + (self.values, self.bytes) = (v, b); + Ok(()) + } + _ => Err(DataError::Malformed(format!( + "the file expands to more than {MAX_FDF_IMPORT_VALUES} values or {} MiB of text", + MAX_FDF_BYTES >> 20 + ))), + } + } +} + +/// 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 { + budget.spend(1, 0)?; if depth > 16 { - return Object::Null; + return Ok(Object::Null); } - match o { - Object::Ref(r) => deep(src, &src.get(*r), depth + 1), - Object::Array(a) => Object::Array(a.iter().map(|x| deep(src, x, depth + 1)).collect()), - Object::Dict(d) => Object::Dict(d.iter().map(|(k, v)| (k.clone(), deep(src, v, depth + 1))).collect()), + Ok(match o { + Object::Ref(r) => { + if path.contains(r) { + return Ok(Object::Null); + } + path.push(*r); + let copy = deep(src, &src.get(*r), depth + 1, path, budget); + path.pop(); + copy? + } + Object::Array(a) => Object::Array(a.iter().map(|x| deep(src, x, depth + 1, path, budget)).collect::>()?), + Object::Dict(d) => { + Object::Dict(d.iter().map(|(k, v)| Ok((k.clone(), deep(src, v, depth + 1, path, budget)?))).collect::>()?) + } Object::Stream(_) => Object::Null, + Object::String(s) => { + budget.spend(0, s.bytes.len())?; + o.clone() + } + Object::Name(n) => { + budget.spend(0, n.len())?; + o.clone() + } other => other.clone(), - } + }) } fn walk_xfdf(node: roxmltree::Node, prefix: &str, out: &mut Vec<(String, Vec)>, depth: usize) -> Result<(), DataError> { @@ -1233,27 +1281,59 @@ fn walk_xfdf(node: roxmltree::Node, prefix: &str, out: &mut Vec<(String, Vec)>, depth: usize) { +/// Read an FDF field tree. A field listed again beneath itself is skipped. +fn walk_fdf( + fdf: &Document, + list: &[Object], + prefix: &str, + out: &mut Vec<(String, Vec)>, + depth: usize, + path: &mut Vec, + budget: &mut ImportBudget, +) -> Result<(), DataError> { if depth > 32 { - return; + return Ok(()); } for f in list { - let Some(d) = fdf.resolve(f).as_dict().cloned() else { continue }; + budget.spend(1, 0)?; + let held; + let field = match f { + Object::Ref(r) if path.contains(r) => continue, + Object::Ref(r) => { + held = fdf.get(*r); + &*held + } + direct => direct, + }; + let Some(d) = field.as_dict() else { continue }; let Some(t) = d.get(b"T").and_then(|t| fdf.resolve(t).as_string().map(|s| s.to_text())) else { continue }; let full = if prefix.is_empty() { t } else { format!("{prefix}.{t}") }; + budget.spend(0, full.len())?; if let Some(v) = d.get(b"V").map(|v| fdf.resolve(v)) { let vals: Vec = match &*v { Object::String(s) => vec![s.to_text()], Object::Name(n) => vec![pdfcraft_forms::name_text(n)], - Object::Array(a) => a.iter().filter_map(|x| fdf.resolve(x).as_string().map(|s| s.to_text())).collect(), + Object::Array(a) => { + budget.spend(a.len(), 0)?; + a.iter().filter_map(|x| fdf.resolve(x).as_string().map(|s| s.to_text())).collect() + } _ => Vec::new(), }; + budget.spend(vals.len(), vals.iter().map(String::len).fold(0, usize::saturating_add))?; out.push((full.clone(), vals)); } - if let Some(kids) = d.get(b"Kids").map(|k| fdf.resolve(k)).and_then(|k| k.as_array().cloned()) { - walk_fdf(fdf, &kids, &full, out, depth + 1); + if let Some(kids) = d.get(b"Kids").map(|k| fdf.resolve(k)) + && let Some(kids) = kids.as_array() + { + let entered = f.as_ref().map(|r| path.push(r)).is_some(); + let walked = walk_fdf(fdf, kids, &full, out, depth + 1, path, budget); + if entered { + path.pop(); + } + walked?; } } + Ok(()) } fn fdf_body(fdf: &Document) -> Result { @@ -1272,8 +1352,9 @@ pub fn data_values(bytes: &[u8]) -> Result)>, DataError let fdf = Document::open(std::sync::Arc::new(bytes.to_vec())).map_err(|e| DataError::Malformed(e.to_string()))?; let body = fdf_body(&fdf)?; let mut values = Vec::new(); + let mut budget = ImportBudget::new(); if let Some(list) = body.get(b"Fields").map(|f| fdf.resolve(f)).and_then(|f| f.as_array().cloned()) { - walk_fdf(&fdf, &list, "", &mut values, 0); + walk_fdf(&fdf, &list, "", &mut values, 0, &mut Vec::new(), &mut budget)?; } return Ok(values); } @@ -1346,10 +1427,11 @@ fn import_fdf(doc: &mut Document, bytes: &[u8]) -> Result { let fdf = Document::open(std::sync::Arc::new(bytes.to_vec())).map_err(|e| DataError::Malformed(e.to_string()))?; let body = fdf_body(&fdf)?; let mut report = Report::default(); + let mut budget = ImportBudget::new(); // Fields (with /Kids for hierarchical names). let mut values = Vec::new(); if let Some(list) = body.get(b"Fields").map(|f| fdf.resolve(f)).and_then(|f| f.as_array().cloned()) { - walk_fdf(&fdf, &list, "", &mut values, 0); + walk_fdf(&fdf, &list, "", &mut values, 0, &mut Vec::new(), &mut budget)?; } // Comments: parents first, then replies (whose /IRT needs the parent's new reference). let pages: Vec = pdfcraft_model::pages(doc).iter().map(|p| p.obj).collect(); @@ -1360,7 +1442,8 @@ fn import_fdf(doc: &mut Document, bytes: &[u8]) -> Result { // The parent is another comment: link it below rather than copying it in. let irt = src.get(b"IRT").cloned(); src.remove(b"IRT"); - let Object::Dict(mut d) = deep(&fdf, &Object::Dict(src), 0) else { continue }; + let mut path: Vec = a.as_ref().into_iter().collect(); + let Object::Dict(mut d) = deep(&fdf, &Object::Dict(src), 0, &mut path, &mut budget)? else { continue }; let Some(page) = d.get(b"Page").and_then(Object::as_int).and_then(|p| usize::try_from(p).ok()).and_then(|p| pages.get(p).copied()) else { continue; }; diff --git a/crates/xfdf/src/tests.rs b/crates/xfdf/src/tests.rs index a93f70c70e..cf9095bf32 100644 --- a/crates/xfdf/src/tests.rs +++ b/crates/xfdf/src/tests.rs @@ -1759,6 +1759,67 @@ fn an_imported_redaction_rect_with_a_non_number_is_not_used() { assert!(mark.get(b"QuadPoints").is_none(), "quads were drawn from a malformed /Rect: {mark:?}"); } +/// An FDF body with `body` as its `/FDF` dictionary contents and `objects` (numbered from 2). +fn fdf_with(body: &str, objects: &[String]) -> Vec { + let mut out = format!("%FDF-1.2\n1 0 obj\n<< /FDF << {body} >> >>\nendobj\n"); + for (i, o) in objects.iter().enumerate() { + out.push_str(&format!("{} 0 obj\n{o}\nendobj\n", i + 2)); + } + out.push_str("trailer\n<< /Root 1 0 R >>\n%%EOF\n"); + out.into_bytes() +} + +/// A field that lists itself as a kid is read once. +#[test] +fn an_fdf_field_listing_itself_is_read_once() { + let cyclic = fdf_with("/Fields [2 0 R]", &["<< /T (a) /V (x) /Kids [2 0 R] >>".into()]); + assert_eq!(data_values(&cyclic).unwrap(), vec![("a".to_string(), vec!["x".to_string()])]); +} + +/// A field tree whose `/Kids` repeat one subtree level after level is refused rather than +/// expanded into millions of rows. +#[test] +fn fdf_field_trees_that_repeat_references_are_bounded() { + let levels = 19; + let mut objects: Vec = (0..levels).map(|i| format!("<< /T (f{i}) /V (x) /Kids [{n} 0 R {n} 0 R] >>", n = i + 3)).collect(); + objects.push("<< /T (leaf) /V (x) >>".into()); + let shared = fdf_with("/Fields [2 0 R]", &objects); + let read = data_values(&shared); + assert!(read.is_err(), "{} rows", read.map_or(0, |v| v.len())); + assert!(matches!(import(&mut blank(), &shared), Err(DataError::Malformed(_)))); +} + +fn fdf_note(data: &str) -> String { + format!("<< /Subtype /Text /Rect [0 0 9 9] /NM (n) /Page 0 /PCData {data} >>") +} + +/// A comment property that refers back to the comment is copied once, not nested level after +/// level. +#[test] +fn an_fdf_comment_referring_to_itself_is_copied_once() { + let cyclic = fdf_with("/Annots [2 0 R]", &[fdf_note("2 0 R")]); + let mut dst = blank(); + assert_eq!(import(&mut dst, &cyclic).unwrap().comments, 1); + let placed = find_by_name(&dst, "n").unwrap(); + let data = dst.get(placed).as_dict().unwrap().get(b"PCData").cloned(); + assert!(!matches!(data, Some(Object::Dict(_))), "{data:?}"); +} + +/// A comment whose properties repeat one reference level after level is refused rather than +/// copied into millions of values. +#[test] +fn fdf_comment_graphs_that_repeat_references_are_bounded() { + let note = fdf_note; + let width = 6; + let mut objects = vec![note("3 0 R")]; + for i in 0..8 { + objects.push(format!("[{}]", vec![format!("{} 0 R", i + 4); width].join(" "))); + } + objects.push("(leaf)".into()); + let shared = fdf_with("/Annots [2 0 R]", &objects); + assert!(matches!(import(&mut blank(), &shared), Err(DataError::Malformed(_)))); +} + /// Line-ending names are attribute values: a name holding a quote or angle bracket cannot add /// attributes or elements to the exported XFDF. #[test] From ae791d667bfaf2785979eaac34b45502dc66c320 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 15:47:11 +0000 Subject: [PATCH 09/30] fix(sign): a time-stamp response must carry the requested nonce and policy parse_response compared only the digest algorithm and imprint with the query, and the token parser dropped the TSTInfo nonce. A genuine token for the same imprint from another request or under another policy was accepted. The token keeps its nonce, and a requested nonce or policy must match. --- crates/sign/src/timestamp.rs | 13 ++++++++++++- crates/sign/tests/it/pdf.rs | 22 ++++++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/crates/sign/src/timestamp.rs b/crates/sign/src/timestamp.rs index 58eda4f049..0eba6bb72c 100644 --- a/crates/sign/src/timestamp.rs +++ b/crates/sign/src/timestamp.rs @@ -62,6 +62,8 @@ pub struct TimestampToken { pub policy: String, pub serial: Vec, pub gen_time: Time, + /// The TSTInfo nonce, when it has one that fits a `u64` (the size requests use). + pub nonce: Option, } impl TimestampToken { @@ -140,6 +142,9 @@ pub fn parse_token(raw: &[u8]) -> Result { .to_vec(); let serial = fields.get(3).ok_or_else(|| bad("TSTInfo has no serial number"))?.uint_bytes().to_vec(); let gen_time = fields.get(4).ok_or_else(|| bad("TSTInfo has no generation time"))?.time()?; + // The nonce is the only INTEGER after genTime (RFC 3161 §2.4.2): accuracy is a SEQUENCE and + // ordering a BOOLEAN. + let nonce = fields.iter().skip(5).find(|f| f.tag == tag::INTEGER).and_then(|f| f.u64().ok()); // The signed messageDigest uses the SignerInfo's digest algorithm, which need not be the // imprint's (RFC 5652 §5.4). let content_digest = token.signer.digest.digest(&[info]); @@ -151,7 +156,7 @@ pub fn parse_token(raw: &[u8]) -> Result { if !token.verify_signature(cert, &content_digest)? { return Err(bad("TSA signature is invalid")); } - Ok(TimestampToken { raw: raw.to_vec(), digest, imprint, policy, serial, gen_time }) + Ok(TimestampToken { raw: raw.to_vec(), digest, imprint, policy, serial, gen_time, nonce }) } /// Parse and validate a `TimeStampResp`, including the CMS signature and message imprint. @@ -170,6 +175,12 @@ pub fn parse_response(bytes: &[u8], query: &TimestampQuery) -> Result, policy: Option<&str>| { + let mut q = TimestampQuery::new(alg, alg.digest(&[b"document"])).unwrap(); + (q.nonce, q.policy) = (nonce, policy.map(str::to_string)); + q + }; + let answer = |q: &TimestampQuery| respond(&tsa.key, &tsa.certificate, &tsa.chain, alg, q, "1.2.3.4", time, 9).unwrap(); + let with_nonce = answer(&query(Some(42), None)); + assert!(parse_response(&with_nonce, &query(Some(42), Some("1.2.3.4"))).is_ok()); + assert!(parse_response(&with_nonce, &query(None, None)).is_ok()); + assert!(parse_response(&with_nonce, &query(Some(43), None)).is_err(), "another request's nonce"); + assert!(parse_response(&with_nonce, &query(Some(42), Some("1.2.3.5"))).is_err(), "another policy"); + let without_nonce = answer(&query(None, None)); + assert!(parse_response(&without_nonce, &query(Some(42), None)).is_err(), "no nonce in the token"); +} + #[cfg(windows)] #[test] fn windows_store_enumeration_and_missing_identity() { From 0a30b4e08107ad1b5809afc83c1f9790a6ef26f8 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 16:01:27 +0000 Subject: [PATCH 10/30] fix(preflight): read content colour one operator at a time content_colour parsed each content stream into a full list of operators before looking at any of them, so a small compressed stream of millions of tiny operators became gigabytes of parsed operators in memory. The content parser's loop is now an iterator (pdfcraft_content::ops), which parse collects; preflight reads it one operator at a time and stops once gray, RGB and CMYK are all found. --- crates/content/src/lib.rs | 197 +++++++++++++++++++++------------- crates/preflight/src/lib.rs | 16 ++- crates/preflight/src/tests.rs | 13 +++ 3 files changed, 151 insertions(+), 75 deletions(-) diff --git a/crates/content/src/lib.rs b/crates/content/src/lib.rs index eb126ffa21..3432f7ff32 100644 --- a/crates/content/src/lib.rs +++ b/crates/content/src/lib.rs @@ -79,90 +79,143 @@ pub fn parse(data: &[u8]) -> Parsed { parse_pieces(data, &[]) } +/// The operators of a (decoded) content stream, read one at a time, as [`parse`] reads them. A +/// caller that stops early or keeps only a summary never holds every operator of the stream. +pub fn ops(data: &[u8]) -> Ops<'_> { + Ops::new(data, &[]) +} + /// Parse `data`, recording which of the piece boundaries `ends` (as in [`Pieces`]) a comment /// crosses. fn parse_pieces(data: &[u8], ends: &[usize]) -> Parsed { - let mut out = Parsed { comment_edges: vec![(false, false); ends.len()], ..Parsed::default() }; - let mut boundary = 0; - let mut lx = Lexer::new(data, 0); - let mut operands: Vec = Vec::new(); - let mut start: Option = None; - loop { - let trivia = lx.pos; - lx.skip_ws(); - // Only the whitespace the lexer skips here holds top-level comments: a `%` inside a - // string, a name, an array or dictionary operand or inline image data is not one. - if boundary < ends.len() { - comment_edges(data, trivia..lx.pos, ends, &mut boundary, &mut out.comment_edges); + let mut reader = Ops::new(data, ends); + let ops = reader.by_ref().collect(); + Parsed { ops, skipped: reader.skipped, comment_edges: reader.comment_edges } +} + +/// The iterator [`ops`] returns. +pub struct Ops<'a> { + data: &'a [u8], + lx: Lexer<'a>, + operands: Vec, + start: Option, + skipped: usize, + ends: &'a [usize], + boundary: usize, + comment_edges: Vec<(bool, bool)>, +} + +impl<'a> Ops<'a> { + fn new(data: &'a [u8], ends: &'a [usize]) -> Self { + Ops { + data, + lx: Lexer::new(data, 0), + operands: Vec::new(), + start: None, + skipped: 0, + ends, + boundary: 0, + comment_edges: vec![(false, false); ends.len()], } - let Some(&b) = data.get(lx.pos) else { break }; - let at = lx.pos; - let operand_start = matches!(b, b'/' | b'(' | b'<' | b'[' | b'+' | b'-' | b'.') || b.is_ascii_digit(); - if operand_start { - match lx.object() { - Ok(o) => { + } + + /// Bytes read so far that could not be read as operands or operators. + pub fn skipped(&self) -> usize { + self.skipped + } +} + +impl Iterator for Ops<'_> { + type Item = Op; + + fn next(&mut self) -> Option { + let Ops { data, lx, operands, start, skipped, ends, boundary, comment_edges: edges } = self; + let (data, ends): (&[u8], &[usize]) = (data, ends); + loop { + let trivia = lx.pos; + lx.skip_ws(); + // Only the whitespace the lexer skips here holds top-level comments: a `%` inside a + // string, a name, an array or dictionary operand or inline image data is not one. + if *boundary < ends.len() { + comment_edges(data, trivia..lx.pos, ends, boundary, edges); + } + let Some(&b) = data.get(lx.pos) else { + // Operands left without an operator. + *skipped += operands.len(); + operands.clear(); + return None; + }; + let at = lx.pos; + let operand_start = matches!(b, b'/' | b'(' | b'<' | b'[' | b'+' | b'-' | b'.') || b.is_ascii_digit(); + if operand_start { + match lx.object() { + Ok(o) => { + start.get_or_insert(at); + operands.push(o); + } + Err(_) if b == b'(' => { + // An unterminated string runs to the end of the stream. + *skipped += data.len() - at; + lx.pos = data.len(); + } + Err(_) => { + // Skip the offending byte and carry on. + lx.pos = at + 1; + *skipped += 1; + } + } + continue; + } + if is_delim(b) { + // A stray `)`, `>`, `]`, `{`, `}`. + lx.pos += 1; + *skipped += 1; + continue; + } + let kw = lx.token(); + if kw.is_empty() { + lx.pos = at + 1; + *skipped += 1; + continue; + } + match kw { + b"true" => { start.get_or_insert(at); - operands.push(o); + operands.push(Object::Bool(true)); } - Err(_) if b == b'(' => { - // An unterminated string runs to the end of the stream. - out.skipped += data.len() - at; - lx.pos = data.len(); + b"false" => { + start.get_or_insert(at); + operands.push(Object::Bool(false)); } - Err(_) => { - // Skip the offending byte and carry on. - lx.pos = at + 1; - out.skipped += 1; + b"null" => { + start.get_or_insert(at); + operands.push(Object::Null); } - } - continue; - } - if is_delim(b) { - // A stray `)`, `>`, `]`, `{`, `}`. - lx.pos += 1; - out.skipped += 1; - continue; - } - let kw = lx.token(); - if kw.is_empty() { - lx.pos = at + 1; - out.skipped += 1; - continue; - } - match kw { - b"true" => { - start.get_or_insert(at); - operands.push(Object::Bool(true)); - } - b"false" => { - start.get_or_insert(at); - operands.push(Object::Bool(false)); - } - b"null" => { - start.get_or_insert(at); - operands.push(Object::Null); - } - b"BI" => match inline_image(data, lx.pos) { - Ok((dict, img, end)) => { - lx.pos = end; - out.ops.push(Op { op: b"BI".to_vec(), operands: Vec::new(), inline: Some((dict, img)), span: start.take().unwrap_or(at)..end }); - operands.clear(); + b"BI" => match inline_image(data, lx.pos) { + Ok((dict, img, end)) => { + lx.pos = end; + operands.clear(); + return Some(Op { + op: b"BI".to_vec(), + operands: Vec::new(), + inline: Some((dict, img)), + span: start.take().unwrap_or(at)..end, + }); + } + Err(end) => { + *skipped += end.saturating_sub(at); + lx.pos = end; + *start = None; + operands.clear(); + } + }, + _ => { + let s = start.take().unwrap_or(at); + return Some(Op { op: kw.to_vec(), operands: std::mem::take(operands), inline: None, span: s..lx.pos }); } - Err(end) => { - out.skipped += end.saturating_sub(at); - lx.pos = end; - start = None; - operands.clear(); - } - }, - _ => { - let s = start.take().unwrap_or(at); - out.ops.push(Op { op: kw.to_vec(), operands: std::mem::take(&mut operands), inline: None, span: s..lx.pos }); } } } - out.skipped += operands.len(); - out } /// Record the piece boundaries crossed by the comments in `trivia` (skipped whitespace and diff --git a/crates/preflight/src/lib.rs b/crates/preflight/src/lib.rs index 2cda29fea1..4e5f7eb2a1 100644 --- a/crates/preflight/src/lib.rs +++ b/crates/preflight/src/lib.rs @@ -339,8 +339,17 @@ fn is_content(d: &Dict) -> bool { /// The device colour a content stream sets. It is read operator by operator, so the bytes of a /// string, a comment or an inline image are never taken for an operator, and an operator right /// before a delimiter (`1 k/GS0 gs`) is still found. -fn content_colour(data: &[u8], used: &mut DeviceColour) { - for op in pdfcraft_content::parse(data).ops { +/// +/// Operators are read one at a time and dropped, so a stream of millions of tiny operators never +/// becomes millions of parsed ones in memory, and reading stops once all three spaces are set. +/// Returns how many operators it read. +fn content_colour(data: &[u8], used: &mut DeviceColour) -> usize { + let mut read = 0; + for op in pdfcraft_content::ops(data) { + if used.gray && used.rgb && used.cmyk { + break; + } + read += 1; match op.op.as_slice() { b"g" | b"G" => used.gray = true, b"rg" | b"RG" => used.rgb = true, @@ -366,6 +375,7 @@ fn content_colour(data: &[u8], used: &mut DeviceColour) { _ => {} } } + read } /// Whether any content or object uses a device colour space. @@ -400,7 +410,7 @@ fn uses_device_colour(doc: &Document, objs: &[(ObjRef, std::sync::Arc)]) && (referenced.contains(r) || is_content(&s.dict)) && let Ok(data) = s.decoded() { - content_colour(&data, &mut used); + let _ = content_colour(&data, &mut used); } } used diff --git a/crates/preflight/src/tests.rs b/crates/preflight/src/tests.rs index 80b1c63c49..290c7695a0 100644 --- a/crates/preflight/src/tests.rs +++ b/crates/preflight/src/tests.rs @@ -417,3 +417,16 @@ fn oversized_xmp_is_refused_while_decoding() { assert_eq!(stream.decoded().unwrap().len(), xmp::MAX_XMP_BYTES + 1, "fixture must actually decompress"); assert!(metadata(&doc).is_none(), "compressed metadata must be limited during decoding"); } + +/// Content is read one operator at a time and no further than it needs: once gray, RGB and CMYK +/// are all set, the rest of the stream is not read. +#[test] +fn content_colour_reads_operators_one_at_a_time_and_stops_when_all_are_found() { + let mut data = b"0 g 0 0 0 rg 0 0 0 0 k\n".to_vec(); + data.extend(b"n\n".repeat(100_000)); + let mut used = DeviceColour::default(); + let read = content_colour(&data, &mut used); + assert!(used.gray && used.rgb && used.cmyk); + assert_eq!(read, 3); +} + From adab150a4558722503506617de545563308c2e2e Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 16:01:51 +0000 Subject: [PATCH 11/30] fix(preflight): find annotation pages from one pass over the pages verify found each annotation's page by copying and searching every page's /Annots array again for each annotation, so a page with many annotations cost quadratic time. The annotation-to-page map is built once (the first page that lists an annotation keeps it, as before). --- crates/preflight/src/lib.rs | 31 ++++++++++++++++++++----------- crates/preflight/src/tests.rs | 18 ++++++++++++++++++ 2 files changed, 38 insertions(+), 11 deletions(-) diff --git a/crates/preflight/src/lib.rs b/crates/preflight/src/lib.rs index 4e5f7eb2a1..731234aa6b 100644 --- a/crates/preflight/src/lib.rs +++ b/crates/preflight/src/lib.rs @@ -416,6 +416,12 @@ fn uses_device_colour(doc: &Document, objs: &[(ObjRef, std::sync::Arc)]) used } +#[cfg(test)] +thread_local! { + /// Page `/Annots` entries read while matching annotations to their pages. + static ANNOT_ENTRIES_READ: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + /// Check `doc` against `level`. pub fn verify(doc: &Document, level: Level) -> Vec { let mut out = Vec::new(); @@ -486,17 +492,20 @@ pub fn verify(doc: &Document, level: Level) -> Vec { } // Pages: annotations and page actions. let pages = pdfcraft_model::pages(doc); - let page_of = |r: ObjRef| { - pages.iter().position(|p| { - p.dict - .get(b"Annots") - .map(|a| doc.resolve(a)) - .and_then(|a| a.as_array().cloned()) - .unwrap_or_default() - .iter() - .any(|x| x.as_ref() == Some(r)) - }) - }; + // Each annotation's page: the first page that lists it. + let mut annot_pages: std::collections::HashMap = std::collections::HashMap::new(); + for (i, p) in pages.iter().enumerate() { + if let Some(a) = p.dict.get(b"Annots").map(|a| doc.resolve(a)) + && let Some(a) = a.as_array() + { + #[cfg(test)] + ANNOT_ENTRIES_READ.with(|n| n.set(n.get() + a.len())); + for r in a.iter().filter_map(Object::as_ref) { + annot_pages.entry(r).or_insert(i); + } + } + } + let page_of = |r: ObjRef| annot_pages.get(&r).copied(); for (i, p) in pages.iter().enumerate() { if p.dict.get(b"AA").is_some() { issue("6.5.2", "Page {} has additional actions", Some((i + 1).to_string()), Some(i), true); diff --git a/crates/preflight/src/tests.rs b/crates/preflight/src/tests.rs index 290c7695a0..a44d47de15 100644 --- a/crates/preflight/src/tests.rs +++ b/crates/preflight/src/tests.rs @@ -430,3 +430,21 @@ fn content_colour_reads_operators_one_at_a_time_and_stops_when_all_are_found() { assert_eq!(read, 3); } +/// Each annotation's page is found from one pass over the pages' `/Annots`, not by searching +/// every page's list again for each annotation. +#[test] +fn annotation_pages_are_found_in_one_pass() { + let n = 2_000; + let annots: Vec = (0..n).map(|i| format!("{} 0 R", i + 4)).collect(); + let page = format!("<< /Type /Page /Parent 2 0 R /Annots [{}] >>", annots.join(" ")); + let mut objs = + vec!["<< /Type /Catalog /Pages 2 0 R >>".to_string(), "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 200 200] >>".into(), page]; + objs.extend((0..n).map(|_| "<< /Type /Annot /Subtype /Link /Rect [0 0 0 0] /P 3 0 R >>".to_string())); + let refs: Vec<&str> = objs.iter().map(String::as_str).collect(); + let doc = build(&refs, ""); + ANNOT_ENTRIES_READ.with(|c| c.set(0)); + let issues = verify(&doc, Level::A2b); + assert!(issues.iter().any(|i| i.clause == "6.3.2" && i.page == Some(0)), "{issues:#?}"); + let read = ANNOT_ENTRIES_READ.with(std::cell::Cell::get); + assert!(read <= 2 * n, "{read} entries read for {n} annotations"); +} From 90982ba9a86233db45e674baea493fc26196151f Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 16:03:50 +0000 Subject: [PATCH 12/30] fix(preflight): check the actions an allowed action chains to Conversion and verification looked only at an action's own /S. An allowed action (a GoTo, say) whose /Next held a JavaScript or other forbidden action written directly inside it was kept, and verification never looked at actions written directly in a catalog /OpenAction or an annotation /A. Both now follow /Next through the actions written inside an action (an indirect /Next is an object of its own, checked as one), within a bound, and conversion removes the whole chain. --- crates/preflight/src/lib.rs | 63 +++++++++++++++++++++++++++-------- crates/preflight/src/tests.rs | 26 +++++++++++++++ 2 files changed, 76 insertions(+), 13 deletions(-) diff --git a/crates/preflight/src/lib.rs b/crates/preflight/src/lib.rs index 731234aa6b..905ba8cbf3 100644 --- a/crates/preflight/src/lib.rs +++ b/crates/preflight/src/lib.rs @@ -515,12 +515,14 @@ pub fn verify(doc: &Document, level: Level) -> Vec { let Some(d) = dict_of(o) else { continue }; let ty = d.name(b"Type"); let sub = d.name(b"Subtype"); - // Actions anywhere (structure elements also have /S: their role). - if let Some(s) = d.name(b"S") - && FORBIDDEN_ACTIONS.contains(&s) - && is_action(d) - { - issue("6.5.1", "A {} action isn't allowed", Some(String::from_utf8_lossy(s).into_owned()), None, true); + // Actions anywhere (structure elements also have /S: their role), with the actions they + // chain to and those written directly in an /A or /OpenAction. + let own = (d.get(b"S").is_some() && is_action(d)).then_some(d); + let direct = [b"A".as_slice(), b"OpenAction"].map(|k| d.get(k).and_then(|a| a.as_dict())); + for a in [own].into_iter().chain(direct).flatten() { + if let Some(s) = chain_forbidden(a) { + issue("6.5.1", "A {} action isn't allowed", Some(s), None, true); + } } // Annotations. if ty == Some(b"Annot") || (sub.is_some() && d.get(b"Rect").is_some() && d.get(b"P").is_some()) { @@ -652,10 +654,39 @@ fn is_action(d: &Dict) -> bool { } } -/// Whether `o` is (or refers to) a forbidden action, or to nothing. +/// How many actions one action and the `/Next` actions written inside it may hold before the +/// whole chain counts as forbidden, since it can't be checked. +const MAX_ACTION_CHAIN: usize = 1024; + +/// The first forbidden action type in the action `a` or in the actions written directly in its +/// `/Next` (ISO 32000-2 12.6.2: a dictionary or an array of them). An action `/Next` refers to +/// is an object of its own, checked and removed as one. +fn chain_forbidden(a: &Dict) -> Option { + let mut stack = vec![a]; + let mut read = 0; + while let Some(d) = stack.pop() { + read += 1; + if read > MAX_ACTION_CHAIN { + return Some("too deeply chained".into()); + } + if let Some(s) = d.name(b"S") + && FORBIDDEN_ACTIONS.contains(&s) + { + return Some(String::from_utf8_lossy(s).into_owned()); + } + match d.get(b"Next") { + Some(Object::Dict(next)) => stack.push(next), + Some(Object::Array(list)) => stack.extend(list.iter().filter_map(Object::as_dict)), + _ => {} + } + } + None +} + +/// Whether `o` is (or refers to) a forbidden action or one chaining to it, or to nothing. fn forbidden(doc: &Document, o: &Object) -> bool { match &*doc.resolve(o) { - Object::Dict(a) => a.name(b"S").is_some_and(|s| FORBIDDEN_ACTIONS.contains(&s)), + Object::Dict(a) => chain_forbidden(a).is_some(), Object::Null => true, _ => false, } @@ -801,14 +832,20 @@ pub fn convert(doc: &mut Document, level: Level) -> Result>"); + let open_action = format!("<< /Type /Catalog /Pages 2 0 R /OpenAction << /S /GoTo /D [3 0 R /Fit] /Next {js} >> >>"); + let goto = format!("<< /Type /Action /S /GoTo /D [3 0 R /Fit] /Next [{js}] >>"); + let cases = [ + build(&[&open_action, pages, page, &link("<< /S /GoTo /D [3 0 R /Fit] >>")], ""), + build(&["<< /Type /Catalog /Pages 2 0 R >>", pages, page, &link("5 0 R"), &goto], ""), + build(&["<< /Type /Catalog /Pages 2 0 R >>", pages, page, &link(&format!("<< /S /GoTo /D [3 0 R /Fit] /Next {js} >>"))], ""), + ]; + for (i, mut doc) in cases.into_iter().enumerate() { + let found = messages(&verify(&doc, Level::A2b)).contains(&"A JavaScript action isn't allowed"); + assert!(found, "case {i}: {:#?}", verify(&doc, Level::A2b)); + let r = convert(&mut doc, Level::A2b).unwrap(); + assert!(!r.remaining.iter().any(|x| x.clause == "6.5.1"), "case {i}: {:#?}", r.remaining); + let plain = pdfcraft_cos::SaveOptions { object_streams: false, ..Default::default() }; + let bytes = pdfcraft_cos::write_full(&doc, &plain).unwrap(); + assert!(!bytes.windows(11).any(|w| w == b"/JavaScript"), "case {i}: {}", String::from_utf8_lossy(&bytes)); + } +} + /// Each annotation's page is found from one pass over the pages' `/Annots`, not by searching /// every page's list again for each annotation. #[test] From 467c8556ec655be08e5b68b1f35ffbbc974415f7 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 16:08:25 +0000 Subject: [PATCH 13/30] fix(engine): encode each image once for Office export Office export encoded and kept a separate copy of an image for every place a page drew it, so a short content stream repeating Do on one large image multiplied it into gigabytes before the writers' limits applied. Each image object is now encoded once and its bytes shared (pdfcraft_export::Image holds an Arc<[u8]>), distinct pictures share a 512 MiB budget whose overflow lists the page as truncated, and RTF, which leaves pictures out, prepares none. --- crates/engine/src/compare.rs | 50 +++++++++++++++---- crates/engine/src/tests.rs | 2 +- crates/engine/tests/it/main.rs | 1 + .../engine/tests/it/office_export_images.rs | 43 ++++++++++++++++ crates/export/src/lib.rs | 9 ++-- crates/export/src/pptx.rs | 10 +++- 6 files changed, 97 insertions(+), 18 deletions(-) create mode 100644 crates/engine/tests/it/office_export_images.rs diff --git a/crates/engine/src/compare.rs b/crates/engine/src/compare.rs index c23db91f1f..095e666c14 100644 --- a/crates/engine/src/compare.rs +++ b/crates/engine/src/compare.rs @@ -1,8 +1,11 @@ //! Compare files: the text differences between two open documents, a report, and the //! differences marked as comments in the newer one. +use std::collections::HashMap; use std::sync::Arc; +use pdfcraft_cos::ObjRef; + pub use pdfcraft_compare::{Change, Comparison, Kind, Side}; // How far Office export reads into form XObjects; `OfficeExport::truncated_pages` lists the pages // that draw more. @@ -10,6 +13,10 @@ pub use pdfcraft_edit::{MAX_FORM_DEPTH, MAX_FORM_VISITS}; use crate::{DocId, Edit, EditError, Markup, NewAnnotation, NoteIcon, Session, Shape}; +/// Most bytes of encoded pictures one Office export prepares, counting each image object once +/// (the Word and PowerPoint writers keep at most 512 MiB of them as well). +pub const MAX_EXPORT_IMAGE_BYTES: usize = 512 << 20; + /// Acrobat's compare colours: replaced blue, inserted green, deleted red. pub fn colour(kind: Kind) -> crate::Rgb { match kind { @@ -163,23 +170,32 @@ pub struct OfficeExport { /// The exported file. pub bytes: Vec, /// Pages (0-based) that draw forms nested deeper than [`MAX_FORM_DEPTH`] or more of them - /// than [`MAX_FORM_VISITS`]: export stops reading there to stay bounded on hostile files, so - /// the text and images of those forms are missing from `bytes`. + /// than [`MAX_FORM_VISITS`], or pictures past [`MAX_EXPORT_IMAGE_BYTES`]: export stops + /// reading there to stay bounded on hostile files, so the text and images of those forms, or + /// those pictures, are missing from `bytes`. pub truncated_pages: Vec, } +/// An image object prepared for Office export: its file extension and encoded bytes, or why it is +/// left out (`Err(true)`: past [`MAX_EXPORT_IMAGE_BYTES`]; `Err(false)`: it can't be encoded). +type PreparedImage = Result<(&'static str, Arc<[u8]>), bool>; + impl crate::Document { /// The pages as paragraphs and images (for Word, HTML and RTF export), including what /// form XObjects draw. pub fn export_pages(&self) -> Vec { - self.export_pages_checked().0 + self.export_pages_checked(true).0 } - /// [`Self::export_pages`], and the pages whose forms were left partly unread - /// ([`OfficeExport::truncated_pages`]). - fn export_pages_checked(&self) -> (Vec, Vec) { + /// [`Self::export_pages`], and the pages whose forms were left partly unread or whose + /// pictures went past [`MAX_EXPORT_IMAGE_BYTES`] ([`OfficeExport::truncated_pages`]). Without + /// `images` no picture is prepared. + fn export_pages_checked(&self, images: bool) -> (Vec, Vec) { let Some(cos) = self.editor.as_ref().map(|e| &e.cos) else { return (Vec::new(), Vec::new()) }; let mut truncated = Vec::new(); + // Each image object is encoded once and shared by every place that draws it. + let mut encoded: HashMap = HashMap::new(); + let mut image_bytes = MAX_EXPORT_IMAGE_BYTES; let pages = self .info .pages @@ -217,13 +233,21 @@ impl crate::Document { } }) .collect(); - let images = pdfcraft_edit::reading_images(cos, i) - .unwrap_or_default() + let placed = if images { pdfcraft_edit::reading_images(cos, i).unwrap_or_default() } else { Vec::new() }; + let mut over_budget = false; + let images = placed .iter() .filter_map(|im| { - let (ext, bytes) = pdfcraft_create::image_file(cos, im.object?).ok()?; + let object = im.object?; + let prepared = encoded.entry(object).or_insert_with(|| { + let (ext, bytes) = pdfcraft_create::image_file(cos, object).map_err(|_| false)?; + image_bytes = image_bytes.checked_sub(bytes.len()).ok_or(true)?; + Ok((if ext == "jpg" { "jpg" } else { "png" }, Arc::from(bytes))) + }); + over_budget |= matches!(prepared, Err(true)); + let (ext, bytes) = prepared.clone().ok()?; Some(pdfcraft_export::Image { - ext: if ext == "jpg" { "jpg" } else { "png" }, + ext, bytes, rect: visible_rect(im.rect), size: [im.matrix[0].hypot(im.matrix[1]) * unit, im.matrix[2].hypot(im.matrix[3]) * unit], @@ -232,6 +256,9 @@ impl crate::Document { }) .collect(); // The page colour (Edit a PDF ▸ Background, a producer's page fill) for formats that keep one. + if over_budget { + truncated.push(i); + } let background = pdfcraft_edit::page_background(cos, i).ok().flatten(); pdfcraft_export::Page { width: info.width as f64, height: info.height as f64, blocks, images, background } }) @@ -247,7 +274,8 @@ impl crate::Document { /// [`Self::export_office`], and the pages whose text it had to leave out /// ([`OfficeExport::truncated_pages`]). pub fn export_office_checked(&self, format: OfficeFormat) -> OfficeExport { - let (pages, truncated_pages) = self.export_pages_checked(); + // RTF leaves pictures out. + let (pages, truncated_pages) = self.export_pages_checked(format != OfficeFormat::Rtf); let title = self.info.title.clone().unwrap_or_else(|| self.name.trim_end_matches(".pdf").to_string()); let bytes = match format { OfficeFormat::Docx => pdfcraft_export::docx(&pages, &title), diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 3d347e6492..47ad482d46 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -6645,7 +6645,7 @@ fn exporting_a_jpeg_logo_to_word_keeps_the_image() { assert!(jpeg.starts_with(&[0xFF, 0xD8])); let docx = d.export_office(compare::OfficeFormat::Docx); assert!(docx.windows(21).any(|w| w == b"word/media/image1.jpg"), "overlap {overlaps}"); - assert!(docx.windows(jpeg.len()).any(|w| w == jpeg.as_slice()), "the JPEG is stored unchanged (overlap {overlaps})"); + assert!(docx.windows(jpeg.len()).any(|w| w == &jpeg[..]), "the JPEG is stored unchanged (overlap {overlaps})"); } } diff --git a/crates/engine/tests/it/main.rs b/crates/engine/tests/it/main.rs index 15e1a8548b..8d9026600d 100644 --- a/crates/engine/tests/it/main.rs +++ b/crates/engine/tests/it/main.rs @@ -8,5 +8,6 @@ mod document_fonts; mod form_rotation; mod marked_content_isolation; mod ocr_edit; +mod office_export_images; mod private_image_resize; mod private_text_glyphs; diff --git a/crates/engine/tests/it/office_export_images.rs b/crates/engine/tests/it/office_export_images.rs new file mode 100644 index 0000000000..4f2ff618fb --- /dev/null +++ b/crates/engine/tests/it/office_export_images.rs @@ -0,0 +1,43 @@ +//! Office export of pages that draw one image many times. + +use std::sync::Arc; + +use pdfcraft_engine::Session; + +fn pdf(objects: &[String]) -> Vec { + let mut out = b"%PDF-1.7\n".to_vec(); + let mut offsets = Vec::new(); + for (i, o) in objects.iter().enumerate() { + offsets.push(out.len()); + out.extend_from_slice(format!("{} 0 obj\n{o}\nendobj\n", i + 1).as_bytes()); + } + let xref = out.len(); + out.extend_from_slice(format!("xref\n0 {}\n0000000000 65535 f \n", objects.len() + 1).as_bytes()); + for o in offsets { + out.extend_from_slice(format!("{o:010} 00000 n \n").as_bytes()); + } + out.extend_from_slice(format!("trailer\n<< /Size {} /Root 1 0 R >>\nstartxref\n{xref}\n%%EOF\n", objects.len() + 1).as_bytes()); + out +} + +/// One image drawn many times is prepared once: every placement shares its encoded bytes. +#[test] +fn an_image_drawn_many_times_is_encoded_once() { + let placements = 500; + let content = "q 10 0 0 10 0 0 cm /Im Do Q\n".repeat(placements); + let bytes = pdf(&[ + "<< /Type /Catalog /Pages 2 0 R >>".into(), + "<< /Type /Pages /Kids [3 0 R] /Count 1 >>".into(), + "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 200 200] /Contents 4 0 R /Resources << /XObject << /Im 5 0 R >> >> >>".into(), + format!("<< /Length {} >>\nstream\n{content}endstream", content.len()), + "<< /Type /XObject /Subtype /Image /Width 2 /Height 2 /ColorSpace /DeviceGray /BitsPerComponent 8 /Length 4 >>\nstream\n\u{0}\u{40}\u{80}\u{ff}\nendstream".into(), + ]); + let mut s = Session::new(); + let id = s.open("many.pdf", None, Arc::new(bytes), None).unwrap(); + let pages = s.get(id).unwrap().export_pages(); + let images = &pages[0].images; + assert_eq!(images.len(), placements); + let first = images[0].bytes.as_ptr(); + assert!(!images[0].bytes.is_empty()); + assert!(images.iter().all(|im| im.bytes.as_ptr() == first), "each placement holds its own copy of the image"); +} diff --git a/crates/export/src/lib.rs b/crates/export/src/lib.rs index f3030ad58d..b4ea9e9652 100644 --- a/crates/export/src/lib.rs +++ b/crates/export/src/lib.rs @@ -35,7 +35,8 @@ pub struct Block { pub struct Image { /// "png" or "jpg". pub ext: &'static str, - pub bytes: Vec, + /// Shared: a picture drawn in many places is held once. + pub bytes: std::sync::Arc<[u8]>, pub rect: [f64; 4], /// Width and height before rotation, in points. pub size: [f64; 2], @@ -932,7 +933,7 @@ mod tests { ], images: vec![Image { ext: "png", - bytes: b"\x89PNG\r\n\x1a\n\0\0\0\rIHDR\0\0\0\x02\0\0\0\x03".to_vec(), + bytes: b"\x89PNG\r\n\x1a\n\0\0\0\rIHDR\0\0\0\x02\0\0\0\x03".as_slice().into(), rect: [72.0, 450.0, 272.0, 600.0], size: [200.0, 150.0], rotation: 0.0, @@ -1374,7 +1375,7 @@ mod tests { /// file in the package, with a content type; JPEG pictures too, inline or placed. #[test] fn word_pictures_resolve_to_media_in_the_package() { - let jpeg = Image { ext: "jpg", bytes: vec![0xFF, 0xD8, 0xFF, 0xD9], ..page().images[0].clone() }; + let jpeg = Image { ext: "jpg", bytes: [0xFF, 0xD8, 0xFF, 0xD9].as_slice().into(), ..page().images[0].clone() }; for placed in [false, true] { let mut p = page(); p.images = vec![jpeg.clone()]; @@ -1401,7 +1402,7 @@ mod tests { fn word_export_bounds_the_number_of_pictures_and_skips_empty_ones() { let mut p = page(); let one = p.images[0].clone(); - p.images = vec![Image { bytes: Vec::new(), ..one.clone() }]; + p.images = vec![Image { bytes: [].as_slice().into(), ..one.clone() }]; p.images.extend(std::iter::repeat_n(one, MAX_MEDIA + 10)); let zip = docx(&[p], "T"); let rels = part(&zip, "word/_rels/document.xml.rels"); diff --git a/crates/export/src/pptx.rs b/crates/export/src/pptx.rs index 1a7250d9e0..886a4bdbde 100644 --- a/crates/export/src/pptx.rs +++ b/crates/export/src/pptx.rs @@ -391,7 +391,7 @@ mod tests { ], images: vec![Image { ext: "png", - bytes: b"\x89PNG\r\n\x1a\n\0\0\0\rIHDR\0\0\0\x02\0\0\0\x03".to_vec(), + bytes: b"\x89PNG\r\n\x1a\n\0\0\0\rIHDR\0\0\0\x02\0\0\0\x03".as_slice().into(), rect: [72.0, 150.0, 300.0, 300.0], size: [228.0, 150.0], rotation: 0.0, @@ -619,7 +619,13 @@ mod tests { blocks.push(huge("inf", f64::INFINITY, 200.0 - k as f64)); } let images = (0..40) - .map(|k| Image { ext: "png", bytes: vec![0; 4], rect: [0.0, -1e300, 1e-300, 1e300 - k as f64], size: [1.0, 1.0], rotation: 0.0 }) + .map(|k| Image { + ext: "png", + bytes: [0; 4].as_slice().into(), + rect: [0.0, -1e300, 1e-300, 1e300 - k as f64], + size: [1.0, 1.0], + rotation: 0.0, + }) .collect(); let p = Page { width: f64::MAX, height: f64::NAN, blocks, images, background: None }; let bytes = pptx(&[p], "Hostile"); From f2f86455ddb0aa538bcb9c159b87effe0342aa08 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 16:09:45 +0000 Subject: [PATCH 14/30] fix(engine): read long underscore runs in one pass Word assembly checked whether the whole word so far was underscores for every glyph it added, so a long run of underscores on one line took quadratic time. The underscore state of the current word is kept as it grows. --- crates/engine/src/js.rs | 41 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 36 insertions(+), 5 deletions(-) diff --git a/crates/engine/src/js.rs b/crates/engine/src/js.rs index a2c444bd62..c80537fbad 100644 --- a/crates/engine/src/js.rs +++ b/crates/engine/src/js.rs @@ -289,6 +289,12 @@ pub(crate) fn page_words(text: &pdfcraft_render::PageText, info: &pdfcraft_rende .collect() } +#[cfg(test)] +thread_local! { + /// Characters read to tell underscore runs from other words. + static UNDERSCORE_CHARS_READ: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + /// A page's words in reading order, their boxes mapped from view space by `map` (and ordered /// again, so that [x0, y0, x1, y1] holds whatever way `map` turns them). pub(crate) fn words_with(text: &pdfcraft_render::PageText, map: impl Fn(f32, f32) -> [f32; 2]) -> Vec<(String, [f32; 4])> { @@ -296,6 +302,8 @@ pub(crate) fn words_with(text: &pdfcraft_render::PageText, map: impl Fn(f32, f32 let mut last_line = u32::MAX; // A space glyph ends the word before it. let mut gap = false; + // Whether the last word is an underscore run (a form's blank), which other glyphs don't join. + let mut last_under = false; for (i, g) in text.glyphs.iter().enumerate() { let line = text.line_of.get(i).copied().unwrap_or(0); if g.text.trim().is_empty() { @@ -303,10 +311,9 @@ pub(crate) fn words_with(text: &pdfcraft_render::PageText, map: impl Fn(f32, f32 continue; } let under = g.text.chars().all(|c| c == '_'); - let breaks = match words.last() { - None => true, - Some((w, _)) => gap || line != last_line || text.space_before.get(i).copied().unwrap_or(false) || w.chars().all(|c| c == '_') != under, - }; + #[cfg(test)] + UNDERSCORE_CHARS_READ.with(|n| n.set(n.get() + g.text.len())); + let breaks = words.is_empty() || gap || line != last_line || text.space_before.get(i).copied().unwrap_or(false) || last_under != under; gap = false; last_line = line; match words.last_mut() { @@ -317,7 +324,10 @@ pub(crate) fn words_with(text: &pdfcraft_render::PageText, map: impl Fn(f32, f32 r[2] = r[2].max(g.rect[2]); r[3] = r[3].max(g.rect[3]); } - _ => words.push((g.text.clone(), g.rect)), + _ => { + words.push((g.text.clone(), g.rect)); + last_under = under; + } } } words @@ -409,6 +419,27 @@ impl Session { mod review_tests { use super::*; + /// A long run of underscores (a form's blank) is read in one pass: each glyph is compared with + /// the word so far without reading the word again. + #[test] + fn a_long_underscore_run_is_read_in_one_pass() { + let n = 10_000; + let glyph = |i: usize, t: &str| pdfcraft_render::TextGlyph { + text: t.into(), + rect: [i as f32, 0.0, i as f32 + 1.0, 10.0], + quad: [[0.0; 2]; 4], + direction: [1.0, 0.0], + }; + let mut glyphs: Vec<_> = (0..n).map(|i| glyph(i, "_")).collect(); + glyphs.push(glyph(n, "x")); + let text = pdfcraft_render::PageText { line_of: vec![0; glyphs.len()], space_before: vec![false; glyphs.len()], glyphs }; + UNDERSCORE_CHARS_READ.with(|c| c.set(0)); + let words = words_with(&text, |x, y| [x, y]); + assert_eq!(words.iter().map(|(w, _)| w.len()).collect::>(), [n, 1]); + let read = UNDERSCORE_CHARS_READ.with(std::cell::Cell::get); + assert!(read <= 4 * n, "{read} characters read for {n} glyphs"); + } + #[test] fn duplicate_field_names_diff_against_first_original() { let first = pdfcraft_js::FieldState::new("duplicate", pdfcraft_js::FieldType::Text, vec!["first".into()]); From 2d5c5054e22b77e1bce18bd43cf8398dfc93f2f9 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 16:11:06 +0000 Subject: [PATCH 15/30] fix(forms): merge a calculation's read names with a set calculation_order merged the names a Calculate script reads into the field's other reads by searching the list so far for each name, so a script naming many fields took quadratic time before the link limits applied. The merge looks names up in a set. --- crates/forms/src/calc_order.rs | 11 ++++++++++- crates/forms/src/tests.rs | 17 +++++++++++++++++ 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/crates/forms/src/calc_order.rs b/crates/forms/src/calc_order.rs index e75a05d475..428c1724d0 100644 --- a/crates/forms/src/calc_order.rs +++ b/crates/forms/src/calc_order.rs @@ -122,6 +122,12 @@ pub(crate) fn run_order(doc: &Document, fields: &[Field]) -> (Vec<(usize, bool)> (run, extra) } +#[cfg(test)] +thread_local! { + /// Read names compared while merging a field's reads. + pub(crate) static NAMES_COMPARED: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + /// The form's calculation order (`fields` as [`fields`] reads them), with each calculated field's /// reads and issues, and Fix order's order. pub fn calculation_order(doc: &Document, fields: &[Field]) -> CalculationOrder { @@ -132,8 +138,11 @@ pub fn calculation_order(doc: &Document, fields: &[Field]) -> CalculationOrder { .map(|(f, _)| { let mut names = f.actions.calculate.reads(); if let Some(js) = &f.actions.scripts.calculate { + let mut known: HashSet = names.iter().cloned().collect(); for name in crate::af::calculation_reads(js) { - if !names.contains(&name) { + #[cfg(test)] + NAMES_COMPARED.with(|n| n.set(n.get() + 1)); + if known.insert(name.clone()) { names.push(name); } } diff --git a/crates/forms/src/tests.rs b/crates/forms/src/tests.rs index b2e4994ecf..2b20428650 100644 --- a/crates/forms/src/tests.rs +++ b/crates/forms/src/tests.rs @@ -2981,3 +2981,20 @@ fn page_walk_skips_repeated_and_cyclic_kids_without_queueing_them() { assert_eq!(walk.next(), None); assert_eq!(page_refs(&doc), [page]); } + +/// A Calculate script naming thousands of fields is merged with the field's other reads in one +/// pass, not by searching the names so far for each one. +#[test] +fn a_script_reading_many_fields_merges_its_reads_in_one_pass() { + use crate::calc_order::NAMES_COMPARED; + let mut doc = one_page(); + add_field(&mut doc, 0, [50.0, 700.0, 250.0, 720.0], &NewField::Text { multiline: false }, Some("total")).unwrap(); + let n = 2_000; + let script: String = (0..n).map(|i| format!("getField(\"f{i}\").value + ")).collect::() + "0"; + set_field_script(&mut doc, "total", "calculate", Some(&format!("event.value = {script};"))).unwrap(); + NAMES_COMPARED.with(|c| c.set(0)); + let order = calculation_order(&doc, &fields(&doc)); + assert_eq!(order.fields[0].reads.len(), n); + let compared = NAMES_COMPARED.with(std::cell::Cell::get); + assert!(compared <= 4 * n, "{compared} names compared for {n} reads"); +} From b6cb15177484df065f7aa87231cf2e4f63785127 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 16:12:06 +0000 Subject: [PATCH 16/30] fix(measure): read a stream's XObject resources once when snapping Every Do operator resolved the stream's /XObject resources again, and resolving a direct dictionary copies it, so a large direct XObject dictionary and many short Do operators multiplied the copying. The dictionary is read on the first Do and kept for the rest of the stream. --- crates/measure/src/snap.rs | 14 +++++++++++++- crates/measure/src/tests.rs | 11 +++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) diff --git a/crates/measure/src/snap.rs b/crates/measure/src/snap.rs index 3cf6bbd17e..37ee2975f7 100644 --- a/crates/measure/src/snap.rs +++ b/crates/measure/src/snap.rs @@ -250,6 +250,8 @@ impl Walker<'_> { let mut current = None; let mut closures = Vec::new(); let mut path = Geometry::default(); + // Read on the first `Do` and kept: resolving a direct dictionary copies it. + let mut xobjects = None; let point = |m: Matrix, x: f64, y: f64| { let (x, y) = m.apply(x, y); [x, y] @@ -384,7 +386,11 @@ impl Walker<'_> { } "Do" => { if let Some(name) = op.name(0) { - let xobjects = resources.get(b"XObject").map(|o| self.doc.resolve(o)); + let xobjects = xobjects.get_or_insert_with(|| { + #[cfg(test)] + XOBJECT_DICT_READS.with(|n| n.set(n.get() + 1)); + resources.get(b"XObject").map(|o| self.doc.resolve(o)) + }); if let Some(o) = xobjects.as_ref().and_then(|o| o.as_dict()).and_then(|d| d.get(name)) { let r = o.as_ref(); if r.is_some_and(|r| self.seen.contains(&r)) { @@ -420,6 +426,12 @@ impl Walker<'_> { Ok(()) } } +#[cfg(test)] +thread_local! { + /// Times a content stream's `/XObject` resources were read. + pub(crate) static XOBJECT_DICT_READS: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + pub fn geometry(doc: &Document, page_index: usize) -> Result { let p = crate::page(doc, page_index)?; let resources = p.dict.get(b"Resources").map(|o| doc.resolve(o)); diff --git a/crates/measure/src/tests.rs b/crates/measure/src/tests.rs index 303f9b4386..06bc1e42db 100644 --- a/crates/measure/src/tests.rs +++ b/crates/measure/src/tests.rs @@ -403,3 +403,14 @@ fn a_scale_ratio_converts_its_page_side_to_points() { assert!(Scale::ratio([0.0, 0.0], [1.0, 0.0], page, PageUnit::Mm, real, "mm", 2).is_err(), "{page} {real}"); } } +/// A content stream reads its `/XObject` resources once, not again (as a copy of a direct +/// dictionary) for every `Do`. +#[test] +fn xobject_resources_are_read_once_per_stream() { + let d = fixture(&"/Missing Do\n".repeat(10_000), "", &[]); + snap::XOBJECT_DICT_READS.with(|c| c.set(0)); + let paths = snap::geometry(&d, 0).unwrap(); + assert!(paths.segments.is_empty()); + let reads = snap::XOBJECT_DICT_READS.with(std::cell::Cell::get); + assert!(reads <= 1, "{reads} reads of the page's XObject resources"); +} From 240d051b4e9973c5d980560657f6377c63f0856b Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 16:12:43 +0000 Subject: [PATCH 17/30] fix(measure): read viewports within bounds when finding a scale scale_at converted every number of a viewport's /BBox before checking it had four, and read every viewport the page listed, so many viewports sharing one long box array took time proportional to their product. A box of any other length is skipped unread, and at most 1024 viewports are read, the most set_scale gives a page. --- crates/measure/src/lib.rs | 20 +++++++++++++++++--- crates/measure/src/tests.rs | 13 +++++++++++++ 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/crates/measure/src/lib.rs b/crates/measure/src/lib.rs index 72b6c4b07b..dda45a75ae 100644 --- a/crates/measure/src/lib.rs +++ b/crates/measure/src/lib.rs @@ -497,6 +497,15 @@ pub fn list(doc: &Document) -> Listing { } out } +/// Most viewports [`set_scale`] gives a page, and most [`scale_at`] reads. +const MAX_VIEWPORTS: usize = 1024; + +#[cfg(test)] +thread_local! { + /// Numbers read from viewport `/BBox` arrays. + pub(crate) static VIEWPORT_NUMBERS_READ: std::cell::Cell = const { std::cell::Cell::new(0) }; +} + /// Last containing viewport wins, using the first point of the measurement (ISO 32000-2 §12.9). /// Outside every viewport the page measures true size in inches. pub fn scale_at(doc: &Document, page_index: usize, at: Point) -> Result { @@ -510,12 +519,17 @@ pub fn scale_at_or(doc: &Document, page_index: usize, at: Point, per_point: f64, if let Some(o) = p.dict.get(b"VP") { let o = doc.resolve(o); let arr = o.as_array().ok_or_else(|| invalid("invalid page viewports"))?; - for viewport in arr.iter().rev() { + // The last viewports win; a page holds at most MAX_VIEWPORTS of its own. + for viewport in arr.iter().rev().take(MAX_VIEWPORTS) { let o = doc.resolve(viewport); let Some(d) = o.as_dict() else { continue }; if let Some(bbox) = d.get(b"BBox") { let o = doc.resolve(bbox); - let b: Option> = o.as_array().and_then(|a| a.iter().map(|v| doc.resolve(v).as_f64()).collect()); + let b: Option> = o.as_array().filter(|a| a.len() == 4).and_then(|a| { + #[cfg(test)] + VIEWPORT_NUMBERS_READ.with(|n| n.set(n.get() + a.len())); + a.iter().map(|v| doc.resolve(v).as_f64()).collect() + }); if let Some(b) = b.filter(|b| b.len() == 4 && b.iter().all(|v| v.is_finite())) && at[0] >= b[0].min(b[2]) && at[0] <= b[0].max(b[2]) @@ -541,7 +555,7 @@ pub fn set_scale(doc: &mut Document, page_index: usize, bbox: [f64; 4], name: &s Some(o) => doc.resolve(o).as_array().cloned().ok_or_else(|| invalid("invalid page viewports"))?, None => Vec::new(), }; - if list.len() >= 1024 { + if list.len() >= MAX_VIEWPORTS { return Err(invalid("page has too many measurement viewports")); } let mut d = Dict::new(); diff --git a/crates/measure/src/tests.rs b/crates/measure/src/tests.rs index 06bc1e42db..259caeb5f3 100644 --- a/crates/measure/src/tests.rs +++ b/crates/measure/src/tests.rs @@ -414,3 +414,16 @@ fn xobject_resources_are_read_once_per_stream() { let reads = snap::XOBJECT_DICT_READS.with(std::cell::Cell::get); assert!(reads <= 1, "{reads} reads of the page's XObject resources"); } +/// Finding the scale under a point reads only well-formed viewport boxes, and only as many +/// viewports as a page can be given, however many a file lists or however long their boxes. +#[test] +fn scale_lookup_reads_viewports_within_bounds() { + let vp = vec!["5 0 R"; 5_000].join(" "); + let long_box = vec!["0"; 2_000].join(" "); + let d = fixture("", &format!("/VP [{vp}]"), &["<< /Type /Viewport /BBox 6 0 R >>", &format!("[{long_box}]")]); + VIEWPORT_NUMBERS_READ.with(|c| c.set(0)); + let scale = scale_at(&d, 0, [10.0, 10.0]).unwrap(); + assert_eq!(scale.unit, "in"); + let read = VIEWPORT_NUMBERS_READ.with(std::cell::Cell::get); + assert!(read <= 4 * 1024, "{read} viewport numbers read"); +} From 05d09de1e337c3dc40bac6286721fe85239e6c8b Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 16:13:45 +0000 Subject: [PATCH 18/30] fix(annot): read a shared page tree Kids array once The page walk pushed a node's /Kids array for every new node, so many page tree nodes sharing one indirect /Kids array re-read it for each node: quadratic work while the seen check skipped every entry. A shared array object is read under the first node that lists it, and the walk stops after four million /Kids entries. --- crates/annot/src/lib.rs | 21 ++++++++++++++++----- crates/annot/src/tests.rs | 22 ++++++++++++++++++++++ 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/crates/annot/src/lib.rs b/crates/annot/src/lib.rs index 6edfbf9cb7..61dd5ff1cc 100644 --- a/crates/annot/src/lib.rs +++ b/crates/annot/src/lib.rs @@ -665,17 +665,20 @@ pub fn page_refs(doc: &Document) -> Result, AnnotError> { /// Depth-first walk over the leaf pages under `top`. It keeps one cursor per open `/Kids` array, /// never one entry per child, so a child listed many times or shared between nodes is skipped by /// the seen check without being queued first (`pdfcraft_model::pages` walks the same way). -/// Cycles end at the seen check and at most a million nodes are visited. +/// Cycles end at the seen check, a `/Kids` array object shared by several nodes is read once, +/// and at most a million nodes and four million `/Kids` entries are read. struct PageWalk<'a> { doc: &'a Document, seen: std::collections::HashSet, pending: Vec<(std::sync::Arc, usize)>, + /// `/Kids` entries read so far. + examined: usize, } impl<'a> PageWalk<'a> { fn new(doc: &'a Document, top: ObjRef) -> Self { let top = std::sync::Arc::new(Object::Array(vec![Object::Ref(top)])); - Self { doc, seen: Default::default(), pending: vec![(top, 0)] } + Self { doc, seen: Default::default(), pending: vec![(top, 0)], examined: 0 } } } @@ -690,8 +693,9 @@ impl Iterator for PageWalk<'_> { }; let node = kid.as_ref(); *next += 1; + self.examined += 1; let Some(node) = node else { continue }; - if self.seen.len() >= 1_000_000 { + if self.seen.len() >= 1_000_000 || self.examined > 4_000_000 { self.pending.clear(); return None; } @@ -700,8 +704,15 @@ impl Iterator for PageWalk<'_> { } let obj = self.doc.get(node); let Some(d) = obj.as_dict() else { continue }; - match d.get(b"Kids").map(|k| self.doc.resolve(k)) { - Some(kids) if d.name(b"Type") != Some(b"Page") => self.pending.push((kids, 0)), + match d.get(b"Kids") { + // A `/Kids` array object shared by several nodes is read under the first: under + // the others it would only list nodes already seen. + Some(Object::Ref(array)) if d.name(b"Type") != Some(b"Page") => { + if self.seen.insert(*array) { + self.pending.push((self.doc.get(*array), 0)); + } + } + Some(kids) if d.name(b"Type") != Some(b"Page") => self.pending.push((self.doc.resolve(kids), 0)), _ => return Some(node), } } diff --git a/crates/annot/src/tests.rs b/crates/annot/src/tests.rs index a659966812..44328bbd06 100644 --- a/crates/annot/src/tests.rs +++ b/crates/annot/src/tests.rs @@ -1504,3 +1504,25 @@ fn typed_signature_uses_nonzero_winding_fill() { assert!(ap.ends_with("h\nf\n"), "expected non-zero winding fill (`f`), got: {ap}"); assert!(!ap.contains("f*"), "must not use even-odd fill (`f*`): {ap}"); } + +/// Many page tree nodes sharing one `/Kids` array read it once, not once per node. +#[test] +fn page_walk_reads_a_shared_kids_array_once() { + let mut doc = fixture(); + let root = ObjRef { num: 2, generation: 0 }; + let (first, second) = (ObjRef { num: 3, generation: 0 }, ObjRef { num: 4, generation: 0 }); + let shared = doc.add(Object::Array([Object::Ref(first), Object::Ref(second)].into_iter().cycle().take(2_000).collect())); + let nodes: Vec = (0..1_000) + .map(|_| { + let mut d = Dict::new(); + d.set(b"Type".to_vec(), Object::name("Pages")); + d.set(b"Kids".to_vec(), Object::Ref(shared)); + Object::Ref(doc.add(d)) + }) + .collect(); + doc.update_dict(root, |d| d.set(b"Kids".to_vec(), Object::Array(nodes))).unwrap(); + let mut walk = PageWalk::new(&doc, root); + let pages: Vec = walk.by_ref().collect(); + assert_eq!(pages, [first, second]); + assert!(walk.examined <= 10_000, "{} kids entries read", walk.examined); +} From c2e96de0b240f6d41ef340cc50c29d9e74198232 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 16:14:34 +0000 Subject: [PATCH 19/30] fix(annot): extend each page's annotation list once when adding many links add_many added links one at a time, and each addition walked the page tree and copied and rewrote the page's whole /Annots array, so a batch of links cost quadratic time. The pages are found once and each page's new links are appended in one update. --- crates/annot/src/lib.rs | 11 ++++++++++- crates/annot/src/links.rs | 37 ++++++++++++++++++++++++++++++------- crates/annot/src/tests.rs | 15 +++++++++++++++ 3 files changed, 55 insertions(+), 8 deletions(-) diff --git a/crates/annot/src/lib.rs b/crates/annot/src/lib.rs index 61dd5ff1cc..d1d74ce410 100644 --- a/crates/annot/src/lib.rs +++ b/crates/annot/src/lib.rs @@ -728,7 +728,16 @@ fn page_ref(doc: &Document, page: usize) -> Result { fn annots(doc: &Document, page: ObjRef) -> Vec { let obj = doc.get(page); let Some(a) = obj.as_dict().and_then(|d| d.get(b"Annots")) else { return Vec::new() }; - doc.resolve(a).as_array().cloned().unwrap_or_default() + let list = doc.resolve(a).as_array().cloned().unwrap_or_default(); + #[cfg(test)] + ANNOTS_ENTRIES_COPIED.with(|n| n.set(n.get() + list.len())); + list +} + +#[cfg(test)] +thread_local! { + /// Entries copied out of pages' `/Annots` arrays. + static ANNOTS_ENTRIES_COPIED: std::cell::Cell = const { std::cell::Cell::new(0) }; } /// Replace a page's `/Annots`, writing into the shared array object when it is indirect. diff --git a/crates/annot/src/links.rs b/crates/annot/src/links.rs index 4d3eef632b..f11eecbd83 100644 --- a/crates/annot/src/links.rs +++ b/crates/annot/src/links.rs @@ -232,6 +232,16 @@ fn checked_rect(rect: [f64; 4]) -> Result<[f64; 4], AnnotError> { /// Add a link over `rect` (user space). Returns its index in the page's `/Annots`. pub fn add(doc: &mut Document, page: usize, rect: [f64; 4], action: &LinkAction, style: &LinkStyle) -> Result { let p = page_ref(doc, page)?; + let obj = new_link(doc, p, rect, action, style)?; + let mut list = annots(doc, p); + list.push(Object::Ref(obj)); + let index = list.len() - 1; + set_annots(doc, p, list)?; + Ok(index) +} + +/// A link annotation object for page `p`, not yet on the page's `/Annots`. +fn new_link(doc: &mut Document, p: ObjRef, rect: [f64; 4], action: &LinkAction, style: &LinkStyle) -> Result { let r = checked_rect(rect)?; if matches!(action, LinkAction::Other(_)) { return Err(AnnotError::Invalid("choose a page or a web address".into())); @@ -244,12 +254,7 @@ pub fn add(doc: &mut Document, page: usize, rect: [f64; 4], action: &LinkAction, d.set(b"F".to_vec(), Object::Int(4)); write_style(&mut d, style); write_action(doc, &mut d, action)?; - let obj = doc.add(Object::Dict(d)); - let mut list = annots(doc, p); - list.push(Object::Ref(obj)); - let index = list.len() - 1; - set_annots(doc, p, list)?; - Ok(index) + Ok(doc.add(Object::Dict(d))) } fn link_at(doc: &Document, page: usize, index: usize) -> Result<(ObjRef, ObjRef), AnnotError> { @@ -414,12 +419,30 @@ pub fn find_urls(chars: &[char]) -> Vec<(std::ops::Range, String)> { /// Create links for found URLs: `(page, quads, uri)` with quads in user space (one link per /// line rectangle). Returns how many links were added. pub fn add_many(doc: &mut Document, links: &[(usize, Vec<[f64; 4]>, String)], style: &LinkStyle) -> Result { + let pages = page_refs(doc)?; + // Each page's `/Annots` is extended once, with its new links in the order given. + let mut added: Vec<(ObjRef, Vec)> = Vec::new(); + let mut slot: std::collections::HashMap = std::collections::HashMap::new(); let mut n = 0; for (page, rects, uri) in links { + let p = pages.get(*page).copied().ok_or(AnnotError::NoSuchPage(*page))?; + let action = LinkAction::Uri(uri.clone()); for r in rects { - add(doc, *page, *r, &LinkAction::Uri(uri.clone()), style)?; + let obj = new_link(doc, p, *r, &action, style)?; + let k = *slot.entry(p).or_insert_with(|| { + added.push((p, Vec::new())); + added.len() - 1 + }); + if let Some((_, list)) = added.get_mut(k) { + list.push(Object::Ref(obj)); + } n += 1; } } + for (p, new) in added { + let mut list = annots(doc, p); + list.extend(new); + set_annots(doc, p, list)?; + } Ok(n) } diff --git a/crates/annot/src/tests.rs b/crates/annot/src/tests.rs index 44328bbd06..cfd80093e1 100644 --- a/crates/annot/src/tests.rs +++ b/crates/annot/src/tests.rs @@ -1526,3 +1526,18 @@ fn page_walk_reads_a_shared_kids_array_once() { assert_eq!(pages, [first, second]); assert!(walk.examined <= 10_000, "{} kids entries read", walk.examined); } + +/// Adding many links to a page extends its `/Annots` once, not once per link. +#[test] +fn many_links_extend_a_page_annotation_list_once() { + let mut doc = fixture(); + let before = annots(&doc, page_refs(&doc).unwrap()[0]).len(); + let n = 2_000; + let rects: Vec<[f64; 4]> = (0..n).map(|i| [10.0, i as f64 * 0.1, 50.0, i as f64 * 0.1 + 5.0]).collect(); + ANNOTS_ENTRIES_COPIED.with(|c| c.set(0)); + let added = links::add_many(&mut doc, &[(0, rects, "https://example.com".into())], &links::LinkStyle::default()).unwrap(); + let copied = ANNOTS_ENTRIES_COPIED.with(std::cell::Cell::get); + assert_eq!(added, n); + assert_eq!(annots(&doc, page_refs(&doc).unwrap()[0]).len(), before + n); + assert!(copied <= 4 * n, "{copied} annotation entries copied for {n} links"); +} From 8adf8e2d3b4a86aaa688f3347e07560f85d11530 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 17:58:47 +0000 Subject: [PATCH 20/30] fix(xfdf): read an indirect /F when deciding a comment is locked against import --- crates/xfdf/src/lib.rs | 7 ++++--- crates/xfdf/src/tests.rs | 23 +++++++++++++++++++++++ 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/crates/xfdf/src/lib.rs b/crates/xfdf/src/lib.rs index 99470b58a1..5c0be52326 100644 --- a/crates/xfdf/src/lib.rs +++ b/crates/xfdf/src/lib.rs @@ -1037,8 +1037,9 @@ const LOCKED: i64 = 128; /// An existing annotation an import must never replace, move or unlock: one the user locked, a /// redaction mark (replacing it would drop the region the user marked to remove), or a form /// widget or field (replacing it would set the field's value and policy around the form setter). -fn import_protected(x: &Dict) -> bool { - x.int(b"F").unwrap_or(0) & LOCKED != 0 || matches!(x.name(b"Subtype"), Some(b"Redact" | b"Widget")) || x.contains(b"FT") +fn import_protected(doc: &Document, x: &Dict) -> bool { + let flags = x.get(b"F").and_then(|f| doc.resolve(f).as_int()).unwrap_or(0); + flags & LOCKED != 0 || matches!(x.name(b"Subtype"), Some(b"Redact" | b"Widget")) || x.contains(b"FT") } fn place(doc: &mut Document, page_ref: ObjRef, mut d: Dict) -> Result { @@ -1089,7 +1090,7 @@ fn place(doc: &mut Document, page_ref: ObjRef, mut d: Dict) -> Result"#; + assert_eq!(import(&mut doc, xfdf).unwrap().comments, 1); + let annots = doc.get(page).as_dict().and_then(|p| p.get(b"Annots").and_then(Object::as_array).cloned()).unwrap(); + assert!(annots.contains(&Object::Ref(note)), "the locked comment was replaced"); + assert_eq!(doc.get(note), before, "the locked comment was changed"); +} From 747fb2dfd888c57d6da2d5d69e103b0c310f7e7b Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 19:26:45 +0000 Subject: [PATCH 21/30] fix(audit): count each word box a line has against the comparison bound covers walks a line's word boxes, so a page with few lines and many words did more work than the line count charged. --- crates/audit/src/lib.rs | 17 +++++++++++++---- crates/audit/src/tests.rs | 7 ++++--- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/crates/audit/src/lib.rs b/crates/audit/src/lib.rs index 4f1877ef67..3ef8f67209 100644 --- a/crates/audit/src/lib.rs +++ b/crates/audit/src/lib.rs @@ -112,7 +112,8 @@ pub fn audit_redactions(doc: &Document) -> Result, AuditError> /// Work and output bounds for one audit. #[derive(Clone, Copy)] struct Limits { - /// Cover-against-line comparisons per page. + /// Cover-against-line comparisons per page, counting a line once per word box it has (at + /// least once), since [`covers`] walks them. comparisons: usize, /// Bytes of covered text across all findings. covered_text: usize, @@ -129,13 +130,21 @@ impl Limits { struct Budget { limits: Limits, comparisons: usize, + /// What comparing one cover with every line of the current page costs. + page_cost: usize, covered_text: usize, } impl Budget { + /// Resets the comparison count for a page with these `lines`. + fn start_page(&mut self, lines: &[TextLine]) { + self.comparisons = 0; + self.page_cost = lines.iter().fold(0, |n, line| n.saturating_add(line.ink.len().max(1))); + } + /// The text of the `lines` that `hides`, joined with single spaces. fn hidden_text(&mut self, lines: &[TextLine], hides: impl Fn(&TextLine) -> bool) -> Result { - self.comparisons = self.comparisons.saturating_add(lines.len()); + self.comparisons = self.comparisons.saturating_add(self.page_cost); if self.comparisons > self.limits.comparisons { return Err(AuditError::TooManyComparisons); } @@ -157,11 +166,11 @@ impl Budget { fn audit(doc: &Document, limits: Limits) -> Result, AuditError> { let copying = doc.permissions().is_none_or(|p| p.copy()); - let mut budget = Budget { limits, comparisons: 0, covered_text: 0 }; + let mut budget = Budget { limits, comparisons: 0, page_cost: 0, covered_text: 0 }; let mut out = Vec::new(); for (n, page) in pdfcraft_model::pages(doc).iter().enumerate() { let lines = pdfcraft_edit::text_lines(doc, n).map_err(|e| AuditError::Text(n, e.to_string()))?; - budget.comparisons = 0; + budget.start_page(&lines); // 1. Opaque filled rectangles over extractable text. for r in cover::filled_rects(doc, &page.dict)? { let covered = budget.hidden_text(&lines, |line| is_cover(&r, line))?; diff --git a/crates/audit/src/tests.rs b/crates/audit/src/tests.rs index de501d01a7..e0ec3888c2 100644 --- a/crates/audit/src/tests.rs +++ b/crates/audit/src/tests.rs @@ -659,7 +659,8 @@ fn covered_text_across_findings_is_bounded() { } /// Covers compared against lines are bounded per page, so content that is quadratic in its -/// covers and lines fails the audit instead of running for hours. +/// covers and lines fails the audit instead of running for hours. A line costs one comparison per +/// word: 20 covers against 20 two-word lines are 800. #[test] fn cover_and_line_comparisons_are_bounded() { let mut content = Vec::new(); @@ -670,9 +671,9 @@ fn cover_and_line_comparisons_are_bounded() { content.extend_from_slice(b"1 g 0 0 10 10 re f\n"); } let doc = open(one_page(&content, "")); - let limits = crate::Limits { comparisons: 399, covered_text: usize::MAX }; + let limits = crate::Limits { comparisons: 799, covered_text: usize::MAX }; assert!(matches!(crate::audit(&doc, limits), Err(crate::AuditError::TooManyComparisons))); - let limits = crate::Limits { comparisons: 400, covered_text: usize::MAX }; + let limits = crate::Limits { comparisons: 800, covered_text: usize::MAX }; assert!(crate::audit(&doc, limits).expect("within the budget").is_empty()); } From 3e16c71a51be74adfb1d4343d25b42975671611f Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 19:27:10 +0000 Subject: [PATCH 22/30] fix(audit): count only the covered text a finding reports against the output bound A copy-restricted document reports no text, so it no longer fails the bound on text it withholds. --- crates/audit/src/lib.rs | 24 +++++++++++++++--------- crates/audit/src/tests.rs | 7 +++++++ 2 files changed, 22 insertions(+), 9 deletions(-) diff --git a/crates/audit/src/lib.rs b/crates/audit/src/lib.rs index 3ef8f67209..509ae977eb 100644 --- a/crates/audit/src/lib.rs +++ b/crates/audit/src/lib.rs @@ -115,7 +115,7 @@ struct Limits { /// Cover-against-line comparisons per page, counting a line once per word box it has (at /// least once), since [`covers`] walks them. comparisons: usize, - /// Bytes of covered text across all findings. + /// Bytes of covered text the findings report. covered_text: usize, } @@ -150,18 +150,24 @@ impl Budget { } let mut covered = String::new(); for line in lines.iter().filter(|l| hides(l)) { - let text = line.text.trim(); - self.covered_text = self.covered_text.saturating_add(text.len() + 1); - if self.covered_text > self.limits.covered_text { - return Err(AuditError::TooMuchOutput); - } if !covered.is_empty() { covered.push(' '); } - covered.push_str(text); + covered.push_str(line.text.trim()); } Ok(covered) } + + /// The text a finding reports for `covered`: none when copying is withheld. Charges what it + /// reports against the output bound. + fn report_text(&mut self, covered: String, copying: bool) -> Result { + let reported = if copying { covered } else { String::new() }; + self.covered_text = self.covered_text.saturating_add(reported.len()); + if self.covered_text > self.limits.covered_text { + return Err(AuditError::TooMuchOutput); + } + Ok(reported) + } } fn audit(doc: &Document, limits: Limits) -> Result, AuditError> { @@ -175,7 +181,7 @@ fn audit(doc: &Document, limits: Limits) -> Result, AuditError for r in cover::filled_rects(doc, &page.dict)? { let covered = budget.hidden_text(&lines, |line| is_cover(&r, line))?; if !covered.trim().is_empty() { - let covered_text = if copying { covered } else { String::new() }; + let covered_text = budget.report_text(covered, copying)?; out.push(AuditFinding { page: n, kind: FindingKind::CoveredText, rect: r.rect, covered_text }); } } @@ -190,7 +196,7 @@ fn audit(doc: &Document, limits: Limits) -> Result, AuditError AnnotationCover::Redact(rect) => (FindingKind::UnappliedMark, rect), AnnotationCover::Square { rect, .. } => (FindingKind::CoveredText, rect), }; - let covered_text = if copying { covered } else { String::new() }; + let covered_text = budget.report_text(covered, copying)?; out.push(AuditFinding { page: n, kind, rect, covered_text }); } } diff --git a/crates/audit/src/tests.rs b/crates/audit/src/tests.rs index e0ec3888c2..659f8de60f 100644 --- a/crates/audit/src/tests.rs +++ b/crates/audit/src/tests.rs @@ -697,8 +697,15 @@ fn copy_restricted_documents_withhold_covered_text() { let findings = audit_redactions(&user).expect("audit must run"); assert_eq!(findings.len(), 1, "the cover is still reported: {findings:?}"); assert_eq!(findings[0].covered_text, ""); + // The output bound counts the text reported, so withheld text costs nothing. + let nothing = crate::Limits { comparisons: usize::MAX, covered_text: 0 }; + assert_eq!(crate::audit(&user, nothing).expect("withheld text is not output").len(), 1); let owner = Document::open_with_password(bytes, Some("owner")).expect("open"); let findings = audit_redactions(&owner).expect("audit must run"); assert_eq!(findings.len(), 1, "{findings:?}"); assert_eq!(findings[0].covered_text, "SECRET DATA 123"); + let exact = crate::Limits { comparisons: usize::MAX, covered_text: "SECRET DATA 123".len() }; + assert!(crate::audit(&owner, exact).is_ok()); + let short = crate::Limits { covered_text: exact.covered_text - 1, ..exact }; + assert!(matches!(crate::audit(&owner, short), Err(crate::AuditError::TooMuchOutput))); } From 68c14c9ddca47a5495d2a271a34370b6f1dcc3ab Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 19:27:10 +0000 Subject: [PATCH 23/30] docs(audit): the comparison bound counts words --- crates/audit/README.md | 2 +- parity/acrobat-features.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/audit/README.md b/crates/audit/README.md index ce2c47915f..cfa2979c21 100644 --- a/crates/audit/README.md +++ b/crates/audit/README.md @@ -33,7 +33,7 @@ Limits, by design and documented rather than hidden: less than 40% of its words (a sliver of a long value) is not flagged. It fails closed: an undecodable content stream, malformed `/Annots`, form XObjects nested deeper -than 32 or in a cycle, a walk past the operator bound, more than 50 million cover-against-line +than 32 or in a cycle, a walk past the operator bound, more than 50 million cover-against-word comparisons on a page, or more than 64 MiB of covered text in the report is an error, never "no findings". diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index 3d209c87f3..6a2d0ccf98 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -5842,7 +5842,7 @@ status = "partial" commands = [] tools = ["doc_audit_redactions"] evidence = ["crates/audit/src/tests.rs::faux_redaction_is_found", "crates/audit/src/tests.rs::background_before_text_is_not_flagged", "crates/audit/src/tests.rs::light_shading_after_text_is_not_flagged", "crates/audit/src/tests.rs::graphics_state_carries_across_streams", "crates/audit/src/tests.rs::audit_fails_closed_on_undecodable_stream", "crates/audit/src/tests.rs::unapplied_redact_mark_is_found", "crates/audit/src/tests.rs::applied_redaction_at_the_end_of_a_line_is_not_flagged", "crates/audit/src/tests.rs::faux_redaction_at_the_end_of_a_line_is_found", "crates/audit/src/tests.rs::faux_redaction_over_a_short_value_after_a_long_label_is_found", "crates/audit/src/tests.rs::faux_redaction_over_mirrored_text_is_found", "crates/audit/src/tests.rs::faux_redaction_over_a_partial_value_after_a_long_label_is_found", "crates/audit/src/tests.rs::faux_redaction_over_a_partial_value_after_a_long_label_on_a_rotated_page_is_found", "crates/audit/src/tests.rs::faux_redactions_with_multiple_covers_lines_and_spacing_are_found", "crates/audit/src/tests.rs::a_cover_over_one_word_of_a_long_line_is_found", "crates/audit/src/tests.rs::a_box_grazing_a_word_is_not_flagged", "crates/automation/tests/it/automation.rs::auditing_faux_redactions_through_tools", "crates/automation/tests/it/automation.rs::an_applied_redaction_at_the_end_of_a_line_audits_clean", "crates/audit/src/tests.rs::rectangle_drawn_as_a_path_covers_text_like_re", "crates/audit/src/tests.rs::non_rectangular_path_is_not_a_cover", "apps/pdfcraft-cli/tests/it/audit.rs::audit_rectangles_match_the_tool_in_top_left_points", "crates/audit/src/tests.rs::rectangle_filled_after_restore_is_a_cover", "crates/audit/src/tests.rs::later_text_on_the_line_does_not_clear_an_earlier_cover", "crates/audit/src/tests.rs::indirect_subtype_is_resolved", "crates/audit/src/tests.rs::copy_restricted_documents_withhold_covered_text", "crates/audit/src/tests.rs::cover_and_line_comparisons_are_bounded", "crates/audit/src/tests.rs::covered_text_across_findings_is_bounded"] -notes = "New pdfcraft-audit crate. A filled rectangle (re + fill, or a single m/l subpath whose corners form an axis-aligned rectangle, #1510; opaque) counts as a cover only when painted after the start of the text line it overlaps and dark enough to hide it (WCAG contrast < 3.0); page backgrounds, table shading and highlights are not flagged. Coverage is measured on the words a line draws, not its box, so the gap an applied redaction leaves is not counted as hidden text; a cover over 40% of the line's box still counts when it has a positive-area intersection with one word, so a short value after a long label is found. A cover hiding half of any one word counts too (#2662), so a box over one number in a long line, or an unapplied mark over it, is found; a sliver under half of every word it touches and under 40% of the line is not flagged. Graphics state carries across a page's content streams. Fails closed: undecodable content, or more than 50 million cover-against-line comparisons on a page or 64 MiB of covered text in the report, is an error, never a clean bill. A document that withholds copying gets every finding with the covered text left out. Also flags /Redact annotations never applied. CLI: pdfcraft-cli audit (fails when findings exist). Tool and CLI report rectangles in points from the top-left of the displayed page, after /Rotate and rounded to 0.01 pt, like the other tools." +notes = "New pdfcraft-audit crate. A filled rectangle (re + fill, or a single m/l subpath whose corners form an axis-aligned rectangle, #1510; opaque) counts as a cover only when painted after the start of the text line it overlaps and dark enough to hide it (WCAG contrast < 3.0); page backgrounds, table shading and highlights are not flagged. Coverage is measured on the words a line draws, not its box, so the gap an applied redaction leaves is not counted as hidden text; a cover over 40% of the line's box still counts when it has a positive-area intersection with one word, so a short value after a long label is found. A cover hiding half of any one word counts too (#2662), so a box over one number in a long line, or an unapplied mark over it, is found; a sliver under half of every word it touches and under 40% of the line is not flagged. Graphics state carries across a page's content streams. Fails closed: undecodable content, or more than 50 million cover-against-word comparisons on a page or 64 MiB of covered text in the report, is an error, never a clean bill. A document that withholds copying gets every finding with the covered text left out. Also flags /Redact annotations never applied. CLI: pdfcraft-cli audit (fails when findings exist). Tool and CLI report rectangles in points from the top-left of the displayed page, after /Rotate and rounded to 0.01 pt, like the other tools." [[feature]] id = "protect.remove-hidden-on-close" From 6394779b81f7ff7c65df139aff12a1a617c41c95 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 20:51:55 +0000 Subject: [PATCH 24/30] fix(annot): check a whole link batch before creating any link object --- crates/annot/src/links.rs | 10 ++++++++++ crates/annot/src/tests.rs | 18 ++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/crates/annot/src/links.rs b/crates/annot/src/links.rs index f11eecbd83..95d4080587 100644 --- a/crates/annot/src/links.rs +++ b/crates/annot/src/links.rs @@ -420,6 +420,16 @@ pub fn find_urls(chars: &[char]) -> Vec<(std::ops::Range, String)> { /// line rectangle). Returns how many links were added. pub fn add_many(doc: &mut Document, links: &[(usize, Vec<[f64; 4]>, String)], style: &LinkStyle) -> Result { let pages = page_refs(doc)?; + // Check the whole batch first, so a bad entry fails it before any link object is created. + for (page, rects, uri) in links { + pages.get(*page).ok_or(AnnotError::NoSuchPage(*page))?; + if uri.trim().is_empty() { + return Err(AnnotError::Invalid("type the web address first".into())); + } + for r in rects { + checked_rect(*r)?; + } + } // Each page's `/Annots` is extended once, with its new links in the order given. let mut added: Vec<(ObjRef, Vec)> = Vec::new(); let mut slot: std::collections::HashMap = std::collections::HashMap::new(); diff --git a/crates/annot/src/tests.rs b/crates/annot/src/tests.rs index cfd80093e1..017e066713 100644 --- a/crates/annot/src/tests.rs +++ b/crates/annot/src/tests.rs @@ -1541,3 +1541,21 @@ fn many_links_extend_a_page_annotation_list_once() { assert_eq!(annots(&doc, page_refs(&doc).unwrap()[0]).len(), before + n); assert!(copied <= 4 * n, "{copied} annotation entries copied for {n} links"); } + +/// A batch with one bad entry adds nothing: no link objects are left behind unattached. +#[test] +fn a_link_batch_with_a_bad_entry_creates_no_objects() { + let mut doc = fixture(); + let page = page_refs(&doc).unwrap()[0]; + let before = (annots(&doc, page).len(), doc.scan_objects().count()); + let good = [10.0, 10.0, 50.0, 20.0]; + let style = links::LinkStyle::default(); + for bad in [ + vec![(0, vec![good], "https://example.com".to_string()), (0, vec![good, [f64::NAN, 0.0, 10.0, 10.0]], "https://example.com".into())], + vec![(0, vec![good], "https://example.com".to_string()), (0, vec![good], " ".into())], + vec![(0, vec![good], "https://example.com".to_string()), (99, vec![good], "https://example.com".into())], + ] { + assert!(links::add_many(&mut doc, &bad, &style).is_err()); + assert_eq!((annots(&doc, page).len(), doc.scan_objects().count()), before); + } +} From aa1c13f71ce8043c76899c108960eaa3be0dec8b Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 20:56:02 +0000 Subject: [PATCH 25/30] fix(engine): list a page truncated for both forms and pictures once --- crates/engine/src/compare.rs | 18 +++++++-------- crates/engine/src/tests.rs | 44 ++++++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 10 deletions(-) diff --git a/crates/engine/src/compare.rs b/crates/engine/src/compare.rs index 095e666c14..1326ccb5e8 100644 --- a/crates/engine/src/compare.rs +++ b/crates/engine/src/compare.rs @@ -184,18 +184,18 @@ impl crate::Document { /// The pages as paragraphs and images (for Word, HTML and RTF export), including what /// form XObjects draw. pub fn export_pages(&self) -> Vec { - self.export_pages_checked(true).0 + self.export_pages_checked(true, MAX_EXPORT_IMAGE_BYTES).0 } /// [`Self::export_pages`], and the pages whose forms were left partly unread or whose - /// pictures went past [`MAX_EXPORT_IMAGE_BYTES`] ([`OfficeExport::truncated_pages`]). Without + /// pictures went past `image_budget` bytes ([`OfficeExport::truncated_pages`]). Without /// `images` no picture is prepared. - fn export_pages_checked(&self, images: bool) -> (Vec, Vec) { + pub(crate) fn export_pages_checked(&self, images: bool, image_budget: usize) -> (Vec, Vec) { let Some(cos) = self.editor.as_ref().map(|e| &e.cos) else { return (Vec::new(), Vec::new()) }; let mut truncated = Vec::new(); // Each image object is encoded once and shared by every place that draws it. let mut encoded: HashMap = HashMap::new(); - let mut image_bytes = MAX_EXPORT_IMAGE_BYTES; + let mut image_bytes = image_budget; let pages = self .info .pages @@ -213,9 +213,7 @@ impl crate::Document { let display_width = f64::from(if info.rotation % 180 == 0 { info.width } else { info.height }); let unit = if crop_width > 0.0 && crop_width.is_finite() && display_width.is_finite() { display_width / crop_width } else { 1.0 }; let reading = pdfcraft_edit::reading_blocks_checked(cos, i).unwrap_or_default(); - if reading.truncated { - truncated.push(i); - } + let forms_unread = reading.truncated; let blocks = reading .blocks .into_iter() @@ -255,10 +253,10 @@ impl crate::Document { }) }) .collect(); - // The page colour (Edit a PDF ▸ Background, a producer's page fill) for formats that keep one. - if over_budget { + if forms_unread || over_budget { truncated.push(i); } + // The page colour (Edit a PDF ▸ Background, a producer's page fill) for formats that keep one. let background = pdfcraft_edit::page_background(cos, i).ok().flatten(); pdfcraft_export::Page { width: info.width as f64, height: info.height as f64, blocks, images, background } }) @@ -275,7 +273,7 @@ impl crate::Document { /// ([`OfficeExport::truncated_pages`]). pub fn export_office_checked(&self, format: OfficeFormat) -> OfficeExport { // RTF leaves pictures out. - let (pages, truncated_pages) = self.export_pages_checked(format != OfficeFormat::Rtf); + let (pages, truncated_pages) = self.export_pages_checked(format != OfficeFormat::Rtf, MAX_EXPORT_IMAGE_BYTES); let title = self.info.title.clone().unwrap_or_else(|| self.name.trim_end_matches(".pdf").to_string()); let bytes = match format { OfficeFormat::Docx => pdfcraft_export::docx(&pages, &title), diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 412bd15de9..3df5279ab5 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -7283,3 +7283,47 @@ fn exporting_office_keeps_dotted_leader_entries_as_paragraphs() { let positions: Vec<_> = entries.iter().map(|entry| html.find(&format!("

{entry}

")).expect("one paragraph per entry")).collect(); assert!(positions.windows(2).all(|pair| pair[0] < pair[1]), "reading order: {html}"); } + +/// A page whose forms go past the reading limit and whose picture goes past the image budget is +/// listed once among the truncated pages. +#[test] +fn a_page_truncated_for_forms_and_pictures_is_listed_once() { + let depth = compare::MAX_FORM_DEPTH + 1; + let mut objs: Vec = vec![ + "<< /Type /Catalog /Pages 2 0 R >>".into(), + "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 200 200] >>".into(), + String::new(), + "<< /Type /XObject /Subtype /Image /Width 2 /Height 2 /ColorSpace /DeviceGray /BitsPerComponent 8 /Length 4 >>\nstream\n\0\x40\u{80}\u{ff}\nendstream".into(), + "<< /Length 34 >>\nstream\nq 10 0 0 10 0 0 cm /Im Do Q /D1 Do\nendstream".into(), + ]; + // Forms D1..D{depth}, objects 6 onwards, each drawing the next. + let xobjects: String = (1..=depth).map(|i| format!("/D{i} {} 0 R ", 5 + i)).collect(); + let resources = format!("<< /XObject << /Im 4 0 R {xobjects}>> >>"); + for i in 1..=depth { + let body = if i == depth { "q Q".to_string() } else { format!("/D{} Do", i + 1) }; + objs.push(format!( + "<< /Type /XObject /Subtype /Form /BBox [0 0 200 200] /Resources {resources} /Length {} >>\nstream\n{body}\nendstream", + body.len() + )); + } + objs[2] = format!("<< /Type /Page /Parent 2 0 R /Contents 5 0 R /Resources {resources} >>"); + let mut bytes = b"%PDF-1.7\n".to_vec(); + let mut offsets = Vec::new(); + for (i, o) in objs.iter().enumerate() { + offsets.push(bytes.len()); + bytes.extend_from_slice(format!("{} 0 obj\n{o}\nendobj\n", i + 1).as_bytes()); + } + let xref = bytes.len(); + bytes.extend_from_slice(format!("xref\n0 {}\n0000000000 65535 f \n", objs.len() + 1).as_bytes()); + for o in offsets { + bytes.extend_from_slice(format!("{o:010} 00000 n \n").as_bytes()); + } + bytes.extend_from_slice(format!("trailer\n<< /Size {} /Root 1 0 R >>\nstartxref\n{xref}\n%%EOF\n", objs.len() + 1).as_bytes()); + let mut s = Session::new(); + let id = s.open("both.pdf", None, Arc::new(bytes), None).unwrap(); + let doc = s.get(id).unwrap(); + assert_eq!(doc.export_pages_checked(false, 0).1, [0], "the forms alone truncate the page"); + let (pages, truncated) = doc.export_pages_checked(true, 0); + assert!(pages[0].images.is_empty(), "the picture is past the budget"); + assert_eq!(truncated, [0]); +} From 62b54fce098b7ab5786350948339ef5741c55057 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 20:56:33 +0000 Subject: [PATCH 26/30] fix(preflight): check the actions in an indirect /Next array --- crates/preflight/src/lib.rs | 43 +++++++++++++++++++++++++++-------- crates/preflight/src/tests.rs | 2 ++ 2 files changed, 35 insertions(+), 10 deletions(-) diff --git a/crates/preflight/src/lib.rs b/crates/preflight/src/lib.rs index 905ba8cbf3..afd6a3bab1 100644 --- a/crates/preflight/src/lib.rs +++ b/crates/preflight/src/lib.rs @@ -520,7 +520,7 @@ pub fn verify(doc: &Document, level: Level) -> Vec { let own = (d.get(b"S").is_some() && is_action(d)).then_some(d); let direct = [b"A".as_slice(), b"OpenAction"].map(|k| d.get(k).and_then(|a| a.as_dict())); for a in [own].into_iter().chain(direct).flatten() { - if let Some(s) = chain_forbidden(a) { + if let Some(s) = chain_forbidden(doc, a) { issue("6.5.1", "A {} action isn't allowed", Some(s), None, true); } } @@ -659,14 +659,31 @@ fn is_action(d: &Dict) -> bool { const MAX_ACTION_CHAIN: usize = 1024; /// The first forbidden action type in the action `a` or in the actions written directly in its -/// `/Next` (ISO 32000-2 12.6.2: a dictionary or an array of them). An action `/Next` refers to -/// is an object of its own, checked and removed as one. -fn chain_forbidden(a: &Dict) -> Option { - let mut stack = vec![a]; +/// `/Next` (ISO 32000-2 12.6.2: a dictionary or an array of them, which may be an object of its +/// own). An action `/Next` refers to is an object of its own, checked and removed as one. +fn chain_forbidden(doc: &Document, a: &Dict) -> Option { let mut read = 0; + let mut seen = std::collections::HashSet::new(); + // Actions written in an indirect `/Next` array, copied out of it. + let mut held: Vec = Vec::new(); + if let Some(s) = walk_chain(doc, a, &mut read, &mut seen, &mut held) { + return Some(s); + } + while let Some(d) = held.pop() { + if let Some(s) = walk_chain(doc, &d, &mut read, &mut seen, &mut held) { + return Some(s); + } + } + None +} + +/// [`chain_forbidden`] for the actions reachable from `a` through direct `/Next` entries. The +/// actions of each indirect `/Next` array (each read once) go to `held`. +fn walk_chain(doc: &Document, a: &Dict, read: &mut usize, seen: &mut std::collections::HashSet, held: &mut Vec) -> Option { + let mut stack = vec![a]; while let Some(d) = stack.pop() { - read += 1; - if read > MAX_ACTION_CHAIN { + *read += 1; + if *read > MAX_ACTION_CHAIN { return Some("too deeply chained".into()); } if let Some(s) = d.name(b"S") @@ -677,6 +694,12 @@ fn chain_forbidden(a: &Dict) -> Option { match d.get(b"Next") { Some(Object::Dict(next)) => stack.push(next), Some(Object::Array(list)) => stack.extend(list.iter().filter_map(Object::as_dict)), + Some(Object::Ref(r)) if seen.insert(*r) => { + if let Object::Array(list) = &*doc.get(*r) { + // More than the chain limit can't be read anyway. + held.extend(list.iter().filter_map(Object::as_dict).take(MAX_ACTION_CHAIN).cloned()); + } + } _ => {} } } @@ -686,7 +709,7 @@ fn chain_forbidden(a: &Dict) -> Option { /// Whether `o` is (or refers to) a forbidden action or one chaining to it, or to nothing. fn forbidden(doc: &Document, o: &Object) -> bool { match &*doc.resolve(o) { - Object::Dict(a) => chain_forbidden(a).is_some(), + Object::Dict(a) => chain_forbidden(doc, a).is_some(), Object::Null => true, _ => false, } @@ -836,11 +859,11 @@ pub fn convert(doc: &mut Document, level: Level) -> Result>")], ""), build(&["<< /Type /Catalog /Pages 2 0 R >>", pages, page, &link("5 0 R"), &goto], ""), build(&["<< /Type /Catalog /Pages 2 0 R >>", pages, page, &link(&format!("<< /S /GoTo /D [3 0 R /Fit] /Next {js} >>"))], ""), + // `/Next` refers to an array that holds the action directly. + build(&["<< /Type /Catalog /Pages 2 0 R >>", pages, page, &link("<< /S /GoTo /D [3 0 R /Fit] /Next 5 0 R >>"), &format!("[{js}]")], ""), ]; for (i, mut doc) in cases.into_iter().enumerate() { let found = messages(&verify(&doc, Level::A2b)).contains(&"A JavaScript action isn't allowed"); From e3f86940a3bf9badfba49bd55b3b763009e7246a Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 21:32:20 +0000 Subject: [PATCH 27/30] M7: redaction audit skips covered-text assembly when copying is withheld --- crates/audit/src/lib.rs | 19 +++++++++++-------- crates/audit/src/tests.rs | 15 +++++++++++++++ 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/crates/audit/src/lib.rs b/crates/audit/src/lib.rs index 509ae977eb..5cbcb75bfb 100644 --- a/crates/audit/src/lib.rs +++ b/crates/audit/src/lib.rs @@ -142,12 +142,17 @@ impl Budget { self.page_cost = lines.iter().fold(0, |n, line| n.saturating_add(line.ink.len().max(1))); } - /// The text of the `lines` that `hides`, joined with single spaces. - fn hidden_text(&mut self, lines: &[TextLine], hides: impl Fn(&TextLine) -> bool) -> Result { + /// The text of the `lines` that `hides`, joined with single spaces, or `None` when they show + /// no text. Without `copying` the text is empty and never assembled: one long line under many + /// covers would otherwise be copied once per cover, outside both bounds. + fn hidden_text(&mut self, lines: &[TextLine], copying: bool, hides: impl Fn(&TextLine) -> bool) -> Result, AuditError> { self.comparisons = self.comparisons.saturating_add(self.page_cost); if self.comparisons > self.limits.comparisons { return Err(AuditError::TooManyComparisons); } + if !copying { + return Ok(lines.iter().any(|line| hides(line) && !line.text.trim().is_empty()).then(String::new)); + } let mut covered = String::new(); for line in lines.iter().filter(|l| hides(l)) { if !covered.is_empty() { @@ -155,7 +160,7 @@ impl Budget { } covered.push_str(line.text.trim()); } - Ok(covered) + Ok((!covered.trim().is_empty()).then_some(covered)) } /// The text a finding reports for `covered`: none when copying is withheld. Charges what it @@ -179,19 +184,17 @@ fn audit(doc: &Document, limits: Limits) -> Result, AuditError budget.start_page(&lines); // 1. Opaque filled rectangles over extractable text. for r in cover::filled_rects(doc, &page.dict)? { - let covered = budget.hidden_text(&lines, |line| is_cover(&r, line))?; - if !covered.trim().is_empty() { + if let Some(covered) = budget.hidden_text(&lines, copying, |line| is_cover(&r, line))? { let covered_text = budget.report_text(covered, copying)?; out.push(AuditFinding { page: n, kind: FindingKind::CoveredText, rect: r.rect, covered_text }); } } // 2. Annotation covers over extractable text. for annot in annots(doc, &page.dict)? { - let covered = budget.hidden_text(&lines, |line| match &annot { + if let Some(covered) = budget.hidden_text(&lines, copying, |line| match &annot { AnnotationCover::Redact(rect) => covers(rect, line), AnnotationCover::Square { rect, rgb } => covers(rect, line) && contrast(line.color, *rgb) < COVER_CONTRAST, - })?; - if !covered.trim().is_empty() { + })? { let (kind, rect) = match annot { AnnotationCover::Redact(rect) => (FindingKind::UnappliedMark, rect), AnnotationCover::Square { rect, .. } => (FindingKind::CoveredText, rect), diff --git a/crates/audit/src/tests.rs b/crates/audit/src/tests.rs index 659f8de60f..ac02251c72 100644 --- a/crates/audit/src/tests.rs +++ b/crates/audit/src/tests.rs @@ -709,3 +709,18 @@ fn copy_restricted_documents_withhold_covered_text() { let short = crate::Limits { covered_text: exact.covered_text - 1, ..exact }; assert!(matches!(crate::audit(&owner, short), Err(crate::AuditError::TooMuchOutput))); } + +/// Withheld text is never assembled: covers over one long line would otherwise copy the line once +/// per cover, a volume neither the comparison nor the output bound charges. +#[test] +fn withheld_covered_text_is_not_assembled() { + let doc = open(one_page(b"BT /F1 24 Tf 100 700 Td (SECRET DATA 123) Tj ET\n", "")); + let lines = pdfcraft_edit::text_lines(&doc, 0).expect("text"); + let mut budget = crate::Budget { limits: crate::Limits::DEFAULT, comparisons: 0, page_cost: 0, covered_text: 0 }; + budget.start_page(&lines); + let withheld = budget.hidden_text(&lines, false, |_| true).expect("within the budget"); + assert_eq!(withheld.map(|t| t.capacity()), Some(0), "a finding, with no text built for it"); + assert_eq!(budget.hidden_text(&lines, false, |_| false).expect("within the budget"), None); + let copied = budget.hidden_text(&lines, true, |_| true).expect("within the budget"); + assert_eq!(copied.as_deref(), Some("SECRET DATA 123")); +} From 35b5abd5b3c73d47c2fac71277d659d185001a2a Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 21:42:02 +0000 Subject: [PATCH 28/30] test(engine): show the truncated-page picture exports within a sufficient budget The fixture's image stream held six bytes under /Length 4, and the test only checked that the picture was missing at a zero budget, which a picture that could not be prepared also passes. The stream is four bytes, and the picture is first shown to export at a 1 MiB budget. --- crates/engine/src/tests.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 3df5279ab5..afe6fd06c2 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -7293,7 +7293,7 @@ fn a_page_truncated_for_forms_and_pictures_is_listed_once() { "<< /Type /Catalog /Pages 2 0 R >>".into(), "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 200 200] >>".into(), String::new(), - "<< /Type /XObject /Subtype /Image /Width 2 /Height 2 /ColorSpace /DeviceGray /BitsPerComponent 8 /Length 4 >>\nstream\n\0\x40\u{80}\u{ff}\nendstream".into(), + "<< /Type /XObject /Subtype /Image /Width 2 /Height 2 /ColorSpace /DeviceGray /BitsPerComponent 8 /Length 4 >>\nstream\n\0\x20\x40\x7f\nendstream".into(), "<< /Length 34 >>\nstream\nq 10 0 0 10 0 0 cm /Im Do Q /D1 Do\nendstream".into(), ]; // Forms D1..D{depth}, objects 6 onwards, each drawing the next. @@ -7323,6 +7323,8 @@ fn a_page_truncated_for_forms_and_pictures_is_listed_once() { let id = s.open("both.pdf", None, Arc::new(bytes), None).unwrap(); let doc = s.get(id).unwrap(); assert_eq!(doc.export_pages_checked(false, 0).1, [0], "the forms alone truncate the page"); + let (exported, _) = doc.export_pages_checked(true, 1 << 20); + assert_eq!(exported[0].images.len(), 1, "the picture exports within a sufficient budget"); let (pages, truncated) = doc.export_pages_checked(true, 0); assert!(pages[0].images.is_empty(), "the picture is past the budget"); assert_eq!(truncated, [0]); From bd9030b27178d9cf9d39226680637d3786a5cac5 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Thu, 22 Oct 2026 22:36:03 -0400 Subject: [PATCH 29/30] chore(sonar): exclude vendored crates and the spec-mandated MD5/RIPEMD-160 (SonarCloud rust:S4790) vendor/ is third-party code kept close to upstream (vendor/README.md), so SonarCloud's 186 findings there are not ours to restyle. MD5 in crates/crypt is what ISO 32000's standard security handler mandates, and RIPEMD-160 in crates/sign is a digest existing signatures can name; both S4790 findings are scoped to those two files. --- sonar-project.properties | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/sonar-project.properties b/sonar-project.properties index 30e80a0c5f..7d0926a248 100644 --- a/sonar-project.properties +++ b/sonar-project.properties @@ -1,2 +1,16 @@ sonar.projectKey=storytold_pdfcraft sonar.organization=storytold + +# vendor/ holds third-party crates carried with small, marked patches (vendor/README.md). +# They are kept close to upstream so each copy can be dropped once upstream releases the +# fix; restyling them would only make that harder, so they are not analysed here. +sonar.exclusions=vendor/** + +# S4790 (weak hash): MD5 is the hash ISO 32000 mandates for the RC4/AES-128 standard security +# handler (Algorithms 1-5), and RIPEMD-160 is a digest existing signatures may name. Both are +# needed to open and verify existing files, not chosen for new security. +sonar.issue.ignore.multicriteria=crypt_md5,sign_ripemd +sonar.issue.ignore.multicriteria.crypt_md5.ruleKey=rust:S4790 +sonar.issue.ignore.multicriteria.crypt_md5.resourceKey=crates/crypt/src/lib.rs +sonar.issue.ignore.multicriteria.sign_ripemd.ruleKey=rust:S4790 +sonar.issue.ignore.multicriteria.sign_ripemd.resourceKey=crates/sign/src/keys.rs From e93f11e81e929a8900aa2ab94fa9f1b04df6a8e6 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Thu, 22 Oct 2026 22:40:05 -0400 Subject: [PATCH 30/30] fix(packaging): use [[ tests and name positional parameters in the scripts (SonarCloud shelldre:S7688, S7679, S7682, S131) Every packaging script runs under bash, so [[ ]] is available. Name the positional parameters of sha256, render and has, end env.sh's functions with an explicit return (bare, so the exit status is unchanged), and give the build-label case a default arm. --- packaging/env.sh | 24 +++++++++++++++--------- packaging/freebsd/package.sh | 4 ++-- packaging/icons.sh | 5 ++++- packaging/linux/flatpak-bundle.sh | 8 ++++---- packaging/linux/package.sh | 21 ++++++++++++--------- packaging/macos/import-cert.sh | 6 +++--- packaging/macos/package.sh | 24 ++++++++++++------------ packaging/web/package.sh | 4 ++-- 8 files changed, 54 insertions(+), 42 deletions(-) diff --git a/packaging/env.sh b/packaging/env.sh index 1832702ebb..eeb5b7230f 100644 --- a/packaging/env.sh +++ b/packaging/env.sh @@ -21,10 +21,11 @@ workspace_version() { /^\[/ { in_pkg = ($0 == "[workspace.package]") ; next } in_pkg && $1 == "version" { gsub(/[" ]/, "", $3); print $3; exit } ' "$ROOT/Cargo.toml" + return } VERSION="${PDFCRAFT_VERSION:-$(workspace_version)}" -if [ -z "$VERSION" ]; then +if [[ -z "$VERSION" ]]; then echo "error: could not read [workspace.package] version from $ROOT/Cargo.toml" >&2 exit 1 fi @@ -32,12 +33,13 @@ export VERSION # A labelled build says so in `--version` and About (pdfcraft_engine::build_info) and in its # package release (packaging/linux/nfpm.yaml), so it can't be mistaken for the release. -if [ -n "${PDFCRAFT_BUILD_LABEL:-}" ]; then +if [[ -n "${PDFCRAFT_BUILD_LABEL:-}" ]]; then case "$PDFCRAFT_BUILD_LABEL" in *[!A-Za-z0-9.]* | .* | *.) echo "error: PDFCRAFT_BUILD_LABEL may hold only letters, digits and inner dots: $PDFCRAFT_BUILD_LABEL" >&2 exit 1 ;; + *) ;; esac export PDFCRAFT_BUILD_LABEL fi @@ -46,7 +48,7 @@ DIST="${DIST:-$ROOT/dist/release}" mkdir -p "$DIST" export DIST -if [ -z "${PDFCRAFT_BUILD_SHA:-}" ]; then +if [[ -z "${PDFCRAFT_BUILD_SHA:-}" ]]; then PDFCRAFT_BUILD_SHA="$(git -C "$ROOT" rev-parse HEAD 2>/dev/null || true)" fi export PDFCRAFT_BUILD_SHA @@ -55,25 +57,27 @@ export CARGO_TARGET_DIR="${CARGO_TARGET_DIR:-$ROOT/target}" # Emit a GitHub Actions warning (plain stderr outside Actions). warn() { - if [ -n "${GITHUB_ACTIONS:-}" ]; then echo "::warning::$*"; else echo "warning: $*" >&2; fi + if [[ -n "${GITHUB_ACTIONS:-}" ]]; then echo "::warning::$*"; else echo "warning: $*" >&2; fi + return } # Copy licence and readme files that exist into a package directory. copy_docs() { local dest="$1" f for f in README.md LICENSE LICENSE-MIT LICENSE-APACHE COPYRIGHT NOTICE; do - if [ -f "$ROOT/$f" ]; then cp "$ROOT/$f" "$dest/"; fi + if [[ -f "$ROOT/$f" ]]; then cp "$ROOT/$f" "$dest/"; fi done copy_font_licences "$dest" + return } # Builds made with the optional craft-fonts input (CRAFT_FONTS_DIR, set for every release) embed # its fonts, so the package carries their licences: fonts//OFL.txt -> OFL-.txt. copy_font_licences() { local dest="$1" f family - [ -n "${CRAFT_FONTS_DIR:-}" ] || return 0 + [[ -n "${CRAFT_FONTS_DIR:-}" ]] || return 0 for f in "$CRAFT_FONTS_DIR"/fonts/*/OFL.txt; do - [ -f "$f" ] || continue + [[ -f "$f" ]] || continue family="$(basename "$(dirname "$f")")" cp "$f" "$dest/OFL-$family.txt" done @@ -90,7 +94,7 @@ stage_models() { dest="$(cd "$dest" && pwd)" (cd "$ROOT" && cargo xtask models "$dest") rm -f "$dest"/*.part - if ! ls "$dest"/*.LICENCE.txt >/dev/null 2>&1 || [ ! -s "$dest/ATTRIBUTION.txt" ]; then + if ! ls "$dest"/*.LICENCE.txt >/dev/null 2>&1 || [[ ! -s "$dest/ATTRIBUTION.txt" ]]; then echo "error: no OCR models in $dest after cargo xtask models" >&2 exit 1 fi @@ -100,5 +104,7 @@ stage_models() { # Portable SHA-256 of a file (prints just the hash). sha256() { - if command -v sha256sum >/dev/null; then sha256sum "$1" | cut -d' ' -f1; else shasum -a 256 "$1" | cut -d' ' -f1; fi + local file="$1" + if command -v sha256sum >/dev/null; then sha256sum "$file" | cut -d' ' -f1; else shasum -a 256 "$file" | cut -d' ' -f1; fi + return } diff --git a/packaging/freebsd/package.sh b/packaging/freebsd/package.sh index 0828058fc8..63b981b24f 100755 --- a/packaging/freebsd/package.sh +++ b/packaging/freebsd/package.sh @@ -14,7 +14,7 @@ LINUX="$ROOT/packaging/linux" APP_ID=ai.storyteller.pdfcraft SKIP_BUILD=0 -while [ $# -gt 0 ]; do +while [[ $# -gt 0 ]]; do case "$1" in --skip-build) SKIP_BUILD=1; shift ;; -h | --help) sed -n '2,9p' "$0"; exit 0 ;; @@ -32,7 +32,7 @@ BASENAME="pdfcraft-$VERSION-freebsd-$ARCH" echo "==> PdfCraft $VERSION for FreeBSD $ARCH" -if [ "$SKIP_BUILD" = 0 ]; then +if [[ "$SKIP_BUILD" = 0 ]]; then (cd "$ROOT" && cargo build --release --locked -p pdfcraft -p pdfcraft-cli) fi BIN="$CARGO_TARGET_DIR/release" diff --git a/packaging/icons.sh b/packaging/icons.sh index da8228ff50..4897803ca3 100755 --- a/packaging/icons.sh +++ b/packaging/icons.sh @@ -27,7 +27,10 @@ MAC="$TMP/macos.svg" echo '' } >"$MAC" -render() { resvg -w "$2" -h "$2" "$1" "$3" &2; exit 1; } +[[ -f "$TARBALL" ]] || { echo "error: $TARBALL not found (run packaging/linux/package.sh --formats tar first)" >&2; exit 1; } for tool in flatpak flatpak-builder; do command -v "$tool" >/dev/null || { echo "error: $tool not found (apt install flatpak flatpak-builder librsvg2-common)" >&2; exit 1; } done @@ -49,7 +49,7 @@ WORK="$CARGO_TARGET_DIR/flatpak-bundle" rm -rf "$WORK" mkdir -p "$WORK/stage" tar -xzf "$TARBALL" -C "$WORK/stage" --strip-components=1 -[ -x "$WORK/stage/bin/pdfcraft" ] || { echo "error: $TARBALL has no bin/pdfcraft" >&2; exit 1; } +[[ -x "$WORK/stage/bin/pdfcraft" ]] || { echo "error: $TARBALL has no bin/pdfcraft" >&2; exit 1; } cp "$HERE/flatpak/$APP_ID.bundle.yml" "$WORK/$APP_ID.yml" flatpak remote-add --user --if-not-exists flathub "$FLATHUB" @@ -62,7 +62,7 @@ OUT="$DIST/$BASENAME.flatpak" flatpak build-bundle --runtime-repo="$FLATHUB" "$WORK/repo" "$OUT" "$APP_ID" stable echo "wrote $OUT" -if [ "$TEST" = 1 ]; then +if [[ "$TEST" = 1 ]]; then flatpak install --user -y --noninteractive --reinstall "$OUT" flatpak info --user "$APP_ID" flatpak run --command=pdfcraft-cli "$APP_ID" --version diff --git a/packaging/linux/package.sh b/packaging/linux/package.sh index 92813c3a82..f87dccd616 100755 --- a/packaging/linux/package.sh +++ b/packaging/linux/package.sh @@ -30,7 +30,7 @@ APP_ID=ai.storyteller.pdfcraft SKIP_BUILD=0 FORMATS="appimage deb rpm tar cli" -while [ $# -gt 0 ]; do +while [[ $# -gt 0 ]]; do case "$1" in --skip-build) SKIP_BUILD=1; shift ;; --formats) FORMATS="$2"; shift 2 ;; @@ -52,8 +52,8 @@ BASENAME="pdfcraft-$VERSION$LABEL_SUFFIX-linux-$ARCH" echo "==> PdfCraft $VERSION$LABEL_SUFFIX for Linux $ARCH ($FORMATS)" -if [ "$SKIP_BUILD" = 0 ]; then - if [ -n "${CROSS_TARGET:-}" ]; then +if [[ "$SKIP_BUILD" = 0 ]]; then + if [[ -n "${CROSS_TARGET:-}" ]]; then (cd "$ROOT" && cargo build --release --locked -p pdfcraft -p pdfcraft-cli --target "$CROSS_TARGET") else (cd "$ROOT" && cargo build --release --locked -p pdfcraft -p pdfcraft-cli) @@ -87,7 +87,10 @@ if command -v appstreamcli >/dev/null; then appstreamcli validate --no-net --explain "$STAGE/usr/share/metainfo/$APP_ID.metainfo.xml" fi -has() { case " $FORMATS " in *" $1 "*) return 0 ;; *) return 1 ;; esac; } +has() { + local format="$1" + case " $FORMATS " in *" $format "*) return 0 ;; *) return 1 ;; esac +} # ---- .tar.gz ------------------------------------------------------------------------------------ if has tar; then @@ -134,9 +137,9 @@ if has appimage; then ln -s "$APP_ID.png" "$APPDIR/.DirIcon" TOOL="${APPIMAGETOOL:-$(command -v appimagetool || true)}" - if [ -z "$TOOL" ]; then + if [[ -z "$TOOL" ]]; then TOOL="$CARGO_TARGET_DIR/appimagetool-$ARCH.AppImage" - if [ ! -x "$TOOL" ]; then + if [[ ! -x "$TOOL" ]]; then curl --proto '=https' -fsSL -o "$TOOL" "https://github.com/AppImage/appimagetool/releases/download/continuous/appimagetool-$ARCH.AppImage" chmod +x "$TOOL" fi @@ -159,16 +162,16 @@ if has appimage; then # package) appimagetool also writes the .zsync, into its working directory, hence the cd. (cd "$DIST" && ARCH="$ARCH" APPIMAGE_EXTRACT_AND_RUN=1 "$TOOL" --no-appstream -u "$UPDATE_INFO" "$APPDIR" "$OUT") echo "wrote $OUT" - if [ -s "$OUT.zsync" ]; then + if [[ -s "$OUT.zsync" ]]; then echo "wrote $OUT.zsync" else warn "zsyncmake not found, so $OUT.zsync was not written; AppImage delta updates need it" fi fi -if [ -z "${CROSS_TARGET:-}" ]; then +if [[ -z "${CROSS_TARGET:-}" ]]; then "$STAGE/usr/bin/pdfcraft-cli" --version -elif [ -n "${EMULATOR:-}" ]; then +elif [[ -n "${EMULATOR:-}" ]]; then "$EMULATOR" "$STAGE/usr/bin/pdfcraft-cli" --version fi echo "==> done" diff --git a/packaging/macos/import-cert.sh b/packaging/macos/import-cert.sh index ae5ee44c6d..b9825da35d 100755 --- a/packaging/macos/import-cert.sh +++ b/packaging/macos/import-cert.sh @@ -12,7 +12,7 @@ umask 077 # shellcheck source=../env.sh . "$(dirname "${BASH_SOURCE[0]}")/../env.sh" -if [ -z "${APPLE_CERTIFICATE:-}" ]; then +if [[ -z "${APPLE_CERTIFICATE:-}" ]]; then warn "APPLE_CERTIFICATE is not set: macOS builds will be ad-hoc signed and not notarized" exit 0 fi @@ -36,13 +36,13 @@ security set-key-partition-list -S apple-tool:,apple:,codesign: -s -k "$KC_PASS" security list-keychains -d user -s "$KEYCHAIN" $(security list-keychains -d user | tr -d '"') IDENTITY="$(security find-identity -v -p codesigning "$KEYCHAIN" | awk '/Developer ID Application/ { print $2; exit }')" -if [ -z "$IDENTITY" ]; then +if [[ -z "$IDENTITY" ]]; then security find-identity -v -p codesigning "$KEYCHAIN" >&2 echo "error: no 'Developer ID Application' identity in APPLE_CERTIFICATE" >&2 exit 1 fi echo "Imported signing identity $IDENTITY" -if [ -n "${GITHUB_ENV:-}" ]; then +if [[ -n "${GITHUB_ENV:-}" ]]; then { echo "MACOS_SIGN_IDENTITY=$IDENTITY"; echo "MACOS_KEYCHAIN=$KEYCHAIN"; } >>"$GITHUB_ENV" else echo "MACOS_SIGN_IDENTITY=$IDENTITY" diff --git a/packaging/macos/package.sh b/packaging/macos/package.sh index 267a87280b..2e2ffc83f5 100755 --- a/packaging/macos/package.sh +++ b/packaging/macos/package.sh @@ -19,7 +19,7 @@ HERE="$ROOT/packaging/macos" ARCH=universal SKIP_BUILD=0 -while [ $# -gt 0 ]; do +while [[ $# -gt 0 ]]; do case "$1" in --arch) ARCH="$2"; shift 2 ;; --skip-build) SKIP_BUILD=1; shift ;; @@ -44,9 +44,9 @@ DMG="$DIST/pdfcraft-$VERSION-macos-$ARCH.dmg" CLI_ZIP="$DIST/pdfcraft-cli-$VERSION-macos-$ARCH.zip" NOTARIZE=0 -if [ "$IDENTITY" = "-" ]; then +if [[ "$IDENTITY" = "-" ]]; then warn "macOS: ad-hoc signing (no MACOS_SIGN_IDENTITY); artifacts are not notarized" -elif [ -n "${APPLE_ID:-}" ] && [ -n "${APPLE_PASSWORD:-}" ] && [ -n "${APPLE_TEAM_ID:-}" ]; then +elif [[ -n "${APPLE_ID:-}" ]] && [[ -n "${APPLE_PASSWORD:-}" ]] && [[ -n "${APPLE_TEAM_ID:-}" ]]; then NOTARIZE=1 else warn "macOS: APPLE_ID / APPLE_PASSWORD / APPLE_TEAM_ID incomplete; signed but not notarized" @@ -55,7 +55,7 @@ fi echo "==> PdfCraft $VERSION for macOS ($ARCH), identity: $IDENTITY, notarize: $NOTARIZE" # ---- build ------------------------------------------------------------------------------------- -if [ "$SKIP_BUILD" = 0 ]; then +if [[ "$SKIP_BUILD" = 0 ]]; then args=() for t in "${TARGETS[@]}"; do args+=(--target "$t"); done (cd "$ROOT" && cargo build --release --locked -p pdfcraft -p pdfcraft-cli "${args[@]}") @@ -74,9 +74,9 @@ done sign() { # Hardened runtime + secure timestamp for a real identity; ad-hoc can't be timestamped. local ts=(--timestamp) - [ "$IDENTITY" = "-" ] && ts=(--timestamp=none) + [[ "$IDENTITY" = "-" ]] && ts=(--timestamp=none) local kc=() - [ -n "${MACOS_KEYCHAIN:-}" ] && kc=(--keychain "$MACOS_KEYCHAIN") + [[ -n "${MACOS_KEYCHAIN:-}" ]] && kc=(--keychain "$MACOS_KEYCHAIN") codesign --force --sign "$IDENTITY" ${kc[@]+"${kc[@]}"} "${ts[@]}" "$@" } @@ -88,8 +88,8 @@ notarize() { echo "$out" id="$(printf '%s' "$out" | plutil -extract id raw -o - - 2>/dev/null || true)" status="$(printf '%s' "$out" | plutil -extract status raw -o - - 2>/dev/null || true)" - if [ "$status" != "Accepted" ]; then - if [ -n "$id" ]; then + if [[ "$status" != "Accepted" ]]; then + if [[ -n "$id" ]]; then xcrun notarytool log "$id" --apple-id "$APPLE_ID" --password "$APPLE_PASSWORD" --team-id "$APPLE_TEAM_ID" || true fi echo "error: notarization of $(basename "$file") failed (status: ${status:-unknown})" >&2 @@ -122,7 +122,7 @@ sign --options runtime --entitlements "$HERE/entitlements.plist" "$APP/Contents/ sign --options runtime --entitlements "$HERE/entitlements.plist" "$APP" codesign --verify --strict --deep --verbose=2 "$APP" -if [ "$NOTARIZE" = 1 ]; then +if [[ "$NOTARIZE" = 1 ]]; then ditto -c -k --keepParent "$APP" "$WORK/PdfCraft-notarize.zip" notarize "$WORK/PdfCraft-notarize.zip" xcrun stapler staple "$APP" @@ -156,7 +156,7 @@ hdiutil_retry() { if hdiutil "$@"; then return 0 fi - [ "$attempt" = 5 ] && { echo "hdiutil $what failed 5 times" >&2; return 1; } + [[ "$attempt" = 5 ]] && { echo "hdiutil $what failed 5 times" >&2; return 1; } warn "hdiutil $what failed (attempt $attempt of 5); retrying in $((attempt * 10))s" sleep $((attempt * 10)) done @@ -174,7 +174,7 @@ if ! codesign --verify --strict --deep --verbose=2 "$MOUNT/PdfCraft.app"; then exit 1 fi hdiutil_retry detach detach "$MOUNT" >/dev/null -if [ "$NOTARIZE" = 1 ]; then +if [[ "$NOTARIZE" = 1 ]]; then notarize "$DMG" xcrun stapler staple "$DMG" xcrun stapler validate "$DMG" @@ -192,7 +192,7 @@ codesign --verify --strict --verbose=2 "$CLI_DIR/pdfcraft-cli" rm -f "$CLI_ZIP" ditto -c -k --keepParent "$CLI_DIR" "$CLI_ZIP" # A bare Mach-O can't carry a stapled ticket; Gatekeeper looks the notarization up online. -if [ "$NOTARIZE" = 1 ]; then notarize "$CLI_ZIP"; fi +if [[ "$NOTARIZE" = 1 ]]; then notarize "$CLI_ZIP"; fi "$WORK/bin/pdfcraft-cli" --version echo "==> done" diff --git a/packaging/web/package.sh b/packaging/web/package.sh index c676721fd9..3a32b394e7 100755 --- a/packaging/web/package.sh +++ b/packaging/web/package.sh @@ -11,13 +11,13 @@ set -euo pipefail . "$(dirname "${BASH_SOURCE[0]}")/../env.sh" HERE="$ROOT/packaging/web" -if [ "${1:-}" != "--skip-build" ]; then +if [[ "${1:-}" != "--skip-build" ]]; then command -v trunk >/dev/null || { echo "error: trunk not found (cargo install trunk --locked)" >&2; exit 1; } (cd "$ROOT/apps/pdfcraft-web" && trunk build --release) fi SITE="$ROOT/dist/web" -[ -f "$SITE/index.html" ] || { echo "error: $SITE/index.html missing; run without --skip-build" >&2; exit 1; } +[[ -f "$SITE/index.html" ]] || { echo "error: $SITE/index.html missing; run without --skip-build" >&2; exit 1; } # Paths must be relative so the site works under any prefix (public_url = "./" in Trunk.toml). if grep -Eq '(src|href)="/[^/]' "$SITE/index.html"; then echo "error: $SITE/index.html has root-absolute URLs; it would break when served from a sub-path" >&2