From 92c979c5b2959294151333f82fdb910d497e309e Mon Sep 17 00:00:00 2001 From: eliets2 <287273517+eliets2@users.noreply.github.com> Date: Tue, 20 Oct 2026 18:25:07 +0300 Subject: [PATCH 1/2] redact: refuse marks with no readable area instead of skipping them A Redact annotation with neither a usable QuadPoints array nor a usable Rect was dropped silently by marks(). Applying the readable marks beside it then reported success, and the text under the dropped mark stayed in the file. apply() now counts such marks on the pages being applied and returns RedactError::UnreadableMark(n) before anything changes. The message goes out through the existing error text: the engine edit error, the automation reply and the UI failure line all print it. Marks that were readable through the existing fallbacks (a Rect alone, a usable QuadPoints array) work as before. Tests: three in crates/redact/src/tests.rs (refusal with the count and no change, Rect-only still redacts, an unreadable mark on another page does not block the selected page) and one end to end in crates/automation/tests/automation.rs (the reply carries the reason and the page keeps its text). On upstream main with only the variant added, the two refusal tests and the automation test fail; the Rect test passes. Reverting each pinned production line turns the tests red again. Gates: cargo fmt --check, clippy -D warnings for pdfcraft-redact and pdfcraft-automation, and the redact test suite pass. --- crates/automation/tests/automation.rs | 42 ++++++++++++++++ crates/redact/src/lib.rs | 25 ++++++++-- crates/redact/src/tests.rs | 69 +++++++++++++++++++++++++++ 3 files changed, 133 insertions(+), 3 deletions(-) diff --git a/crates/automation/tests/automation.rs b/crates/automation/tests/automation.rs index cbf40f196..204f8c789 100644 --- a/crates/automation/tests/automation.rs +++ b/crates/automation/tests/automation.rs @@ -3415,6 +3415,48 @@ fn redacting_through_tools() { assert!(matches!(a.call("redact_mark", &json!({ "doc": doc })), Err(ToolError::InvalidArgs(_)))); } +/// One page with the text "Secret 1234" and a Redact annotation that has neither a usable +/// QuadPoints array nor a Rect, so there is no area to redact. +fn redact_pdf_with_unreadable_mark() -> Vec { + let body = "BT /F1 24 Tf 20 150 Td (Secret 1234) Tj ET"; + let objs: Vec = vec![ + "<< /Type /Catalog /Pages 2 0 R >>".into(), + "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 200 300] >>".into(), + "<< /Type /Page /Parent 2 0 R /Contents 4 0 R /Resources << /Font << /F1 5 0 R >> >> /Annots [6 0 R] >>".into(), + format!("<< /Length {} >>\nstream\n{body}\nendstream", body.len()), + "<< /Type /Font /Subtype /Type1 /BaseFont /Helvetica >>".into(), + "<< /Type /Annot /Subtype /Redact /QuadPoints /oops >>".into(), + ]; + 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()); + bytes +} + +/// A mark that can't be read is refused by `redact_apply`, with the reason in the reply, and the +/// page keeps its text. (Before, the mark was skipped and the reply said there were no marks.) +#[test] +fn an_unreadable_redaction_mark_is_refused_with_the_reason() { + let dir = workdir("redact-unreadable"); + std::fs::write(dir.join("broken.pdf"), redact_pdf_with_unreadable_mark()).unwrap(); + let mut a = auto(&dir); + let doc = ok(&mut a, "doc_open", json!({ "path": "broken.pdf" }))["doc"].as_u64().unwrap(); + let Err(ToolError::Failed(reason)) = a.call("redact_apply", &json!({ "doc": doc })) else { + panic!("redact_apply must refuse a mark with no readable area"); + }; + assert!(reason.contains("1 redaction mark(s) have no readable area") && reason.contains("nothing was redacted"), "{reason}"); + assert!(page_text(&mut a, doc)[0].contains("Secret 1234"), "the page is unchanged"); +} + #[test] fn redacting_with_codes_through_tools() { let dir = workdir("redact-codes"); diff --git a/crates/redact/src/lib.rs b/crates/redact/src/lib.rs index fdac37dbe..51afc85d9 100644 --- a/crates/redact/src/lib.rs +++ b/crates/redact/src/lib.rs @@ -38,6 +38,12 @@ pub enum RedactError { NothingToApply, #[error("page {0} has content that can't be read, so it can't be redacted safely")] Unreadable(usize), + /// Redact annotations with no readable area (neither usable `/QuadPoints` nor a usable `/Rect`). + /// Skipping them would leave the text under them in the file while the others report success. + #[error( + "{0} redaction mark(s) have no readable area (neither usable QuadPoints nor a usable Rect), so the text under them can't be known; nothing was redacted. Remove those marks or mark the text again, then apply again" + )] + UnreadableMark(usize), #[error("redaction could not be verified: {0} item(s) were still found under the marks, so nothing was changed")] Residue(usize), /// A mark's overlay text has a character Helvetica can't draw; nothing was redacted. @@ -155,7 +161,13 @@ pub(crate) fn annots_of(doc: &Document, page: &Dict) -> Vec { /// Every redaction mark in the document, page by page. pub fn marks(doc: &Document) -> Vec { - let mut out = Vec::new(); + marks_checked(doc).0 +} + +/// [`marks`], and the 0-based page of each Redact annotation that has no readable area, which +/// [`apply`] refuses rather than skipping. +pub(crate) fn marks_checked(doc: &Document) -> (Vec, Vec) { + let (mut out, mut unreadable) = (Vec::new(), Vec::new()); for (pi, p) in pdfcraft_model::pages(doc).iter().enumerate() { for a in annots_of(doc, &p.dict) { let Some(r) = a.as_ref() else { continue }; @@ -190,13 +202,14 @@ pub fn marks(doc: &Document) -> Vec { rects.push(r); } if rects.is_empty() { + unreadable.push(pi); continue; } let overlay = d.get(b"OverlayText").and_then(|t| doc.resolve(t).as_string().map(|s| s.to_text())).unwrap_or_default(); out.push(Mark { page: pi, obj: r, rects, fill: color(doc, d, b"IC"), overlay, look: overlay_look(doc, d) }); } } - out + (out, unreadable) } /// A page's content streams: (the references or inline objects as listed, their decoded data). @@ -319,7 +332,13 @@ fn localize_resources(doc: &mut Document, selected: &[usize]) -> Result<(), pdfc /// sensitive content still present. Call this on a working clone and replace the original only /// on success. On any error, discard the working clone; do not save or reuse it. pub fn apply(doc: &mut Document, pages: Option<&[usize]>) -> Result { - let all_marks = marks(doc); + let (all_marks, unreadable) = marks_checked(doc); + // A mark with no readable area is not applied, and applying the others would look complete: + // refuse before anything changes. Only the pages being applied count. + let unreadable = unreadable.iter().filter(|page| pages.is_none_or(|p| p.contains(page))).count(); + if unreadable > 0 { + return Err(RedactError::UnreadableMark(unreadable)); + } let chosen: Vec<&Mark> = all_marks.iter().filter(|m| pages.is_none_or(|p| p.contains(&m.page))).collect(); if chosen.is_empty() { return Err(RedactError::NothingToApply); diff --git a/crates/redact/src/tests.rs b/crates/redact/src/tests.rs index da1ecf12a..5641ddb7c 100644 --- a/crates/redact/src/tests.rs +++ b/crates/redact/src/tests.rs @@ -643,6 +643,75 @@ fn unreadable_content_fails_closed() { assert_eq!(apply(&mut doc, None), Err(RedactError::Unreadable(1))); } +/// Leaves a mark with nothing readable: /QuadPoints becomes a name and /Rect is removed. +fn break_area(doc: &mut Document, obj: ObjRef) { + doc.update_dict(obj, |d| { + d.set(b"QuadPoints".to_vec(), Object::name("oops")); + d.remove(b"Rect"); + }) + .unwrap(); +} + +/// A Redact annotation with no readable area (neither usable QuadPoints nor a usable Rect) used to +/// vanish: `marks` skipped it, so applying the readable mark next to it reported success and left +/// the text under the broken mark in place. Now the whole operation is refused before anything +/// changes, and the count is of the unreadable marks. +#[test] +fn a_mark_with_no_readable_area_is_refused_instead_of_vanishing() { + let mut doc = one_page(b"BT /F1 10 Tf 10 100 Td (AB1234CD) Tj ET", "", vec![]); + mark(&mut doc, 0, &[[40.0, 95.0, 50.0, 110.0]], ""); + mark(&mut doc, 0, &[[0.0, 95.0, 10.0, 110.0]], ""); + for m in marks(&doc) { + break_area(&mut doc, m.obj); + } + mark(&mut doc, 0, &[[20.0, 95.0, 40.0, 110.0]], ""); + assert_eq!(marks(&doc).len(), 1, "only the readable mark is listed"); + assert_eq!(apply(&mut doc, None), Err(RedactError::UnreadableMark(2))); + // Nothing changed: the readable mark is still pending and its text is still drawn. + assert_eq!(marks(&doc).len(), 1); + assert!(content(&doc, 0).contains("(AB1234CD) Tj"), "{}", content(&doc, 0)); +} + +/// A mark whose QuadPoints can't be used still redacts through its Rect, as it always did. +#[test] +fn a_mark_with_only_a_usable_rect_still_redacts() { + let mut doc = one_page(b"BT /F1 10 Tf 10 100 Td (AB1234CD) Tj ET", "", vec![]); + mark(&mut doc, 0, &[[20.0, 95.0, 40.0, 110.0]], ""); + let m = marks(&doc).swap_remove(0); + doc.update_dict(m.obj, |d| { + d.set(b"QuadPoints".to_vec(), Object::name("oops")); + d.set(b"Rect".to_vec(), Object::Array(vec![Object::Int(20), Object::Int(95), Object::Int(40), Object::Int(110)])); + }) + .unwrap(); + assert_eq!(marks(&doc).len(), 1, "the Rect keeps the mark readable"); + let r = apply(&mut doc, None).unwrap(); + assert_eq!((r.marks, r.glyphs), (1, 4)); + assert!(!content(&doc, 0).contains("1234")); +} + +/// Only the pages being applied decide the refusal: an unreadable mark on another page does not +/// stop the marks of the selected page from being applied. +#[test] +fn an_unreadable_mark_on_another_page_does_not_stop_the_selected_page() { + let mut doc = pdf(vec![ + b"<< /Type /Catalog /Pages 2 0 R >>".to_vec(), + b"<< /Type /Pages /Kids [3 0 R 7 0 R] /Count 2 >>".to_vec(), + b"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Contents 4 0 R /Resources << /Font << /F1 5 0 R >> >> >>".to_vec(), + stream("", b"BT /F1 10 Tf 10 100 Td (AB1234CD) Tj ET"), + FONT.replace("95 0 R", "6 0 R").into_bytes(), + widths(), + b"<< /Type /Page /Parent 2 0 R /MediaBox [0 0 300 300] /Contents 8 0 R /Resources << /Font << /F1 5 0 R >> >> >>".to_vec(), + stream("", b"BT /F1 10 Tf 10 100 Td (XY) Tj ET"), + ]); + mark(&mut doc, 1, &[[10.0, 95.0, 30.0, 110.0]], ""); + let broken = marks(&doc).swap_remove(0); + break_area(&mut doc, broken.obj); + mark(&mut doc, 0, &[[20.0, 95.0, 40.0, 110.0]], ""); + assert_eq!(apply(&mut doc, None), Err(RedactError::UnreadableMark(1))); + assert_eq!(apply(&mut doc, Some(&[0])).map(|r| r.glyphs), Ok(4)); + assert!(!content(&doc, 0).contains("1234")); +} + /// A document with something in every hidden-information category. fn hidden_fixture() -> Document { let content = b"BT /F1 10 Tf 10 100 Td (Visible) Tj 3 Tr (Hidden) Tj 0 Tr ET BT /F1 10 Tf 500 500 Td (Offpage) Tj ET /OC /L1 BDC BT /F1 10 Tf 10 50 Td (Layer) Tj ET EMC"; From 71e03461868417cb5d5ea973dfa5f83227148fda Mon Sep 17 00:00:00 2001 From: eliets2 <287273517+eliets2@users.noreply.github.com> Date: Tue, 20 Oct 2026 18:25:15 +0300 Subject: [PATCH 2/2] docs(redact): say that a mark with no readable area fails apply The apply section of the redact README described the fail-closed rules but not the new refusal of a Redact annotation with neither a usable QuadPoints array nor a usable Rect. Documented here with the error name and the page scope of the count. No behaviour change in this commit. --- crates/redact/README.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/crates/redact/README.md b/crates/redact/README.md index 7cd5ded1e..3332da14f 100644 --- a/crates/redact/README.md +++ b/crates/redact/README.md @@ -32,7 +32,9 @@ clear_marks(&mut doc, None)?; // remove marks without app A verification pass re-reads each redacted page and fails the operation if any glyph or inline image is still under an area. Content that can't be decoded makes the operation fail rather than -leave it unredacted. +leave it unredacted. A Redact annotation with neither a usable `/QuadPoints` array nor a usable +`/Rect` makes it fail with `RedactError::UnreadableMark` before anything changes, instead of +skipping that mark. The count covers the marks on the pages being applied. Before changing any page, `apply` prepares the redaction appearances with a shared 8 MiB budget for their uncompressed PDF content across the selected pages. It bounds repeated-text