From 459379ef9b127648cc30c0b2fe2b20c2b2375f04 Mon Sep 17 00:00:00 2001 From: Hamza Naveed <45464935+Hamza0101@users.noreply.github.com> Date: Mon, 19 Oct 2026 22:03:54 +0500 Subject: [PATCH] Edit text: refuse a rewrite of text that is a clipping path (#3447) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Edit text: refuse a rewrite of text that is a clipping path Editing text in a Photoshop-exported PDF destroyed the artwork on the page. Such exports draw text in `7 Tr`: the glyphs are not ink but a mask, and a fill or image is painted through them. Rewriting the run removed the glyphs and so the mask, the fill behind flooded its own rectangle, and the file still opened without a warning (#3429). Measured on a page whose only text is a `7 Tr` mask over a red rectangle: the fill covered 3.4% of the page before the edit and 100% after it. `clear_block` already refuses these runs -- "text used as a clipping path cannot be previewed for editing" -- but `replace_line` and `rewrite_block` did not, so the destructive path was the one the editor offered. Both refuse modes 4 to 7 now, and the document is left untouched. Modes 0 to 2 paint ink and are rewritten as before. This does not keep the design: preserving it means emitting the replacement inside the same `BT … ET` with the mode intact, which needs the writer to place glyphs in a clipping object, as both reporters noted. Refusing turns silent data loss into an error while that is written. Found and analysed by @RavenCrowe666 and @mtlcl410, who converged on this fix. * Check every run the rewrite removes, not the line's first one The guard read `target.origin.state.render_mode`, the mode the line's first run set. Two paths got a clipping run past it: A line's runs need not share a rendering mode. Nothing in the join in `text_lines` requires one -- it asks for the same stream piece, scope, text object or neighbouring word, font, size, weight, slant and fill colour -- so a line that starts as ink can continue as a mask, and the whole line's operators are what the rewrite removes: BT /F1 48 Tf 40 60 Td 0 Tr (TITLE) Tj 7 Tr (MASK) Tj ET reads as one line "TITLEMASK" whose origin is `0 Tr`. Replacing it passed the guard and took the mask away with the ink. `coincident_ops` drops another drawing of the line on the same baseline, so that the replacement is all that shows -- a fake-bold second copy, an invisible OCR layer. It matches on text, overlap and baseline, not on rendering mode, so that copy can be the mask while the line being replaced is ink. Both rewrite paths could drop it. `TextLine` now carries whether any of its runs clips, and the guard reads every line whose operators are in the removal set, the way `clear_block` already checks every run it drops. In `replace_line_with_limit` the removal set is computed before the replacement font is generated, so a refusal still leaves the document untouched. Both cases have a test; on the previous commit each returns `Ok(LineEdit { substituted: None })`. * Edit text: keep existing fixtures on text_page; build the clipping fixture on it Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Brandon Thomas Co-authored-by: Claude Opus 5.5 --- crates/edit/src/tests.rs | 75 ++++++++++++++++++++++++++++++++++++++++ crates/edit/src/text.rs | 39 ++++++++++++++++++--- 2 files changed, 110 insertions(+), 4 deletions(-) diff --git a/crates/edit/src/tests.rs b/crates/edit/src/tests.rs index 691e01c98a..879ebc6c66 100644 --- a/crates/edit/src/tests.rs +++ b/crates/edit/src/tests.rs @@ -5143,3 +5143,78 @@ fn flattening_comments_keeps_annotations_that_are_not_comments() { let without_widget: Vec<&str> = all.iter().copied().filter(|s| *s != "Widget").collect(); assert_eq!(left(&reopen(&doc)), without_widget, "flattening fields touches no annotation that isn't a widget"); } + +/// A page whose text is a clipping mask (`mode` Tr) with a fill painted through it. +fn clipping_text_page(mode: i64) -> Document { + text_page(&format!("q\nBT {mode} Tr /F1 48 Tf 40 60 Td (MASK) Tj ET\n1 0 0 rg 0 0 300 150 re f\nQ\n")) +} + +/// Rewriting text that is a clipping path destroyed the artwork it masked: the glyphs went, the +/// mask with them, and the fill behind flooded its own rectangle, in a file that still opened +/// without a warning (#3429). Photoshop exports draw every text object this way. The rewrite is +/// refused instead, as `clear_block` already refuses a preview. +#[test] +fn rewriting_clipping_path_text_is_refused_for_every_clipping_mode() { + for mode in [4, 5, 6, 7] { + let before = text::text_lines(&clipping_text_page(mode), 0).unwrap(); + assert_eq!(before.len(), 1, "mode {mode}: the mask is one line"); + + let mut doc = clipping_text_page(mode); + let line = text::replace_line(&mut doc, 0, 0, "EDITED"); + assert!(matches!(&line, Err(EditError::Invalid(m)) if m.contains("clipping path")), "mode {mode}: {line:?}"); + + let mut doc = clipping_text_page(mode); + let block = text::rewrite_block(&mut doc, 0, 0, Some("EDITED"), &BlockStyle::default()); + assert!(matches!(&block, Err(EditError::Invalid(m)) if m.contains("clipping path")), "mode {mode}: {block:?}"); + + // Refused means untouched: the mask is still there to shape what is painted through it. + let after = text::text_lines(&doc, 0).unwrap(); + assert_eq!(after.len(), 1, "mode {mode}"); + assert_eq!(after[0].text, "MASK", "mode {mode}"); + } +} + +/// The modes that paint ink are still editable: only 4 to 7 add to the clip path. +#[test] +fn painting_modes_are_still_rewritten() { + for mode in [0, 1, 2] { + let mut doc = clipping_text_page(mode); + assert!(text::replace_line(&mut doc, 0, 0, "EDITED").is_ok(), "mode {mode}"); + assert_eq!(text::text_lines(&doc, 0).unwrap()[0].text, "EDITED", "mode {mode}"); + } +} + +/// A line's runs need not share a rendering mode: nothing in the join requires it, so a line that +/// starts as ink can continue as a mask. The refusal reads the whole line, not the mode its first +/// run set -- otherwise the rewrite passed the guard and took the mask away with the ink. +#[test] +fn a_line_that_starts_as_ink_and_continues_as_a_mask_is_refused() { + let content = "q\nBT /F1 48 Tf 40 60 Td 0 Tr (TITLE) Tj 7 Tr (MASK) Tj ET\n1 0 0 rg 0 0 300 150 re f\nQ\n"; + let before = text::text_lines(&text_page(content), 0).unwrap(); + assert_eq!(before.len(), 1, "the two runs are one line: {:?}", before.iter().map(|l| &l.text).collect::>()); + assert_eq!(before[0].text, "TITLEMASK", "and the mask is the second half of it"); + + let mut doc = text_page(content); + let line = text::replace_line(&mut doc, 0, 0, "EDITED"); + assert!(matches!(&line, Err(EditError::Invalid(m)) if m.contains("clipping path")), "{line:?}"); + let block = text::rewrite_block(&mut doc, 0, 0, Some("EDITED"), &BlockStyle::default()); + assert!(matches!(&block, Err(EditError::Invalid(m)) if m.contains("clipping path")), "{block:?}"); + assert_eq!(text::text_lines(&doc, 0).unwrap()[0].text, "TITLEMASK", "refused means untouched"); +} + +/// Another drawing of the line on the same baseline goes with it, so that the replacement is all +/// that shows -- and that copy can be the mask while the line being replaced is ink. Dropping it +/// destroys the artwork as surely as rewriting the mask itself, so this is refused too. +#[test] +fn a_clipping_copy_on_the_same_baseline_is_refused() { + let content = "q\nBT 0 Tr /F1 48 Tf 40 60 Td (MASK) Tj ET\nBT 7 Tr /F1 48 Tf 40 60 Td (MASK) Tj ET\n1 0 0 rg 0 0 300 150 re f\nQ\n"; + let before = text::text_lines(&text_page(content), 0).unwrap(); + assert_eq!(before.len(), 2, "two drawings of the same line"); + + let mut doc = text_page(content); + let line = text::replace_line(&mut doc, 0, 0, "EDITED"); + assert!(matches!(&line, Err(EditError::Invalid(m)) if m.contains("clipping path")), "{line:?}"); + let block = text::rewrite_block(&mut doc, 0, 0, Some("EDITED"), &BlockStyle::default()); + assert!(matches!(&block, Err(EditError::Invalid(m)) if m.contains("clipping path")), "{block:?}"); + assert_eq!(text::text_lines(&doc, 0).unwrap().len(), 2, "refused means untouched"); +} diff --git a/crates/edit/src/text.rs b/crates/edit/src/text.rs index e93ead68f5..2672592178 100644 --- a/crates/edit/src/text.rs +++ b/crates/edit/src/text.rs @@ -68,6 +68,10 @@ pub struct TextLine { /// Some of the line's text is `/ActualText`, which is already in logical order: visual-order /// Indic recovery would scramble it. actual_text: bool, + /// Whether any of the line's runs draws in a clipping mode (`4 Tr` to `7 Tr`). The join below + /// does not require one rendering mode, so a line that starts as ink can continue as a mask; + /// [`refuse_clipping_text`] reads this rather than the origin's mode alone. + clips: bool, } impl TextLine { @@ -1145,6 +1149,7 @@ pub fn text_lines(doc: &Document, page: usize) -> Result, EditErro l.decodable &= s.decodable; l.visual_order |= s.visual_order; l.actual_text |= s.actual_text; + l.clips |= s.state.render_mode >= 4; // Spaces may use a separate subset font; they do not change the visible style. } else { lines.push(TextLine { @@ -1165,6 +1170,7 @@ pub fn text_lines(doc: &Document, page: usize) -> Result, EditErro scope: s.scope, logical_order: s.logical_order, actual_text: s.actual_text, + clips: s.state.render_mode >= 4, ops: std::iter::once(s.op).chain(s.extra_ops.iter().copied()).collect(), origin: Origin { tm: s.tm.0, @@ -1235,6 +1241,7 @@ fn add_shown(lines: &mut Vec, last: &mut Option, s: Shown, stream l.decodable &= s.decodable; l.visual_order |= s.visual_order; l.actual_text |= s.actual_text; + l.clips |= s.state.render_mode >= 4; } else { lines.push(TextLine { text: s.text.clone(), @@ -1254,6 +1261,7 @@ fn add_shown(lines: &mut Vec, last: &mut Option, s: Shown, stream scope: s.scope, logical_order: s.logical_order, actual_text: s.actual_text, + clips: s.state.render_mode >= 4, ops: std::iter::once(s.op).chain(s.extra_ops.iter().copied()).collect(), origin: Origin { tm: s.tm.0, @@ -1837,11 +1845,33 @@ pub fn replace_line(doc: &mut Document, page: usize, line: usize, text: &str) -> replace_line_with_limit(doc, page, line, text, MAX_TYPE3_GLYPHS) } +/// Refuses a rewrite that would remove text drawn as a clipping path (`4 Tr` to `7 Tr`). Such +/// glyphs are not ink but a mask, and whatever is painted through them takes their shape. +/// Removing the glyphs removes the mask, and the fill or image behind floods its own rectangle +/// instead -- the artwork is destroyed, in a file that still opens without a warning (#3429). +/// `clear_block` already refuses these for the same reason. +/// +/// Every line the rewrite drops is checked, not only the one being replaced: a line's runs need +/// not share a rendering mode (see [`TextLine::clips`]), and [`coincident_ops`] drops a whole +/// overlapping line without regard to its mode -- so a mask could go with text that is ink. +fn refuse_clipping_text(lines: &[TextLine], dropped: &std::collections::HashSet) -> Result<(), EditError> { + if lines.iter().any(|l| l.clips && l.ops.iter().any(|op| dropped.contains(op))) { + return Err(EditError::Invalid("text used as a clipping path cannot be edited: its glyphs mask the artwork behind them".into())); + } + Ok(()) +} + /// [`replace_line`] with the most unique characters one generated font may hold (tests lower it). pub(crate) fn replace_line_with_limit(doc: &mut Document, page: usize, line: usize, text: &str, max_glyphs: usize) -> Result { let text = text.replace(['\n', '\r'], " "); let lines = text_lines(doc, page)?; let target = lines.get(line).cloned().ok_or_else(|| EditError::Invalid(format!("page {} has no line {}", page + 1, line + 1)))?; + // Copies of the line drawn on the same baseline go with it, so the replacement is all that + // shows. Which operators go is settled here, before any font is generated, so that a refusal + // leaves the document untouched. + let mut drops = coincident_ops(&lines, &[&target]); + drops.extend(target.ops.iter().copied()); + refuse_clipping_text(&lines, &drops)?; let p = page_dict(doc, page)?; let mut res = p.dict.get(b"Resources").map(|r| doc.resolve(r)).and_then(|r| r.as_dict().cloned()).unwrap_or_default(); let mut fonts_res = res.get(b"Font").map(|f| doc.resolve(f)).and_then(|f| f.as_dict().cloned()).unwrap_or_default(); @@ -1915,10 +1945,7 @@ pub(crate) fn replace_line_with_limit(doc: &mut Document, page: usize, line: usi } } } - // Rebuild: the line's first operator becomes the replacement, its others go. Copies of the - // line drawn on the same baseline go too, so the replacement is all that shows. - let mut drops = coincident_ops(&lines, &[&target]); - drops.extend(target.ops.iter().copied()); + // Rebuild: the line's first operator becomes the replacement, its others go. let new_data = splice(&pieces, &ops, |i| if i == first { (std::mem::take(&mut replacement), false) } else { (Vec::new(), !drops.contains(&i)) }); let contents = rewritten_contents(doc, &streams, new_data); let page_ref = p.obj; @@ -2179,6 +2206,10 @@ fn rewrite_block_from( let blocks = group_blocks(&lines); let b = blocks.get(block).cloned().ok_or_else(|| EditError::Invalid(format!("page {} has no paragraph {}", page + 1, block + 1)))?; let members: Vec<&TextLine> = b.lines.iter().map(|i| &lines[*i]).collect(); + // One clipping run is enough: rewriting the paragraph would drop its mask with the rest. + let mut dropped = coincident_ops(&lines, &members); + dropped.extend(members.iter().flat_map(|l| l.ops.iter().copied())); + refuse_clipping_text(&lines, &dropped)?; let first = members.first().copied().ok_or_else(|| EditError::Invalid("the paragraph has no source runs".into()))?; let p = page_dict(doc, page)?; let mut res = p.dict.get(b"Resources").map(|r| doc.resolve(r)).and_then(|r| r.as_dict().cloned()).unwrap_or_default();