From a8a689d682da98cd407d7134a8a1084b8657e680 Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Wed, 14 Oct 2026 16:12:55 +0200 Subject: [PATCH] fix(server): keep each team library version in its own file (#206) --- apps/figcraft-server/src/hardening_tests.rs | 38 +++++++++++++++++++++ apps/figcraft-server/src/store.rs | 36 +++++++++++++++---- docs/collab-protocol.md | 6 ++-- docs/server.md | 3 +- 4 files changed, 74 insertions(+), 9 deletions(-) diff --git a/apps/figcraft-server/src/hardening_tests.rs b/apps/figcraft-server/src/hardening_tests.rs index 6a74e617..086539ca 100644 --- a/apps/figcraft-server/src/hardening_tests.rs +++ b/apps/figcraft-server/src/hardening_tests.rs @@ -800,3 +800,41 @@ fn the_server_forgets_addresses_whose_connections_are_gone() { assert!(server.blocks_remembered() <= 2, "{} blocks remembered", server.blocks_remembered()); server.stop(); } + +#[test] +fn a_library_publish_that_fails_keeps_serving_the_old_version() { + let dir = TempDir::new("libfail"); + let mut store = Store::open(dir.path(), config(dir.path()).store()).unwrap(); + let (file, ..) = store.publish("Library", &snapshot("Library")).unwrap(); + // Version 1, then a changed library as version 2. + let mut s = Session::new(); + let f = s.execute("frame.create", &json!({"x": 0, "y": 0, "width": 50, "height": 20, "name": "Button"})).unwrap()["id"].as_u64().unwrap(); + let c = s.execute("component.create", &json!({"ids": [f]})).unwrap()["id"].as_u64().unwrap(); + s.execute("library.publish", &json!({})).unwrap(); + let v1 = figcraft_engine::library_server::archive(&s.doc().unwrap().published.clone().unwrap()).unwrap(); + s.execute("node.set", &json!({"id": c, "props": {"fill": "#00ff00"}})).unwrap(); + s.execute("library.publish", &json!({})).unwrap(); + let v2 = figcraft_engine::library_server::archive(&s.doc().unwrap().published.clone().unwrap()).unwrap(); + assert_eq!(store.publish_library(&file.id, &v1).unwrap(), 1); + let files = dir.path().join(crate::store::FILES).join(&file.id); + // The description can't be written: a directory sits where its temporary file goes. + let tmp = crate::disk::temp_of(&files.join(crate::disk::META)); + std::fs::create_dir(&tmp).unwrap(); + assert!(store.publish_library(&file.id, &v2).is_err()); + let fetch = |s: &mut Store| s.library(ip("198.51.100.1"), &file.id, Access::View, LibraryReq::Fetch { have: None, usage: None }); + let served = |reply: LibraryReply| match reply { + LibraryReply::Library { version, archive } => (version, archive), + other => panic!("{other:?}"), + }; + assert_eq!(served(fetch(&mut store)), (1, v1.clone()), "version 1, with its own bytes"); + // A crash right after the archive was written leaves it on disk, the description unchanged. + std::fs::write(files.join(crate::store::library_name(2)), &v2).unwrap(); + drop(store); + std::fs::remove_dir(&tmp).unwrap(); + let mut store = Store::open(dir.path(), config(dir.path()).store()).unwrap(); + assert_eq!(served(fetch(&mut store)), (1, v1), "still version 1 after a restart"); + // Publishing again works, and serves the new version. + assert_eq!(store.publish_library(&file.id, &v2).unwrap(), 2); + assert_eq!(served(fetch(&mut store)), (2, v2)); + assert!(!files.join(crate::store::library_name(1)).exists(), "the old version is deleted"); +} diff --git a/apps/figcraft-server/src/store.rs b/apps/figcraft-server/src/store.rs index e5e56b8f..15408251 100644 --- a/apps/figcraft-server/src/store.rs +++ b/apps/figcraft-server/src/store.rs @@ -10,7 +10,8 @@ //! and a join code's hash and wrapped token (see `codes`) //! /files//snapshot.bin the session state at the last compaction //! /files//log.bin what changed since (see `disk`) -//! /files//library.bin its team library, when one was published (a `.figcraft` archive) +//! /files//library-.bin its team library, when one was published (a `.figcraft` +//! archive; `library.bin` from servers before versioned names) //! /files//usage.json how other files use that library, as their apps report it //! /plugins/ the plugin directory (see `directory`) //! /blocked.json addresses the operator blocked @@ -179,7 +180,7 @@ impl Meta { } } -/// What a file's team library is (`library.bin` holds it). +/// What a file's team library is ([`library_file`] holds it). #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] struct LibraryMeta { id: String, @@ -919,11 +920,12 @@ impl Store { /// A file's team library for `ip`, unless its version is `have`; within what one address and /// everyone may download (the size is known before anything is read). fn fetch_library(&mut self, ip: IpAddr, file: &str, have: Option) -> Result { - let (path, max) = (self.dir(file).join(LIBRARY), self.cfg.max_file_bytes); + let (dir, max) = (self.dir(file), self.cfg.max_file_bytes); let Some(lib) = self.entry(file)?.meta.library.clone() else { return Ok(LibraryReply::Empty) }; if have == Some(lib.version) { return Ok(LibraryReply::Unchanged { version: lib.version }); } + let path = library_file(&dir, lib.version); let size = fs::metadata(&path).map_or(0, |m| m.len()); let now = Instant::now(); if !self.abuse.allow(ip, Action::Download(usize::try_from(size).unwrap_or(usize::MAX)), now) || !self.served.take_n(now, size as f64) { @@ -965,14 +967,24 @@ impl Store { return Err(format!("version {} is on the server already: publish again to follow it", old.version)); } } - // The archive first: a description never names a version whose library isn't there (a - // failed description write leaves the older number, which the next publish follows). - disk::write_atomic(&dir.join(LIBRARY), archive).map_err(|e| format!("couldn't keep the library: {e}"))?; + // Each version has a file of its own, written before the description names it: a failed + // description write or a crash in between leaves the previous version served whole, never + // the new bytes under the old number. + let path = dir.join(library_name(m.version)); + disk::write_atomic(&path, archive).map_err(|e| format!("couldn't keep the library: {e}"))?; let before = entry.meta.library.replace(LibraryMeta { id: m.id.clone(), name: m.name.clone(), version: m.version }); if let Err(e) = write_json(&dir.join(META), &entry.meta) { + // Never served (the description doesn't name it); the next publish of it replaces it. + if let Err(r) = fs::remove_file(&path) { + log::warn!("file {file}: an unpublished library version couldn't be deleted: {r}"); + } entry.meta.library = before; return Err(format!("couldn't keep the library: {e}")); } + // The previous version isn't served any more. + if let Some(Err(e)) = before.map(|old| fs::remove_file(library_file(&dir, old.version))) { + log::warn!("file {file}: the previous library version couldn't be deleted: {e}"); + } log::info!("file {file}: published library version {} ({} bytes)", m.version, archive.len()); Ok(m.version) } @@ -1249,6 +1261,18 @@ fn flush_log(id: &str, log: &mut Log) -> Result { } } +/// The name of the file holding version `version` of a file's team library. +pub fn library_name(version: u64) -> String { + format!("library-{version}.bin") +} + +/// Where version `version` of the team library in `dir` is: its own file, or `library.bin` for a +/// version published before each version had one. +fn library_file(dir: &Path, version: u64) -> PathBuf { + let path = dir.join(library_name(version)); + if path.exists() { path } else { dir.join(LIBRARY) } +} + fn write_json(path: &Path, value: &T) -> Result<(), String> { let text = serde_json::to_vec_pretty(value).map_err(|e| e.to_string())?; disk::write_atomic(path, &text).map_err(|e| e.to_string()) diff --git a/docs/collab-protocol.md b/docs/collab-protocol.md index 7888241d..874988a3 100644 --- a/docs/collab-protocol.md +++ b/docs/collab-protocol.md @@ -726,8 +726,10 @@ resuming with its offline edits, and leaving the session going back to snapshot - **What is kept.** The published library a library file holds (manifest and snapshot with per-asset versions) travels as a small `.figcraft` archive holding only it. The server reads it in full with the limits a library file is read with, requires the file's library id once one is - kept and a newer version, then writes it beside the file (`library.bin`, atomically) and records - id, name and version in the file's description. Publishing from the app first fetches the + kept and a newer version, then writes it beside the file in a file of its own + (`library-.bin`, atomically) and only then records id, name and version in the file's + description, so a failed write or a crash in between leaves the previous version served whole; + the previous version's file is deleted after. Publishing from the app first fetches the server's version and publishes the next one from it, so per-asset versions continue whoever published last; a race is refused and the next publish follows. - **Using it.** A file enables the library with the link in place of a library file's path diff --git a/docs/server.md b/docs/server.md index 88d99928..f2e4820c 100644 --- a/docs/server.md +++ b/docs/server.md @@ -320,7 +320,8 @@ behaves as before (blocked addresses, report rates and the limits on a link with a join code its stretched hash and the wrapped token /files//snapshot.bin its state at the last compaction (with a checksum) /files//log.bin what changed since, one checksummed record per change -/files//library.bin its team library, when one was published (a .figcraft archive) +/files//library-.bin its team library, when one was published (a .figcraft + archive; library.bin when it was published by an older server) /files//usage.json which files use that library's components, as their apps report it /plugins/index.json the plugin directory: every version kept (manifest fields, size, SHA-256) /plugins/.bin a plugin bundle, named by the SHA-256 of its bytes