fix(render): clear asset caches between watch renders

crates/rustmotion-components/src/gif.rs

@@ -102,8 +102,26 @@ fn stall_decode_for_tests() {
     }
 }
 
+/// Hard ceiling on one composed GIF canvas frame's byte size
+/// (`width × height × 4`), independent of the render's own video dimensions.
+/// A GIF's logical-screen descriptor is two `u16` fields straight from the
+/// file header — up to 65535×65535, a 17.2 GiB single allocation — and
+/// nothing validated them before this cap existed.
+const MAX_GIF_CANVAS_BYTES: u64 = 128 * 1024 * 1024;
+
+/// Hard ceiling on the number of animation frames one GIF decodes into.
+/// Real GIFs rarely exceed a few hundred; this keeps a maliciously (or just
+/// accidentally) long frame count from growing the decoded frame list
+/// without bound even when each individual frame is well under the canvas
+/// budget above.
+const MAX_GIF_FRAMES: usize = 600;
+
 /// Decode a GIF into full-canvas RGBA frames, their cumulative end times, and
-/// the total duration.
+/// the total duration. `max_canvas_w`/`max_canvas_h` are the render's own
+/// video dimensions: a GIF canvas larger than that can never be usefully
+/// drawn (it only ever gets scaled into the component's layout box, which is
+/// at most the video frame), so it is rejected the same way an
+/// over-budget canvas is.
 ///
 /// Every frame after the first is usually a *sub-rectangle* holding only the
 /// pixels that changed, so frames must be composed onto a persistent canvas
