Merge pull request #4601 from storytold/fix/pdf-repeated-form-budget

Fix #3569: a PDF that draws one form over and over can't exhaust memory or hang the import

crates/engine/tests/import_fuzz.rs

@@ -3791,6 +3791,26 @@ fn pdf_streams_the_reader_would_fail_on_are_refused() {
     }
 }
 
+/// A page drawing a thousand times one form that decodes to 64 MiB (of spaces, then a square):
+/// each drawing reads the form again, so the import stops at the page's form content instead of
+/// decoding 64 GB (#3569, which hung).
+#[test]
+fn pdf_a_huge_form_drawn_over_and_over_is_refused_in_bounded_time() {
+    use std::io::Write as _;
+    let mut e = flate2::write::ZlibEncoder::new(Vec::new(), flate2::Compression::best());
+    for _ in 0..64 {
+        e.write_all(&[b' '; 1 << 20]).unwrap();
+    }
+    e.write_all(b"0 0 5 5 re f").unwrap();
+    let form = packed_as_is(FORM, &e.finish().unwrap());
+    let bytes = pdf_with(&"/F Do ".repeat(1000), "<< /XObject << /F 5 0 R >> >>", &[form]);
+    let start = std::time::Instant::now();
+    let e = vectorcraft_engine::cmd::fileio::load("repeated.pdf", &bytes).err().map(|e| e.to_string());
+    let took = start.elapsed();
+    assert!(e.as_deref().is_some_and(|e| e.contains("no document was imported")), "{e:?}");
+    assert!(took < std::time::Duration::from_secs(60), "took {took:?}");
+}
+
 #[test]
 fn pdf_art_that_draws_itself() {
     let own = "<< /XObject << /F 5 0 R >> /Pattern << /P 5 0 R >> /Font << /F1 5 0 R >> /ExtGState << /G << /SMask << /S /Alpha /G 5 0 R >> >> >> >>";

crates/pdf/src/import.rs

@@ -43,5 +43,5 @@ pub use entry::{
     import, import_with_report, is_glyph_outlines, is_outlines_note,
 };
 #[cfg(test)]
-pub(crate) use entry::{import_with_reader_options, read};
+pub(crate) use entry::{TEST_MAX_OBJECTS, import_with_reader_options, read};
 pub(crate) use state::{closed_subpaths, round3};

crates/pdf/src/import/builder.rs

@@ -1,5 +1,8 @@
 use super::cid::{CidText, FontInfo, comparable, font_chars, identify_glyph, matching_face, replacement, unify_radical};
-use super::entry::{MAX_NESTED, OVERPRINT_UNREAD, OVERPRINT_ZERO_MODE, PATTERN_REUSE, TEXT_OUTLINES, UNMATCHED_GLYPHS_NOTE, UNNAMED_CHARACTERS_NOTE};
+use super::entry::{
+    MAX_NESTED, OVERPRINT_UNREAD, OVERPRINT_ZERO_MODE, PATTERN_REUSE, TEXT_OUTLINES, TOO_MANY_OBJECTS, UNMATCHED_GLYPHS_NOTE,
+    UNNAMED_CHARACTERS_NOTE, max_objects,
+};
 use super::state::{Builder, Frame, FrameKind, Saved, bounds, grouped, line_width, mean_scale, paints, round3, stroke_layer};
 use super::{
     Affine, Appearance, AppearanceItem, Arc, BezPath, BfString, BlendMode, CacheKey, Color, Colors, Families, FillLayer, FillRule, FunctionBudget,
@@ -41,6 +44,7 @@ impl<'p> Builder<'p> {
             pattern_keys: HashMap::new(),
             function_paints: HashMap::new(),
             function_budget: FunctionBudget::new(),
+            objects_left: max_objects(),
             taken: vec![],
             scan: Scan::default(),
             tag_at: 0,
@@ -66,6 +70,9 @@ impl<'p> Builder<'p> {
 
     /// A deep copy of `node` with fresh ids.
     fn reid(&mut self, node: &Node) -> Node {
+        // Each copy counts against the import's objects: once they are spent nothing more is
+        // drawn through a mask, so the copying stops too.
+        self.objects_left = self.objects_left.saturating_sub(1);
         let mut n = node.clone();
         n.id = self.id();
         if let Some(ch) = n.children_mut() {
@@ -191,7 +198,15 @@ impl<'p> Builder<'p> {
         }
     }
 
+    /// Add `n`, drawn now, to the art: left out (with a warning) once the import's objects are
+    /// spent ([`TOO_MANY_OBJECTS`]).
     pub(super) fn push_node(&mut self, mut n: Node) {
+        let Some(left) = self.objects_left.checked_sub(1) else {
+            self.lines.remove(&n.id);
+            self.warn(TOO_MANY_OBJECTS);
+            return;
+        };
+        self.objects_left = left;
         if self.blend != BlendMode::Normal && !n.is_container() {
             n.blend = self.blend;
         }

crates/pdf/src/import/entry.rs

@@ -25,6 +25,35 @@ pub(super) const MAX_NESTED: u32 = 8;
 /// A tiling pattern read this many times with the same art is taken to always draw it.
 pub(super) const PATTERN_REUSE: u32 = 16;
 
+/// The most objects one import makes (paths, images, lines of type, and the copies of mask art
+/// each masked object takes): half of what a saved document may hold to open again
+/// ([`vectorcraft_doc::node::MAX_READ_NODES`]: 4M, 512K in the web build), leaving room for the
+/// groups, clips and layers around them. A page of a million paths still comes in whole, as an
+/// SVG or CAD drawing of two million objects does. What it stops is a small file drawing one form
+/// over and over, which neither the scan's operator limit nor the interpreter's budgets bound
+/// (#3569: 1 KB drawing 25 squares 32,000 times took 877 MB, about 1 KB an object, so this is
+/// about 2 GB).
+const MAX_OBJECTS: usize = vectorcraft_doc::node::MAX_READ_NODES / 2;
+
+/// Warning: the import reached [`MAX_OBJECTS`].
+pub(super) const TOO_MANY_OBJECTS: &str = "the file draws more objects than can be imported: the rest were left out";
+
+/// [`MAX_OBJECTS`], lowered by tests that need an import past it.
+#[cfg(not(test))]
+pub(super) fn max_objects() -> usize {
+    MAX_OBJECTS
+}
+
+#[cfg(test)]
+thread_local! {
+    pub(crate) static TEST_MAX_OBJECTS: std::cell::Cell<usize> = const { std::cell::Cell::new(MAX_OBJECTS) };
+}
+
+#[cfg(test)]
+pub(super) fn max_objects() -> usize {
+    TEST_MAX_OBJECTS.with(std::cell::Cell::get)
+}
+
 /// Stands for the group of art whose group couldn't be told (see [`Builder::unsure`]): it goes
 /// to a hidden, non-printing layer of its page.
 pub(super) const UNSORTED: usize = usize::MAX;

crates/pdf/src/import/state.rs

@@ -122,6 +122,8 @@ pub(super) struct Builder<'p> {
     pub(super) function_paints: HashMap<(u128, Option<Vec<String>>), Paint>,
     /// What is left of the import's function evaluations for those shadings.
     pub(super) function_budget: FunctionBudget,
+    /// What is left of the import's objects ([`super::entry::max_objects`]).
+    pub(super) objects_left: usize,
     /// Swatch names in use.
     pub(super) taken: Vec<String>,
     /// The page as walked by [`scan_page`], and how far the interpreter is through it.

crates/pdf/src/import_scan.rs

@@ -36,6 +36,14 @@ const MAX_STATES: usize = 65_536;
 /// takes a few million; the walk reads about 15 million a second, twice as fast as the
 /// interpreter draws them.
 const MAX_OPS: usize = 64_000_000;
+/// The most bytes of form XObject content walked per page, a form drawn again counted again:
+/// [`MAX_OPS`] guards content that is mostly operators, this content that isn't (a form of long
+/// strings, inline image data or spaces). An operator takes some 10 to 40 bytes (a curve's six
+/// numbers about 40), so a page of ordinary content within `MAX_OPS` walks up to about this much;
+/// a page that draws a form decoding to 64 MiB a thousand times (#3569, which hung) stops at its
+/// 33rd reading. The interpreter reads every form again too, and nothing else bounds what it
+/// decodes. (2 GiB: within a 32-bit `usize`, for the web build.)
+const MAX_FORM_CONTENT: usize = 2 << 30;
 /// The most marked-content sequences noted per page (24 bytes each): past it the rest of the
 /// page isn't told apart either.
 const MAX_TAGS: usize = 4_000_000;
@@ -461,6 +469,8 @@ struct Walker<'w, 'p> {
     ops: usize,
     max_ops: usize,
     max_form_bytes: usize,
+    /// The bytes of form content walked so far ([`MAX_FORM_CONTENT`]).
+    form_content: usize,
     out: Scan,
     /// A form XObject hidden by its own optional content was met.
     hidden_form: bool,
@@ -819,6 +829,11 @@ impl Walker<'_, '_> {
                         self.out.cut = true;
                         return;
                     };
+                    self.form_content = self.form_content.saturating_add(data.len());
+                    if self.form_content > MAX_FORM_CONTENT {
+                        self.out.cut = true;
+                        return;
+                    }
                     // Ordinary filtered form decoders may return a prefix at their limit.
                     // Reaching that bound cannot certify a complete form, even at an operator boundary.
                     if !s.filters().is_empty() && data.len() >= self.max_form_bytes {
@@ -1038,6 +1053,7 @@ fn walk_page<'w, 'p>(page: &Page<'p>, ocgs: &'w mut Ocgs, all_on: bool, fonts: &
         ops: 0,
         max_ops: max_ops(),
         max_form_bytes,
+        form_content: 0,
         out: Scan::default(),
         hidden_form: false,
         states: vec![State::default()],

crates/pdf/src/tests/import_fidelity.rs

@@ -616,6 +616,30 @@ fn function_based_shadings_share_one_budget_per_import() {
     assert!(r.warnings.iter().any(|w| w.contains("too many to read")), "{:?}", r.warnings);
 }
 
+/// A page drawing one small form over and over makes objects only up to the import's limit, then
+/// leaves the rest out with a warning, instead of taking memory for every drawing (#3569).
+#[test]
+fn a_form_drawn_over_and_over_stops_at_the_import_s_objects() {
+    let squares: String = (0..25).map(|i| format!("{} {} 2 2 re f ", i % 5 * 4, i / 5 * 4)).collect();
+    let form = stream("/Type /XObject /Subtype /Form /BBox [0 0 20 20]", &squares);
+    let bytes = one_page(&"/F Do ".repeat(200), &format!("/XObject << /F {} 0 R >>", first_extra(1)), &[&form]);
+    // With the import's objects lowered to `limit` (on this thread: the limit is this thread's).
+    let open_within = |limit: usize| {
+        let old = crate::import::TEST_MAX_OBJECTS.replace(limit);
+        let r = crate::import::read(&bytes, &ImportOptions::default());
+        crate::import::TEST_MAX_OBJECTS.set(old);
+        r.unwrap()
+    };
+    let paths = |r: &ImportReport| all(&r.document).iter().filter(|n| matches!(n.kind, NodeKind::Path { .. })).count();
+    let left_out = |r: &ImportReport| r.warnings.iter().any(|w| w.contains("the rest were left out"));
+    let whole = open_within(10_000);
+    assert_eq!(paths(&whole), 5000);
+    assert!(!left_out(&whole), "{:?}", whole.warnings);
+    let cut = open_within(1000);
+    assert_eq!(paths(&cut), 1000);
+    assert!(left_out(&cut), "{:?}", cut.warnings);
+}
+
 /// A colour that changes in step with the sampling grid along y only (one colour at every grid
 /// point) is still read along y, not as a flat gradient along x.
 #[test]