diff --git a/packages/cultcache-py/tests/test_cultcache.py b/packages/cultcache-py/tests/test_cultcache.py index c2f0fca8..e1d28354 100644 --- a/packages/cultcache-py/tests/test_cultcache.py +++ b/packages/cultcache-py/tests/test_cultcache.py @@ -903,6 +903,35 @@ class CultCacheTests(unittest.TestCase): SingleFileMessagePackBackingStore(store_path).push_all(envelopes) self.assertEqual(store_path.read_bytes(), before, f"{vector.name} was rewritten") + def test_single_file_push_all_rewrites_only_a_file_that_is_exactly_one_store(self) -> None: + import msgpack + + envelopes = SingleFileMessagePackBackingStore(self._C2A_VECTORS / "v3-base.msgpack").pull_all() + whole = (self._C2A_VECTORS / "v3-base.msgpack").read_bytes() + refused = { + "truncated": whole[:-1], + "trailing": whole + bytes([1, 2, 3]), + "scalar": bytes([0x01]), + "map": bytes([0x80]), + "string first": msgpack.packb(["hello", 1]), + } + for name, content in refused.items(): + with tempfile.TemporaryDirectory() as tmp: + store_path = Path(tmp) / "store.msgpack" + store_path.write_bytes(content) + with self.assertRaises(ValueError, msg=name): + SingleFileMessagePackBackingStore(store_path).push_all(envelopes) + self.assertEqual(store_path.read_bytes(), content, f"{name} was rewritten") + + # An empty file and a legacy envelope array (empty included) carry no header and are rewritten unmarked. + legacy = msgpack.packb([{"key": "old", "type": "old", "payload": b"", "storedAt": "then"}], use_bin_type=True) + for name, content in {"empty": b"", "empty array": bytes([0x90]), "legacy": legacy}.items(): + with tempfile.TemporaryDirectory() as tmp: + store_path = Path(tmp) / "store.msgpack" + store_path.write_bytes(content) + SingleFileMessagePackBackingStore(store_path).push_all(envelopes) + self.assertEqual(msgpack.unpackb(store_path.read_bytes(), raw=False)[0], "cultcache.store.v1", name) + if __name__ == "__main__": unittest.main() diff --git a/packages/cultcache-rs/src/lib.rs b/packages/cultcache-rs/src/lib.rs index 3f8e86e6..3b19dd45 100644 --- a/packages/cultcache-rs/src/lib.rs +++ b/packages/cultcache-rs/src/lib.rs @@ -321,8 +321,9 @@ const STORE_FORMAT_ELEMENT_IDS: &str = "cultcache.store.v3"; /// The store header string, `None` for the legacy envelope array (its first slot is not a string), and an error for /// bytes that are not one complete MessagePack array: a truncated store is refused as the snapshot reader refuses it, -/// never taken for a legacy file. One pass reads the first slot and walks the rest to prove the file whole. -fn store_header(bytes: &[u8]) -> Result> { +/// never taken for a legacy file. One pass reads the first slot and walks the rest to prove the file whole. A rewrite +/// asks for the file to be `whole`: exactly that one array, so bytes after it are refused too. +fn store_header(bytes: &[u8], whole: bool) -> Result> { #[derive(serde::Deserialize)] #[serde(untagged)] enum FirstSlot { @@ -353,9 +354,12 @@ fn store_header(bytes: &[u8]) -> Result> { deserializer.deserialize_seq(HeaderVisitor) } } - rmp_serde::from_slice::
(bytes) - .map(|header| header.0) - .map_err(|error| anyhow!("the store is not one complete MessagePack array: {error}")) + let mut cursor = std::io::Cursor::new(bytes); + let header =
::deserialize(&mut rmp_serde::Deserializer::new(&mut cursor)) + .map_err(|error| anyhow!("the store is not one complete MessagePack array: {error}"))?; + let after = bytes.len() - cursor.position() as usize; + ensure!(!whole || after == 0, "the store has {after} bytes after its MessagePack array"); + Ok(header.0) } #[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] @@ -737,7 +741,7 @@ impl SingleFileMessagePackBackingStore { if bytes.is_empty() { return Ok(Vec::new()); } - match store_header(&bytes).ok().flatten() { + match store_header(&bytes, false).ok().flatten() { Some(header) if header.starts_with("cultcache.store.") => decode_store_snapshot(&bytes), _ => rmp_serde::from_slice(&bytes).map_err(anyhow::Error::from), } @@ -768,7 +772,7 @@ impl SingleFileMessagePackBackingStore { // would drop them. let format = match fs::read(&self.path) { Ok(bytes) if bytes.is_empty() => STORE_FORMAT_V1, - Ok(bytes) => match store_header(&bytes) + Ok(bytes) => match store_header(&bytes, true) .map_err(|error| anyhow!("{}: {error:#}; the store is not rewritten", self.path.display()))? { Some(header) => readable_store_format(&header)?, @@ -2624,7 +2628,7 @@ fn readable_store_format(header: &str) -> Result<&'static str> { } fn decode_store_snapshot(bytes: &[u8]) -> Result> { - if let Ok(Some(header)) = store_header(bytes) { + if let Ok(Some(header)) = store_header(bytes, false) { readable_store_format(&header)?; } let snapshot: PersistedStoreSnapshot = @@ -4751,7 +4755,7 @@ mod tests { let keys: Vec<_> = envelopes.iter().map(|e| e.key.as_str()).collect(); assert_eq!(keys, vec!["alpha", "beta"]); store.push(&envelopes[0])?; - Ok(store_header(&std::fs::read(&path)?)?.expect("a store header")) + Ok(store_header(&std::fs::read(&path)?, true)?.expect("a store header")) } #[test] @@ -4837,6 +4841,41 @@ mod tests { } Ok(()) } + // A rewrite replaces only a file that is exactly one store: bytes after the array, a value that is not an array, or a + // first slot that is a string but not a header are refused and left be. An empty file and a legacy envelope array + // (first slot not a string, empty included) carry no header and are written v1. + #[test] + fn push_all_rewrites_only_a_file_that_is_exactly_one_store() -> Result<()> { + let source = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("../../tests/vectors/document-variants-c2a/v3-base.msgpack"); + let mut trailing = std::fs::read(source)?; + trailing.extend_from_slice(&[1, 2, 3]); + let refused: Vec<(&str, Vec)> = vec![ + ("trailing", trailing), + ("scalar", vec![0x01]), + ("map", vec![0x80]), + ("string first", rmp_serde::to_vec(&("hello", 1))?), + ]; + for (name, bytes) in refused { + let temp = tempfile::tempdir()?; + let path = temp.path().join("store.msgpack"); + std::fs::write(&path, &bytes)?; + let mut store = SingleFileMessagePackBackingStore::new(&path); + let result = store.push_all(&[snapshot_envelope("x", b"one")], PushAllOptions::default()); + assert!(result.is_err(), "{name} was accepted"); + assert_eq!(std::fs::read(&path)?, bytes, "{name} was rewritten"); + } + + for (name, bytes) in [("empty array", vec![0x90u8]), ("legacy", vec![0x91u8, 0x80])] { + let temp = tempfile::tempdir()?; + let path = temp.path().join("store.msgpack"); + std::fs::write(&path, &bytes)?; + let mut store = SingleFileMessagePackBackingStore::new(&path); + store.push_all(&[snapshot_envelope("x", b"one")], PushAllOptions::default())?; + assert_eq!(store_header(&std::fs::read(&path)?, true)?.as_deref(), Some(STORE_FORMAT_V1), "{name}"); + } + Ok(()) + } + #[test] fn a_rewrite_of_a_v1_store_stays_v1() -> Result<()> { assert_eq!( diff --git a/packages/cultcache-ts/src/single-file-messagepack-backing-store.ts b/packages/cultcache-ts/src/single-file-messagepack-backing-store.ts index 8c4b35c9..4aafea26 100644 --- a/packages/cultcache-ts/src/single-file-messagepack-backing-store.ts +++ b/packages/cultcache-ts/src/single-file-messagepack-backing-store.ts @@ -4,7 +4,7 @@ import { dirname, resolve } from "node:path"; import { decode, encode } from "@msgpack/msgpack"; import { z } from "zod"; -import { STORE_FORMAT_VERSION, type StoreFormat, isStoreSnapshot, requireV1RecordSlots } from "./store-format"; +import { STORE_FORMAT_VERSION, type StoreFormat, isStoreSnapshot, requireV1RecordSlots, storeHeader } from "./store-format"; import type { CacheBackingStore, CultCacheEnvelope, @@ -143,7 +143,8 @@ export class SingleFileMessagePackBackingStore implements CacheBackingStore { } // A flush of the whole store writes the header the file on disk carries, read now: a file marked for element ids - // stays marked, and one that is not (or is gone) is written unmarked. + // stays marked, and one that is not (or is gone, empty or legacy) is written unmarked. A file that is not exactly one + // store this runtime reads is refused and left as it is. await this.#readDiskFormat(); await this.#writeAll(entries); }); @@ -152,8 +153,7 @@ export class SingleFileMessagePackBackingStore implements CacheBackingStore { async #readDiskFormat(): Promise { try { const data = await readFile(this.filePath); - const decoded = data.length === 0 ? undefined : decode(data); - this.#format = isStoreSnapshot(decoded) ? decoded[0] : STORE_FORMAT_VERSION; + this.#format = (data.length === 0 ? undefined : storeHeader(decode(data))) ?? STORE_FORMAT_VERSION; } catch (error) { if ((error as NodeJS.ErrnoException).code !== "ENOENT") { throw error; diff --git a/packages/cultcache-ts/src/store-format.ts b/packages/cultcache-ts/src/store-format.ts index 02d74c1c..681f1eeb 100644 --- a/packages/cultcache-ts/src/store-format.ts +++ b/packages/cultcache-ts/src/store-format.ts @@ -31,6 +31,28 @@ export function isStoreSnapshot(decoded: unknown): decoded is [StoreFormat, ...u return true; } +/** + * The header of a decoded store file about to be rewritten: `undefined` for a legacy envelope array (an array whose first + * slot is not a string, empty included), the header otherwise. A value that is not an array, or whose first slot is a string + * but not a header this runtime reads, is refused, so a writer never overwrites a file it cannot see. Bytes after the array + * are refused by `decode` itself. + */ +export function storeHeader(decoded: unknown): StoreFormat | undefined { + if (!Array.isArray(decoded)) { + throw new Error("CultCache store is not a MessagePack array; it is not rewritten."); + } + + if (typeof decoded[0] !== "string") { + return undefined; + } + + if (!isStoreSnapshot(decoded)) { + throw new Error(`CultCache store's first slot is the string "${decoded[0]}", not a store header; it is not rewritten.`); + } + + return decoded[0]; +} + /** Refuses a persisted record with more slots than v1 defines, naming its key and schema id. */ export function requireV1RecordSlots(record: unknown[]): void { if (record.length > PERSISTED_RECORD_SLOTS) { diff --git a/packages/cultcache-ts/test/cult-cache.test.ts b/packages/cultcache-ts/test/cult-cache.test.ts index 3379b068..027f1f43 100644 --- a/packages/cultcache-ts/test/cult-cache.test.ts +++ b/packages/cultcache-ts/test/cult-cache.test.ts @@ -1581,3 +1581,36 @@ test("SingleFileMessagePackBackingStore pushAll refuses a store whose header it assert.deepEqual(await readFile(file), before, `${vector} was rewritten`); } }); + +test("SingleFileMessagePackBackingStore pushAll rewrites only a file that is exactly one store and leaves anything else untouched", async () => { + const dir = await mkdtemp(join(tmpdir(), "cultcache-pushall-whole-")); + const seed = await copyVector(dir, join(c2aVectors, "v3-base.msgpack"), "seed.msgpack"); + const whole = await readFile(seed); + const envelopes = await new SingleFileMessagePackBackingStore(seed).pullAll(); + const refused: Array<[string, Uint8Array]> = [ + ["truncated", whole.subarray(0, whole.length - 1)], + ["trailing", Buffer.concat([whole, Buffer.from([1, 2, 3])])], + ["scalar", Uint8Array.of(0x01)], + ["map", Uint8Array.of(0x80)], + ["string-first", encode(["hello", 1])], + ]; + for (const [name, bytes] of refused) { + const file = join(dir, `${name}.msgpack`); + await writeFile(file, bytes); + await assert.rejects(() => new SingleFileMessagePackBackingStore(file).pushAll(envelopes), Error, name); + assert.ok(Buffer.from(bytes).equals(await readFile(file)), `${name} was rewritten`); + } + + // An empty file and a legacy envelope array (first slot not a string, empty included) carry no header and are rewritten. + const accepted: Array<[string, Uint8Array]> = [ + ["empty", new Uint8Array()], + ["empty-array", Uint8Array.of(0x90)], + ["legacy", encode([{ key: "old", type: "old", payload: new Uint8Array(), storedAt: "then" }])], + ]; + for (const [name, bytes] of accepted) { + const file = join(dir, `${name}.msgpack`); + await writeFile(file, bytes); + await new SingleFileMessagePackBackingStore(file).pushAll(envelopes); + assert.equal((decode(await readFile(file)) as unknown[])[0], "cultcache.store.v1", name); + } +}); diff --git a/src/GameCult.Caching.MessagePack/CultDocumentMessagePackSerialization.cs b/src/GameCult.Caching.MessagePack/CultDocumentMessagePackSerialization.cs index da5fad89..b25627b4 100644 --- a/src/GameCult.Caching.MessagePack/CultDocumentMessagePackSerialization.cs +++ b/src/GameCult.Caching.MessagePack/CultDocumentMessagePackSerialization.cs @@ -243,31 +243,37 @@ public static class CultDocumentMessagePackSerialization /// /// The format header of a store file about to be replaced by a writer that does not read the store's records: null for - /// an empty file or a legacy envelope array, the header string otherwise. The header is read from the prefix, but a file - /// that is not one complete MessagePack value or whose header this runtime cannot read is refused, never taken for a - /// legacy file, so a writer cannot overwrite what it cannot see. + /// an empty file or a legacy envelope array (an array whose first slot is not a string), the header string otherwise. + /// A file is a store only when it is exactly one complete MessagePack array: a truncated file, bytes after the array, a + /// value that is not an array, or a first slot that is a string but not a header this runtime reads are all refused, + /// never taken for a legacy file, so a writer cannot overwrite what it cannot see. /// public static string? ReadStoreHeader(byte[] bytes) { if (bytes.Length == 0) return null; + var whole = new MessagePackReader(bytes); try { - var whole = new MessagePackReader(bytes); + if (whole.NextMessagePackType != MessagePackType.Array) + throw new NotSupportedException($"The store is a MessagePack {whole.NextMessagePackType}, not an array; it is not rewritten."); whole.Skip(); } catch (Exception ex) when (ex is EndOfStreamException or MessagePackSerializationException or InvalidOperationException) { - throw new NotSupportedException("The store is not one complete MessagePack value; it is not rewritten.", ex); + throw new NotSupportedException("The store is not one complete MessagePack array; it is not rewritten.", ex); } + if (!whole.End) + throw new NotSupportedException("The store has bytes after its MessagePack array; it is not rewritten."); + var reader = new MessagePackReader(bytes); - if (reader.NextMessagePackType != MessagePackType.Array || reader.ReadArrayHeader() == 0 || - reader.NextMessagePackType != MessagePackType.String) + if (reader.ReadArrayHeader() == 0 || reader.NextMessagePackType != MessagePackType.String) return null; - var header = reader.ReadString(); - if (header != null && header.StartsWith("cultcache.store.", StringComparison.Ordinal)) - RequireSingleFileFormat(new CultPersistedStoreSnapshot { FormatVersion = header }); + var header = reader.ReadString() ?? string.Empty; + if (!header.StartsWith("cultcache.store.", StringComparison.Ordinal)) + throw new NotSupportedException($"The store's first slot is the string '{header}', not a store header; it is not rewritten."); + RequireSingleFileFormat(new CultPersistedStoreSnapshot { FormatVersion = header }); return header; } diff --git a/tests/GameCult.Caching.Tests/ElementIdTests.cs b/tests/GameCult.Caching.Tests/ElementIdTests.cs index 9394c3d4..fb941453 100644 --- a/tests/GameCult.Caching.Tests/ElementIdTests.cs +++ b/tests/GameCult.Caching.Tests/ElementIdTests.cs @@ -785,25 +785,24 @@ namespace GameCult.Caching.Tests Assert.That(HeaderOf(path), Is.EqualTo("cultcache.store.v3"), "read without one, it cannot be seen, so it is marked"); } - // A flush replaces the whole file, but never one whose header it cannot read (Rust, TypeScript and Python refuse the same): - // it throws and the file is left as it was. An empty file and a legacy envelope array carry no header and are replaced. + // A flush replaces the whole file, but only a file that is exactly one store this runtime reads (Rust, TypeScript and Python + // refuse the same): anything else throws and is left as it was. An empty file and a legacy envelope array (an array whose + // first slot is not a string, empty included) carry no header and are replaced. [TestCase("truncated", false)] [TestCase("v9", false)] + [TestCase("trailing", false)] + [TestCase("scalar", false)] + [TestCase("map", false)] + [TestCase("string-first", false)] [TestCase("empty", true)] [TestCase("legacy", true)] - public void AFlushRefusesAFileWhoseHeaderItCannotReadAndLeavesItUntouched(string file, bool replaced) + [TestCase("empty-array", true)] + public void AFlushRefusesAFileThatIsNotExactlyOneStoreAndLeavesItUntouched(string file, bool replaced) { var path = PathOf($"flush-over-{file}.cc"); using var cache = Open(path); cache.Commit(batch => batch.Upsert(typeof(IdDeck), Deck("d"), new CultRecordKey("d"))); - var whole = File.ReadAllBytes(path); - var bytes = file switch - { - "truncated" => whole[..^1], - "v9" => MessagePackSerializer.Serialize(new object[] { "cultcache.store.v9", Array.Empty(), Array.Empty() }), - "empty" => Array.Empty(), - _ => MessagePackSerializer.Serialize(new object[] { new Dictionary { ["key"] = "old", ["type"] = "old", ["storedAt"] = "then" } }) - }; + var bytes = NotAStore(file, File.ReadAllBytes(path)); File.WriteAllBytes(path, bytes); if (replaced) @@ -817,6 +816,35 @@ namespace GameCult.Caching.Tests Assert.That(File.ReadAllBytes(path), Is.EqualTo(bytes), "the file was rewritten"); } + // An unconditional commit rewrites the file it read: a store with bytes after its array is refused, not rewritten. + [Test] + public void AnUnconditionalCommitRefusesAStoreWithBytesAfterItsArray() + { + var path = PathOf("commit-over-trailing.cc"); + using var cache = Open(path); + cache.Commit(batch => batch.Upsert(typeof(IdDeck), Deck("d"), new CultRecordKey("d"))); + var bytes = NotAStore("trailing", File.ReadAllBytes(path)); + File.WriteAllBytes(path, bytes); + + Assert.Throws(() => cache.Commit(batch => batch.Upsert(typeof(IdDeck), Deck("e"), new CultRecordKey("e")))); + Assert.That(File.ReadAllBytes(path), Is.EqualTo(bytes), "the file was rewritten"); + } + + // What a store file is not, built from a whole store file. + private static byte[] NotAStore(string file, byte[] whole) => file switch + { + "truncated" => whole[..^1], + "v9" => MessagePackSerializer.Serialize(new object[] { "cultcache.store.v9", Array.Empty(), Array.Empty() }), + "trailing" => whole.Concat(new byte[] { 1, 2, 3 }).ToArray(), + "scalar" => new byte[] { 0x01 }, + "map" => new byte[] { 0x80 }, + "string-first" => MessagePackSerializer.Serialize(new object[] { "hello", 1 }), + "empty" => Array.Empty(), + "empty-array" => new byte[] { 0x90 }, + "legacy" => MessagePackSerializer.Serialize(new object[] { new Dictionary { ["key"] = "old", ["type"] = "old", ["storedAt"] = "then" } }), + _ => throw new ArgumentOutOfRangeException(nameof(file)) + }; + [Test] public void ADirectoryStoreIsMarkedByTheIdsItWritesAndAnEmptyDeckDoesNotMarkIt() { diff --git a/tests/GameCult.Mesh.Tests/CultMeshSingleFileElementIdTests.cs b/tests/GameCult.Mesh.Tests/CultMeshSingleFileElementIdTests.cs index 4636059f..777993d5 100644 --- a/tests/GameCult.Mesh.Tests/CultMeshSingleFileElementIdTests.cs +++ b/tests/GameCult.Mesh.Tests/CultMeshSingleFileElementIdTests.cs @@ -106,14 +106,32 @@ public sealed class CultMeshSingleFileElementIdTests File.WriteAllBytes(truncated, whole[..^1]); var unknown = Path_("v9.cc"); File.WriteAllBytes(unknown, MessagePackSerializer.Serialize(new object[] { "cultcache.store.v9", Array.Empty(), Array.Empty() })); + // A file is a store only when it is exactly one MessagePack array whose first slot, if a string, is a store header. + var trailing = Path_("trailing.cc"); + File.WriteAllBytes(trailing, whole.Concat(new byte[] { 1, 2, 3 }).ToArray()); + var scalar = Path_("scalar.cc"); + File.WriteAllBytes(scalar, new byte[] { 0x01 }); + var map = Path_("map.cc"); + File.WriteAllBytes(map, new byte[] { 0x80 }); + var stringFirst = Path_("string-first.cc"); + File.WriteAllBytes(stringFirst, MessagePackSerializer.Serialize(new object[] { "hello", 1 })); - foreach (var path in new[] { truncated, unknown }) + foreach (var path in new[] { truncated, unknown, trailing, scalar, map, stringFirst }) { var before = File.ReadAllBytes(path); Assert.Throws(() => CultMesh.WriteSingleFileDocumentPayload(path, Key, schema, null, new byte[] { 0x90 }), path); File.ReadAllBytes(path).Should().Equal(before, path + " was rewritten"); } + // An empty file and a legacy envelope array (first slot not a string, empty included) carry no header and are written. + foreach (var (name, bytes) in new[] { ("empty.cc", Array.Empty()), ("empty-array.cc", new byte[] { 0x90 }), ("legacy.cc", new byte[] { 0x91, 0x80 }) }) + { + var path = Path_(name); + File.WriteAllBytes(path, bytes); + CultMesh.WriteSingleFileDocumentPayload(path, Key, schema, null, new byte[] { 0x90 }); + HeaderOf(path).Should().Be("cultcache.store.v1", name); + } + CultMesh.WriteSingleFileDocumentPayload(marked, Key, schema, null, new byte[] { 0x90 }); HeaderOf(marked).Should().Be("cultcache.store.v3", "a whole marked file is still written and stays marked"); }