From 86b31e71d36e48dbba4ab18b12652f169734e1f8 Mon Sep 17 00:00:00 2001 From: aiongg Date: Fri, 16 Oct 2026 17:32:35 +0000 Subject: [PATCH 1/4] Sessions: follow stored source paths only on this computer or the session's share Audio source and movie paths stored in a .scraft session go through io::stored_path (craft-docpath) before they reach the file system: a relative path is taken from the session's folder, and paths on other computers, devices and URLs are refused. A refused source is reported as missing with the reason; a refused movie is marked offline and the Video track and Video window show why instead of opening it. Import Session Data and crash recovery resolve the same way. --- Cargo.lock | 5 ++ Cargo.toml | 3 + crates/engine/Cargo.toml | 1 + crates/engine/src/autosave.rs | 12 ++- crates/engine/src/cmd/setup_more.rs | 30 +++++--- crates/engine/src/cmd/video.rs | 6 +- crates/engine/src/io.rs | 114 +++++++++++++++++++++++++--- crates/model/src/session.rs | 1 + crates/model/src/source.rs | 3 + crates/ui-egui/src/video_track.rs | 12 ++- crates/ui-egui/src/video_window.rs | 6 +- 11 files changed, 159 insertions(+), 34 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b8eb7655..21b131eb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -846,6 +846,10 @@ dependencies = [ "windows 0.54.0", ] +[[package]] +name = "craft-docpath" +version = "0.1.0" + [[package]] name = "craft-ui" version = "0.1.0" @@ -3654,6 +3658,7 @@ dependencies = [ name = "soundcraft-engine" version = "0.7.0" dependencies = [ + "craft-docpath", "log", "proptest", "serde", diff --git a/Cargo.toml b/Cargo.toml index 362e6a9a..8e950a9c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -42,6 +42,9 @@ rustfft = "6" zip = { version = "9", default-features = false } cpal = "0.16" +# Which file paths stored in a session may be opened. +craft-docpath = { path = "../secfix-craft-libs/crates/docpath" } + # ui craft-ui = { version = "0.1.0", git = "https://github.com/storytold/craft-libs", rev = "5c874d51eeb3de70e735c8ce87a59a7dedd47675", features = ["serde"] } egui = "0.36" diff --git a/crates/engine/Cargo.toml b/crates/engine/Cargo.toml index 9b097a00..48a03408 100644 --- a/crates/engine/Cargo.toml +++ b/crates/engine/Cargo.toml @@ -23,6 +23,7 @@ serde_json.workspace = true thiserror.workspace = true log.workspace = true zip.workspace = true +craft-docpath.workspace = true [dev-dependencies] proptest.workspace = true diff --git a/crates/engine/src/autosave.rs b/crates/engine/src/autosave.rs index cb7faac1..01899d2f 100644 --- a/crates/engine/src/autosave.rs +++ b/crates/engine/src/autosave.rs @@ -1,7 +1,7 @@ //! Crash-recovery copies of open sessions, one folder each, written in the background, and the //! recovery of the ones left behind with unsaved work. -use crate::io::{Recording, open_session, sanitize_name}; +use crate::io::{Recording, open_session_linked, sanitize_name}; use crate::{Engine, EngineError, Result}; use soundcraft_audio_io::{BitDepth, EncodeOptions, FileFormat}; use soundcraft_model::{Session, SourceAudio, SourceId}; @@ -197,7 +197,7 @@ impl Autosave { let file = if Path::new(&src.path).is_absolute() { Some(PathBuf::from(&src.path)) } else { base.as_ref().map(|b| b.join(&src.path)) }; let audio = s.pool.get(src.id); if let Some(f) = &file - && (!src.unsaved && f.is_file() || audio.is_none()) + && (audio.is_none() || !src.unsaved && f.is_file()) { src.path = f.to_string_lossy().into_owned(); continue; @@ -291,7 +291,9 @@ pub fn leftovers(root: &Path) -> Vec { /// media that couldn't be loaded. pub fn recover(e: &mut Engine, left: &Leftover) -> Result> { let mut r = Engine::default(); - let mut missing = open_session(&mut r, &left.dir.join(SESSION).to_string_lossy())?; + let session = left.dir.join(SESSION); + let links = left.origin.as_deref().map_or(session.as_path(), Path::new); + let mut missing = open_session_linked(&mut r, &session.to_string_lossy(), links)?; let mut s = r.session().clone(); let unfinished = unfinished_takes(left, &s, &mut missing); if let Some(home) = left.origin.as_deref().and_then(|o| Path::new(o).parent()) { @@ -315,7 +317,8 @@ pub fn recover(e: &mut Engine, left: &Leftover) -> Result> { /// is left untouched and added to `skipped`. fn unfinished_takes(left: &Leftover, s: &Session, skipped: &mut Vec) -> Option<(Recording, Vec<(soundcraft_model::TrackId, PathBuf)>)> { let plan: Recording = serde_json::from_str(&std::fs::read_to_string(left.dir.join(RECORDING)).ok()?).ok()?; - let known: Vec = s.sources.iter().filter_map(|src| std::fs::canonicalize(&src.path).ok()).collect(); + let known: Vec = + s.sources.iter().filter(|src| s.pool.get(src.id).is_some()).filter_map(|src| std::fs::canonicalize(&src.path).ok()).collect(); let folders = take_folders(left); let mut takes = Vec::new(); for t in &plan.takes { @@ -413,6 +416,7 @@ fn io_error(path: &Path, err: std::io::Error) -> EngineError { #[cfg(test)] mod tests { use super::*; + use crate::io::open_session; use serde_json::json; use soundcraft_audio_io::AudioBuffer; use soundcraft_model::{ChannelFormat, Clip, TrackKind}; diff --git a/crates/engine/src/cmd/setup_more.rs b/crates/engine/src/cmd/setup_more.rs index 3d436759..aee13ba1 100644 --- a/crates/engine/src/cmd/setup_more.rs +++ b/crates/engine/src/cmd/setup_more.rs @@ -7,7 +7,7 @@ use crate::cmd; use serde_json::json; use soundcraft_model::{BusId, ChannelFormat, ClipContent, OutputPath, Route, Session, SourceId, TrackId, TrackKind}; use soundcraft_time::Timecode; -use std::path::{Path, PathBuf}; +use std::path::Path; macro_rules! option { ($id:literal, $label:literal, $flag:literal) => { @@ -407,7 +407,6 @@ fn import_session_data(e: &mut Engine, p: &Value) -> Result { let path = str_param(p, "path").filter(|x| !x.trim().is_empty()).ok_or_else(|| bad(id, "`path` required"))?.to_string(); let text_v = crate::io::read_text(&path)?; let other = Session::from_json(&text_v).map_err(|err| EngineError::Io(format!("{path}: {err}")))?; - let dir = Path::new(&path).parent().map(Path::to_path_buf).unwrap_or_default(); let names: Option> = p.get("tracks").and_then(Value::as_array).map(|a| a.iter().filter_map(Value::as_str).map(str::to_string).collect()); let picked: Vec = match &names { @@ -435,20 +434,27 @@ fn import_session_data(e: &mut Engine, p: &Value) -> Result { let mut map: std::collections::BTreeMap = Default::default(); let mut missing = Vec::new(); for src in needed.iter().filter_map(|sid| other.source(*sid)) { - let abs = if Path::new(&src.path).is_absolute() { PathBuf::from(&src.path) } else { dir.join(&src.path) }; let mut ns = src.clone(); ns.id = SourceId(s.alloc()); - ns.path = abs.to_string_lossy().into_owned(); - match crate::io::decode_file(&mut files, &abs, sr) { - Ok(audio) => { - ns.frames = audio.buffer.frames() as u64; - ns.sample_rate = sr; - ns.unsaved = true; - s.pool.insert(ns.id, audio); + match crate::io::stored_path(&src.path, Path::new(&path)) { + Ok(abs) => { + ns.path = abs.to_string_lossy().into_owned(); + match crate::io::decode_file(&mut files, &abs, sr) { + Ok(audio) => { + ns.frames = audio.buffer.frames() as u64; + ns.sample_rate = sr; + ns.unsaved = true; + s.pool.insert(ns.id, audio); + } + Err(_) => { + ns.unsaved = false; + missing.push(ns.path.clone()); + } + } } - Err(_) => { + Err(why) => { ns.unsaved = false; - missing.push(ns.path.clone()); + missing.push(format!("{}: {why}", ns.path)); } } map.insert(src.id.0, ns.id); diff --git a/crates/engine/src/cmd/video.rs b/crates/engine/src/cmd/video.rs index f361e0d4..c65e40d6 100644 --- a/crates/engine/src/cmd/video.rs +++ b/crates/engine/src/cmd/video.rs @@ -28,7 +28,7 @@ pub fn specs() -> Vec { always, offset ), - cmd!(query "video.list", "Movies", [], None, "{} — movies used by Video tracks (path, size, rate, codec, online file)", always, |e, _| { + cmd!(query "video.list", "Movies", [], None, "{} — movies used by Video tracks (path, size, rate, codec, online file, why it is offline)", always, |e, _| { let s = e.session(); let rows: Vec = s .videos @@ -36,7 +36,8 @@ pub fn specs() -> Vec { .map(|v| { let clips = s.tracks.iter().flat_map(|t| t.clips()).filter(|c| c.video_source() == Some(v.id)).count(); json!({"id": v.id, "name": v.name, "path": v.path, "width": v.width, "height": v.height, "frame_rate": v.frame_rate, - "duration": v.duration, "codec": v.codec, "clips": clips, "file_found": crate::io::file_found(&v.path)}) + "duration": v.duration, "codec": v.codec, "clips": clips, + "file_found": v.offline.is_none() && crate::io::file_found(&v.path), "offline": v.offline}) }) .collect(); Ok(json!({"videos": rows, "online": !s.edit.flag(OFFLINE_FLAG)})) @@ -92,6 +93,7 @@ mod tests { frame_rate: 24.0, duration: 2.0, codec: "h264".into(), + offline: None, }); let c = s.new_clip_id(); crate::edit::place_clip(s, t, Clip::video(c, "m", vid, 0, 1000, 96_000)); diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index 7b26e0b1..80e8e4f8 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -60,6 +60,23 @@ pub fn file_found(path: impl AsRef) -> bool { crate::file_access::check_read(path).is_ok() && path.is_file() } +/// The file a path stored in the session file `session` refers to, or why it is not opened. +/// +/// Every path a session names (audio sources, movies) goes through here before it reaches the +/// file system. A relative path is taken from the session's folder, and only files on this +/// computer or on the network share the session itself is on are opened (`craft_docpath`). +/// Paths the person picks in this session (imports) are not stored paths and skip this. +pub fn stored_path(stored: &str, session: &Path) -> std::result::Result { + let session = std::path::absolute(session).unwrap_or_else(|_| session.to_path_buf()); + let path = craft_docpath::resolve(stored, Some(&session)).map_err(|why| format!("not opened: {why}"))?; + // A path that is still relative would be taken from the working directory, which the session + // does not choose. + if path.is_relative() { + return Err("not opened: the path is not absolute".into()); + } + Ok(path) +} + /// Create `dir` and its parents, inside the automation write root when there is one. pub(crate) fn make_dir(dir: impl AsRef) -> Result<()> { let dir = dir.as_ref(); @@ -206,6 +223,7 @@ pub fn import_video_bytes(e: &mut Engine, path: &str, bytes: Vec, at: Sample frame_rate: info.frame_rate, duration: info.duration, codec: info.codec.clone(), + offline: None, }); // Reuse the first Video track when the new clip does not overlap its clips; else add one at the top. let range = Range::new(at, at.saturating_add(len)); @@ -235,22 +253,35 @@ pub fn import_video_bytes(e: &mut Engine, path: &str, bytes: Vec, at: Sample })) } -/// Resolve each referenced movie: keep it when its path exists, else look for a file of the same -/// name next to the session (moved projects) and relink. Returns the movies still missing. -pub fn relink_videos(s: &mut Session, session_dir: &Path) -> Vec { +/// Resolve each movie referenced by the session file `session` ([`stored_path`]): keep the +/// resolved path when its file exists, else look for a file of the same name next to the session +/// (moved projects) and relink. A movie whose stored path is refused and has no such copy is +/// marked offline. Returns the movies still missing. +pub fn relink_videos(s: &mut Session, session: &Path) -> Vec { + let session_dir = session.parent().unwrap_or(Path::new("")); let mut missing = Vec::new(); for v in &mut s.videos { - let p = if Path::new(&v.path).is_absolute() { PathBuf::from(&v.path) } else { session_dir.join(&v.path) }; - if file_found(&p) { + v.offline = None; + let p = stored_path(&v.path, session); + // The UI opens `path` as it is: keep the checked path, not one relative to the working + // directory. + if let Ok(found) = &p + && file_found(found) + { + v.path = found.to_string_lossy().into_owned(); continue; } let name = Path::new(&v.path).file_name().map(PathBuf::from).unwrap_or_default(); let near = [session_dir.join(&name), session_dir.join("Video Files").join(&name)] .into_iter() .find(|c| !name.as_os_str().is_empty() && file_found(c)); - match near { - Some(c) => v.path = c.to_string_lossy().into_owned(), - None => missing.push(format!("{} (video)", p.display())), + match (near, p) { + (Some(c), _) => v.path = c.to_string_lossy().into_owned(), + (None, Ok(p)) => missing.push(format!("{} (video)", p.display())), + (None, Err(why)) => { + missing.push(format!("{} (video): {why}", v.path)); + v.offline = Some(why); + } } } missing @@ -1096,19 +1127,26 @@ fn store_wav(dir: &Path, stem: &str, bytes: &[u8]) -> Result<(String, bool)> { /// Open a `.scraft` session and decode its audio files (missing files are reported, not fatal). pub fn open_session(e: &mut Engine, path: &str) -> Result> { + open_session_linked(e, path, Path::new(path)) +} + +/// [`open_session`], following the media paths in it as if they were stored in the session file +/// `links` (crash recovery: the session written beside the autosave, its paths from the original). +pub(crate) fn open_session_linked(e: &mut Engine, path: &str, links: &Path) -> Result> { let text = read_text(path)?; let mut s = Session::from_json(&text).map_err(|err| EngineError::Io(err.to_string()))?; let dir = Path::new(path).parent().map(Path::to_path_buf).unwrap_or_default(); let mut missing = Vec::new(); let mut files = DecodedFiles::new(); for src in &s.sources { - let p = if Path::new(&src.path).is_absolute() { PathBuf::from(&src.path) } else { dir.join(&src.path) }; - match decode_file(&mut files, &p, s.sample_rate.hz()) { + let decoded = + stored_path(&src.path, links).map_err(|why| format!("{}: {why}", src.path)).and_then(|p| decode_file(&mut files, &p, s.sample_rate.hz())); + match decoded { Ok(audio) => s.pool.insert(src.id, audio), Err(err) => missing.push(err), } } - missing.extend(relink_videos(&mut s, &dir)); + missing.extend(relink_videos(&mut s, links)); s.folder = Some(dir); e.replace_session(s); e.path = Some(path.to_string()); @@ -1731,6 +1769,60 @@ mod open_tests { assert_eq!(std::fs::read_dir(dir.join("copy/Audio Files")).unwrap().count(), 1); let _ = std::fs::remove_dir_all(&dir); } + #[test] + fn stored_media_paths_are_followed_only_on_this_computer() { + let dir = std::env::temp_dir().join(format!("soundcraft-stored-paths-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + let audio = AudioBuffer { sample_rate: 48_000, channels: vec![vec![0.25; 480]] }; + std::fs::write(dir.join("k.wav"), soundcraft_audio_io::encode(&audio, &EncodeOptions::default()).unwrap()).unwrap(); + let mut saved = Session::default(); + let t = saved.add_track(TrackKind::Audio, ChannelFormat::Mono, None); + let src = add_source(&mut saved, "k", audio, Some("../k.wav"), FileFormat::Wav); + let id = saved.new_clip_id(); + crate::edit::place_clip(&mut saved, t, Clip::audio(id, "k", src, 0, 0, 480)); + // In a folder of its own, so a movie named `k.wav` is not relinked to a file beside it. + std::fs::create_dir_all(dir.join("s")).unwrap(); + let path = dir.join("s").join("Song.scraft").to_string_lossy().into_owned(); + let mut stored = vec![r"\\192.0.2.1\share\x.wav".to_string(), "file://192.0.2.1/s/x.wav".to_string()]; + // Linux reads `//tmp/…` as the local `/tmp/…`, so only the path check keeps it from being read. + if cfg!(unix) { + stored.push(format!("/{}", dir.join("k.wav").display())); + } + let refused = |m: &str| m.contains("not opened: the path is on another computer"); + for p in stored { + let mut s = saved.clone(); + s.sources[0].path.clone_from(&p); + let id = SourceId(s.alloc()); + s.videos.push(soundcraft_model::VideoSource { + id, + name: "x".into(), + path: p.clone(), + width: 0, + height: 0, + frame_rate: 0.0, + duration: 0.0, + codec: String::new(), + offline: None, + }); + std::fs::write(&path, s.to_json().unwrap()).unwrap(); + let mut r = Engine::default(); + let missing = open_session(&mut r, &path).unwrap(); + assert_eq!(missing.iter().filter(|m| refused(m)).count(), 2, "{p}: {missing:?}"); + assert!(r.session().pool.get(r.session().sources[0].id).is_none(), "{p}"); + assert!(r.session().videos[0].offline.is_some(), "{p}"); + let mut into = Engine::default(); + let imported = into.execute("file.import_session_data", &json!({"path": path})).unwrap(); + assert!(imported["missing"].as_array().unwrap().iter().any(|m| m.as_str().is_some_and(refused)), "{p}: {imported}"); + assert!(into.session().pool.get(into.session().sources[0].id).is_none(), "{p}"); + } + // A path relative to the session opens. + std::fs::write(&path, saved.to_json().unwrap()).unwrap(); + let mut r = Engine::default(); + assert!(open_session(&mut r, &path).unwrap().is_empty()); + assert!(r.session().pool.get(r.session().sources[0].id).is_some()); + let _ = std::fs::remove_dir_all(&dir); + } } #[cfg(test)] diff --git a/crates/model/src/session.rs b/crates/model/src/session.rs index 449d82da..df141a70 100644 --- a/crates/model/src/session.rs +++ b/crates/model/src/session.rs @@ -695,6 +695,7 @@ mod tests { frame_rate: 23.976, duration: 12.5, codec: "prores".into(), + offline: None, }); let cid = s.new_clip_id(); if let Some(p) = s.track_mut(t).and_then(Track::playlist_mut) { diff --git a/crates/model/src/source.rs b/crates/model/src/source.rs index 5da35e76..c7c8a341 100644 --- a/crates/model/src/source.rs +++ b/crates/model/src/source.rs @@ -46,6 +46,9 @@ pub struct VideoSource { /// Codec id (`h264`, `prores`, `mjpeg`, `hevc`, …). #[serde(default)] pub codec: String, + /// Why the movie is not opened, when its stored path was refused on opening the session. + #[serde(skip)] + pub offline: Option, } impl VideoSource { diff --git a/crates/ui-egui/src/video_track.rs b/crates/ui-egui/src/video_track.rs index fdf5d87b..eb6dfca1 100644 --- a/crates/ui-egui/src/video_track.rs +++ b/crates/ui-egui/src/video_track.rs @@ -404,8 +404,9 @@ pub fn movie_secs(s: &Session, clip_start: Samples, offset: Samples, at: Samples (at.saturating_sub(clip_start).saturating_add(offset) as f64 - sync) / s.sample_rate.as_f64().max(1.0) } -/// The picture at `at`: the topmost active Video track with an unmuted clip there. -pub fn picture_at(s: &Session, at: Samples) -> Option<(MovieKey, String, f64, f64)> { +/// The picture at `at`: the topmost active Video track with an unmuted clip there. The path is +/// the movie's, or why it is offline. +pub fn picture_at(s: &Session, at: Samples) -> Option<(MovieKey, Result, f64, f64)> { for t in s.tracks.iter().filter(|t| t.kind == soundcraft_model::TrackKind::Video && !t.inactive) { for c in t.clips() { if c.muted || !c.range().contains(at) { @@ -414,7 +415,8 @@ pub fn picture_at(s: &Session, at: Samples) -> Option<(MovieKey, String, f64, f6 if let ClipContent::Video { source, offset } = c.content && let Some(v) = s.video(source) { - return Some((MovieKey::new(source, &v.path), v.path.clone(), movie_secs(s, c.start, offset, at), v.frame_rate)); + let path = v.offline.clone().map_or_else(|| Ok(v.path.clone()), Err); + return Some((MovieKey::new(source, &v.path), path, movie_secs(s, c.start, offset, at), v.frame_rate)); } } } @@ -450,6 +452,10 @@ pub fn draw_lane(app: &mut SoundApp, painter: &Painter, s: &Session, track: &Tra p.text(vis.center(), egui::Align2::CENTER_CENTER, "Video Track Offline", regular(10.0), Color32::from_gray(170)); continue; } + if let Some(why) = &v.offline { + p.text(vis.center(), egui::Align2::CENTER_CENTER, format!("media offline: {why}"), regular(10.0), Color32::from_rgb(220, 120, 120)); + continue; + } let key = MovieKey::new(source, &v.path); if let Some(Err(e)) = app.video.info(key) { p.text(vis.center(), egui::Align2::CENTER_CENTER, format!("media offline: {e}"), regular(10.0), Color32::from_rgb(220, 120, 120)); diff --git a/crates/ui-egui/src/video_window.rs b/crates/ui-egui/src/video_window.rs index 2f2aa7f9..61809e78 100644 --- a/crates/ui-egui/src/video_window.rs +++ b/crates/ui-egui/src/video_window.rs @@ -24,7 +24,7 @@ fn video(app: &mut SoundApp, ctx: &egui::Context) { let s = app.engine.session_arc(); let pic = picture_at(&s, pos); let on = online(&s); - if on && let Some((key, path, secs, fps)) = &pic { + if on && let Some((key, Ok(path), secs, fps)) = &pic { app.video.request_frame(ctx, *key, path, *secs, *fps); } let aspect = pic @@ -39,6 +39,7 @@ fn video(app: &mut SoundApp, ctx: &egui::Context) { ui.horizontal(|ui| { crate::widgets::checkbox(ui, &mut burn, "Timecode burn-in"); let text = match &pic { + Some((_, Err(why), ..)) => why.clone(), Some((k, ..)) => match app.video.info(*k) { Some(Ok(i)) => format!("{}x{} {:.3} fps {}", i.width, i.height, i.frame_rate, i.codec_detail), Some(Err(e)) => e.clone(), @@ -64,6 +65,7 @@ fn video(app: &mut SoundApp, ctx: &egui::Context) { } else { match &pic { None => None, + Some((_, Err(why), ..)) => Some(why.clone()), Some((k, ..)) => match (app.video.current(*k), app.video.current_error(*k)) { (Some(tex), _) => { painter.image(tex.id(), img, Rect::from_min_max(pos2(0.0, 0.0), pos2(1.0, 1.0)), Color32::WHITE); @@ -130,7 +132,7 @@ fn universe(app: &mut SoundApp, ctx: &egui::Context) { let key = MovieKey::new(source, &v.path); let cr = Rect::from_min_max(pos2(x_at(c.start), r.min.y + 4.0), pos2(x_at(c.end()).max(x_at(c.start) + 2.0), r.max.y - 4.0)); painter.rect_filled(cr, 0.0, Color32::from_rgb(30, 30, 34)); - if !on { + if !on || v.offline.is_some() { continue; } let tw = (cr.height() * v.aspect() as f32).max(8.0); From bde132e0bf71c8e29281023dc5f116980d6854e5 Mon Sep 17 00:00:00 2001 From: aiongg Date: Fri, 16 Oct 2026 17:34:40 +0000 Subject: [PATCH 2/4] ProRes: check frame dimensions before allocating planes decode_frame_into refuses a frame header larger than the movie picture limit before sizing any plane, and Movie refuses a ProRes frame whose header size differs from the track's declared size before decoding it. --- crates/video/src/movie.rs | 7 ++++++- crates/video/src/prores/decode.rs | 4 ++++ crates/video/src/tests.rs | 22 ++++++++++++++++++++++ 3 files changed, 32 insertions(+), 1 deletion(-) diff --git a/crates/video/src/movie.rs b/crates/video/src/movie.rs index 00541407..c5755ca3 100644 --- a/crates/video/src/movie.rs +++ b/crates/video/src/movie.rs @@ -10,7 +10,7 @@ use std::sync::Arc; pub type SharedSource = Arc; /// Largest picture we decode (bounds every allocation derived from the file). -const MAX_PIXELS: u64 = 8192 * 8192; +pub(crate) const MAX_PIXELS: u64 = 8192 * 8192; /// A decoded picture, packed RGBA (8 bits per channel, `width * height * 4` bytes). #[derive(Debug, Clone, PartialEq)] @@ -395,6 +395,11 @@ impl Movie { self.decoded += 1; match self.codec { Codec::ProRes => { + let (hdr, ..) = crate::prores::FrameHeader::parse(&data).map_err(|e| VideoError::Decode(e.to_string()))?; + let (w, h) = (u32::from(hdr.width), u32::from(hdr.height)); + if (w, h) != (self.info.width, self.info.height) { + return Err(VideoError::Decode(format!("{w}x{h} frame in a {}x{} track", self.info.width, self.info.height))); + } let opts = crate::prores::DecodeOptions { bit_depth: Some(8), threads: false }; let f = crate::prores::decode_frame_with(&data, &opts).map_err(|e| VideoError::Decode(e.to_string()))?; if u64::from(f.width) * u64::from(f.height) > MAX_PIXELS { diff --git a/crates/video/src/prores/decode.rs b/crates/video/src/prores/decode.rs index 11f8fc1d..305a5b1b 100644 --- a/crates/video/src/prores/decode.rs +++ b/crates/video/src/prores/decode.rs @@ -109,6 +109,10 @@ pub fn decode_frame_with(data: &[u8], opts: &DecodeOptions) -> Result { /// a sequence). On error the contents of `out` are unspecified. pub fn decode_frame_into(data: &[u8], opts: &DecodeOptions, out: &mut Frame) -> Result<()> { let (hdr, mut off, frame_size) = FrameHeader::parse(data)?; + // The planes are sized from the header: refuse a size no movie may have before allocating. + if u64::from(hdr.width) * u64::from(hdr.height) > crate::movie::MAX_PIXELS { + return Err(Error::Unsupported("frame size")); + } let data = &data[..frame_size]; let depth = opts.bit_depth.unwrap_or(match hdr.chroma { ChromaFormat::Yuv422 => 10, diff --git a/crates/video/src/tests.rs b/crates/video/src/tests.rs index 58db4b3c..695a44ae 100644 --- a/crates/video/src/tests.rs +++ b/crates/video/src/tests.rs @@ -96,6 +96,28 @@ fn prores_and_mjpeg_decode() { } } +/// A 64x48 ProRes frame whose header claims `w`x`h`. +fn prores_frame_claiming(w: u16, h: u16) -> Vec { + use crate::prores::{ChromaFormat, Encoder, Frame, Profile}; + let mut data = Encoder::new(Profile::Hq, 64, 48).encode(&Frame::new(64, 48, ChromaFormat::Yuv422, 10, false)).unwrap(); + data[16..18].copy_from_slice(&w.to_be_bytes()); + data[18..20].copy_from_slice(&h.to_be_bytes()); + data +} + +#[test] +fn prores_frame_size_is_checked_before_decoding() { + use crate::prores::{DecodeOptions, Error, decode_frame_with}; + let huge = decode_frame_with(&prores_frame_claiming(65535, 65535), &DecodeOptions::default()); + assert!(matches!(huge, Err(Error::Unsupported("frame size"))), "{huge:?}"); + // A frame that differs from the track's declared size is refused, not decoded at its own size. + let mut mw = Mp4Writer::new(Cursor::new(Vec::new()), WriterOptions::new(Brand::Mov)).unwrap(); + let t = mw.add_track(TrackConfig::new(SampleEntry::prores(crate::isobmff::FourCc(*b"apch"), 64, 48), 25)).unwrap(); + mw.write_sample(t, WriteSample { data: &prores_frame_claiming(56, 48), duration: 1, composition_offset: 0, is_sync: true }).unwrap(); + let mut m = Movie::open(mw.finish().unwrap().into_inner()).unwrap(); + assert!(matches!(m.frame(0), Err(VideoError::Decode(e)) if e.contains("56x48 frame in a 64x48 track"))); +} + #[test] fn unsupported_codec_opens_but_does_not_decode() { let mut mw = Mp4Writer::new(Cursor::new(Vec::new()), WriterOptions::new(Brand::Mp4)).unwrap(); From 1e036eaa174b5d9db472ed2694ed1b4903294715 Mon Sep 17 00:00:00 2001 From: aiongg Date: Sat, 17 Oct 2026 02:20:54 +0000 Subject: [PATCH 3/4] Build: use craft-docpath from craft-libs --- Cargo.lock | 1 + Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/Cargo.lock b/Cargo.lock index 21b131eb..a1facf5e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -849,6 +849,7 @@ dependencies = [ [[package]] name = "craft-docpath" version = "0.1.0" +source = "git+https://github.com/aiongg/craft-libs?rev=12281d07fedff5c955bb68344fb287aa5d87971e#12281d07fedff5c955bb68344fb287aa5d87971e" [[package]] name = "craft-ui" diff --git a/Cargo.toml b/Cargo.toml index 8e950a9c..a9e7f200 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -43,7 +43,7 @@ zip = { version = "9", default-features = false } cpal = "0.16" # Which file paths stored in a session may be opened. -craft-docpath = { path = "../secfix-craft-libs/crates/docpath" } +craft-docpath = { git = "https://github.com/aiongg/craft-libs", version = "0.1.0", rev = "12281d07fedff5c955bb68344fb287aa5d87971e" } # ui craft-ui = { version = "0.1.0", git = "https://github.com/storytold/craft-libs", rev = "5c874d51eeb3de70e735c8ce87a59a7dedd47675", features = ["serde"] } From 52bd33bb06d5d001f7271d9272eca5d688f80ab5 Mon Sep 17 00:00:00 2001 From: skylar taylor-barrick Date: Sat, 17 Oct 2026 12:04:05 -0600 Subject: [PATCH 4/4] Build: take craft-docpath from the merged craft-libs rev craft-libs#169 is merged, so the fork pin is no longer needed. Pinned to storytold/craft-libs 82a24b006eefa036b43c09a67fb9a3fbd17ffe63 (main after the merge) rather than the older 12281d07, which predates resolve_absolute and the Windows colon refusal. --- Cargo.lock | 2 +- Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a1facf5e..463a213b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -849,7 +849,7 @@ dependencies = [ [[package]] name = "craft-docpath" version = "0.1.0" -source = "git+https://github.com/aiongg/craft-libs?rev=12281d07fedff5c955bb68344fb287aa5d87971e#12281d07fedff5c955bb68344fb287aa5d87971e" +source = "git+https://github.com/storytold/craft-libs?rev=82a24b006eefa036b43c09a67fb9a3fbd17ffe63#82a24b006eefa036b43c09a67fb9a3fbd17ffe63" [[package]] name = "craft-ui" diff --git a/Cargo.toml b/Cargo.toml index a9e7f200..4e2ed5b0 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -43,7 +43,7 @@ zip = { version = "9", default-features = false } cpal = "0.16" # Which file paths stored in a session may be opened. -craft-docpath = { git = "https://github.com/aiongg/craft-libs", version = "0.1.0", rev = "12281d07fedff5c955bb68344fb287aa5d87971e" } +craft-docpath = { version = "0.1.0", git = "https://github.com/storytold/craft-libs", rev = "82a24b006eefa036b43c09a67fb9a3fbd17ffe63" } # ui craft-ui = { version = "0.1.0", git = "https://github.com/storytold/craft-libs", rev = "5c874d51eeb3de70e735c8ce87a59a7dedd47675", features = ["serde"] }