diff --git a/src/snapshot.rs b/src/snapshot.rs index 4e6da89..9907a6b 100644 --- a/src/snapshot.rs +++ b/src/snapshot.rs @@ -783,9 +783,17 @@ fn parse_record(data: &[u8]) -> Result<(Record<'_>, usize), RecordError> { other => { // Trailing NUL slack (prealloc / torn copy) is a discarded tail, // not an invalid file. Mid-file unknown types still fail-stop. + // crc==0 && type==0 is trailing slack ONLY if the rest of the + // file is zeros; a hole in front of a CRC-valid suffix is + // mid-file junk (load must not compact that suffix away). let crc = u32::from_le_bytes([data[0], data[1], data[2], data[3]]); if crc == 0 && other == 0 { - return Err(RecordError::Truncated); + if data.iter().all(|&b| b == 0) { + return Err(RecordError::Truncated); + } + return Err(RecordError::Invalid( + "mid-file zero hole before a non-zero suffix".into(), + )); } Err(RecordError::Invalid(format!( "unknown record type: {other:#x}" diff --git a/tests/dst_invariants.rs b/tests/dst_invariants.rs index f3d57bb..6fd4cce 100644 --- a/tests/dst_invariants.rs +++ b/tests/dst_invariants.rs @@ -2,7 +2,7 @@ //! //! These assert the *correct* behavior. They fail on current main (B1/B3/B4). -use slipstream::snapshot::{SnapshotError, SnapshotWriter, load}; +use slipstream::snapshot::{load, SnapshotError, SnapshotWriter}; use slipstream::{AppendLogSnapshot, KvEntry, KvUpdate, SnapshotStore, VersionToken, WatchCursor}; use std::path::Path; use tempfile::TempDir; @@ -136,3 +136,57 @@ fn trailing_zero_bytes_do_not_reject_the_prefix() { assert_eq!(snap.entries.len(), 3, "all three keys must survive"); assert_eq!(snap.cursor.as_u64(), Some(3)); } + +/// Five NULs at a record boundary look like `crc=0, type=0`. After the B4 +/// fix that is `Truncated`, so a *mid-file* hole (prealloc / leftover +/// version bytes / torn copy) stops replay and `load` rewrites the suffix +/// away. Each PUT/CURSOR on either side is CRC-valid — the zeros are not +/// inside any frame. +#[test] +fn mid_file_nuls_must_not_drop_a_crc_valid_suffix() { + let dir = TempDir::new().unwrap(); + let path = dir.path().join("s.snap"); + + let mut w = SnapshotWriter::open(&path, u64::MAX).unwrap(); + w.write_update(&put("a", b"x", 1)).unwrap(); + w.checkpoint(&WatchCursor::from_u64(1)).unwrap(); + w.write_update(&put("b", b"y", 2)).unwrap(); + w.checkpoint(&WatchCursor::from_u64(2)).unwrap(); + drop(w); + + let orig = std::fs::read(&path).unwrap(); + // First record: PUT a (key 1, value 1, ver 8) = 4+1+2+1+4+1+1+8 = 22, + // then CURSOR 1 = 4+1+1+8 = 14. Insert after those two. + let rec1 = 22usize; + let cur1 = 14usize; + let insert_at = 6 + rec1 + cur1; + assert!( + insert_at < orig.len(), + "need a suffix after the first put+cursor" + ); + let mut punched = orig[..insert_at].to_vec(); + punched.extend_from_slice(&[0u8; 5]); + punched.extend_from_slice(&orig[insert_at..]); + std::fs::write(&path, &punched).unwrap(); + + match load(&path) { + Err(SnapshotError::Corrupted) | Err(SnapshotError::InvalidFormat(_)) => { + // Fail-stop on mid-file junk: file must stay intact so a suffix + // of CRC-valid records is not burned by compact-on-load. + let stayed = std::fs::read(&path).unwrap(); + assert_eq!( + stayed, punched, + "fail-stop must not rewrite the suffix away" + ); + } + Ok(Some(snap)) => { + assert!( + snap.entries.contains_key("b") && snap.cursor.as_u64() == Some(2), + "mid-file NULs must not silent-drop a CRC-valid suffix; got keys={:?} cursor={:?}", + snap.entries.keys().collect::>(), + snap.cursor.as_u64() + ); + } + other => panic!("unexpected {other:?}"), + } +}