From c2251a4e1665fdeb4881fc19cf20160ada4d6490 Mon Sep 17 00:00:00 2001 From: mtlcl410 <2368091+mtlcl410@users.noreply.github.com> Date: Sun, 1 Nov 2026 16:38:13 +0800 Subject: [PATCH] M5: a pop-up /Rect too large for f32 reads as no pop-up; reversed corners are tested (#2137 review) A file's huge number became infinite as f32 and reached the window's place. Both readers (annot summaries and the render inspector) now take such a rectangle as no pop-up, as one that isn't four numbers; opening it gives it a new place. Reversed corners were already put in order; tests now cover both, read from a file and saved through set_popup. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01Pq7HyCfwzyRXj4VfQAVVdv --- crates/annot/src/info.rs | 4 +++ crates/annot/src/tests.rs | 31 ++++++++++++++++++++++++ crates/render/src/inspect/annotations.rs | 8 ++++-- crates/render/src/inspect/tests.rs | 16 ++++++++++++ 4 files changed, 57 insertions(+), 2 deletions(-) diff --git a/crates/annot/src/info.rs b/crates/annot/src/info.rs index 18a395a848..b6fa661482 100644 --- a/crates/annot/src/info.rs +++ b/crates/annot/src/info.rs @@ -76,7 +76,11 @@ pub(crate) fn popup_info(doc: &Document, own: Option, annot: &Dict) -> O let p = doc.get(r); let p = p.as_dict()?; let rect: Vec = doc.resolve(p.get(b"Rect")?).as_array()?.iter().filter_map(|v| doc.resolve(v).as_f64().map(|x| x as f32)).collect(); + // Four numbers that are finite as `f32` (a huge one becomes infinite), corners in any order. let [x0, y0, x1, y1] = rect[..] else { return None }; + if !rect.iter().all(|v| v.is_finite()) { + return None; + } let open = p.get(b"Open").map(|o| doc.resolve(o)).is_some_and(|o| matches!(&*o, Object::Bool(true))); Some(PopupInfo { rect: [x0.min(x1), y0.min(y1), x0.max(x1), y0.max(y1)], open }) } diff --git a/crates/annot/src/tests.rs b/crates/annot/src/tests.rs index 4e45d1d777..d25cbacdb9 100644 --- a/crates/annot/src/tests.rs +++ b/crates/annot/src/tests.rs @@ -2320,3 +2320,34 @@ fn opening_a_pop_up_without_a_rect_gives_it_one() { assert!(popup.rect[2] - popup.rect[0] >= 100.0 && popup.rect[3] - popup.rect[1] >= 100.0, "a window's size: {:?}", popup.rect); assert_eq!(annot_dict(&doc, r).reference(b"Popup"), Some(pr), "the same pop-up, not a new one"); } + +/// #2137 review: a pop-up rectangle with its corners reversed is read, and saved, with them in +/// order (ISO 32000-2 §7.9.5: a reader normalises a rectangle). One whose numbers aren't finite +/// as `f32` (a file's huge number) reads as no pop-up, and opening it gives it a new place. +#[test] +fn a_pop_up_rect_with_reversed_corners_is_put_in_order_and_a_huge_one_is_none() { + let mut doc = fixture(); + let note = add_annotation(&mut doc, &new(0, Shape::Note { at: [20.0, 280.0], icon: NoteIcon::Comment }), &meta("n1")).unwrap(); + let (_, r) = annot_ref(&mut doc, 0, note).unwrap(); + let pr = annot_dict(&doc, r).reference(b"Popup").expect("new notes have a pop-up"); + let popup = |doc: &Document| summaries(doc).into_iter().find(|s| s.index == note).unwrap().popup; + let set_rect = |doc: &mut Document, rect: [f64; 4]| doc.update_dict(pr, |p| p.set(b"Rect".to_vec(), num_array(&rect))).unwrap(); + + // Reversed in the file: read in order. + set_rect(&mut doc, [290.0, 200.0, 150.0, 100.0]); + assert_eq!(popup(&doc), Some(PopupInfo { rect: [150.0, 100.0, 290.0, 200.0], open: false })); + // Reversed through `set_popup`: saved in order. + set_popup(&mut doc, 0, note, Some([300.0, 220.0, 160.0, 120.0]), None, &meta("n1")).unwrap(); + let saved: Vec = + doc.get(pr).as_dict().and_then(|p| p.get(b"Rect")).and_then(Object::as_array).unwrap().iter().filter_map(Object::as_f64).collect(); + assert_eq!(saved, [160.0, 120.0, 300.0, 220.0]); + + // A number too large for `f32` (or infinite): no pop-up, and opening it gives it a place. + for huge in [1e300, f64::INFINITY] { + set_rect(&mut doc, [0.0, 0.0, huge, 100.0]); + assert_eq!(popup(&doc), None, "{huge}"); + } + set_popup(&mut doc, 0, note, None, Some(true), &meta("n1")).unwrap(); + let given = popup(&doc).expect("the pop-up reads again"); + assert!(given.open && given.rect.iter().all(|v| v.is_finite()), "{given:?}"); +} diff --git a/crates/render/src/inspect/annotations.rs b/crates/render/src/inspect/annotations.rs index 36e0556f65..7139a874ae 100644 --- a/crates/render/src/inspect/annotations.rs +++ b/crates/render/src/inspect/annotations.rs @@ -166,12 +166,16 @@ impl<'a> Inspector<'a> { && p.get(b"Parent") .map_or(true, |parent| parent.as_reference().is_ok() && self.resolved_id(parent).is_some_and(|id| Some(id) == own)) }; - // A pop-up without a `/Rect` of four numbers is none, as `pdfcraft_annot::summaries` - // reads it: both lists of the comments agree. + // A pop-up without a `/Rect` of four numbers finite as `f32` is none, as + // `pdfcraft_annot::summaries` reads it: both lists of the comments agree. Its corners + // may come in any order. let popup = d.get(b"Popup").ok().and_then(|o| self.dict(o)).filter(is_own).and_then(|p| { let rect = self.resolve(p.get(b"Rect").ok()?).as_array().ok()?; let nums: Vec = rect.iter().filter_map(|v| self.resolve(v).as_float().ok()).collect(); let [x0, y0, x1, y1] = nums[..] else { return None }; + if !nums.iter().all(|v| v.is_finite()) { + return None; + } Some(Popup { rect: [x0.min(x1), y0.min(y1), x0.max(x1), y0.max(y1)], open: p.get(b"Open").ok().and_then(|o| self.resolve(o).as_bool().ok()).unwrap_or(false), diff --git a/crates/render/src/inspect/tests.rs b/crates/render/src/inspect/tests.rs index 62bc130290..69466b0e07 100644 --- a/crates/render/src/inspect/tests.rs +++ b/crates/render/src/inspect/tests.rs @@ -1916,3 +1916,19 @@ fn a_pop_up_without_a_four_number_rect_is_none() { assert_eq!(note.popup.is_some(), read, "{rect:?}"); } } + +/// #2137 review: a pop-up `/Rect` with its corners reversed reads with them in order (ISO +/// 32000-2 §7.9.5), and one with a number too large for `f32` reads as no pop-up, as +/// `pdfcraft_annot::summaries` reads both. +#[test] +fn a_pop_up_rect_with_reversed_corners_is_put_in_order_and_a_huge_one_is_none() { + let bytes = "%PDF-1.7\n1 0 obj << /Type /Catalog /Pages 2 0 R >> endobj\n2 0 obj << /Type /Pages /Kids [3 0 R] /Count 1 >> endobj\n3 0 obj << /Type /Page /Parent 2 0 R /MediaBox [0 0 200 300] /Annots [4 0 R 5 0 R] >> endobj\n4 0 obj << /Type /Annot /Subtype /Text /Rect [20 260 40 280] /Contents (Hi) /Popup 5 0 R >> endobj\n5 0 obj << /Type /Annot /Subtype /Popup /Rect RECT /Parent 4 0 R /Open true >> endobj\ntrailer << /Root 1 0 R >>\n%%EOF\n"; + let popup = |rect: &str| { + let info = inspect(Arc::new(bytes.replace("RECT", rect).into_bytes()), None).expect("opens"); + info.annotations.iter().find(|a| a.subtype == "Text").expect("the note").popup.map(|p| p.rect) + }; + assert_eq!(popup("[150 200 50 100]"), Some([50.0, 100.0, 150.0, 200.0]), "reversed"); + let huge = format!("1{}", "0".repeat(40)); + assert_eq!(popup(&format!("[0 0 {huge} 100]")), None, "a number past f32"); + assert_eq!(popup(&format!("[0 0 {huge}.0 100]")), None, "a real past f32"); +}