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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Pq7HyCfwzyRXj4VfQAVVdv

crates/annot/src/info.rs

@@ -76,7 +76,11 @@ pub(crate) fn popup_info(doc: &Document, own: Option<ObjRef>, annot: &Dict) -> O
     let p = doc.get(r);
     let p = p.as_dict()?;
     let rect: Vec<f32> = 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 })
 }

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<f64> =
+        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:?}");
+}

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<f32> = 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),

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");
+}