shifathosn
/
figcraft
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(server): never persist a snapshot the reader refuses (#188) (#236) 

Snapshots hold more than the document (recent batches, deletions), so near
the cap compaction could write one over max_file_bytes, which both readers
then refused: the file vanished on restart. Reading now allows twice the
cap, and compaction refuses a snapshot beyond that, keeping the old
snapshot and log.

Co-authored-by: Justin John <babyjjbabyjj549@gmail.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Justin John 2026-10-14 09:01:30 +05:30 committed by GitHub
parent 118ca7c60b
commit 8e12e5eeda
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 28 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
apps/figcraft-server/src/hardening_tests.rs
View File

@ -235,6 +235,21 @@ fn a_file_cant_grow_past_its_cap_through_batches() {
// Its snapshot holds the document and the recent batches kept for reconnecting clients (at
// most a quarter of the cap), and the last batch let in may finish what the cap allows.
assert!(on_disk <= max + max / 4 + 210_000, "{on_disk} bytes on disk");
drop(raw);
server.stop();
// Everything confirmed opens again with the same settings, after compacting on the way out.
let kept = client.document().nodes().count();
let mut server = crate::start(&cfg).unwrap();
let mut raw = Raw::connect(server.addr());
raw.send(&Client::hello(&file.id, &edit.token, "Grower", ""));
let welcome = loop {
match raw.next() {
w @ ServerMsg::Welcome { .. } => break w,
ServerMsg::Refused { reason } => panic!("the file doesn't open again: {reason}"),
_ => {}
}
};
assert_eq!(Client::join(welcome).unwrap().document().nodes().count(), kept);
server.stop();
}

15
apps/figcraft-server/src/store.rs
View File

@ -426,7 +426,7 @@ impl Store {
let meta: Meta = serde_json::from_slice(&disk::read_capped(&path.join(META), 1 << 20)?).map_err(|e| format!("{META} is damaged ({e})"))?;
let links: Vec<Link> =
serde_json::from_slice(&disk::read_capped(&path.join(LINKS), 16 << 20)?).map_err(|e| format!("{LINKS} is damaged ({e})"))?;
let generation = disk::check_snapshot(&disk::read_capped(&path.join(SNAPSHOT), self.cfg.max_file_bytes)?)?;
let generation = disk::check_snapshot(&disk::read_capped(&path.join(SNAPSHOT), self.snapshot_cap())?)?;
// A damaged log only loses its end; it is reported when the file opens.
if let Ok(bytes) = disk::read_capped(&path.join(LOG), self.cfg.max_file_bytes)
&& let Ok(log) = disk::read_log(&bytes)
@ -1086,7 +1086,7 @@ impl Store {
fn load(&mut self, file: &str) -> Result<Server, String> {
self.touch(file);
let dir = self.dir(file);
let (generation, state) = disk::decode_snapshot(&disk::read_capped(&dir.join(SNAPSHOT), self.cfg.max_file_bytes)?)?;
let (generation, state) = disk::decode_snapshot(&disk::read_capped(&dir.join(SNAPSHOT), self.snapshot_cap())?)?;
let mut server = Server::restore(state, "", self.cfg.core).map_err(|e| format!("the snapshot can't be used: {e}"))?;
let mut issues = Vec::new();
let mut fresh_log = true;
@ -1130,11 +1130,22 @@ impl Store {
Ok(server)
}
/// The largest snapshot read back: a file's cap plus room for what its state keeps beside the
/// document (recent batches for reconnecting clients, deletions), which may take it over.
fn snapshot_cap(&self) -> u64 {
self.cfg.max_file_bytes.saturating_mul(2)
}
/// Write `server`'s state as snapshot `generation` and start an empty log of it.
fn rewrite(&mut self, file: &str, generation: u64, server: &Server) -> Result<Log, String> {
let dir = self.dir(file);
let state = server.state().map_err(|e| e.to_string())?;
let bytes = disk::encode_snapshot(generation, &state)?;
// Never replace a readable snapshot with one that can't be opened again: the old snapshot
// and log still hold everything.
if bytes.len() as u64 > self.snapshot_cap() {
return Err(format!("the snapshot would be {} MiB, more than can be opened again", bytes.len() >> 20));
}
if bytes.len() as u64 > self.cfg.max_file_bytes {
log::warn!("file {file} is {} MiB, over the {} MiB cap (kept anyway)", bytes.len() >> 20, self.cfg.max_file_bytes >> 20);
}