Edit text: refuse a rewrite of text that is a clipping path (#3447)

* 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 <noreply@anthropic.com>

---------

Co-authored-by: Brandon Thomas <echelon@gmail.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Hamza Naveed
2026-10-19 13:03:54 -04:00
committed by GitHub
co-authored by Claude Opus 5.5 Brandon Thomas
parent e5033243f4
commit 459379ef9b
2 changed files with 110 additions and 4 deletions
+75
View File
@@ -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::<Vec<_>>());
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");
}
+35 -4
View File
@@ -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<Vec<TextLine>, 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<Vec<TextLine>, 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<TextLine>, last: &mut Option<Pen>, 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<TextLine>, last: &mut Option<Pen>, 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<usize>) -> 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<LineEdit, EditError> {
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();