From 2153da064438b461ded6e5d605a65752d301a4de Mon Sep 17 00:00:00 2001 From: Zinadin Zidan Date: Sat, 7 Nov 2026 03:06:01 +0600 Subject: [PATCH] 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 | 19 +++++++++++++++---- crates/edit/src/tests.rs | 8 ++++++++ 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/crates/edit/src/added/store.rs b/crates/edit/src/added/store.rs index df0b697ff9..3555910f04 100644 --- a/crates/edit/src/added/store.rs +++ b/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 { 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 = cmyk.as_array()?.iter().map(Object::as_f64).collect::>()?; - 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(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 { let r = nums(doc, d.get(b"Rect")); let rect: [f64; 4] = r.try_into().ok()?; diff --git a/crates/edit/src/tests.rs b/crates/edit/src/tests.rs index 1edcb65c25..8d9f5c58d2 100644 --- a/crates/edit/src/tests.rs +++ b/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"))]),