@@ -113,7 +131,7 @@ fn stall_decode_for_tests() {
 /// why only the first frame ever appeared (issue #185).
 ///
 /// `None` means nothing can be drawn, and the reason has already been reported.
-fn decode_composed_frames(src: &str) -> Option<DecodedGif> {
+fn decode_composed_frames(src: &str, max_canvas_w: u32, max_canvas_h: u32) -> Option<DecodedGif> {
     let file = match std::fs::File::open(src) {
         Ok(f) => f,
         Err(e) => {
@@ -139,6 +157,18 @@ fn decode_composed_frames(src: &str) -> Option<DecodedGif> {
     let canvas_w = decoder.width() as u32;
     let canvas_h = decoder.height() as u32;
 
+    let canvas_bytes = canvas_w as u64 * canvas_h as u64 * 4;
+    if canvas_bytes > MAX_GIF_CANVAS_BYTES || canvas_w > max_canvas_w || canvas_h > max_canvas_h {
+        if crate::warn_once_for(&format!("gif-oversized:{src}")) {
+            eprintln!(
+                "rustmotion: gif '{src}' declares a {canvas_w}x{canvas_h} canvas ({canvas_bytes} \
+                 bytes/frame), over the {MAX_GIF_CANVAS_BYTES}-byte budget or larger than this \
+                 render's own {max_canvas_w}x{max_canvas_h} video — refusing to decode it."
+            );
+        }
+        return None;
+    }
+
     #[cfg(test)]
     stall_decode_for_tests();
 
@@ -148,6 +178,16 @@ fn decode_composed_frames(src: &str) -> Option<DecodedGif> {
     let mut composed = vec![0u8; canvas_w as usize * canvas_h as usize * 4];
 
     while let Ok(Some(frame)) = decoder.read_next_frame() {
+        if frames.len() >= MAX_GIF_FRAMES {
+            if crate::warn_once_for(&format!("gif-frame-cap:{src}")) {
+                eprintln!(
+                    "rustmotion: gif '{src}' has more than {MAX_GIF_FRAMES} frames — truncating \
+                     the decoded animation at the cap."
+                );
+            }
+            break;
+        }
+
         // `Previous` disposal restores what was there before this frame, so it
         // has to be captured before compositing.
         let restore = (frame.dispose == gif::DisposalMethod::Previous).then(|| composed.clone());
@@ -189,10 +229,14 @@ fn decode_composed_frames(src: &str) -> Option<DecodedGif> {
 /// failure (`decode_composed_frames` returning `None`) leaves the entry
 /// vacant — `or_try_insert_with` never calls `insert` on its `Err` path —
 /// so a broken source is retried rather than permanently cached as absent.
-fn cached_decode(src: &str) -> Option<Arc<DecodedGif>> {
+fn cached_decode(src: &str, max_canvas_w: u32, max_canvas_h: u32) -> Option<Arc<DecodedGif>> {
     gif_cache()
         .entry(src.to_string())
-        .or_try_insert_with(|| decode_composed_frames(src).map(Arc::new).ok_or(()))
+        .or_try_insert_with(|| {
+            decode_composed_frames(src, max_canvas_w, max_canvas_h)
+                .map(Arc::new)
+                .ok_or(())
+        })
         .ok()
         .map(|entry| entry.clone())
 }
@@ -205,7 +249,7 @@ impl Painter for Gif {
         _props: &AnimatedProperties,
         ctx: &PaintCtx,
     ) {
-        let Some(cached) = cached_decode(&self.src) else {
+        let Some(cached) = cached_decode(&self.src, ctx.video_width, ctx.video_height) else {
             return;
         };
 
@@ -282,7 +326,8 @@ mod tests {
         write_two_frame_gif(&path);
 
         let (frames, times, total) =
-            decode_composed_frames(path.to_str().expect("utf-8 path")).expect("gif must decode");
+            decode_composed_frames(path.to_str().expect("utf-8 path"), 1920, 1080)
+                .expect("gif must decode");
         std::fs::remove_file(&path).ok();
 
         assert_eq!(frames.len(), 2, "both frames must be drawable");
@@ -312,7 +357,7 @@ mod tests {
     #[test]
     fn a_missing_file_reports_instead_of_returning_nothing() {
         let missing = std::env::temp_dir().join("rustmotion_gif_absent_xyz.gif");
-        assert!(decode_composed_frames(missing.to_str().expect("utf-8")).is_none());
+        assert!(decode_composed_frames(missing.to_str().expect("utf-8"), 1920, 1080).is_none());
         // The warn-once slot must have been claimed — silence is the bug.
         assert!(
             !crate::warn_once_for(&format!("gif-open:{}", missing.to_str().expect("utf-8"))),
@@ -371,7 +416,7 @@ mod tests {
                 let src = src.clone();
                 std::thread::spawn(move || {
                     barrier.wait();
-                    cached_decode(&src)
+                    cached_decode(&src, 4, 2)
                 })
             })
             .collect();
@@ -392,4 +437,117 @@ mod tests {
             );
         }
     }
+
+    /// Writes a GIF whose logical-screen descriptor declares `w`×`h` but
+    /// whose only frame is 1×1 — the crafted-header shape a decompression
+    /// bomb takes: a file of a few hundred bytes that asks the decoder to commit to a
+    /// canvas orders of magnitude larger than anything it actually encodes.
+    fn write_oversized_header_gif(path: &std::path::Path, w: u16, h: u16) {
+        let palette: &[u8] = &[0, 0, 0, 255, 255, 255];
+        let mut file = std::fs::File::create(path).expect("create gif fixture");
+        let mut encoder = gif::Encoder::new(&mut file, w, h, palette).expect("gif encoder");
+        let frame = gif::Frame::from_indexed_pixels(1, 1, vec![0], None);
+        encoder.write_frame(&frame).expect("write frame");
+    }
+
+    /// A 6000×6000 canvas is 144 MiB per frame — comfortably over
+    /// `MAX_GIF_CANVAS_BYTES` (128 MiB) regardless of how large the render's
+    /// own video is, so passing generous `max_w`/`max_h` here isolates the
+    /// byte-budget check from the video-dimensions check exercised by the
+    /// next test. 144 MiB is deliberately far short of the 65535×65535
+    /// (~17 GiB) header a real crafted file could declare — large enough to
+    /// prove the budget check fires, small enough that running this test
+    /// never risks the allocation it is asserting never happens.
+    #[test]
+    fn a_canvas_over_the_byte_budget_is_rejected_without_allocating_it() {
+        let path = std::env::temp_dir().join(format!(
+            "rustmotion_gif_bomb_budget_{}_{}.gif",
+            std::process::id(),
+            std::time::SystemTime::now()
+                .duration_since(std::time::UNIX_EPOCH)
+                .unwrap()
+                .as_nanos(),
+        ));
+        write_oversized_header_gif(&path, 6000, 6000);
+
+        let result = decode_composed_frames(path.to_str().expect("utf-8"), 8192, 8192);
+        std::fs::remove_file(&path).ok();
+
+        assert!(
+            result.is_none(),
+            "a 144 MiB canvas must be refused, not decoded"
+        );
+        assert!(
+            !crate::warn_once_for(&format!("gif-oversized:{}", path.to_str().unwrap())),
+            "the rejection must have reported once"
+        );
+    }
+
+    /// The other half of the same guard: a canvas larger than the render's
+    /// own video dimensions can never be usefully drawn either. A
+    /// 3000×2000 canvas is only 24 MiB (well under the byte budget alone)
+    /// but bigger than the render's own 1920×1080 video, so this isolates
+    /// the video-dimensions check from the byte-budget one above.
+    #[test]
+    fn a_canvas_larger_than_the_video_is_rejected_even_under_the_byte_budget() {
+        let path = std::env::temp_dir().join(format!(
+            "rustmotion_gif_bomb_dims_{}_{}.gif",
+            std::process::id(),
+            std::time::SystemTime::now()
+                .duration_since(std::time::UNIX_EPOCH)
+                .unwrap()
+                .as_nanos(),
+        ));
+        write_oversized_header_gif(&path, 3000, 2000);
+
+        let result = decode_composed_frames(path.to_str().expect("utf-8"), 1920, 1080);
+        std::fs::remove_file(&path).ok();
+
+        assert!(
+            result.is_none(),
+            "a canvas bigger than the video's own dimensions must be refused"
+        );
+    }
+
+    /// Writes `count` tiny (2×2) frames — cheap regardless of `count`, so
+    /// the frame-count cap can be tested without the per-frame size mattering.
+    fn write_many_frame_gif(path: &std::path::Path, count: u32) {
+        let palette: &[u8] = &[0, 0, 0, 255, 255, 255];
+        let mut file = std::fs::File::create(path).expect("create gif fixture");
+        let mut encoder = gif::Encoder::new(&mut file, 2, 2, palette).expect("gif encoder");
+        for _ in 0..count {
+            let mut frame = gif::Frame::from_indexed_pixels(2, 2, vec![0, 1, 1, 0], None);
+            frame.delay = 1;
+            encoder.write_frame(&frame).expect("write frame");
+        }
+    }
+
+    /// The decode loop had no frame-count limit at all — a
+    /// small canvas with a pathologically large frame count still grows the
+    /// decoded animation without bound. `MAX_GIF_FRAMES` truncates it
+    /// instead of rejecting the whole GIF: a real, merely-too-long animation
+    /// still plays (up to the cap), it just doesn't keep growing memory.
+    #[test]
+    fn frame_count_beyond_the_cap_is_truncated_not_unbounded() {
+        let path = std::env::temp_dir().join(format!(
+            "rustmotion_gif_many_frames_{}_{}.gif",
+            std::process::id(),
+            std::time::SystemTime::now()
+                .duration_since(std::time::UNIX_EPOCH)
+                .unwrap()
+                .as_nanos(),
+        ));
+        write_many_frame_gif(&path, MAX_GIF_FRAMES as u32 + 50);
+
+        let (frames, times, _total) = decode_composed_frames(path.to_str().expect("utf-8"), 10, 10)
+            .expect("a small, merely-long gif must still decode");
+        std::fs::remove_file(&path).ok();
+
+        assert_eq!(
+            frames.len(),
+            MAX_GIF_FRAMES,
+            "frame count must be truncated to the cap, not grow past it"
+        );
+        assert_eq!(times.len(), MAX_GIF_FRAMES);
+    }
 }

crates/rustmotion/src/cli/commands/render.rs

@@ -8,6 +8,20 @@ use std::path::{Path, PathBuf};
 
 use crate::cli::commands::validation::{self, ValidationSource};
 
+/// Every process-global decode cache a `--watch` iteration must forget
+/// before re-rendering, so an edited asset is never served from a stale
+/// entry keyed only on its path. `ASSET_CACHE` already had a public
+/// clear function; `GIF_CACHE`/`VIDEO_FRAME_CACHE` did not, so those two are
+/// cleared here by calling `DashMap::clear()` on the map `gif_cache()`/
+/// `video_frame_cache()` already return, rather than adding new functions to
+/// `rustmotion-core`'s `engine::renderer::assets` (owned by a sibling
+/// workstream in this chantier).
+fn clear_all_media_caches() {
+    engine::clear_asset_cache();
+    engine::gif_cache().clear();
+    engine::video_frame_cache().clear();
+}
+
 /// Load + validate a scenario for watch mode. On validation failure prints the
 /// report and returns the typed error so the caller can decide how to handle it.
 ///
@@ -373,7 +387,7 @@ pub fn cmd_watch(
                     Err(e) => eprintln!("Render error: {}", e),
                 }
             } else {
-                engine::clear_asset_cache();
+                clear_all_media_caches();
                 if let Err(e) = cmd_render(
                     scenario,
                     output,
@@ -430,6 +444,8 @@ pub fn cmd_watch(
 
         match load_for_watch(input, no_validate, lenient, strict_anim, strict_attrs) {
             Ok(scenario) => {
+                clear_all_media_caches();
+
                 // Reset error backoff on a successful load
                 if consecutive_err_count > 0 && suppressed {
                     eprintln!("Recovered from previous errors.");
@@ -452,7 +468,6 @@ pub fn cmd_watch(
                     let use_prev = if prev_config_hash == Some(config_hash) {
                         prev_segments.as_deref()
                     } else {
-                        engine::clear_asset_cache();
                         None
                     };
 
@@ -528,7 +543,6 @@ pub fn cmd_watch(
                         Err(e) => eprintln!("Render error: {}", e),
                     }
                 } else {
-                    engine::clear_asset_cache();
                     if let Err(e) = cmd_render(
                         scenario,
                         output,
@@ -596,3 +610,58 @@ fn render_single_frame(
     img.save(output)?;
     Ok(())
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use rustmotion_core::engine::renderer::{asset_cache, gif_cache, video_frame_cache};
+
+    fn unique_marker(label: &str) -> String {
+        format!(
+            "rustmotion-audit-ws-d-rm25-{label}-{}-{}",
+            std::process::id(),
+            std::time::SystemTime::now()
+                .duration_since(std::time::UNIX_EPOCH)
+                .expect("system clock before UNIX epoch")
+                .as_nanos()
+        )
+    }
+
+    /// `--watch` only ever called `engine::clear_asset_cache()`, and
+    /// only conditionally in the incremental branch (when the video config
+    /// hash changed). `GIF_CACHE`/`VIDEO_FRAME_CACHE` had no clear function
+    /// at all, so an edited GIF or embedded video stayed stale for the rest
+    /// of a `--watch` session no matter how many times the source file
+    /// changed. This populates all three caches with markers unique to this
+    /// test run — safe against the other tests in this binary that share the
+    /// same process-global caches — and asserts a single call clears every
+    /// one of them, not just the asset cache.
+    #[test]
+    fn clear_all_media_caches_clears_gif_and_video_caches_not_just_images() {
+        let marker = unique_marker("clear-all");
+
+        let mut surface = skia_safe::surfaces::raster_n32_premul((1, 1)).expect("raster surface");
+        asset_cache().insert(marker.clone(), surface.image_snapshot());
+        gif_cache().insert(
+            marker.clone(),
+            std::sync::Arc::new((Vec::new(), Vec::new(), 0.0)),
+        );
+        video_frame_cache().insert(marker.clone(), std::sync::Arc::new(Vec::new()));
+
+        assert!(asset_cache().contains_key(&marker));
+        assert!(gif_cache().contains_key(&marker));
+        assert!(video_frame_cache().contains_key(&marker));
+
+        clear_all_media_caches();
+
+        assert!(!asset_cache().contains_key(&marker));
+        assert!(
+            !gif_cache().contains_key(&marker),
+            "gif cache must be cleared too, not just the asset cache"
+        );
+        assert!(
+            !video_frame_cache().contains_key(&marker),
+            "video frame cache must be cleared too, not just the asset cache"
+        );
+    }
+}