Add text: a stored ink is read without copying a long array (review)

Reading an item's /CMYK resolved the value, which copied a direct array,
then collected every entry before checking there were four. A file with a
huge numeric /CMYK paid for that on each content listing and item lookup.
Now a direct value is read where it is, a reference is followed without a
copy, and the array's length is checked before any entry is read. /Gray is
read the same way. A long /CMYK, direct or by reference, and a /Gray that is
a long array still read as no ink.

crates/edit/src/added/store.rs

@@ -8,15 +8,26 @@ use super::{Content, Dict, Document, EditError, GlyphError, MarkKind, ObjRef, Ob
 /// text paints in its RGB `/Color`.
 fn ink(doc: &Document, d: &Dict) -> Option<Ink> {
     let ink = if let Some(g) = d.get(b"Gray") {
-        Ink::Gray(doc.resolve(g).as_f64()?)
+        Ink::Gray(read_value(doc, g, Object::as_f64)?)
     } else {
-        let cmyk = doc.resolve(d.get(b"CMYK")?);
-        let parts: Vec<f64> = cmyk.as_array()?.iter().map(Object::as_f64).collect::<Option<_>>()?;
-        Ink::Cmyk(parts.try_into().ok()?)
+        // Four entries or none: a longer array is never copied or read through.
+        read_value(doc, d.get(b"CMYK")?, |cmyk| {
+            let [c, m, y, k] = cmyk.as_array()?.as_slice() else { return None };
+            Some(Ink::Cmyk([c.as_f64()?, m.as_f64()?, y.as_f64()?, k.as_f64()?]))
+        })?
     };
     Some(ink).filter(|i| i.valid())
 }
 
+/// `f` of `o`, a reference followed. A direct value is read where it is, never copied, so a
+/// large one a file puts there costs nothing to look at.
+fn read_value<T>(doc: &Document, o: &Object, f: impl FnOnce(&Object) -> T) -> T {
+    match o {
+        Object::Ref(_) => f(&doc.resolve(o)),
+        direct => f(direct),
+    }
+}
+
 fn parse(doc: &Document, d: &Dict) -> Option<Content> {
     let r = nums(doc, d.get(b"Rect"));
     let rect: [f64; 4] = r.try_into().ok()?;

crates/edit/src/tests.rs

@@ -1120,6 +1120,10 @@ fn a_hand_edited_ink_reads_back_only_when_it_is_the_stored_colours_look() {
     add_content(&mut doc, 0, &Content::Text(AddedText { rect: [72.0, 600.0, 300.0, 700.0], text: "Proof".into(), ..AddedText::default() })).unwrap();
     let obj = list_added(&doc)[0].obj;
     let real = |v: &[f64]| Object::Array(v.iter().map(|x| Object::Real(*x)).collect());
+    // A `/CMYK` far longer than four numbers, as a hostile file might store it: an ink first.
+    let long = real(&[0.0, 0.0, 0.0, 1.0].repeat(50_000));
+    let long_ref = Object::Ref(doc.add(long.clone()));
+    let ink_ref = Object::Ref(doc.add(real(&[0.0, 0.0, 0.0, 1.0])));
     let read_text = |entries: &[(&[u8], Object)]| {
         let mut edited = doc.clone();
         let Object::Stream(mut s) = (*edited.get(obj)).clone() else { panic!("an item is a stream") };
@@ -1136,6 +1140,7 @@ fn a_hand_edited_ink_reads_back_only_when_it_is_the_stored_colours_look() {
     // As PDFCraft writes it: the ink stands.
     assert_eq!(read(&[(b"Color", real(&[0.5; 3])), (b"Gray", Object::Real(0.5))]), Some(Ink::Gray(0.5)));
     assert_eq!(read(&[(b"Color", real(&[0.0; 3])), (b"CMYK", real(&[0.0, 0.0, 0.0, 1.0]))]), Some(Ink::Cmyk([0.0, 0.0, 0.0, 1.0])));
+    assert_eq!(read(&[(b"Color", real(&[0.0; 3])), (b"CMYK", ink_ref)]), Some(Ink::Cmyk([0.0, 0.0, 0.0, 1.0])), "by reference");
     // Finer than it paints, it reads back as painted (`0.5 g`), its look with it.
     let fine = read_text(&[(b"Color", real(&[0.50049; 3])), (b"Gray", Object::Real(0.50049))]);
     assert_eq!((fine.ink, fine.color), (Some(Ink::Gray(0.5)), [0.5; 3]));
@@ -1143,6 +1148,9 @@ fn a_hand_edited_ink_reads_back_only_when_it_is_the_stored_colours_look() {
         ("a colour the ink isn't the look of", vec![(&b"Color"[..], real(&[1.0, 0.0, 0.0])), (b"CMYK", real(&[0.0, 0.0, 0.0, 1.0]))]),
         ("three CMYK numbers", vec![(b"Color", real(&[0.0; 3])), (b"CMYK", real(&[0.0, 0.0, 1.0]))]),
         ("five CMYK numbers", vec![(b"Color", real(&[0.0; 3])), (b"CMYK", real(&[0.0, 0.0, 0.0, 1.0, 0.0]))]),
+        ("a long CMYK", vec![(b"Color", real(&[0.0; 3])), (b"CMYK", long.clone())]),
+        ("a long CMYK by reference", vec![(b"Color", real(&[0.0; 3])), (b"CMYK", long_ref.clone())]),
+        ("gray a long array", vec![(b"Color", real(&[0.0; 3])), (b"Gray", long.clone())]),
         ("CMYK not an array", vec![(b"Color", real(&[0.0; 3])), (b"CMYK", Object::Real(1.0))]),
         ("gray out of range", vec![(b"Color", real(&[1.0; 3])), (b"Gray", Object::Real(2.0))]),
         ("gray not a number", vec![(b"Color", real(&[0.0; 3])), (b"Gray", Object::name("Black"))]),