From 4240ffda42f17d939be9678aca35148834cfef70 Mon Sep 17 00:00:00 2001 From: Zinadin Zidan Date: Thu, 5 Nov 2026 11:37:18 +0600 Subject: [PATCH] fix(engine): cap sessions at 256 MiB and parse them without a Value tree (#892) A session was parsed into a serde_json Value and then converted, which takes many times the file's size in memory, and nothing limited how big a session file could be. Session::from_json now reads only format_version first (every other field is skipped) and then parses the text straight into the session. A missing format_version and a newer version give the errors they gave before. Other type errors now end with the place they were found ("at line L column C"). A field of the document given twice in one object, which used to load with the last value winning, is now refused with "duplicate field `key`" (a twice-given format_version too, not "missing format_version"): keeping the old leniency would need the tree this change removes, and no writer repeats a field. Keys of maps (plugin params, edit values and the like) and unknown keys still keep the last value given, as before. A track's height was read through an untagged enum, which buffers the whole value before it looks at it; a huge array there took about 27 times its size. It now has its own reader that takes exactly the forms that loaded before ("Micro" to "Extreme", also as {"Micro": null}, {"Custom": n}, {"Fit": n}, and the old bare "Fit" or {"Fit": null}) and refuses anything else at its first token. Opening a session (a .scraft file, the browser's bytes, the session in a bundle) and Import Session Data refuse one over 256 MiB by its size, before reading it. Save, Save Copy In, Save As Template, autosave and the browser's bundle refuse to write a session over that size, so SoundCraft never saves one it then won't open. Save and autosave check before they write any audio, so a refused save leaves nothing behind and a refused autosave keeps the last one and its audio copies as they were; the refusal is reported like any other autosave failure. Save checks against the longest name each new audio file can get, so it may refuse a session within a few bytes per new file of the limit. Crash recovery reads only the name of a leftover autosave, and none from one over the cap. Audio files are read without that cap, as before. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/autosave.rs | 106 +++++++++++++++-- crates/engine/src/cmd/file.rs | 2 + crates/engine/src/cmd/setup_more.rs | 2 +- crates/engine/src/io.rs | 174 +++++++++++++++++++++++++--- crates/engine/src/io/bundle.rs | 39 ++++++- crates/engine/src/io/session.rs | 94 ++++++++++++++- crates/model/src/session.rs | 76 +++++++++++- crates/model/src/track.rs | 123 ++++++++++++++------ 8 files changed, 541 insertions(+), 75 deletions(-) diff --git a/crates/engine/src/autosave.rs b/crates/engine/src/autosave.rs index 8ef38e27..d86219f5 100644 --- a/crates/engine/src/autosave.rs +++ b/crates/engine/src/autosave.rs @@ -215,6 +215,8 @@ impl Autosave { // Copies made in this write, by the audio they hold: sources sharing one decoded buffer // (a session may name one file under many ids) share one file. let mut written: BTreeMap<*const SourceAudio, String> = BTreeMap::new(); + // Copies still to make, written once the size is checked; their numbers are taken then too. + let (mut fresh, mut next) = (Vec::new(), self.next); for src in &mut s.sources { 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); @@ -225,15 +227,13 @@ impl Autosave { continue; } let Some(audio) = audio else { continue }; - let name = match self.copies.remove(&src.id) { - Some((copy, name)) if copy.upgrade().is_some_and(|a| Arc::ptr_eq(&a, audio)) => name, + let name = match self.copies.get(&src.id) { + Some((copy, name)) if copy.upgrade().is_some_and(|a| Arc::ptr_eq(&a, audio)) => name.clone(), _ if written.contains_key(&Arc::as_ptr(audio)) => written.get(&Arc::as_ptr(audio)).cloned().unwrap_or_default(), _ => { - let name = format!("{} {}.wav", sanitize_name(&src.name), self.next); - self.next += 1; - let opts = EncodeOptions { format: FileFormat::Wav, bit_depth: BitDepth::Float32, dither: false, bwf: None }; - let bytes = soundcraft_audio_io::encode(&audio.buffer, &opts).map_err(|err| EngineError::Io(err.to_string()))?; - create_folder(&audio_dir).and_then(|()| write_synced(&audio_dir.join(&name), &bytes)).map_err(|err| io_error(&audio_dir, err))?; + let name = format!("{} {}.wav", sanitize_name(&src.name), next); + next += 1; + fresh.push((name.clone(), Arc::clone(audio))); name } }; @@ -243,6 +243,15 @@ impl Autosave { copies.insert(src.id, (Arc::downgrade(audio), name)); } let text = s.to_json().map_err(|err| EngineError::Io(err.to_string()))?; + // An autosave too large to recover is not written, nor its new copies: the last autosave + // stays as it was, and the failure is reported (#892). + crate::io::session_size("the autosave", text.len() as u64, true)?; + self.next = next; + for (name, audio) in &fresh { + let opts = EncodeOptions { format: FileFormat::Wav, bit_depth: BitDepth::Float32, dither: false, bwf: None }; + let bytes = soundcraft_audio_io::encode(&audio.buffer, &opts).map_err(|err| EngineError::Io(err.to_string()))?; + create_folder(&audio_dir).and_then(|()| write_synced(&audio_dir.join(name), &bytes)).map_err(|err| io_error(&audio_dir, err))?; + } write_atomically(&self.dir.join(ORIGIN), origin.unwrap_or_default().as_bytes())?; write_atomically(&self.dir.join(SESSION), text.as_bytes())?; for (_, name) in std::mem::replace(&mut self.copies, copies).into_values() { @@ -297,9 +306,7 @@ pub fn leftovers(root: &Path) -> Vec { drop(lock); match saved { Ok(saved) if recording || origin_saved.is_none_or(|o| saved > o) => { - let text = std::fs::read_to_string(dir.join(SESSION)).unwrap_or_default(); - let named = serde_json::from_str::(&text).ok().and_then(|v| v.get("name")?.as_str().map(str::to_string)); - let name = named.unwrap_or_else(|| dir.file_name().unwrap_or_default().to_string_lossy().into_owned()); + let name = saved_name(&dir.join(SESSION)).unwrap_or_else(|| dir.file_name().unwrap_or_default().to_string_lossy().into_owned()); out.push(Leftover { dir, name, origin, saved, recording }); } _ => { @@ -311,6 +318,21 @@ pub fn leftovers(root: &Path) -> Vec { out } +/// The name of the session saved in `file`, read without keeping the rest of it; `None` when the +/// file is missing, not a session, or too large to open (#892). +fn saved_name(file: &Path) -> Option { + use std::io::Read as _; + #[derive(serde::Deserialize)] + struct Named { + name: String, + } + if std::fs::metadata(file).ok()?.len() > crate::io::session_limit() { + return None; + } + let read = std::fs::File::open(file).ok()?.take(crate::io::session_limit()); + serde_json::from_reader::<_, Named>(std::io::BufReader::new(read)).ok().map(|n| n.name) +} + /// Open `left` as an unsaved version of the session it came from, so Save writes it back there, /// with the takes of a recording that never finished placed as Stop would have. Returns the /// media that couldn't be loaded. @@ -699,6 +721,70 @@ mod tests { let _ = std::fs::remove_dir_all(&dir); } + /// A test folder, removed when the test ends, passed or not. + struct Gone(PathBuf); + + impl Drop for Gone { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.0); + } + } + + #[test] + fn an_autosave_too_large_to_open_writes_nothing_and_keeps_the_last() { + // #892, with the limit lowered for this test: a refused autosave makes no audio copies + // and keeps the last autosave and its copies as they were. + let dir = Gone(folder("autosave-too-large")); + let mut e = Engine::default(); + record(&mut e, 0.5); + let mut a = Autosave::create(&dir.0.join("Autosave"), "Song").unwrap(); + a.write(e.session(), None, None).unwrap(); + let state = |a: &Autosave| { + let mut files: Vec = + std::fs::read_dir(a.dir.join("Audio Files")).unwrap().map(|f| f.unwrap().file_name().to_string_lossy().into_owned()).collect(); + files.sort(); + let copies: Vec<(SourceId, String)> = a.copies.iter().map(|(id, (_, name))| (*id, name.clone())).collect(); + (files, copies, a.next, std::fs::read_to_string(a.dir.join(SESSION)).unwrap()) + }; + let before = state(&a); + record(&mut e, 0.25); + { + let _limit = crate::io::SessionLimit::set(before.3.len() as u64); + for _ in 0..2 { + let err = a.write(e.session(), None, None).unwrap_err().to_string(); + assert!(err.starts_with("the autosave is too large to save"), "{err}"); + assert!(state(&a) == before); + } + // The writer thread reports it as it reports any failed autosave. + let failure = Arc::new(Mutex::new(None)); + let pending = Some((e.session_arc(), None, None)); + let mut w = Writer { root: dir.0.join("Writer"), failure: Arc::clone(&failure), folder: None, pending, last: None, written: Ok(()) }; + w.write_pending(); + let reported = failure.lock().unwrap().take().unwrap_or_default(); + assert!(reported.starts_with("the autosave is too large to save"), "{reported}"); + } + a.write(e.session(), None, None).unwrap(); + let after = state(&a); + assert_eq!((after.0.len(), after.1.len()), (2, 2)); + assert!(after.0.contains(&before.0[0])); + } + + #[test] + fn a_leftover_is_named_from_its_session_without_reading_a_huge_one() { + // #892: the name is read on its own, and an autosave too large to open is not read. + let dir = Gone(folder("autosave-names")); + std::fs::create_dir_all(&dir.0).unwrap(); + let file = dir.0.join(SESSION); + let s = Session { name: "Song".into(), ..Session::default() }; + std::fs::write(&file, s.to_json().unwrap()).unwrap(); + assert_eq!(saved_name(&file).as_deref(), Some("Song")); + std::fs::write(&file, r#"{"name": 5}"#).unwrap(); + assert_eq!(saved_name(&file), None); + std::fs::File::create(&file).unwrap().set_len(crate::io::MAX_SESSION_BYTES + 1).unwrap(); + assert_eq!(saved_name(&file), None); + assert_eq!(saved_name(&dir.0.join("none")), None); + } + #[test] fn sessions_started_together_get_folders_of_their_own() { // #579: the folder name has millisecond resolution, so two sessions created back to back diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index 6cac6607..90974749 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -45,6 +45,8 @@ pub fn specs() -> Vec { s.sources.clear(); s.videos.clear(); let text = s.to_json().map_err(|err| EngineError::Io(err.to_string()))?; + // A template too large to open is not saved (#892). + crate::io::session_size(file.display(), text.len() as u64, true)?; if cfg!(target_arch = "wasm32") { crate::io::write_file(&file.to_string_lossy(), text.as_bytes())?; } else { diff --git a/crates/engine/src/cmd/setup_more.rs b/crates/engine/src/cmd/setup_more.rs index 3d07deb1..376bd472 100644 --- a/crates/engine/src/cmd/setup_more.rs +++ b/crates/engine/src/cmd/setup_more.rs @@ -472,7 +472,7 @@ fn midi_inputs(e: &mut Engine, p: &Value) -> Result { fn import_session_data(e: &mut Engine, p: &Value) -> Result { let id = "file.import_session_data"; 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 text_v = crate::io::read_session_text(&path)?; let other = Session::from_json(&text_v).map_err(|err| EngineError::Io(format!("{path}: {err}")))?; 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()); diff --git a/crates/engine/src/io.rs b/crates/engine/src/io.rs index 67d6e4a9..571e7ffd 100644 --- a/crates/engine/src/io.rs +++ b/crates/engine/src/io.rs @@ -42,19 +42,122 @@ pub use strip::{StripMode, StripNames, StripScan, bounded_to_track, name_strippe /// one. Only a regular file qualifies: a device or a pipe named by a session or a command would /// never end or never answer. Errors name the file. pub fn read_file(path: impl AsRef) -> Result> { - let path = path.as_ref(); + read_file_within(path.as_ref(), false) +} + +/// [`read_file`]; a `session` file is refused ([`session_size`]) past [`session_limit`]. +fn read_file_within(path: &Path, session: bool) -> Result> { + use std::io::Read as _; crate::file_access::check_read(path).map_err(EngineError::Io)?; let err = |err: std::io::Error| EngineError::Io(format!("{}: {err}", path.display())); - if !std::fs::metadata(path).map_err(err)?.is_file() { + let meta = std::fs::metadata(path).map_err(err)?; + if !meta.is_file() { return Err(EngineError::Io(format!("{}: not a regular file", path.display()))); } - std::fs::read(path).map_err(err) + if !session { + return std::fs::read(path).map_err(err); + } + session_size(path.display(), meta.len(), false)?; + // The file may have grown since: never read more than one byte past the limit. + let mut data = Vec::new(); + std::fs::File::open(path).map_err(err)?.take(session_limit().saturating_add(1)).read_to_end(&mut data).map_err(err)?; + session_size(path.display(), data.len() as u64, false)?; + Ok(data) } /// [`read_file`] as UTF-8 text. pub fn read_text(path: impl AsRef) -> Result { let path = path.as_ref(); - String::from_utf8(read_file(path)?).map_err(|err| EngineError::Io(format!("{}: {err}", path.display()))) + utf8(path, read_file(path)?) +} + +fn utf8(path: &Path, bytes: Vec) -> Result { + String::from_utf8(bytes).map_err(|err| EngineError::Io(format!("{}: {err}", path.display()))) +} + +/// The largest session (`.scraft`) SoundCraft opens or saves. A session is read whole and parsed +/// in one go, so a larger one is refused by its size before it is read, and never written (it +/// could not be opened again); real sessions are far smaller. +pub const MAX_SESSION_BYTES: u64 = 256 << 20; + +#[cfg(test)] +thread_local! { + /// A smaller limit than [`MAX_SESSION_BYTES`] for the tests on this thread ([`SessionLimit`]). + static TEST_SESSION_LIMIT: std::cell::Cell> = const { std::cell::Cell::new(None) }; +} + +/// [`MAX_SESSION_BYTES`] (in tests, the limit a [`SessionLimit`] set on this thread). +pub(crate) fn session_limit() -> u64 { + #[cfg(test)] + if let Some(limit) = TEST_SESSION_LIMIT.with(std::cell::Cell::get) { + return limit; + } + MAX_SESSION_BYTES +} + +/// While it lives, sessions on this thread are opened and saved up to the limit given to +/// [`SessionLimit::set`] instead of [`MAX_SESSION_BYTES`], so tests reach it with small sessions. +#[cfg(test)] +pub(crate) struct SessionLimit; + +#[cfg(test)] +impl SessionLimit { + pub(crate) fn set(limit: u64) -> SessionLimit { + TEST_SESSION_LIMIT.with(|l| l.set(Some(limit))); + SessionLimit + } +} + +#[cfg(test)] +impl Drop for SessionLimit { + fn drop(&mut self) { + TEST_SESSION_LIMIT.with(|l| l.set(None)); + } +} + +/// Refuses the session `name` of `len` bytes when it is larger than [`session_limit`]: too large +/// to open, or, when `saving`, to save. +pub(crate) fn session_size(name: impl std::fmt::Display, len: u64, saving: bool) -> Result<()> { + let limit = session_limit(); + if len <= limit { + return Ok(()); + } + let most = if limit >= 1 << 20 { format!("{} MiB", limit >> 20) } else { format!("{limit} bytes") }; + Err(EngineError::Io(if saving { + format!("{name} is too large to save: over {most}, more than SoundCraft opens") + } else { + format!("{name} is too large to open (over {most})") + })) +} + +/// [`read_text`] for a session file: refused, unread, past [`session_limit`]. +pub(crate) fn read_session_text(path: impl AsRef) -> Result { + let path = path.as_ref(); + utf8(path, read_file_within(path, true)?) +} + +/// A bare `.scraft` handed over as bytes (the browser) as text: refused, uncopied, past +/// [`session_limit`]. +pub(crate) fn session_bytes_text(name: &str, bytes: &[u8]) -> Result { + session_size(name, bytes.len() as u64, false)?; + String::from_utf8(bytes.to_vec()).map_err(|_| EngineError::Io(format!("{name}: not a SoundCraft session"))) +} + +/// How many bytes [`Session::to_json`] makes of `s`, counted without keeping them. +fn json_len(s: &Session) -> u64 { + struct Count(u64); + impl std::io::Write for Count { + fn write(&mut self, buf: &[u8]) -> std::io::Result { + self.0 = self.0.saturating_add(buf.len() as u64); + Ok(buf.len()) + } + fn flush(&mut self) -> std::io::Result<()> { + Ok(()) + } + } + let mut n = Count(0); + // A session that cannot be written counts as nothing here: `to_json` reports that error. + serde_json::to_writer_pretty(&mut n, s).map_or(0, |()| n.0) } /// Is there a file at `path` the automation read root lets us see? A path outside the root is @@ -497,7 +600,6 @@ fn write_session(session: &Session, path: &str, overwrite: bool) -> Result<(Sess let mut s = session.clone(); let from = s.folder.clone(); let same_dir = from.as_deref().is_some_and(|f| same_path(f, &dir)); - let mut written = 0; // A file can stay put for all the sources that name it, unless their audio differs. let shared_paths: Vec = { let mut audio_by_path: HashMap<&str, HashSet>> = HashMap::new(); @@ -506,8 +608,11 @@ fn write_session(session: &Session, path: &str, overwrite: bool) -> Result<(Sess } s.sources.iter().map(|src| audio_by_path.get(src.path.as_str()).is_some_and(|a| a.len() > 1)).collect() }; - // Files written for audio during this save: sources playing the same audio share one. + // Files for audio in this save: sources playing the same audio share one. The files still to + // write (their stem, audio and the sources naming them) are written once the size is checked. let mut files: HashMap<*const SourceAudio, String> = HashMap::new(); + let mut queued: HashMap<*const SourceAudio, usize> = HashMap::new(); + let mut pending: Vec<(String, Arc, Vec)> = Vec::new(); for (i, src) in s.sources.iter_mut().enumerate() { if !same_dir && s.pool.get(src.id).is_none() @@ -533,7 +638,14 @@ fn write_session(session: &Session, path: &str, overwrite: bool) -> Result<(Sess src.format = FileFormat::Wav; continue; } - make_dir(&audio_dir)?; + if let Some((stem, _, sources)) = queued.get(&Arc::as_ptr(audio)).and_then(|&k| pending.get_mut(k)) { + // As long as any name the file can get, so the size checked is an upper bound. + src.path = format!("Audio Files/{stem}_9999.wav"); + src.unsaved = false; + src.format = FileFormat::Wav; + sources.push(i); + continue; + } let mut stem = sanitize_name(&src.name); if overwrite { // One file per source id so autosaves never collide or pile up. @@ -545,23 +657,46 @@ fn write_session(session: &Session, path: &str, overwrite: bool) -> Result<(Sess continue; } } - let opts = EncodeOptions { format: FileFormat::Wav, bit_depth: soundcraft_audio_io::BitDepth::Float32, dither: false, bwf: None }; - let bytes = soundcraft_audio_io::encode(&audio.buffer, &opts).map_err(|err| EngineError::Io(err.to_string()))?; - let (name, new) = store_wav(&audio_dir, &stem, &bytes)?; - src.path = format!("Audio Files/{name}"); + src.path = format!("Audio Files/{stem}_9999.wav"); src.unsaved = false; src.format = FileFormat::Wav; - files.insert(Arc::as_ptr(audio), src.path.clone()); - written += usize::from(new); - } - if written > 0 { - sync_dir(&audio_dir); + queued.insert(Arc::as_ptr(audio), pending.len()); + pending.push((stem, Arc::clone(audio), vec![i])); } s.folder = Some(dir.clone()); if !overwrite && let Some(stem) = p.file_stem().and_then(OsStr::to_str) { s.name = stem.to_string(); } - let text = s.to_json().map_err(|err| EngineError::Io(err.to_string()))?; + // Before any audio is written: a session too large to open again is not saved (#892). With no + // audio to write, the text checked is the text saved. + let checked = if pending.is_empty() { + let text = s.to_json().map_err(|err| EngineError::Io(err.to_string()))?; + session_size(p.display(), text.len() as u64, true)?; + Some(text) + } else { + session_size(p.display(), json_len(&s), true)?; + None + }; + let mut written = 0; + for (stem, audio, sources) in &pending { + make_dir(&audio_dir)?; + let opts = EncodeOptions { format: FileFormat::Wav, bit_depth: soundcraft_audio_io::BitDepth::Float32, dither: false, bwf: None }; + let bytes = soundcraft_audio_io::encode(&audio.buffer, &opts).map_err(|err| EngineError::Io(err.to_string()))?; + let (name, new) = store_wav(&audio_dir, stem, &bytes)?; + for &i in sources { + if let Some(src) = s.sources.get_mut(i) { + src.path = format!("Audio Files/{name}"); + } + } + written += usize::from(new); + } + if written > 0 { + sync_dir(&audio_dir); + } + let text = match checked { + Some(text) => text, + None => s.to_json().map_err(|err| EngineError::Io(err.to_string()))?, + }; make_dir(&dir)?; // Atomic-ish write: temp file then rename. let tmp = p.with_extension("scraft.tmp"); @@ -762,10 +897,13 @@ fn read_bundle(bytes: &[u8], limits: BundleLimits) -> Result<(String, std::colle if total > limits.bytes { return Err(EngineError::Io("bundle: too large".into())); } + let is_session = !rel.contains('/') && Path::new(&rel).extension().and_then(OsStr::to_str) == Some(soundcraft_model::SESSION_EXTENSION); + if is_session && text.is_none() { + session_size(format_args!("bundle: {rel}"), size, false)?; + } let mut data = Vec::new(); // The declared size is hostile too: never read past it. f.by_ref().take(size).read_to_end(&mut data).map_err(|err| EngineError::Io(format!("bundle: {rel}: {err}")))?; - let is_session = !rel.contains('/') && Path::new(&rel).extension().and_then(OsStr::to_str) == Some(soundcraft_model::SESSION_EXTENSION); if is_session && text.is_none() { text = Some(String::from_utf8(data).map_err(|_| EngineError::Io(format!("bundle: {rel} is not text")))?); } else { diff --git a/crates/engine/src/io/bundle.rs b/crates/engine/src/io/bundle.rs index b906333d..ea928aca 100644 --- a/crates/engine/src/io/bundle.rs +++ b/crates/engine/src/io/bundle.rs @@ -69,6 +69,8 @@ fn session_bundle_within(session: &Session, limits: BundleLimits) -> Result<(Vec src.format = FileFormat::Wav; } let text = s.to_json().map_err(|err| EngineError::Io(err.to_string()))?; + // Its session too: one the reader would refuse is not saved (#892). + super::session_size("the session", text.len() as u64, true)?; fits(text.len())?; w.start_file(format!("{}.{}", sanitize_name(&s.name), soundcraft_model::SESSION_EXTENSION), opts).map_err(zerr)?; w.write_all(text.as_bytes()).map_err(ioerr)?; @@ -100,10 +102,7 @@ pub fn open_session_bytes(e: &mut Engine, name: &str, bytes: &[u8]) -> Result Result) -> Result Result> { - let text = read_text(path)?; + let text = read_session_text(path)?; let mut s = Session::from_json(&text).map_err(|err| EngineError::Io(err.to_string()))?; let dir = path.parent().map(Path::to_path_buf).unwrap_or_default(); let mut missing = Vec::new(); @@ -98,6 +98,96 @@ mod save_tests { use super::*; use serde_json::json; + /// A new, empty scratch folder, removed again when the test ends, passed or not. + struct Scratch(std::path::PathBuf); + + impl Scratch { + fn new(tag: &str) -> Scratch { + let dir = std::env::temp_dir().join(format!("soundcraft-{tag}-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + Scratch(dir) + } + } + + impl Drop for Scratch { + fn drop(&mut self) { + let _ = std::fs::remove_dir_all(&self.0); + } + } + + #[test] + fn a_session_file_too_large_to_parse_is_refused_unread() { + // #892: a session file is read whole and parsed; past the cap it is refused by its size. + // The file's length is set, not written, so it takes no time or space to make. + let dir = Scratch::new("huge-session"); + let path = dir.0.join("Huge.scraft"); + std::fs::File::create(&path).unwrap().set_len(crate::io::MAX_SESSION_BYTES + 1).unwrap(); + let mut e = crate::demo::demo_engine(); + let err = open_session(&mut e, &path).unwrap_err().to_string(); + assert_eq!(err, format!("{} is too large to open (over 256 MiB)", path.display())); + assert_eq!(e.session().tracks.len(), crate::demo::demo_engine().session().tracks.len()); + let err = e.execute("file.import_session_data", &json!({"path": path.to_string_lossy()})).unwrap_err().to_string(); + assert!(err.contains("too large to open"), "{err}"); + } + + #[test] + fn a_session_too_large_to_open_is_not_saved_and_leaves_nothing_behind() { + // #892: what SoundCraft saves, it opens again. A refused save writes no audio and no + // session; the limit is lowered for this test so a small session reaches it. + let dir = Scratch::new("refused-save"); + let path = |p: &str| dir.0.join(p).to_string_lossy().into_owned(); + save_session(&mut with_take(0.5), &path("A/Song.scraft")).unwrap(); + let saved = std::fs::metadata(path("A/Song.scraft")).unwrap().len(); + let untouched = |sub: &str| assert!(!dir.0.join(sub).exists(), "{sub} was written"); + { + let _limit = crate::io::SessionLimit::set(saved - 1); + let mut e = with_take(0.5); + let err = save_session(&mut e, &path("B/Song.scraft")).unwrap_err().to_string(); + assert_eq!(err, format!("{} is too large to save: over {} bytes, more than SoundCraft opens", path("B/Song.scraft"), saved - 1)); + assert!(e.is_dirty() && e.path.is_none()); + let err = e.execute("session.save_copy", &json!({"path": path("C/Song.scraft")})).unwrap_err().to_string(); + assert!(err.contains("too large to save"), "{err}"); + for sub in ["B", "C"] { + untouched(sub); + } + // An open refuses the session saved before, now over the limit. + let err = open_session(&mut Engine::default(), path("A/Song.scraft")).unwrap_err().to_string(); + assert!(err.contains("too large to open"), "{err}"); + } + { + // A template holds no audio or clips: a smaller limit refuses it. + let _limit = crate::io::SessionLimit::set(100); + std::fs::create_dir_all(dir.0.join("D")).unwrap(); + let err = with_take(0.5).execute("session.save_template", &json!({"path": path("D/Song.scraft")})).unwrap_err().to_string(); + assert!(err.contains("too large to save"), "{err}"); + untouched("D/Song.scraft"); + } + // Checked before the audio file is named, against the longest name it can get: a + // session at the limit plus that margin is saved, and is the same size as before. + let _limit = crate::io::SessionLimit::set(saved + 5); + save_session(&mut with_take(0.5), &path("B/Song.scraft")).unwrap(); + assert_eq!(std::fs::metadata(path("B/Song.scraft")).unwrap().len(), saved); + assert_eq!(reopen(&path("B/Song.scraft")).session().sources.len(), 1); + } + + #[test] + fn a_saved_session_parses_as_it_did_through_a_tree() { + // #892: parsing the text directly gives the document the `Value` round trip gave. + let mut e = crate::demo::demo_engine(); + e.execute("mix.volume", &json!({"track": "Kick", "db": -7.0})).unwrap(); + let text = e.session().to_json().unwrap(); + let mut old: Session = serde_json::from_value(serde_json::from_str(&text).unwrap()).unwrap(); + old.sanitize(); + assert_eq!(Session::from_json(&text).unwrap(), old); + let mut r = Engine::default(); + let dir = Scratch::new("reparse"); + let path = dir.0.join("Song.scraft").to_string_lossy().into_owned(); + save_session(&mut e, &path).unwrap(); + assert_eq!(open_session(&mut r, &path).unwrap(), Vec::::new()); + assert_eq!(r.session().tracks, e.session().tracks); + } + #[test] fn autosave_copy_leaves_document_untouched_and_reuses_audio() { let mut e = crate::demo::demo_engine(); diff --git a/crates/model/src/session.rs b/crates/model/src/session.rs index 8d7d4998..45fa7913 100644 --- a/crates/model/src/session.rs +++ b/crates/model/src/session.rs @@ -724,13 +724,36 @@ impl Session { } /// Parse a session; decoded audio must be attached separately (`pool`). + /// + /// The text is parsed straight into the document, never into a `serde_json::Value` tree + /// first (which takes many times the file's size). Only the version is read before that. + /// A field of the document given twice in one object is refused (`duplicate field`), as no + /// writer repeats one. Keys of maps (plugin params, edit values, Auto Rename schemes, MIDI + /// originals) and unknown keys keep the last value given, as before. pub fn from_json(text: &str) -> Result { - let v: serde_json::Value = serde_json::from_str(text).map_err(|e| SessionError::Format(e.to_string()))?; - let ver = v.get("format_version").and_then(serde_json::Value::as_u64).ok_or_else(|| SessionError::Format("missing format_version".into()))?; + /// The version alone: every other field is skipped, not kept. + #[derive(serde::Deserialize)] + struct Head { + format_version: Option, + } + // `Head` would also take a JSON array as its fields in order: only an object has a version. + let object = text.trim_start_matches([' ', '\t', '\n', '\r']).starts_with('{'); + let version = match object.then(|| serde_json::from_str::(text)) { + Some(Ok(head)) => head.format_version, + // A version given twice is that error, not a missing version. + Some(Err(e)) if e.to_string().starts_with("duplicate field") => return Err(SessionError::Format(e.to_string())), + // Valid JSON of another shape (not an object, or a version that is not a number) + // has no version; anything else is not JSON. + _ => { + serde_json::from_str::(text).map_err(|e| SessionError::Format(e.to_string()))?; + None + } + }; + let ver = version.and_then(|v| v.as_u64()).ok_or_else(|| SessionError::Format("missing format_version".into()))?; if ver > u64::from(Self::FORMAT_VERSION) { return Err(SessionError::Version(u32::try_from(ver).unwrap_or(u32::MAX))); } - let mut s: Session = serde_json::from_value(v).map_err(|e| SessionError::Format(e.to_string()))?; + let mut s: Session = serde_json::from_str(text).map_err(|e| SessionError::Format(e.to_string()))?; s.sanitize(); s.validate()?; Ok(s) @@ -1140,6 +1163,53 @@ mod tests { assert!(matches!(Session::from_json(&v.to_string()), Err(SessionError::Version(999)))); } + #[test] + fn the_version_is_checked_as_before() { + // #892: the version is read without building the whole document as a tree; the errors + // stay what they were. + let format = |t: &str| match Session::from_json(t) { + Err(SessionError::Format(m)) => m, + other => panic!("{t}: {other:?}"), + }; + for t in [ + "{}", + "[]", + "[5]", + " \n[1, {}]", + "\"x\"", + "5", + "null", + r#"{"format_version":"1"}"#, + r#"{"format_version":-1}"#, + r#"{"format_version":1.5}"#, + r#"{"format_version":null}"#, + r#"{"format_version":[0,0,0]}"#, + r#"{"name":"x","other":{"format_version":1}}"#, + ] { + assert_eq!(format(t), "missing format_version", "{t}"); + } + for t in ["not json", r#"{"format_version":1"#, r#"{"format_version":1} x"#, r#"{"format_version":"1", ]"#] { + let want = serde_json::from_str::(t).unwrap_err().to_string(); + assert_eq!(format(t), want, "{t}"); + } + let mut v = serde_json::to_value(Session::default()).unwrap(); + for (ver, want) in [(serde_json::json!(2), 2), (serde_json::json!(1u64 << 40), u32::MAX)] { + v["format_version"] = ver; + assert_eq!(Session::from_json(&v.to_string()), Err(SessionError::Version(want))); + } + // A field given twice is refused by name, the version too. + let twice = |key: &str| format(&format!(r#"{{"format_version":1,"{key}":"a","{key}":"b"}}"#)); + assert!(twice("name").starts_with("duplicate field `name` at line 1 column "), "{}", twice("name")); + assert!(twice("format_version").starts_with("duplicate field `format_version` at line 1 column "), "{}", twice("format_version")); + // A newer version is refused even when the rest would not parse. + assert_eq!(Session::from_json(r#"{"format_version":7,"tracks":5}"#), Err(SessionError::Version(7))); + // An older or the current version loads as it did. + for ver in [0, 1] { + v["format_version"] = serde_json::json!(ver); + assert_eq!(Session::from_json(&v.to_string()).map(|s| s.format_version), Ok(ver)); + } + } + #[test] fn duplicate_ids_are_rejected_on_load() { let mut s = Session::default(); diff --git a/crates/model/src/track.rs b/crates/model/src/track.rs index 663a6a4b..38f35b1b 100644 --- a/crates/model/src/track.rs +++ b/crates/model/src/track.rs @@ -83,8 +83,7 @@ impl TrackKind { } /// Track heights (View › Track Height presets). -#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Default, serde::Serialize, serde::Deserialize)] -#[serde(from = "TrackHeightRepr")] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, Default, serde::Serialize)] pub enum TrackHeight { /// Any height from Micro to Extreme, in points. Custom(u16), @@ -100,49 +99,69 @@ pub enum TrackHeight { Fit(u16), } -/// What a saved track height may look like: the current form, or the bare `"Fit"` string older sessions used. -#[derive(serde::Deserialize)] -#[serde(untagged)] -enum TrackHeightRepr { - Current(TrackHeightCurrent), - LegacyFit(LegacyFit), +/// A saved track height: `"Micro"` … `"Extreme"` (or `{"Micro": null}` …), `{"Custom": points}`, +/// `{"Fit": points}`, or the bare `"Fit"` older sessions used. Read as it comes, never buffered (as an +/// untagged enum would), so anything else in a session file is refused at its first token. +impl<'de> serde::Deserialize<'de> for TrackHeight { + fn deserialize>(d: D) -> Result { + d.deserialize_any(HeightVisitor) + } } -#[derive(serde::Deserialize)] -enum LegacyFit { - Fit, +const HEIGHT_VARIANTS: &[&str] = &["Custom", "Micro", "Mini", "Small", "Medium", "Large", "Jumbo", "Extreme", "Fit"]; + +struct HeightVisitor; + +impl HeightVisitor { + /// The height a variant without a size names. + fn preset(name: &str) -> Option { + Some(match name { + "Micro" => TrackHeight::Micro, + "Mini" => TrackHeight::Mini, + "Small" => TrackHeight::Small, + "Medium" => TrackHeight::Medium, + "Large" => TrackHeight::Large, + "Jumbo" => TrackHeight::Jumbo, + "Extreme" => TrackHeight::Extreme, + _ => return None, + }) + } } -/// Mirror of `TrackHeight` that derives the current serialized form. -#[derive(serde::Deserialize)] -enum TrackHeightCurrent { - Custom(u16), - Micro, - Mini, - Small, - Medium, - Large, - Jumbo, - Extreme, - Fit(u16), -} +impl<'de> serde::de::Visitor<'de> for HeightVisitor { + type Value = TrackHeight; -impl From for TrackHeight { - fn from(repr: TrackHeightRepr) -> Self { - match repr { - TrackHeightRepr::Current(TrackHeightCurrent::Custom(p)) => TrackHeight::Custom(p), - TrackHeightRepr::Current(TrackHeightCurrent::Micro) => TrackHeight::Micro, - TrackHeightRepr::Current(TrackHeightCurrent::Mini) => TrackHeight::Mini, - TrackHeightRepr::Current(TrackHeightCurrent::Small) => TrackHeight::Small, - TrackHeightRepr::Current(TrackHeightCurrent::Medium) => TrackHeight::Medium, - TrackHeightRepr::Current(TrackHeightCurrent::Large) => TrackHeight::Large, - TrackHeightRepr::Current(TrackHeightCurrent::Jumbo) => TrackHeight::Jumbo, - TrackHeightRepr::Current(TrackHeightCurrent::Extreme) => TrackHeight::Extreme, - TrackHeightRepr::Current(TrackHeightCurrent::Fit(p)) => TrackHeight::Fit(p), + fn expecting(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result { + f.write_str("a track height") + } + + fn visit_str(self, v: &str) -> Result { + match v { // Older sessions stored Fit with no size, drawn 88 points tall: the default Medium height. - TrackHeightRepr::LegacyFit(LegacyFit::Fit) => TrackHeight::Medium, + "Fit" => Ok(TrackHeight::Medium), + "Custom" => Err(E::invalid_type(serde::de::Unexpected::UnitVariant, &"a height in points")), + other => Self::preset(other).ok_or_else(|| E::unknown_variant(other, HEIGHT_VARIANTS)), } } + + fn visit_map>(self, mut map: A) -> Result { + use serde::de::Error as _; + let Some(key) = map.next_key::()? else { return Err(A::Error::invalid_length(0, &"one variant")) }; + let h = match key.as_str() { + "Custom" => TrackHeight::Custom(map.next_value()?), + // `{"Fit": null}` is the old size-less Fit too, as above. + "Fit" => map.next_value::>()?.map_or(TrackHeight::Medium, TrackHeight::Fit), + other => { + let h = Self::preset(other).ok_or_else(|| A::Error::unknown_variant(other, HEIGHT_VARIANTS))?; + map.next_value::<()>()?; + h + } + }; + if map.next_key::()?.is_some() { + return Err(A::Error::invalid_length(2, &"one variant")); + } + Ok(h) + } } impl TrackHeight { @@ -383,6 +402,36 @@ mod tests { assert_eq!(serde_json::from_str::("{\"Custom\":50}").unwrap(), TrackHeight::Custom(50)); } + #[test] + fn every_saved_height_form_loads_and_nothing_else() { + for h in TrackHeight::ALL.into_iter().chain([TrackHeight::Custom(50), TrackHeight::Fit(120)]) { + assert_eq!(serde_json::from_str::(&serde_json::to_string(&h).unwrap()).unwrap(), h); + assert_eq!(serde_json::from_value::(serde_json::to_value(h).unwrap()).unwrap(), h); + } + for h in TrackHeight::ALL { + let null = format!("{{{}:null}}", serde_json::to_string(&h).unwrap()); + assert_eq!(serde_json::from_str::(&null).unwrap(), h, "{null}"); + } + assert_eq!(serde_json::from_str::(r#"{"Fit":null}"#).unwrap(), TrackHeight::Medium); + assert!(serde_json::from_str::(r#"{"Custom":null}"#).is_err()); + for bad in + [r#""Custom""#, r#""Huge""#, "7", "[]", "null", "{}", r#"{"Fit":"x"}"#, r#"{"Fit":70000}"#, r#"{"Large":1}"#, r#"{"Fit":1,"Fit":2}"#] + { + assert!(serde_json::from_str::(bad).is_err(), "{bad}"); + } + } + + #[test] + fn a_huge_height_is_refused_at_its_first_token() { + // #892: the height was read through an untagged enum, which buffers the whole value + // first; a session could hold a huge array there. It is refused where it starts. + for (open, rest) in [("[", "0,".repeat(100_000) + "0]"), (r#"{"Fit":["#, "0,".repeat(100_000) + "0]}")] { + let text = format!("{open}{rest}"); + let err = serde_json::from_str::(&text).unwrap_err(); + assert!(err.is_data() && err.line() == 1 && err.column() <= open.len() + 1, "{open}: {err}"); + } + } + #[test] fn new_track_defaults() { let t = Track::new(TrackId(1), "Gtr", TrackKind::Audio, ChannelFormat::Stereo, TRACK_COLORS[0]);