From b52bc303281b58eb0bd28134cc7a1dc764177007 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 11 Nov 2026 14:47:56 -0500 Subject: [PATCH] fix(annot): count ink points as strokes resolve and cap the eraser's samples (#6001) * fix(annot): count ink points as strokes resolve and cap the eraser's samples Resizing and erasing a drawing resolved every /InkList entry without a total, so one stroke array listed many times expanded freely; the eraser densified every segment into up to 2,000 samples with no overall limit. * review: refuse an eraser result that exceeds the stored ink point cap * review: say the erasure would make the drawing too large * address review: the eraser counts each stroke's final point against its sample total --- crates/annot/src/modify.rs | 69 ++++++++++++++++++++++++++------------ crates/annot/src/tests.rs | 67 ++++++++++++++++++++++++++++++++++++ 2 files changed, 114 insertions(+), 22 deletions(-) diff --git a/crates/annot/src/modify.rs b/crates/annot/src/modify.rs index ae2222b387..6566337ca1 100644 --- a/crates/annot/src/modify.rs +++ b/crates/annot/src/modify.rs @@ -857,19 +857,38 @@ pub fn set_points(doc: &mut Document, page: usize, index: usize, points: &[[f64; /// Scale a drawing's strokes into `rect`. The margin between the strokes and the old `/Rect` (half /// the line width and a little more) stays the same on each side, so the line isn't clipped. +/// Most stroke points a stored drawing may have for resizing or erasing it. Counted as each +/// stroke is resolved, so one stroke array referenced again and again can't expand past it. +const MAX_STORED_INK_POINTS: usize = 1_000_000; +/// Most points the eraser densifies a drawing into before testing them against its path. +const MAX_ERASE_SAMPLES: usize = 4_000_000; + +/// The strokes of an ink annotation's `/InkList`, at most [`MAX_STORED_INK_POINTS`] points. +fn ink_strokes(doc: &Document, d: &Dict) -> Result>, AnnotError> { + let too_many = || AnnotError::Invalid(format!("the drawing has more than {MAX_STORED_INK_POINTS} points")); + let list = d.get(b"InkList").map(|l| doc.resolve(l)); + let mut points = 0usize; + let mut strokes = Vec::new(); + for s in list.as_deref().and_then(Object::as_array).map(Vec::as_slice).unwrap_or_default() { + let stroke = doc.resolve(s); + let Some(a) = stroke.as_array() else { + strokes.push(Vec::new()); + continue; + }; + // Charged before the coordinates are read: a shared array costs each time it is listed. + points = points.saturating_add(a.len() / 2); + if points > MAX_STORED_INK_POINTS { + return Err(too_many()); + } + let v: Vec = a.iter().filter_map(|x| doc.resolve(x).as_f64()).collect(); + strokes.push(v.as_chunks::<2>().0.iter().map(|p| [p[0], p[1]]).collect()); + } + Ok(strokes) +} + fn resize_ink(doc: &mut Document, r: ObjRef, rect: [f64; 4], meta: &Meta) -> Result<(), AnnotError> { let d = annot_dict(doc, r); - let strokes: Vec> = d - .get(b"InkList") - .map(|l| doc.resolve(l)) - .and_then(|l| l.as_array().cloned()) - .unwrap_or_default() - .iter() - .map(|s| { - let v: Vec = doc.resolve(s).as_array().map(|a| a.iter().filter_map(|x| doc.resolve(x).as_f64()).collect()).unwrap_or_default(); - v.as_chunks::<2>().0.iter().map(|p| [p[0], p[1]]).collect() - }) - .collect(); + let strokes = ink_strokes(doc, &d)?; let Some(inner) = bounds(strokes.iter().flatten().copied()) else { return Err(AnnotError::Invalid("the drawing has no strokes".into())); }; @@ -1475,17 +1494,7 @@ pub fn erase_ink(doc: &mut Document, page: usize, index: usize, path: &[[f64; 2] if path.is_empty() || !path.iter().all(|p| finite(p)) || !radius.is_finite() || radius <= 0.0 { return Err(AnnotError::Invalid("invalid eraser path".into())); } - let strokes: Vec> = d - .get(b"InkList") - .map(|l| doc.resolve(l)) - .and_then(|l| l.as_array().cloned()) - .unwrap_or_default() - .iter() - .map(|s| { - let v: Vec = doc.resolve(s).as_array().map(|a| a.iter().filter_map(|x| doc.resolve(x).as_f64()).collect()).unwrap_or_default(); - v.as_chunks::<2>().0.iter().map(|p| [p[0], p[1]]).collect() - }) - .collect(); + let strokes = ink_strokes(doc, &d)?; // Distance from a point to the eraser's path (segments, or a single point). let near = |p: [f64; 2]| -> bool { if path.len() == 1 { @@ -1501,17 +1510,27 @@ pub fn erase_ink(doc: &mut Document, page: usize, index: usize, path: &[[f64; 2] }; let mut kept: Vec> = Vec::new(); let mut erased = false; + let mut samples = 0usize; for s in &strokes { // Densify so long segments are cut where the eraser crosses them. let mut pts: Vec<[f64; 2]> = Vec::new(); for w in s.windows(2) { let n = ((w[1][0] - w[0][0]).hypot(w[1][1] - w[0][1]) / (radius / 2.0)).ceil().clamp(1.0, 2000.0) as usize; + samples = samples.saturating_add(n); + if samples > MAX_ERASE_SAMPLES { + return Err(AnnotError::Invalid("the drawing is too large to erase at this eraser size".into())); + } for k in 0..n { let t = k as f64 / n as f64; pts.push([w[0][0] + (w[1][0] - w[0][0]) * t, w[0][1] + (w[1][1] - w[0][1]) * t]); } } if let Some(last) = s.last() { + // The final point is a sample too. + samples = samples.saturating_add(1); + if samples > MAX_ERASE_SAMPLES { + return Err(AnnotError::Invalid("the drawing is too large to erase at this eraser size".into())); + } pts.push(*last); } let mut run: Vec<[f64; 2]> = Vec::new(); @@ -1537,6 +1556,12 @@ pub fn erase_ink(doc: &mut Document, page: usize, index: usize, path: &[[f64; 2] delete_annotation(doc, page, index)?; return Ok(true); } + // The retained runs are stored as the new `/InkList`: past the cap, writing them would leave a + // drawing that later `resize_ink` and `erase_ink` refuse to read. + let points: usize = kept.iter().map(Vec::len).sum(); + if points > MAX_STORED_INK_POINTS { + return Err(AnnotError::Invalid(format!("erasing would leave a drawing with more than {MAX_STORED_INK_POINTS} points"))); + } let width = d.get(b"BS").and_then(|b| b.as_dict()).and_then(|b| b.get(b"W")).and_then(|w| w.as_f64()).unwrap_or(1.0); let rect = grow(bounds(kept.iter().flatten().copied()).unwrap_or_default(), width / 2.0 + 1.0); doc.update_dict(r, |d| { diff --git a/crates/annot/src/tests.rs b/crates/annot/src/tests.rs index ce5a8c1604..980959d4d9 100644 --- a/crates/annot/src/tests.rs +++ b/crates/annot/src/tests.rs @@ -4797,6 +4797,73 @@ fn freehand_highlight_is_compatible_ink_with_marker_blending() { } } +/// A drawing whose /InkList lists one stroke array over and over is refused once its points +/// pass the cap, counted as each stroke is resolved, before they are collected or scaled. +#[test] +fn ink_points_are_counted_as_strokes_resolve() { + let numbers: String = (0..2000).map(|k| format!(" {}", k % 100)).collect(); + let refs = " 5 0 R".repeat(501); + let doc_with = |refs: &str| { + doc_of(&[ + "<< /Type /Catalog /Pages 2 0 R >>".into(), + "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 612 792] >>".into(), + "<< /Type /Page /Parent 2 0 R /Annots [4 0 R] >>".into(), + format!("<< /Type /Annot /Subtype /Ink /Rect [0 0 100 100] /InkList [{refs}] >>"), + format!("[{numbers}]"), + ]) + }; + // 501 × 1,000 points is under the cap: the points are read (this bare drawing then has no + // appearance it can be redrawn with, which is another matter). + let mut doc = doc_with(&refs); + let under = set_rect(&mut doc, 0, 0, [0.0, 0.0, 50.0, 50.0], &meta("")); + assert!(!matches!(&under, Err(AnnotError::Invalid(m)) if m.contains("points")), "{under:?}"); + // 1,001 × 1,000 points is over it. + let mut doc = doc_with(&" 5 0 R".repeat(1001)); + match set_rect(&mut doc, 0, 0, [0.0, 0.0, 50.0, 50.0], &meta("")) { + Err(AnnotError::Invalid(m)) => assert!(m.contains("more than 1000000 points"), "{m}"), + other => panic!("refused: {other:?}"), + } + match erase_ink(&mut doc, 0, 0, &[[1.0, 1.0]], 1.0, &meta("")) { + Err(AnnotError::Invalid(m)) => assert!(m.contains("more than 1000000 points"), "{m}"), + other => panic!("refused: {other:?}"), + } +} + +/// The eraser's densified samples have a total: a tiny eraser over long strokes is refused +/// instead of interpolating millions of points. +#[test] +fn the_eraser_densifies_within_a_total() { + let zigzag: String = (0..2100).map(|k| format!(" {} {}", if k % 2 == 0 { 0 } else { 5000 }, k)).collect(); + let mut doc = doc_of(&[ + "<< /Type /Catalog /Pages 2 0 R >>".into(), + "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 612 792] >>".into(), + "<< /Type /Page /Parent 2 0 R /Annots [4 0 R] >>".into(), + format!("<< /Type /Annot /Subtype /Ink /Rect [0 0 5000 2100] /InkList [[{zigzag}]] >>"), + ]); + match erase_ink(&mut doc, 0, 0, &[[1.0, 1.0]], 0.01, &meta("")) { + Err(AnnotError::Invalid(m)) => assert!(m.contains("too large to erase"), "{m}"), + other => panic!("refused: {other:?}"), + } +} + +/// Each stroke's final point is a sample too: 500,000 strokes of two points at 8 samples a segment +/// are 4,000,000 densified samples and as many final points again, past the total. +#[test] +fn the_eraser_counts_each_strokes_final_point() { + let refs = " 5 0 R".repeat(500_000); + let mut doc = doc_of(&[ + "<< /Type /Catalog /Pages 2 0 R >>".into(), + "<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 612 792] >>".into(), + "<< /Type /Page /Parent 2 0 R /Annots [4 0 R] >>".into(), + format!("<< /Type /Annot /Subtype /Ink /Rect [0 0 100 100] /InkList [{refs}] >>"), + "[0 0 8 0]".into(), + ]); + match erase_ink(&mut doc, 0, 0, &[[1.0, 1.0]], 2.0, &meta("")) { + Err(AnnotError::Invalid(m)) => assert!(m.contains("too large to erase"), "{m}"), + other => panic!("refused: {other:?}"), + } +} + /// A 3D annotation is PDF 1.6 and a PRC model 1.7 (ISO 32000-2 §13.6): an older document's /// catalog says so, and a newer one is left as it is. #[test]