diff --git a/crates/catalog/src/journal.rs b/crates/catalog/src/journal.rs index 289ee4fb8..edcfae22a 100644 --- a/crates/catalog/src/journal.rs +++ b/crates/catalog/src/journal.rs @@ -271,7 +271,7 @@ fn resync(line: &[u8], last: u64) -> Option<(usize, (u64, Op))> { const START: &[u8] = b"{\"seq\":"; (1..line.len().saturating_sub(START.len() - 1)).filter(|&i| line[i..].starts_with(START)).find_map(|i| { let rec = std::str::from_utf8(&line[i..]).ok().map(|l| l.trim_end_matches('\r')).and_then(decode_record)?; - (rec.0 <= last + 1).then_some((i, rec)) + (rec.0 <= last.saturating_add(1)).then_some((i, rec)) }) } @@ -298,6 +298,9 @@ fn write_snapshot(store: &mut dyn Store, seq: u64, catalog: &Catalog) -> std::io Ok(SnapshotTiming { serialize_ms, write_sync_ms: (ms_since(t0) - serialize_ms).max(0.0), bytes, ..Default::default() }) } +/// The highest `seq` a snapshot may hold; beyond it the counter is treated as corrupt, far from overflow. +const MAX_SEQ: u64 = u64::MAX / 2; + /// Just the identification of `catalog.snap` (read before the catalog itself). #[derive(serde::Deserialize)] struct SnapHeader { @@ -331,6 +334,10 @@ impl Journal { return Err(newer(format!("{SNAPSHOT} is catalog format v{}", h.version))); } let s: SnapFile = serde_json::from_slice(&bytes).map_err(|e| CatalogError::Corrupt(format!("{SNAPSHOT}: {e}")))?; + // a seq this high can't come from real use; the next record's seq would wrap + if s.seq > MAX_SEQ { + return Err(CatalogError::Corrupt(format!("{SNAPSHOT}: seq {} is out of range", s.seq))); + } let mut catalog = s.catalog; report.repaired = catalog.repair(); for r in &report.repaired { @@ -392,7 +399,7 @@ impl Journal { let _ = op; report.stale += 1; } - Some((seq, op)) if seq == j.seq + 1 => { + Some((seq, op)) if j.seq.checked_add(1) == Some(seq) => { if catalog.apply(op).is_err() { report.failed += 1; } else { @@ -476,7 +483,7 @@ impl Journal { let mut buf = String::new(); let mut seq = self.seq; for op in ops { - seq += 1; + seq = seq.checked_add(1).ok_or_else(|| CatalogError::Corrupt(format!("{LOG}: op counter overflow")))?; buf.push_str(&encode_record(seq, op)); buf.push('\n'); } @@ -527,7 +534,7 @@ impl Journal { log::warn!("catalog: background snapshot failed ({e}); writing one now"); } let t0 = web_time::Instant::now(); - let seq = self.seq + unlogged; + let seq = self.seq.checked_add(unlogged).ok_or_else(|| CatalogError::Corrupt(format!("{SNAPSHOT}: op counter overflow")))?; let mut timing = match write_snapshot(self.store.as_mut(), seq, catalog) { Ok(t) => t, Err(e) => { diff --git a/crates/catalog/src/tests_journal.rs b/crates/catalog/src/tests_journal.rs index 8570bd077..26b4e902a 100644 --- a/crates/catalog/src/tests_journal.rs +++ b/crates/catalog/src/tests_journal.rs @@ -376,3 +376,18 @@ fn a_damaged_process_value_loads_as_v1_with_the_edits() { let d = &loaded.photo(id).unwrap().develop; assert_eq!((d.process, d.light.exposure), (ProcessVersion::V1, 0.8)); } + +/// A snapshot whose `seq` is near `u64::MAX` is refused as corrupt: the next record's seq would +/// wrap (and every later edit would be skipped as stale on reopen) or overflow-panic in debug. +#[test] +fn snapshot_with_seq_near_u64_max_is_corrupt() { + for seq in [u64::MAX, u64::MAX - 1, u64::MAX / 2 + 1] { + let m = MemStore::new(); + m.set( + SNAPSHOT, + format!("{{\"format\":\"lightcraft-catalog\",\"version\":1,\"seq\":{seq},\"catalog\":{}}}\n", Catalog::new().to_snapshot()).into_bytes(), + ); + let e = Journal::open(Box::new(m)).err(); + assert!(matches!(e, Some(CatalogError::Corrupt(_))), "seq {seq}: {e:?}"); + } +}