fix(server): keep each team library version in its own file (#206)

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");
+}

apps/figcraft-server/src/store.rs

@@ -10,7 +10,8 @@
 //!                                and a join code's hash and wrapped token (see `codes`)
 //! <data>/files/<id>/snapshot.bin the session state at the last compaction
 //! <data>/files/<id>/log.bin      what changed since (see `disk`)
-//! <data>/files/<id>/library.bin  its team library, when one was published (a `.figcraft` archive)
+//! <data>/files/<id>/library-<version>.bin  its team library, when one was published (a `.figcraft`
+//!                                archive; `library.bin` from servers before versioned names)
 //! <data>/files/<id>/usage.json   how other files use that library, as their apps report it
 //! <data>/plugins/                the plugin directory (see `directory`)
 //! <data>/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<u64>) -> Result<LibraryReply, String> {
-        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<u64, ()> {
     }
 }
 
+/// 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<T: Serialize + ?Sized>(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())

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-<version>.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

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
 <data>/files/<id>/snapshot.bin its state at the last compaction (with a checksum)
 <data>/files/<id>/log.bin      what changed since, one checksummed record per change
-<data>/files/<id>/library.bin  its team library, when one was published (a .figcraft archive)
+<data>/files/<id>/library-<version>.bin  its team library, when one was published (a .figcraft
+                               archive; library.bin when it was published by an older server)
 <data>/files/<id>/usage.json   which files use that library's components, as their apps report it
 <data>/plugins/index.json      the plugin directory: every version kept (manifest fields, size, SHA-256)
 <data>/plugins/<sha256>.bin    a plugin bundle, named by the SHA-256 of its bytes