From 9ed589bb66d7d8c1ec144bfdfaac040fd052faa6 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Tue, 3 Nov 2026 21:11:54 +0000 Subject: [PATCH] fix(edit): refuse a mark on top of content too large to read for open marked content --- crates/edit/src/apply.rs | 2 +- crates/edit/src/marked_isolation_tests.rs | 13 ++++++++++++ crates/edit/src/place.rs | 24 +++++++++++++++++++---- 3 files changed, 34 insertions(+), 5 deletions(-) diff --git a/crates/edit/src/apply.rs b/crates/edit/src/apply.rs index 8b52c81c76..4d5909e2df 100644 --- a/crates/edit/src/apply.rs +++ b/crates/edit/src/apply.rs @@ -320,7 +320,7 @@ pub fn remove_marks(doc: &mut Document, pages: &[usize], kind: MarkKind) -> Resu if before != kept.len() { let wrappers = match isolation(doc, &kept) { Isolation::Clean { wrappers, .. } => wrappers, - Isolation::Unknown { .. } | Isolation::TooDeep => { + Isolation::Unknown { .. } | Isolation::TooDeep | Isolation::TooLarge => { let mut opens = Vec::new(); let mut pairs = Vec::new(); for (i, o) in kept.iter().enumerate() { diff --git a/crates/edit/src/marked_isolation_tests.rs b/crates/edit/src/marked_isolation_tests.rs index 227371bcac..312d92e0ce 100644 --- a/crates/edit/src/marked_isolation_tests.rs +++ b/crates/edit/src/marked_isolation_tests.rs @@ -132,6 +132,19 @@ fn depth_budget_bounds_emitted_closures_and_overflow_is_refused_on_top() { } } +/// Content too large to read in full might leave a marked-content sequence (hidden optional +/// content) open, so nothing is put on top of it, and the page stays as it was. +#[test] +fn a_mark_is_refused_on_top_of_content_too_large_to_read() { + let mut big = b"/OC /Hidden BDC\n".to_vec(); + big.extend(b"0 g\n".repeat(1_000_001)); + let mut doc = fixture(&[&big]); + let page = page_list(&doc).remove(0).obj; + let before = (*doc.get(page)).clone(); + assert!(matches!(stamp(&mut doc, 0, "Probe", b"q Q\n".to_vec()), Err(EditError::Invalid(m)) if m.contains("too large"))); + assert_eq!((*doc.get(page)).clone(), before); +} + /// Marks refused on top of content nested past the depth budget leave the page as it was: old /// marks are not removed and no resources are added before the refusal. #[test] diff --git a/crates/edit/src/place.rs b/crates/edit/src/place.rs index ae2faf8386..9ffefb8c29 100644 --- a/crates/edit/src/place.rs +++ b/crates/edit/src/place.rs @@ -192,6 +192,9 @@ pub(crate) enum Isolation { /// Marked content nests past [`ISOLATION_MAX_DEPTH`]: how many sequences a mark on top would /// have to close (some may be hidden optional content) is unknown. TooDeep, + /// The content is too large to read in full (too many streams, bytes or operators), so + /// whether it leaves a marked-content sequence open is unknown. + TooLarge, } const ISOLATION_MAX_STREAMS: usize = 100_000; @@ -205,21 +208,29 @@ pub(crate) const ISOLATION_MAX_DEPTH: usize = 10_000; pub(crate) fn isolation(doc: &Document, list: &[Object]) -> Isolation { let unknown = Isolation::Unknown { unclosed: 0, marked: 0 }; if list.len() > ISOLATION_MAX_STREAMS { - return unknown; + return Isolation::TooLarge; } let mut remaining = ISOLATION_MAX_BYTES; let mut data = Vec::with_capacity(list.len()); for object in list { let resolved = doc.resolve(object); let Object::Stream(stream) = &*resolved else { return unknown }; - let Ok(bytes) = stream.decoded_within(remaining) else { return unknown }; - let Some(left) = remaining.checked_sub(bytes.len().saturating_add(1)) else { return unknown }; + // Past the byte allowance, as opposed to a stream that does not decode at all. + let bytes = match stream.decoded_within(remaining) { + Ok(bytes) => bytes, + Err(e) if e.to_string().contains("exceeds the limit") => return Isolation::TooLarge, + Err(_) => return unknown, + }; + let Some(left) = remaining.checked_sub(bytes.len().saturating_add(1)) else { return Isolation::TooLarge }; remaining = left; data.push(bytes); } let pieces = pdfcraft_content::Pieces::join(&data); let parsed = pieces.parse(); - if parsed.skipped != 0 || parsed.ops.len() > ISOLATION_MAX_OPS { + if parsed.ops.len() > ISOLATION_MAX_OPS { + return Isolation::TooLarge; + } + if parsed.skipped != 0 { return unknown; } let edges = pieces.comment_edges(&parsed); @@ -357,6 +368,7 @@ pub(crate) fn place_tagged(doc: &mut Document, page: &pdfcraft_model::Page, tag: Isolation::Unknown { unclosed, marked } => (behind_marks < list.len()).then_some((behind_marks, unclosed, marked)), // Content left inside an optional-content sequence it cannot close could be hidden. Isolation::TooDeep => return Err(too_deep_on_top()), + Isolation::TooLarge => return Err(too_large_on_top()), } }; let mark_ref = doc.add(Object::Stream(Stream::flate(tagged(tag), &content))); @@ -452,6 +464,9 @@ pub(crate) fn revealed_under_paint(doc: &Document, page: usize, invisible: bool, /// none otherwise. The page was checked to take them ([`check_on_top`]) before the edit began. pub(crate) fn lift_on_top(over: bool, paragraph: &mut Vec) -> Vec { if over { std::mem::take(paragraph) } else { Vec::new() } + +fn too_large_on_top() -> EditError { + EditError::Invalid("the page's content is too large to check for open marked content before adding anything on top of it".into()) } fn too_deep_on_top() -> EditError { @@ -470,6 +485,7 @@ pub(crate) fn check_on_top(doc: &Document, page: &pdfcraft_model::Page) -> Resul }; match isolation(doc, &list) { Isolation::TooDeep => Err(too_deep_on_top()), + Isolation::TooLarge => Err(too_large_on_top()), _ => Ok(()), } }