From eaeffa748b2ce5aa798352cb85690e2d44838ebf Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Sun, 8 Nov 2026 19:40:21 -0500 Subject: [PATCH] fix(measure): charge snapping per filter stage and bound listing work (#5920) --- crates/measure/src/lib.rs | 47 +++++++++++++++++++++-- crates/measure/src/snap.rs | 39 +++++++++++-------- crates/measure/src/tests.rs | 74 +++++++++++++++++++++++++++++++++++++ 3 files changed, 141 insertions(+), 19 deletions(-) diff --git a/crates/measure/src/lib.rs b/crates/measure/src/lib.rs index d52d31dcc6..7ff8bfdcf1 100644 --- a/crates/measure/src/lib.rs +++ b/crates/measure/src/lib.rs @@ -452,6 +452,13 @@ pub struct Listing { pub truncated: bool, } const MAX_ANNOTATIONS: usize = 100_000; +/// Vertex pairs one listing compares for self-intersection in all: an area's boundary check +/// is quadratic in its vertices, and many areas (or many annotations sharing one indirect vertex +/// array) must not multiply that into a stall. +const MAX_LISTED_WORK: usize = 50_000_000; +/// Scales already read by one listing, by the indirect `/Measure` object: one scale shared by +/// many annotations is read (and its ratio decoded) once, failures included. +type Scales = std::collections::HashMap>; /// Points and text one listing keeps, as counted by [`retained`]: about a thousand /// measurements of [`MAX_POINTS`] points each. const MAX_LISTED_BYTES: usize = 32 << 20; @@ -462,8 +469,18 @@ fn retained(m: &Measurement) -> usize { .into_iter() .fold(m.points.len().saturating_mul(std::mem::size_of::()), usize::saturating_add) } -fn read_measurement(doc: &Document, d: &Dict, kind: Kind, page: usize, index: usize) -> Result { - let scale = Scale::read(doc, d.get(b"Measure").ok_or_else(|| invalid("measurement has no scale"))?)?; +/// The scale a measurement annotation reads with, through the listing's cache: a scale given by +/// reference is read once, and its error kept, however many annotations name it. +fn scale_of(doc: &Document, d: &Dict, scales: &mut Scales) -> Result { + let measure = d.get(b"Measure").ok_or_else(|| invalid("measurement has no scale"))?; + match measure.as_ref() { + Some(r) => scales.entry(r).or_insert_with(|| Scale::read(doc, measure).map_err(|e| e.to_string())).clone().map_err(|e| invalid(&e)), + None => Scale::read(doc, measure), + } +} + +fn read_measurement(doc: &Document, d: &Dict, kind: Kind, page: usize, index: usize, scales: &mut Scales) -> Result { + let scale = scale_of(doc, d, scales)?; let key = if kind == Kind::Distance { &b"L"[..] } else { b"Vertices" }; let o = doc.resolve(d.get(key).ok_or_else(|| invalid("measurement has no vertices"))?); let a = o.as_array().ok_or_else(|| invalid("invalid measurement vertices"))?; @@ -482,7 +499,9 @@ pub fn list(doc: &Document) -> Listing { let mut out = Listing::default(); let mut count = 0usize; let mut kept = 0usize; + let mut work = 0usize; let mut seen = std::collections::HashSet::new(); + let mut scales = Scales::new(); for (page, p) in pdfcraft_model::pages(doc).iter().enumerate() { let Some(o) = p.dict.get(b"Annots") else { continue }; let o = doc.resolve(o); @@ -507,7 +526,29 @@ pub fn list(doc: &Document) -> Listing { Some(b"PolygonDimension") => Kind::Area, _ => continue, }; - match read_measurement(doc, d, kind, page, index) { + // An area's boundary check compares every pair of edges: charge it before reading the + // vertices, so a listing stops at its work bound rather than at a stall. Other kinds + // read their vertices once. + let vertices = d.get(if kind == Kind::Distance { b"L".as_slice() } else { b"Vertices" }).map(|v| doc.resolve(v)); + let n = vertices.as_deref().and_then(Object::as_array).map_or(0, |a| a.len() / 2).min(MAX_POINTS); + // An area's boundary check compares every pair of edges: charge it before reading the + // vertices, so a listing stops at its work bound rather than at a stall. The scale is + // resolved first, as `read_measurement` reads it: one that is rejected reads nothing at + // all, so it must not spend the work that later, valid measurements need. + if kind == Kind::Area { + if let Err(e) = scale_of(doc, d, &mut scales) { + out.unsupported.push(Unsupported { page, index, reason: e.to_string() }); + continue; + } + work = work.saturating_add(n.saturating_mul(n)); + } else { + work = work.saturating_add(n.max(1)); + } + if work > MAX_LISTED_WORK { + out.truncated = true; + return out; + } + match read_measurement(doc, d, kind, page, index, &mut scales) { Ok(m) => { kept = kept.saturating_add(retained(&m)); if kept > MAX_LISTED_BYTES { diff --git a/crates/measure/src/snap.rs b/crates/measure/src/snap.rs index b26c389e96..c39d811d9f 100644 --- a/crates/measure/src/snap.rs +++ b/crates/measure/src/snap.rs @@ -1,7 +1,7 @@ //! Bounded path extraction and snapping in PDF user space, including nested Form XObjects. use crate::{Point, Result, check_points, distance, invalid}; -use pdfcraft_content::{Matrix, parse}; -use pdfcraft_cos::{Dict, Document, ObjRef, Object}; +use pdfcraft_content::{Matrix, ops}; +use pdfcraft_cos::{DecodeBudget, Dict, Document, ObjRef, Object}; use serde::Serialize; use std::collections::HashSet; @@ -199,7 +199,9 @@ struct Walker<'a> { doc: &'a Document, geometry: Geometry, seen: HashSet, - bytes: usize, + /// Decoded bytes left for the page: every filter stage's output counts, so a stream that + /// expands and then shrinks pays for the expansion each time it is drawn. + budget: DecodeBudget, streams: usize, } impl Walker<'_> { @@ -215,46 +217,44 @@ impl Walker<'_> { for part in arr { let s = self.doc.resolve(part); if let Object::Stream(s) = s.as_ref() { - let remaining = MAX_BYTES.saturating_sub(self.bytes).saturating_sub(joined.len()); + let remaining = self.budget.remaining(); if remaining == 0 { self.geometry.truncated = true; break; } // An undecodable part is skipped; the parts before it still snap. - match s.decoded_within(remaining) { + match self.budget.decode_within(s, remaining) { Ok(data) => { joined.extend(data); // A space, as the renderer joins pieces: a trailing comment runs on. joined.push(b' '); } - Err(_) => self.decode_failed(remaining), + Err(_) => self.decode_failed(), } } } return self.ops(&joined, resources, initial, depth); } let Object::Stream(stream) = o.as_ref() else { return Ok(()) }; - let remaining = MAX_BYTES.saturating_sub(self.bytes); + let remaining = self.budget.remaining(); if remaining == 0 { self.geometry.truncated = true; return Ok(()); } - match stream.decoded_within(remaining) { + match self.budget.decode_within(stream, remaining) { Ok(data) => self.ops(&data, resources, initial, depth), Err(_) => { - self.decode_failed(remaining); + self.decode_failed(); Ok(()) } } } - /// A tolerant decode fails on an unsupported filter or once it has produced `remaining` bytes, - /// so a failure uses up the byte budget: repeating a failing stream cannot repeat that work. - fn decode_failed(&mut self, remaining: usize) { + /// A failed decode has paid the budget for what it decoded before failing (all of it, past + /// the bound): repeating a failing stream cannot repeat that work for free. + fn decode_failed(&mut self) { self.geometry.unreadable = self.geometry.unreadable.saturating_add(1); - self.bytes = self.bytes.saturating_add(remaining); } fn ops(&mut self, data: &[u8], resources: &Dict, initial: Matrix, depth: usize) -> Result<()> { - self.bytes = self.bytes.saturating_add(data.len()); self.streams = self.streams.saturating_add(1); let mut matrix = initial; let mut stack = Vec::new(); @@ -268,7 +268,8 @@ impl Walker<'_> { let (x, y) = m.apply(x, y); [x, y] }; - for op in parse(data).ops { + // One operator at a time: a stream of millions of short operators is never held whole. + for op in ops(data) { if self.geometry.segments.len().saturating_add(path.segments.len()) >= MAX_SEGMENTS || self.geometry.targets().saturating_add(path.targets()) >= MAX_TARGETS { @@ -425,6 +426,11 @@ impl Walker<'_> { self.seen.insert(r); } self.walk(o, res, m, depth + 1)?; + // A form that hit a bound ends this walk too: more `Do`s would + // only repeat the work of copying its resources. + if self.geometry.truncated { + break; + } if let Some(r) = r { self.seen.remove(&r); } @@ -449,7 +455,8 @@ pub fn geometry(doc: &Document, page_index: usize) -> Result { let resources = p.dict.get(b"Resources").map(|o| doc.resolve(o)); let empty = Dict::new(); let resources = resources.as_ref().and_then(|o| o.as_dict()).unwrap_or(&empty); - let mut walker = Walker { doc, geometry: Geometry::default(), seen: HashSet::new(), bytes: 0, streams: 0 }; + let mut walker = + Walker { doc, geometry: Geometry::default(), seen: HashSet::new(), budget: DecodeBudget::new(MAX_BYTES, usize::MAX), streams: 0 }; // Extraction is lenient: limits and unreadable streams are reported in the geometry. if let Some(contents) = p.dict.get(b"Contents") { walker.walk(contents, resources, Matrix::IDENTITY, 0)?; diff --git a/crates/measure/src/tests.rs b/crates/measure/src/tests.rs index 88906910c7..f265f086ec 100644 --- a/crates/measure/src/tests.rs +++ b/crates/measure/src/tests.rs @@ -471,3 +471,77 @@ fn a_stream_over_the_byte_budget_uses_it_up_once() { assert!(g.truncated); assert_eq!(g.segments.len(), 1, "the parts before it still snap"); } + +/// A content stream drawn again and again pays for every filter stage's output each time, not +/// only for what the last stage leaves: `/ASCIIHexDecode /RunLengthDecode` turns 4 MiB of hex +/// into 2 MiB and then 1 MiB, and eight draws of it are 24 MiB of decoding against a 16 MiB bound. +#[test] +fn repeated_content_pays_for_every_filter_stage() { + let hex = "0058".repeat(1 << 20); + let pdf = Pdf::new() + .obj("<< /Type /Catalog /Pages 2 0 R >>") + .obj("<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 612 792] >>") + .obj("<< /Type /Page /Parent 2 0 R /Contents [4 0 R 4 0 R 4 0 R 4 0 R 4 0 R 4 0 R 4 0 R 4 0 R] >>") + .stream("/Filter [/ASCIIHexDecode /RunLengthDecode]", &hex); + let d = Document::open(Arc::new(pdf.build())).unwrap(); + let g = snap::geometry(&d, 0).unwrap(); + assert!(g.truncated, "eight draws decode past the page's bound"); +} + +/// One scale shared by many measurements is read once, and the self-intersection check of many +/// areas is charged against one work bound rather than run in full for each. +#[test] +fn listing_reads_a_shared_scale_once_and_bounds_boundary_checks() { + let measure = "<< /Type /Measure /Subtype /RL /R (1 pt = 1 pt) /X [<< /Type /NumberFormat /U (pt) /C 1 /D 100 >>] /D [<< /Type /NumberFormat /U (pt) /C 1 /D 100 >>] /A [<< /Type /NumberFormat /U (pt) /C 1 /D 100 >>] >>"; + // A convex 2,048-gon: every one of its edges is checked against every other. + let n = MAX_POINTS; + let verts: Vec = (0..n) + .map(|i| { + let a = i as f64 / n as f64 * std::f64::consts::TAU; + format!("{:.3} {:.3}", 300.0 + 100.0 * a.cos(), 400.0 + 100.0 * a.sin()) + }) + .collect(); + let verts = verts.join(" "); + let areas = 40; + let annots: Vec = (0..areas).map(|i| format!("{} 0 R", 6 + i)).collect(); + let mut pdf = Pdf::new() + .obj("<< /Type /Catalog /Pages 2 0 R >>") + .obj("<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 612 792] >>") + .obj(format!("<< /Type /Page /Parent 2 0 R /Contents 4 0 R /Annots [{}] >>", annots.join(" "))) + .stream("", "") + .obj(measure); + for _ in 0..areas { + pdf = pdf.obj(format!("<< /Type /Annot /Subtype /Polygon /IT /PolygonDimension /Rect [0 0 612 792] /Vertices [{verts}] /Measure 5 0 R >>")); + } + let d = Document::open(Arc::new(pdf.build())).unwrap(); + let l = list(&d); + // 40 areas of 2,048 vertices are 168 million pairs: the listing stops at its bound, with the + // areas before it listed. + assert!(l.truncated, "{} listed", l.measurements.len()); + assert!((5..areas).contains(&l.measurements.len()), "{} listed", l.measurements.len()); + assert!(l.unsupported.is_empty(), "{:?}", l.unsupported); +} + +/// Perimeters and distances read their vertices once: only areas pay for the pairwise boundary +/// check, so many maximal perimeters list in full. +#[test] +fn perimeters_are_charged_linear_work() { + let measure = "<< /Type /Measure /Subtype /RL /R (1 pt = 1 pt) /X [<< /Type /NumberFormat /U (pt) /C 1 /D 100 >>] /D [<< /Type /NumberFormat /U (pt) /C 1 /D 100 >>] /A [<< /Type /NumberFormat /U (pt) /C 1 /D 100 >>] >>"; + let verts: Vec = (0..MAX_POINTS).map(|i| format!("{} {}", 10 + i % 500, 10 + i / 500 * 3 + (i % 2))).collect(); + let verts = verts.join(" "); + let lines = 40; + let annots: Vec = (0..lines).map(|i| format!("{} 0 R", 6 + i)).collect(); + let mut pdf = Pdf::new() + .obj("<< /Type /Catalog /Pages 2 0 R >>") + .obj("<< /Type /Pages /Kids [3 0 R] /Count 1 /MediaBox [0 0 612 792] >>") + .obj(format!("<< /Type /Page /Parent 2 0 R /Contents 4 0 R /Annots [{}] >>", annots.join(" "))) + .stream("", "") + .obj(measure); + for _ in 0..lines { + pdf = pdf.obj(format!("<< /Type /Annot /Subtype /PolyLine /IT /PolyLineDimension /Rect [0 0 612 792] /Vertices [{verts}] /Measure 5 0 R >>")); + } + let d = Document::open(Arc::new(pdf.build())).unwrap(); + let l = list(&d); + assert!(!l.truncated, "{} listed", l.measurements.len()); + assert_eq!(l.measurements.len() + l.unsupported.len(), lines, "{:?}", l.unsupported.first()); +}