From 1a660feea46296b67adb7ff9c71f487587da11d8 Mon Sep 17 00:00:00 2001 From: Libby Date: Thu, 12 Nov 2026 01:17:56 +0000 Subject: [PATCH 1/7] Red: a payload is one MessagePack value; bytes after it are refused (tests only) Co-Authored-By: Claude Sonnet 5.5 --- .../BytesAfterVectorTests.cs | 152 ++++++++++++++++++ .../PayloadDepthTests.cs | 21 ++- 2 files changed, 170 insertions(+), 3 deletions(-) create mode 100644 tests/GameCult.Caching.Tests/BytesAfterVectorTests.cs diff --git a/tests/GameCult.Caching.Tests/BytesAfterVectorTests.cs b/tests/GameCult.Caching.Tests/BytesAfterVectorTests.cs new file mode 100644 index 00000000..9eea0651 --- /dev/null +++ b/tests/GameCult.Caching.Tests/BytesAfterVectorTests.cs @@ -0,0 +1,152 @@ +#nullable enable +using System; +using System.IO; +using System.Linq; +using GameCult.Caching.MessagePack; +using MessagePack; +using NUnit.Framework; +using UnionSurface = GameCult.Caching.Tests.UnionParityVectorTests.UnionSurface; + +namespace GameCult.Caching.Tests +{ + // A record's payload is exactly one MessagePack value. The vectors are union-surface.msgpack followed by a tail; the C# + // reference is their only writer (VectorFiles, CULTCACHE_WRITE_VECTORS=1) and every other runtime is held to bytes-after-outcomes.msgpack. + public class BytesAfterVectorTests + { + private const string Vector = "union-parity"; + private const string Canary = "CANARY-bytes-after-7d1e"; + private const string Clause = "bytes after its first MessagePack value"; + private const int Limit = CultMessagePackSecurity.MaximumPayloadDepth; + + private static readonly CultDocumentRegistry Registry = CultDocumentRegistry.ForTypes(new[] { typeof(UnionSurface) }); + + private static byte[] Base => VectorFiles.Committed(Vector, "union-surface.msgpack"); + + private static byte[] DeepTail() + { + var text = System.Text.Encoding.ASCII.GetBytes(Canary); + return Enumerable.Repeat((byte)0x91, 300).Concat(new[] { (byte)(0xa0 | text.Length) }).Concat(text).ToArray(); + } + + // file, tail + private static (string File, byte[] Tail)[] Cases() => new (string, byte[])[] + { + ("bytes-after-nil.msgpack", new byte[] { 0xc0 }), + ("bytes-after-never-used.msgpack", new byte[] { 0xc1, 0xc1 }), + ("bytes-after-deep-tail.msgpack", DeepTail()), + ("bytes-after-second-document.msgpack", Base), + }; + + private static byte[] Raw(Action write) + { + var buffer = new System.Buffers.ArrayBufferWriter(); + var writer = new MessagePackWriter(buffer); + write(writer); + writer.Flush(); + return buffer.WrittenSpan.ToArray(); + } + + // The bounds row shape: [file, outcome, union, key|nil, detail]. + private static byte[] OutcomesFile() => Raw(w => + { + var cases = Cases(); + w.WriteArrayHeader(cases.Length); + foreach (var (file, _) in cases) + { + w.WriteArrayHeader(5); + w.Write(file); + w.Write("refused"); + w.Write("UnionSurface"); + w.WriteNil(); + w.Write("bytes-after"); + } + }); + + private static string Message(Action action) + { + try { action(); } + catch (Exception exception) + { + var message = ""; + for (var e = exception; e != null; e = e.InnerException) message += e.Message + "\n"; + return message; + } + + throw new AssertionException("the payload was accepted"); + } + + [Test] + public void VectorsMatchAndRefuse() + { + var baseBytes = Base; + foreach (var (file, tail) in Cases()) VectorFiles.Check(Vector, file, baseBytes.Concat(tail).ToArray()); + VectorFiles.Check(Vector, "bytes-after-outcomes.msgpack", OutcomesFile()); + + // From here only the committed files speak. + var reader = new MessagePackReader(VectorFiles.Committed(Vector, "bytes-after-outcomes.msgpack")); + Assert.That(reader.ReadArrayHeader(), Is.EqualTo(4)); + for (var i = 0; i < 4; i++) + { + Assert.That(reader.ReadArrayHeader(), Is.EqualTo(5)); + var file = reader.ReadString()!; + Assert.That(reader.ReadString(), Is.EqualTo("refused")); + Assert.That(reader.ReadString(), Is.EqualTo("UnionSurface")); + Assert.That(reader.TryReadNil(), Is.True); + Assert.That(reader.ReadString(), Is.EqualTo("bytes-after")); + + var payload = VectorFiles.Committed(Vector, file); + Assert.That(payload.Take(baseBytes.Length), Is.EqualTo(baseBytes), $"{file} begins with union-surface.msgpack"); + var count = payload.Length - baseBytes.Length; + var message = Message(() => CultDocumentMessagePackSerialization.DeserializeUntyped(typeof(UnionSurface), payload, Registry)); + Assert.That(message, Does.Contain(typeof(UnionSurface).FullName).And.Contain($"{count} {Clause}").And.Contain("refused on read"), file); + Assert.That(message, Does.Not.Contain(Canary).And.Not.Contain("the limit is"), file); + } + } + + [Test] + public void StoreLoadRefuses() + { + var work = Path.Combine(Path.GetTempPath(), $"cultlib-bytesafter-{Guid.NewGuid():N}"); + Directory.CreateDirectory(work); + try + { + var snapshot = CultDocumentMessagePackSerialization.DeserializeSnapshot(VectorFiles.Committed(Vector, "union-surface.cc")); + snapshot.Records[0].Payload = VectorFiles.Committed(Vector, "bytes-after-nil.msgpack"); + var scratch = Path.Combine(work, "tail.cc"); + File.WriteAllBytes(scratch, CultDocumentMessagePackSerialization.SerializeSnapshot(snapshot)); + var load = Message(() => CultCacheMessagePack.Create(scratch, new CultCacheOpenOptions { Registry = Registry }).Dispose()); + Assert.That(load, Does.Contain("surface").And.Contain($"1 {Clause}")); + } + finally + { + Directory.Delete(work, recursive: true); + } + } + + public sealed class TwoValueFormatter : global::MessagePack.Formatters.IMessagePackFormatter + { + public void Serialize(ref MessagePackWriter writer, TwoValues value, MessagePackSerializerOptions options) + { + writer.WriteArrayHeader(0); + writer.Write(7); + } + + public TwoValues Deserialize(ref MessagePackReader reader, MessagePackSerializerOptions options) { reader.Skip(); return new TwoValues(); } + } + + [CultDocument("tests.two_values", "tests.two_values.v1")] + [MessagePackFormatter(typeof(TwoValueFormatter))] + public sealed class TwoValues + { + public string Id = string.Empty; + } + + [Test] + public void OwnFormatterWritingTwoValuesRefusedOnWrite() + { + var registry = CultDocumentRegistry.ForTypes(new[] { typeof(TwoValues) }); + var message = Message(() => CultDocumentMessagePackSerialization.SerializeUntyped(new TwoValues(), typeof(TwoValues), registry)); + Assert.That(message, Does.Contain(typeof(TwoValues).FullName).And.Contain($"1 {Clause}").And.Contain("refused on write")); + } + } +} diff --git a/tests/GameCult.Caching.Tests/PayloadDepthTests.cs b/tests/GameCult.Caching.Tests/PayloadDepthTests.cs index 91aa9a6f..1d3db1d0 100644 --- a/tests/GameCult.Caching.Tests/PayloadDepthTests.cs +++ b/tests/GameCult.Caching.Tests/PayloadDepthTests.cs @@ -148,10 +148,25 @@ namespace GameCult.Caching.Tests } [Test] - public void OnlyTheFirstValueIsThePayload() + public void OnlyOneValueIsAPayload() { - Accepts(Cat(Bytes(0x90), Chain(Limit + 10)), "bytes after the first value are not walked"); - Accepts(Cat(Bytes(0x01), Chain(Limit + 10)), "nor after a scalar"); + var tails = new[] + { + (Cat(Bytes(0x90), Chain(Limit + 10)), Chain(Limit + 10).Length), + (Cat(Bytes(0x01), Chain(Limit + 10)), Chain(Limit + 10).Length), + (Bytes(0x90, 0xc1, 0xc1), 2), + }; + foreach (var (payload, count) in tails) + foreach (var direction in new[] { PayloadDirection.Read, PayloadDirection.Write }) + { + var message = Assert.Throws(() => CultPayloadDepth.Check(payload, typeof(PayloadDepthTests), direction))!.Message; + var word = direction == PayloadDirection.Read ? "read" : "write"; + Assert.That(message, Is.EqualTo($"{typeof(PayloadDepthTests).FullName} payload has {count} bytes after its first MessagePack value (refused on {word}).")); + } + + // The tail is not walked: a tail nested past the limit is refused as bytes after the value, not as depth. + var deep = Assert.Throws(() => CultPayloadDepth.Check(Cat(Bytes(0x90), Chain(Limit + 10)), typeof(object), PayloadDirection.Read))!; + Assert.That(deep.Message, Does.Not.Contain("the limit is")); } } } -- 2.47.3 From 7d3e9c77b3205c369b0d354bb0808ea8abc9b05c Mon Sep 17 00:00:00 2001 From: Libby Date: Thu, 12 Nov 2026 01:18:59 +0000 Subject: [PATCH 2/7] Refuse a document payload with bytes after its first MessagePack value; bytes-after vectors, contract, changelog Co-Authored-By: Claude Sonnet 5.5 --- .../CultPayloadDepth.cs | 13 ++++++++--- .../Contracts/cultcache-persistence-format.md | 2 ++ .../BytesAfterVectorTests.cs | 21 ++++++++---------- tests/vectors/union-parity/README.md | 17 ++++++++++++++ .../bytes-after-deep-tail.msgpack | Bin 0 -> 601 bytes .../bytes-after-never-used.msgpack | Bin 0 -> 279 bytes .../union-parity/bytes-after-nil.msgpack | Bin 0 -> 278 bytes .../union-parity/bytes-after-outcomes.msgpack | 1 + .../bytes-after-second-document.msgpack | Bin 0 -> 554 bytes unity/org.gamecult.cultlib/CHANGELOG.md | 1 + 10 files changed, 40 insertions(+), 15 deletions(-) create mode 100644 tests/vectors/union-parity/bytes-after-deep-tail.msgpack create mode 100644 tests/vectors/union-parity/bytes-after-never-used.msgpack create mode 100644 tests/vectors/union-parity/bytes-after-nil.msgpack create mode 100644 tests/vectors/union-parity/bytes-after-outcomes.msgpack create mode 100644 tests/vectors/union-parity/bytes-after-second-document.msgpack diff --git a/src/GameCult.Caching.MessagePack/CultPayloadDepth.cs b/src/GameCult.Caching.MessagePack/CultPayloadDepth.cs index 258663e8..6179f8be 100644 --- a/src/GameCult.Caching.MessagePack/CultPayloadDepth.cs +++ b/src/GameCult.Caching.MessagePack/CultPayloadDepth.cs @@ -7,11 +7,12 @@ namespace GameCult.Caching.MessagePack; internal enum PayloadDirection { Read, Write } /// -/// The one payload nesting count. Walks the MessagePack headers of a document payload iteratively, counting every array and +/// The one walk of a payload's shape: its depth and its one value. Walks the MessagePack headers of a document payload iteratively, counting every array and /// map (read by a formatter or skipped past it; the outermost array is depth 1) and refusing the first container past /// . Scalars are skipped by length, nothing is allocated per /// element, and the count stack is bounded by the limit. A refusal names the type, the depth reached and the limit, -/// never payload bytes. A payload that ends early is the deserializer's to refuse. +/// never payload bytes. A payload that ends early is the deserializer's to refuse; bytes after the first value are refused +/// with their count and never walked. /// internal static class CultPayloadDepth { @@ -73,7 +74,13 @@ internal static class CultPayloadDepth // One element is complete; close every container it was the last element of. while (depth > 0 && --remaining[depth - 1] == 0) depth--; - if (depth == 0) return; + if (depth == 0) + { + if (position < bytes.Length) + throw new MessagePackSerializationException( + $"{type.FullName} payload has {bytes.Length - position} bytes after its first MessagePack value (refused on {(direction == PayloadDirection.Read ? "read" : "write")})."); + return; + } } } diff --git a/src/GameCult.Caching/Contracts/cultcache-persistence-format.md b/src/GameCult.Caching/Contracts/cultcache-persistence-format.md index 5ac6c3b5..0f3c6009 100644 --- a/src/GameCult.Caching/Contracts/cultcache-persistence-format.md +++ b/src/GameCult.Caching/Contracts/cultcache-persistence-format.md @@ -403,6 +403,8 @@ runtime enforces the same bound on read and on write, and the refusal names the the depth and the limit, never payload bytes. The vectors are in `tests/vectors/union-parity` (`bounds-outcomes.msgpack`). +A record's payload is exactly one MessagePack value: every runtime refuses a payload with bytes after its first value wherever it reads a payload under the nesting bound, and on write where a write takes bytes the library did not encode itself (a C# type's own formatter, a Rust raw envelope), without walking those bytes, so a tail past the limit is refused as bytes after the value and not as depth; the refusal names the type and the count of bytes after the value, never the bytes (vectors: `tests/vectors/union-parity/bytes-after-outcomes.msgpack`). + ## Variant Records A variant record is a record whose document is a base record's document with some diff --git a/tests/GameCult.Caching.Tests/BytesAfterVectorTests.cs b/tests/GameCult.Caching.Tests/BytesAfterVectorTests.cs index 9eea0651..b61487d5 100644 --- a/tests/GameCult.Caching.Tests/BytesAfterVectorTests.cs +++ b/tests/GameCult.Caching.Tests/BytesAfterVectorTests.cs @@ -37,18 +37,11 @@ namespace GameCult.Caching.Tests ("bytes-after-second-document.msgpack", Base), }; - private static byte[] Raw(Action write) + // The bounds row shape: [file, outcome, union, key|nil, detail]. + private static byte[] OutcomesFile() { var buffer = new System.Buffers.ArrayBufferWriter(); - var writer = new MessagePackWriter(buffer); - write(writer); - writer.Flush(); - return buffer.WrittenSpan.ToArray(); - } - - // The bounds row shape: [file, outcome, union, key|nil, detail]. - private static byte[] OutcomesFile() => Raw(w => - { + var w = new MessagePackWriter(buffer); var cases = Cases(); w.WriteArrayHeader(cases.Length); foreach (var (file, _) in cases) @@ -60,7 +53,10 @@ namespace GameCult.Caching.Tests w.WriteNil(); w.Write("bytes-after"); } - }); + + w.Flush(); + return buffer.WrittenSpan.ToArray(); + } private static string Message(Action action) { @@ -135,10 +131,11 @@ namespace GameCult.Caching.Tests } [CultDocument("tests.two_values", "tests.two_values.v1")] + [MessagePackObject] [MessagePackFormatter(typeof(TwoValueFormatter))] public sealed class TwoValues { - public string Id = string.Empty; + [Key(0)] [CultName] public string Id = string.Empty; } [Test] diff --git a/tests/vectors/union-parity/README.md b/tests/vectors/union-parity/README.md index fbf9c6c1..b2fe1df0 100644 --- a/tests/vectors/union-parity/README.md +++ b/tests/vectors/union-parity/README.md @@ -170,3 +170,20 @@ arrays around a canary string, which no formatter reads. | `depth-skipped-past-limit.msgpack` | 257 | refused, depth | refuse on read though the typed depth is under 10 | A walk of the payload must not recurse: 100000 nested arrays in an extra slot are refused on a 256 KiB stack. + +## Bytes after the value (bytes-after-outcomes.msgpack) + +A record's payload is exactly one MessagePack value. Each file is `union-surface.msgpack` followed by a tail the first value does not own; the C# reference writes them and every other runtime is held to the same refusal. + +| File | Tail after the value | outcome, detail | +|---|---|---| +| `bytes-after-nil.msgpack` | `c0` (1 byte) | refused, bytes-after | +| `bytes-after-never-used.msgpack` | `c1 c1` (2 bytes, the never-used code) | refused, bytes-after | +| `bytes-after-deep-tail.msgpack` | 300 nested fixarray(1) around a canary string (324 bytes), nested past the limit | refused, bytes-after | +| `bytes-after-second-document.msgpack` | a second copy of `union-surface.msgpack` | refused, bytes-after | + +`bytes-after-outcomes.msgpack` is an array of `[file, "refused", "UnionSurface", nil, "bytes-after"]` rows, the row shape of `bounds-outcomes.msgpack` but in its own file, so the bounds walkers (which assert 12 rows) are unchanged. + +Must: refuse on read, naming the type and the count of bytes after the first value (`1`, `2`, the tail length) in a clause that says "bytes after its first MessagePack value"; never echo the canary; never report the depth refusal ("the limit is") for any of them, because the tail is not walked; and a single-file store holding `bytes-after-nil.msgpack` as a record payload refuses on open. + +What a runtime walks: only the first value of the payload, to find where it ends (a leaf counts as complete at its own length, a container when its last child completes). It compares that end with the payload length; a payload that ends early is not bytes after the value and stays the decoder's to refuse. It never walks the tail, so the tail's own shape (a stray `c1`, 300 levels) is never judged. diff --git a/tests/vectors/union-parity/bytes-after-deep-tail.msgpack b/tests/vectors/union-parity/bytes-after-deep-tail.msgpack new file mode 100644 index 0000000000000000000000000000000000000000..2dcc82f9cf5235956c37ed6c9e429bfca556b629 GIT binary patch literal 601 zcmbQfytuR|EipNjc@o2%6!aU5${fLW866AE~O}j2j2oI3a+MgjvPa?*y267{?Fp%UW~* literal 0 HcmV?d00001 diff --git a/tests/vectors/union-parity/bytes-after-nil.msgpack b/tests/vectors/union-parity/bytes-after-nil.msgpack new file mode 100644 index 0000000000000000000000000000000000000000..3bbdaa3321d1d4def22b1d20c9bb3a044b211d1c GIT binary patch literal 278 zcmZ9GK@P$o5Jk0W+<2D)-Fcf4&^Cc!D2*ON2NSm%(QZ5hN0Wsy;H+kT{(pb2V?Wp} za5=*XzR7Pm@i;96`fR0A1}mT27zS?iz6=Y3WmwSr5f_&%DA+jIZe7<9AE4Y{nWUjt w#>EmV*{Ou^<_p=xn@`?wfX-zlK+_w`Twqw8^SeogR|{H8yGm==NwD)6Ka^HkvH$=8 literal 0 HcmV?d00001 diff --git a/tests/vectors/union-parity/bytes-after-outcomes.msgpack b/tests/vectors/union-parity/bytes-after-outcomes.msgpack new file mode 100644 index 00000000..8237924e --- /dev/null +++ b/tests/vectors/union-parity/bytes-after-outcomes.msgpack @@ -0,0 +1 @@ +”•·bytes-after-nil.msgpack§refused¬UnionSurfaceÀ«bytes-after•¾bytes-after-never-used.msgpack§refused¬UnionSurfaceÀ«bytes-after•½bytes-after-deep-tail.msgpack§refused¬UnionSurfaceÀ«bytes-after•Ù#bytes-after-second-document.msgpack§refused¬UnionSurfaceÀ«bytes-after \ No newline at end of file diff --git a/tests/vectors/union-parity/bytes-after-second-document.msgpack b/tests/vectors/union-parity/bytes-after-second-document.msgpack new file mode 100644 index 0000000000000000000000000000000000000000..80d136fa80a0680a2d93d8a5b7a42a3adbc660e1 GIT binary patch literal 554 zcmbQfytuR|EipNjc@o2GE Dx2>_` literal 0 HcmV?d00001 diff --git a/unity/org.gamecult.cultlib/CHANGELOG.md b/unity/org.gamecult.cultlib/CHANGELOG.md index a2a1efae..4ecb63d0 100644 --- a/unity/org.gamecult.cultlib/CHANGELOG.md +++ b/unity/org.gamecult.cultlib/CHANGELOG.md @@ -6,6 +6,7 @@ All notable changes to this package are documented in this file. ### Breaking +- A document payload with bytes after its first MessagePack value is refused on read and on write (`MessagePackSerializationException` naming the type and the count of bytes after the value, never the bytes); the tail is not walked, so a tail nested past the limit is refused as bytes after the value, not as depth. Before, the tail was ignored on read and dropped on the next write. A payload that ends early is still the deserializer's to refuse. - A single-file store holding two records under one key is unreadable: the reader refuses it with `CultStoreUnreadableException` naming the key (keys compare by ordinal code units, no case folding or normalisation), so a cache, `ReadStoreSnapshot` and a flush refuse the file and leave it as it is. Before, the later record silently replaced the earlier. A directory store reads its manifest through the same reader and refuses a duplicate key the same way. Repair the duplicate with the runtime that wrote it before reading the store here. - A record whose payload carries a slot past the slots its reading type declares, at any depth (the record, a nested object, a list element, a union arm or a unit arm), is loaded read-only: it is served, and every staged write or removal of its key (`Upsert`, `UpsertVariant`, `Remove`, a batch, and `Flatten` of a variant) is refused with `CultSchemaConflictException` before anything is staged, and the store keeps the record byte for byte. Before, the extra slots were skipped on read and dropped on the next write. Declaring an id compatible consents only to depth-0 slots the schema comparison reports ignored under that id; any other extra slot keeps the record read-only whatever is declared, including under the reader's own id. A variant overriding a slot its type does not have is read-only too (it used to be shed on flush). Write such a record from the build that declares those slots. Missing trailing slots still default and stay writable. - `Flatten` (and `FlattenAsync`) of a variant whose base chain holds a record its store does not own (a record held read-only for extra slots, or undeclared drift that drops slots) is refused with `CultSchemaConflictException` naming that base record, and the store is left as it was. Before, the flatten wrote the base's decoded view into a plain record and the base's unread data was lost for that variant. Writing and removing the variant itself are still accepted: they persist only its delta. -- 2.47.3 From 1ef05106ae32a0568479d990c111b9f3a345ccc2 Mon Sep 17 00:00:00 2001 From: Libby Date: Thu, 12 Nov 2026 01:39:58 +0000 Subject: [PATCH 3/7] Keep the CDN manifest contract: DeserializeBoundedUntyped reads under the depth bound alone; Check stays the record-payload owner Co-Authored-By: Claude Sonnet 5.5 --- .../CultDocumentMessagePackSerialization.cs | 14 ++++++ .../CultPayloadDepth.cs | 43 +++++++++++-------- .../Contracts/cultcache-persistence-format.md | 2 +- .../BytesAfterVectorTests.cs | 10 +++++ .../CultMeshContentVectorTests.cs | 4 +- tests/vectors/union-parity/README.md | 2 + unity/org.gamecult.cultlib/CHANGELOG.md | 2 +- 7 files changed, 56 insertions(+), 21 deletions(-) diff --git a/src/GameCult.Caching.MessagePack/CultDocumentMessagePackSerialization.cs b/src/GameCult.Caching.MessagePack/CultDocumentMessagePackSerialization.cs index 15b70c9c..4665879b 100644 --- a/src/GameCult.Caching.MessagePack/CultDocumentMessagePackSerialization.cs +++ b/src/GameCult.Caching.MessagePack/CultDocumentMessagePackSerialization.cs @@ -227,6 +227,20 @@ public static class CultDocumentMessagePackSerialization return new CultDecoded(document, scope.Finding()); } + /// + /// Reads a document payload that is not a stored record under the nesting bound alone: bytes after the first value are + /// ignored, as the CultMesh CDN manifest contract has them (contracts/cultmesh/content-acceptance-vectors.json). A record + /// payload goes through , which refuses them. + /// + public static object DeserializeBoundedUntyped(Type type, byte[] payload, CultDocumentRegistry registry) + { + if (registry == null) throw new ArgumentNullException(nameof(registry)); + registry.GetRequired(type); + CultPayloadDepth.CheckDepth(payload, type, PayloadDirection.Read); + return MessagePackSerializer.Deserialize(type, payload, OptionsForDocument(type)) + ?? throw new InvalidOperationException($"MessagePack returned null for Cult document type {type.FullName}."); + } + /// /// Replaces slots of a serialized document payload (a MessagePack array indexed by slot) with already-encoded member /// values, leaving every other slot byte for byte as it was. A slot past the end extends the array with nil. diff --git a/src/GameCult.Caching.MessagePack/CultPayloadDepth.cs b/src/GameCult.Caching.MessagePack/CultPayloadDepth.cs index 6179f8be..4daa0d46 100644 --- a/src/GameCult.Caching.MessagePack/CultPayloadDepth.cs +++ b/src/GameCult.Caching.MessagePack/CultPayloadDepth.cs @@ -16,7 +16,20 @@ internal enum PayloadDirection { Read, Write } /// internal static class CultPayloadDepth { + /// A record's payload: the depth bound, and exactly one value. Bytes after the first value are refused, never walked. public static void Check(ReadOnlySpan bytes, Type type, PayloadDirection direction) + { + var end = Walk(bytes, type, direction); + if (end < bytes.Length) + throw new MessagePackSerializationException( + $"{type.FullName} payload has {bytes.Length - end} bytes after its first MessagePack value (refused on {(direction == PayloadDirection.Read ? "read" : "write")})."); + } + + /// The depth bound alone, for a reader whose contract tolerates bytes after the value (the CDN manifest vectors). + public static void CheckDepth(ReadOnlySpan bytes, Type type, PayloadDirection direction) => Walk(bytes, type, direction); + + // The one walk: refuses past the depth limit and returns where the first value ends (the payload's length when it ends early). + private static long Walk(ReadOnlySpan bytes, Type type, PayloadDirection direction) { const int limit = CultMessagePackSecurity.MaximumPayloadDepth; Span remaining = stackalloc long[limit]; @@ -35,12 +48,12 @@ internal static class CultPayloadDepth case <= 0x8f: container = true; header = 1; children = 2L * (code & 0x0f); break; case <= 0x9f: container = true; header = 1; children = code & 0x0f; break; case <= 0xbf: skip = 1 + (code & 0x1f); break; - case 0xc4 or 0xd9: if (!Length(bytes, position, 1, out var l8)) return; skip = 2 + l8; break; - case 0xc5 or 0xda: if (!Length(bytes, position, 2, out var l16)) return; skip = 3 + l16; break; - case 0xc6 or 0xdb: if (!Length(bytes, position, 4, out var l32)) return; skip = 5 + l32; break; - case 0xc7: if (!Length(bytes, position, 1, out var e8)) return; skip = 3 + e8; break; - case 0xc8: if (!Length(bytes, position, 2, out var e16)) return; skip = 4 + e16; break; - case 0xc9: if (!Length(bytes, position, 4, out var e32)) return; skip = 6 + e32; break; + case 0xc4 or 0xd9: if (!Length(bytes, position, 1, out var l8)) return bytes.Length; skip = 2 + l8; break; + case 0xc5 or 0xda: if (!Length(bytes, position, 2, out var l16)) return bytes.Length; skip = 3 + l16; break; + case 0xc6 or 0xdb: if (!Length(bytes, position, 4, out var l32)) return bytes.Length; skip = 5 + l32; break; + case 0xc7: if (!Length(bytes, position, 1, out var e8)) return bytes.Length; skip = 3 + e8; break; + case 0xc8: if (!Length(bytes, position, 2, out var e16)) return bytes.Length; skip = 4 + e16; break; + case 0xc9: if (!Length(bytes, position, 4, out var e32)) return bytes.Length; skip = 6 + e32; break; case 0xca or 0xce or 0xd2: skip = 5; break; case 0xcb or 0xcf or 0xd3: skip = 9; break; case 0xcc or 0xd0: skip = 2; break; @@ -49,10 +62,10 @@ internal static class CultPayloadDepth case 0xd6: skip = 6; break; case 0xd7: skip = 10; break; case 0xd8: skip = 18; break; - case 0xdc: if (!Length(bytes, position, 2, out var a16)) return; container = true; header = 3; children = a16; break; - case 0xdd: if (!Length(bytes, position, 4, out var a32)) return; container = true; header = 5; children = a32; break; - case 0xde: if (!Length(bytes, position, 2, out var m16)) return; container = true; header = 3; children = 2 * m16; break; - default: if (!Length(bytes, position, 4, out var m32)) return; container = true; header = 5; children = 2 * m32; break; + case 0xdc: if (!Length(bytes, position, 2, out var a16)) return bytes.Length; container = true; header = 3; children = a16; break; + case 0xdd: if (!Length(bytes, position, 4, out var a32)) return bytes.Length; container = true; header = 5; children = a32; break; + case 0xde: if (!Length(bytes, position, 2, out var m16)) return bytes.Length; container = true; header = 3; children = 2 * m16; break; + default: if (!Length(bytes, position, 4, out var m32)) return bytes.Length; container = true; header = 5; children = 2 * m32; break; } if (container) @@ -74,14 +87,10 @@ internal static class CultPayloadDepth // One element is complete; close every container it was the last element of. while (depth > 0 && --remaining[depth - 1] == 0) depth--; - if (depth == 0) - { - if (position < bytes.Length) - throw new MessagePackSerializationException( - $"{type.FullName} payload has {bytes.Length - position} bytes after its first MessagePack value (refused on {(direction == PayloadDirection.Read ? "read" : "write")})."); - return; - } + if (depth == 0) return position; } + + return bytes.Length; } // The big-endian length that follows the code byte, when the payload is long enough to hold it. diff --git a/src/GameCult.Caching/Contracts/cultcache-persistence-format.md b/src/GameCult.Caching/Contracts/cultcache-persistence-format.md index 0f3c6009..db3fd932 100644 --- a/src/GameCult.Caching/Contracts/cultcache-persistence-format.md +++ b/src/GameCult.Caching/Contracts/cultcache-persistence-format.md @@ -403,7 +403,7 @@ runtime enforces the same bound on read and on write, and the refusal names the the depth and the limit, never payload bytes. The vectors are in `tests/vectors/union-parity` (`bounds-outcomes.msgpack`). -A record's payload is exactly one MessagePack value: every runtime refuses a payload with bytes after its first value wherever it reads a payload under the nesting bound, and on write where a write takes bytes the library did not encode itself (a C# type's own formatter, a Rust raw envelope), without walking those bytes, so a tail past the limit is refused as bytes after the value and not as depth; the refusal names the type and the count of bytes after the value, never the bytes (vectors: `tests/vectors/union-parity/bytes-after-outcomes.msgpack`). +A record's payload is exactly one MessagePack value: every runtime refuses a payload with bytes after its first value wherever it reads a payload under the nesting bound, and on write where a write takes bytes the library did not encode itself (a C# type's own formatter, a Rust raw envelope), without walking those bytes, so a tail past the limit is refused as bytes after the value and not as depth; the refusal names the type and the count of bytes after the value, never the bytes (vectors: `tests/vectors/union-parity/bytes-after-outcomes.msgpack`). The CultMesh CDN manifest is a different contract: its wire bytes may carry bytes after the value (`contracts/cultmesh/content-acceptance-vectors.json`), so its reader keeps the nesting bound through `DeserializeBoundedUntyped` and does not refuse them. ## Variant Records diff --git a/tests/GameCult.Caching.Tests/BytesAfterVectorTests.cs b/tests/GameCult.Caching.Tests/BytesAfterVectorTests.cs index b61487d5..1c6e1355 100644 --- a/tests/GameCult.Caching.Tests/BytesAfterVectorTests.cs +++ b/tests/GameCult.Caching.Tests/BytesAfterVectorTests.cs @@ -99,6 +99,16 @@ namespace GameCult.Caching.Tests } } + [Test] + public void TheBoundedEntryKeepsTheDepthBoundAndToleratesATail() + { + foreach (var (file, _) in Cases()) + Assert.That(CultDocumentMessagePackSerialization.DeserializeBoundedUntyped(typeof(UnionSurface), VectorFiles.Committed(Vector, file), Registry), Is.TypeOf(), file); + + var deep = VectorFiles.Committed(Vector, "depth-past-limit.msgpack"); + Assert.That(Message(() => CultDocumentMessagePackSerialization.DeserializeBoundedUntyped(typeof(UnionSurface), deep, Registry)), Does.Contain($"the limit is {Limit}")); + } + [Test] public void StoreLoadRefuses() { diff --git a/tests/GameCult.Mesh.Tests/CultMeshContentVectorTests.cs b/tests/GameCult.Mesh.Tests/CultMeshContentVectorTests.cs index da764cc5..77407520 100644 --- a/tests/GameCult.Mesh.Tests/CultMeshContentVectorTests.cs +++ b/tests/GameCult.Mesh.Tests/CultMeshContentVectorTests.cs @@ -462,9 +462,9 @@ public sealed class CultMeshContentVectorTests private static byte[] Reencode(CultMeshContentChunkResponseMessage message) => CultNetSchemaMessageSerialization.Serialize(message); - /// The one reader of manifest bytes: the document reader production stores and reads manifests with (CultPayloadDepth 256 included). + /// The one reader of manifest bytes: the bounded document reader (CultPayloadDepth 256 included): the CDN manifest contract tolerates bytes after the value, a record payload does not. private static CultMeshCdnArtifactManifest ReadManifest(byte[] bytes) => - (CultMeshCdnArtifactManifest)CultDocumentMessagePackSerialization.DeserializeUntyped(typeof(CultMeshCdnArtifactManifest), bytes); + (CultMeshCdnArtifactManifest)CultDocumentMessagePackSerialization.DeserializeBoundedUntyped(typeof(CultMeshCdnArtifactManifest), bytes, CultDocumentRegistry.Shared); private static byte[] Reencode(CultMeshCdnArtifactManifest manifest) => CultDocumentMessagePackSerialization.SerializeUntyped(manifest, typeof(CultMeshCdnArtifactManifest)); diff --git a/tests/vectors/union-parity/README.md b/tests/vectors/union-parity/README.md index b2fe1df0..9468d64d 100644 --- a/tests/vectors/union-parity/README.md +++ b/tests/vectors/union-parity/README.md @@ -187,3 +187,5 @@ A record's payload is exactly one MessagePack value. Each file is `union-surface Must: refuse on read, naming the type and the count of bytes after the first value (`1`, `2`, the tail length) in a clause that says "bytes after its first MessagePack value"; never echo the canary; never report the depth refusal ("the limit is") for any of them, because the tail is not walked; and a single-file store holding `bytes-after-nil.msgpack` as a record payload refuses on open. What a runtime walks: only the first value of the payload, to find where it ends (a leaf counts as complete at its own length, a container when its last child completes). It compares that end with the payload length; a payload that ends early is not bytes after the value and stays the decoder's to refuse. It never walks the tail, so the tail's own shape (a stray `c1`, 300 levels) is never judged. + +The CultMesh CDN manifest is not covered by these vectors: its contract (`contracts/cultmesh/content-acceptance-vectors.json`) accepts bytes after the value, and its reader keeps the depth bound through `DeserializeBoundedUntyped`. diff --git a/unity/org.gamecult.cultlib/CHANGELOG.md b/unity/org.gamecult.cultlib/CHANGELOG.md index 4ecb63d0..ff74007f 100644 --- a/unity/org.gamecult.cultlib/CHANGELOG.md +++ b/unity/org.gamecult.cultlib/CHANGELOG.md @@ -6,7 +6,7 @@ All notable changes to this package are documented in this file. ### Breaking -- A document payload with bytes after its first MessagePack value is refused on read and on write (`MessagePackSerializationException` naming the type and the count of bytes after the value, never the bytes); the tail is not walked, so a tail nested past the limit is refused as bytes after the value, not as depth. Before, the tail was ignored on read and dropped on the next write. A payload that ends early is still the deserializer's to refuse. +- A document payload with bytes after its first MessagePack value is refused on read and on write (`MessagePackSerializationException` naming the type and the count of bytes after the value, never the bytes); the tail is not walked, so a tail nested past the limit is refused as bytes after the value, not as depth. Before, the tail was ignored on read and dropped on the next write. A payload that ends early is still the deserializer's to refuse. The CultMesh CDN manifest contract still accepts bytes after the value; `CultDocumentMessagePackSerialization.DeserializeBoundedUntyped` (new) reads under the nesting bound alone for it. - A single-file store holding two records under one key is unreadable: the reader refuses it with `CultStoreUnreadableException` naming the key (keys compare by ordinal code units, no case folding or normalisation), so a cache, `ReadStoreSnapshot` and a flush refuse the file and leave it as it is. Before, the later record silently replaced the earlier. A directory store reads its manifest through the same reader and refuses a duplicate key the same way. Repair the duplicate with the runtime that wrote it before reading the store here. - A record whose payload carries a slot past the slots its reading type declares, at any depth (the record, a nested object, a list element, a union arm or a unit arm), is loaded read-only: it is served, and every staged write or removal of its key (`Upsert`, `UpsertVariant`, `Remove`, a batch, and `Flatten` of a variant) is refused with `CultSchemaConflictException` before anything is staged, and the store keeps the record byte for byte. Before, the extra slots were skipped on read and dropped on the next write. Declaring an id compatible consents only to depth-0 slots the schema comparison reports ignored under that id; any other extra slot keeps the record read-only whatever is declared, including under the reader's own id. A variant overriding a slot its type does not have is read-only too (it used to be shed on flush). Write such a record from the build that declares those slots. Missing trailing slots still default and stay writable. - `Flatten` (and `FlattenAsync`) of a variant whose base chain holds a record its store does not own (a record held read-only for extra slots, or undeclared drift that drops slots) is refused with `CultSchemaConflictException` naming that base record, and the store is left as it was. Before, the flatten wrote the base's decoded view into a plain record and the base's unread data was lost for that variant. Writing and removing the variant itself are still accepted: they persist only its delta. -- 2.47.3 From 2dbb41dee792664a0dece7371f78d2cebe9c645e Mon Sep 17 00:00:00 2001 From: Libby Date: Thu, 12 Nov 2026 02:06:56 +0000 Subject: [PATCH 4/7] No manifest exemption: one entry decides one value (ruling trailing-bytes-refuse); mesh reads through the production binding; per-path tail tests; TS decodeManifest refuses tails Co-Authored-By: Claude Sonnet 5.5 --- .../cultmesh/content-acceptance-vectors.json | 18 +++--- packages/cultnet-ts/CHANGELOG.md | 4 ++ packages/cultnet-ts/README.md | 4 +- packages/cultnet-ts/src/content.ts | 4 +- .../CultDocumentMessagePackSerialization.cs | 14 ----- .../CultPayloadDepth.cs | 43 +++++-------- .../Contracts/cultcache-persistence-format.md | 2 +- .../BytesAfterVectorTests.cs | 63 +++++++++++++++---- .../BytesAfterPathTests.cs | 61 ++++++++++++++++++ .../CultMeshContentVectorTests.cs | 6 +- .../BytesAfterPathTests.cs | 62 ++++++++++++++++++ tests/vectors/union-parity/README.md | 2 +- unity/org.gamecult.cultlib/CHANGELOG.md | 2 +- 13 files changed, 216 insertions(+), 69 deletions(-) create mode 100644 tests/GameCult.Mesh.Tests/BytesAfterPathTests.cs create mode 100644 tests/GameCult.Networking.Tests/BytesAfterPathTests.cs diff --git a/contracts/cultmesh/content-acceptance-vectors.json b/contracts/cultmesh/content-acceptance-vectors.json index d9868a63..b4d92cde 100644 --- a/contracts/cultmesh/content-acceptance-vectors.json +++ b/contracts/cultmesh/content-acceptance-vectors.json @@ -899,23 +899,23 @@ { "label": "trailing_nil", "inputBase64": "mqNhL2KlYXNzZXShMdlAZTNiMGM0NDI5OGZjMWMxNDlhZmJmNGM4OTk2ZmI5MjQyN2FlNDFlNDY0OWI5MzRjYTQ5NTk5MWI3ODUyYjg1NQCvYXBwbGljYXRpb24veC10oXSRlNlAZTNiMGM0NDI5OGZjMWMxNDlhZmJmNGM4OTk2ZmI5MjQyN2FlNDFlNDY0OWI5MzRjYTQ5NTk5MWI3ODUyYjg1NQAAoWuRoXiBoWuhdsA=", - "accepted": true, - "reencodedBase64": "mqNhL2KlYXNzZXShMdlAZTNiMGM0NDI5OGZjMWMxNDlhZmJmNGM4OTk2ZmI5MjQyN2FlNDFlNDY0OWI5MzRjYTQ5NTk5MWI3ODUyYjg1NQCvYXBwbGljYXRpb24veC10oXSRldlAZTNiMGM0NDI5OGZjMWMxNDlhZmJmNGM4OTk2ZmI5MjQyN2FlNDFlNDY0OWI5MzRjYTQ5NTk5MWI3ODUyYjg1NQAAoWugkaF4gaFroXY=", - "recordKey": "mesh:cdn:artifact:a-b:1:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" + "accepted": false, + "reencodedBase64": null, + "recordKey": null }, { "label": "trailing_reserved_code", "inputBase64": "mqNhL2KlYXNzZXShMdlAZTNiMGM0NDI5OGZjMWMxNDlhZmJmNGM4OTk2ZmI5MjQyN2FlNDFlNDY0OWI5MzRjYTQ5NTk5MWI3ODUyYjg1NQCvYXBwbGljYXRpb24veC10oXSRlNlAZTNiMGM0NDI5OGZjMWMxNDlhZmJmNGM4OTk2ZmI5MjQyN2FlNDFlNDY0OWI5MzRjYTQ5NTk5MWI3ODUyYjg1NQAAoWuRoXiBoWuhdsE=", - "accepted": true, - "reencodedBase64": "mqNhL2KlYXNzZXShMdlAZTNiMGM0NDI5OGZjMWMxNDlhZmJmNGM4OTk2ZmI5MjQyN2FlNDFlNDY0OWI5MzRjYTQ5NTk5MWI3ODUyYjg1NQCvYXBwbGljYXRpb24veC10oXSRldlAZTNiMGM0NDI5OGZjMWMxNDlhZmJmNGM4OTk2ZmI5MjQyN2FlNDFlNDY0OWI5MzRjYTQ5NTk5MWI3ODUyYjg1NQAAoWugkaF4gaFroXY=", - "recordKey": "mesh:cdn:artifact:a-b:1:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" + "accepted": false, + "reencodedBase64": null, + "recordKey": null }, { "label": "trailing_truncated_array", "inputBase64": "mqNhL2KlYXNzZXShMdlAZTNiMGM0NDI5OGZjMWMxNDlhZmJmNGM4OTk2ZmI5MjQyN2FlNDFlNDY0OWI5MzRjYTQ5NTk5MWI3ODUyYjg1NQCvYXBwbGljYXRpb24veC10oXSRlNlAZTNiMGM0NDI5OGZjMWMxNDlhZmJmNGM4OTk2ZmI5MjQyN2FlNDFlNDY0OWI5MzRjYTQ5NTk5MWI3ODUyYjg1NQAAoWuRoXiBoWuhdpI=", - "accepted": true, - "reencodedBase64": "mqNhL2KlYXNzZXShMdlAZTNiMGM0NDI5OGZjMWMxNDlhZmJmNGM4OTk2ZmI5MjQyN2FlNDFlNDY0OWI5MzRjYTQ5NTk5MWI3ODUyYjg1NQCvYXBwbGljYXRpb24veC10oXSRldlAZTNiMGM0NDI5OGZjMWMxNDlhZmJmNGM4OTk2ZmI5MjQyN2FlNDFlNDY0OWI5MzRjYTQ5NTk5MWI3ODUyYjg1NQAAoWugkaF4gaFroXY=", - "recordKey": "mesh:cdn:artifact:a-b:1:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855" + "accepted": false, + "reencodedBase64": null, + "recordKey": null }, { "label": "top_nil", diff --git a/packages/cultnet-ts/CHANGELOG.md b/packages/cultnet-ts/CHANGELOG.md index 56a15c7b..cd5a32f2 100644 --- a/packages/cultnet-ts/CHANGELOG.md +++ b/packages/cultnet-ts/CHANGELOG.md @@ -5,6 +5,10 @@ All notable changes to `@gamecult/cultnet-ts` are recorded here, following ## Unreleased +### Breaking + +- `decodeManifest` refuses a manifest with bytes after its first MessagePack value (`CDN artifact manifest has N bytes after its first MessagePack value.`), as the C# reference does; before, the tail was ignored. `contracts/cultmesh/content-acceptance-vectors.json` now holds `trailing_nil`, `trailing_reserved_code` and `trailing_truncated_array` as refused. + ### Added - Fork card (`contracts/cultnet/governance/fork-card.md`): `raised_at`, a required key holding an RFC3339 instant string or nil (the question's first admission time), outside `options_hash`; the card vectors are re-pinned with the key, three negatives (epoch integer, date-only string, missing key) and a nil-`raised_at` valid card; every `optionsHash` digest is unchanged. diff --git a/packages/cultnet-ts/README.md b/packages/cultnet-ts/README.md index 7d1b9a6a..92231d52 100644 --- a/packages/cultnet-ts/README.md +++ b/packages/cultnet-ts/README.md @@ -144,8 +144,8 @@ imports no `node:` module, so a browser bundle can take it. past the shared depth limit are refused. - The decoder reads what MessagePack-CSharp reads and refuses what it refuses: nil in a string, tag or metadata slot is `null` and is written back as nil, - a missing trailing slot takes the reference's default, extra slots and trailing - bytes are skipped (a chunk reference's fifth slot is its element id, a string + a missing trailing slot takes the reference's default, extra slots are skipped and bytes + after the manifest's first value are refused (a chunk reference's fifth slot is its element id, a string or nil, `""` when absent; `packContent` writes the decimal offset as the id, as the reference does), a float-encoded integer is refused, and invalid UTF-8 decodes to U+FFFD. A nil `chunks` is refused, as the reference's diff --git a/packages/cultnet-ts/src/content.ts b/packages/cultnet-ts/src/content.ts index 6f9fd873..683de8a0 100644 --- a/packages/cultnet-ts/src/content.ts +++ b/packages/cultnet-ts/src/content.ts @@ -249,7 +249,7 @@ class Reader { export function decodeManifest(bytes: Uint8Array): CultMeshCdnArtifactManifest { checkPayloadDepth(bytes, "CDN artifact manifest"); const reader = new Reader(bytes); - // A slot the array does not reach keeps the property initialiser; a slot past the tenth is skipped. Trailing bytes are not read. + // A slot the array does not reach keeps the property initialiser; a slot past the tenth is skipped. const count = reader.array("manifest"); let index = 0; const slot = (read: () => T, initial: T): T => index++ < count ? read() : initial; @@ -267,6 +267,8 @@ export function decodeManifest(bytes: Uint8Array): CultMeshCdnArtifactManifest { [reader.string("metadata") ?? wrongType("metadata"), reader.string("metadata")] as const), []), }; for (; index < count; index += 1) reader.skip(); + // A manifest is one record payload: exactly one MessagePack value, as the reference's one-value rule has it. + if (reader.position < bytes.length) throw new Error(`CDN artifact manifest has ${bytes.length - reader.position} bytes after its first MessagePack value.`); validateManifest(manifest); return manifest; } diff --git a/src/GameCult.Caching.MessagePack/CultDocumentMessagePackSerialization.cs b/src/GameCult.Caching.MessagePack/CultDocumentMessagePackSerialization.cs index 4665879b..15b70c9c 100644 --- a/src/GameCult.Caching.MessagePack/CultDocumentMessagePackSerialization.cs +++ b/src/GameCult.Caching.MessagePack/CultDocumentMessagePackSerialization.cs @@ -227,20 +227,6 @@ public static class CultDocumentMessagePackSerialization return new CultDecoded(document, scope.Finding()); } - /// - /// Reads a document payload that is not a stored record under the nesting bound alone: bytes after the first value are - /// ignored, as the CultMesh CDN manifest contract has them (contracts/cultmesh/content-acceptance-vectors.json). A record - /// payload goes through , which refuses them. - /// - public static object DeserializeBoundedUntyped(Type type, byte[] payload, CultDocumentRegistry registry) - { - if (registry == null) throw new ArgumentNullException(nameof(registry)); - registry.GetRequired(type); - CultPayloadDepth.CheckDepth(payload, type, PayloadDirection.Read); - return MessagePackSerializer.Deserialize(type, payload, OptionsForDocument(type)) - ?? throw new InvalidOperationException($"MessagePack returned null for Cult document type {type.FullName}."); - } - /// /// Replaces slots of a serialized document payload (a MessagePack array indexed by slot) with already-encoded member /// values, leaving every other slot byte for byte as it was. A slot past the end extends the array with nil. diff --git a/src/GameCult.Caching.MessagePack/CultPayloadDepth.cs b/src/GameCult.Caching.MessagePack/CultPayloadDepth.cs index 4daa0d46..6179f8be 100644 --- a/src/GameCult.Caching.MessagePack/CultPayloadDepth.cs +++ b/src/GameCult.Caching.MessagePack/CultPayloadDepth.cs @@ -16,20 +16,7 @@ internal enum PayloadDirection { Read, Write } /// internal static class CultPayloadDepth { - /// A record's payload: the depth bound, and exactly one value. Bytes after the first value are refused, never walked. public static void Check(ReadOnlySpan bytes, Type type, PayloadDirection direction) - { - var end = Walk(bytes, type, direction); - if (end < bytes.Length) - throw new MessagePackSerializationException( - $"{type.FullName} payload has {bytes.Length - end} bytes after its first MessagePack value (refused on {(direction == PayloadDirection.Read ? "read" : "write")})."); - } - - /// The depth bound alone, for a reader whose contract tolerates bytes after the value (the CDN manifest vectors). - public static void CheckDepth(ReadOnlySpan bytes, Type type, PayloadDirection direction) => Walk(bytes, type, direction); - - // The one walk: refuses past the depth limit and returns where the first value ends (the payload's length when it ends early). - private static long Walk(ReadOnlySpan bytes, Type type, PayloadDirection direction) { const int limit = CultMessagePackSecurity.MaximumPayloadDepth; Span remaining = stackalloc long[limit]; @@ -48,12 +35,12 @@ internal static class CultPayloadDepth case <= 0x8f: container = true; header = 1; children = 2L * (code & 0x0f); break; case <= 0x9f: container = true; header = 1; children = code & 0x0f; break; case <= 0xbf: skip = 1 + (code & 0x1f); break; - case 0xc4 or 0xd9: if (!Length(bytes, position, 1, out var l8)) return bytes.Length; skip = 2 + l8; break; - case 0xc5 or 0xda: if (!Length(bytes, position, 2, out var l16)) return bytes.Length; skip = 3 + l16; break; - case 0xc6 or 0xdb: if (!Length(bytes, position, 4, out var l32)) return bytes.Length; skip = 5 + l32; break; - case 0xc7: if (!Length(bytes, position, 1, out var e8)) return bytes.Length; skip = 3 + e8; break; - case 0xc8: if (!Length(bytes, position, 2, out var e16)) return bytes.Length; skip = 4 + e16; break; - case 0xc9: if (!Length(bytes, position, 4, out var e32)) return bytes.Length; skip = 6 + e32; break; + case 0xc4 or 0xd9: if (!Length(bytes, position, 1, out var l8)) return; skip = 2 + l8; break; + case 0xc5 or 0xda: if (!Length(bytes, position, 2, out var l16)) return; skip = 3 + l16; break; + case 0xc6 or 0xdb: if (!Length(bytes, position, 4, out var l32)) return; skip = 5 + l32; break; + case 0xc7: if (!Length(bytes, position, 1, out var e8)) return; skip = 3 + e8; break; + case 0xc8: if (!Length(bytes, position, 2, out var e16)) return; skip = 4 + e16; break; + case 0xc9: if (!Length(bytes, position, 4, out var e32)) return; skip = 6 + e32; break; case 0xca or 0xce or 0xd2: skip = 5; break; case 0xcb or 0xcf or 0xd3: skip = 9; break; case 0xcc or 0xd0: skip = 2; break; @@ -62,10 +49,10 @@ internal static class CultPayloadDepth case 0xd6: skip = 6; break; case 0xd7: skip = 10; break; case 0xd8: skip = 18; break; - case 0xdc: if (!Length(bytes, position, 2, out var a16)) return bytes.Length; container = true; header = 3; children = a16; break; - case 0xdd: if (!Length(bytes, position, 4, out var a32)) return bytes.Length; container = true; header = 5; children = a32; break; - case 0xde: if (!Length(bytes, position, 2, out var m16)) return bytes.Length; container = true; header = 3; children = 2 * m16; break; - default: if (!Length(bytes, position, 4, out var m32)) return bytes.Length; container = true; header = 5; children = 2 * m32; break; + case 0xdc: if (!Length(bytes, position, 2, out var a16)) return; container = true; header = 3; children = a16; break; + case 0xdd: if (!Length(bytes, position, 4, out var a32)) return; container = true; header = 5; children = a32; break; + case 0xde: if (!Length(bytes, position, 2, out var m16)) return; container = true; header = 3; children = 2 * m16; break; + default: if (!Length(bytes, position, 4, out var m32)) return; container = true; header = 5; children = 2 * m32; break; } if (container) @@ -87,10 +74,14 @@ internal static class CultPayloadDepth // One element is complete; close every container it was the last element of. while (depth > 0 && --remaining[depth - 1] == 0) depth--; - if (depth == 0) return position; + if (depth == 0) + { + if (position < bytes.Length) + throw new MessagePackSerializationException( + $"{type.FullName} payload has {bytes.Length - position} bytes after its first MessagePack value (refused on {(direction == PayloadDirection.Read ? "read" : "write")})."); + return; + } } - - return bytes.Length; } // The big-endian length that follows the code byte, when the payload is long enough to hold it. diff --git a/src/GameCult.Caching/Contracts/cultcache-persistence-format.md b/src/GameCult.Caching/Contracts/cultcache-persistence-format.md index db3fd932..ad3c06f5 100644 --- a/src/GameCult.Caching/Contracts/cultcache-persistence-format.md +++ b/src/GameCult.Caching/Contracts/cultcache-persistence-format.md @@ -403,7 +403,7 @@ runtime enforces the same bound on read and on write, and the refusal names the the depth and the limit, never payload bytes. The vectors are in `tests/vectors/union-parity` (`bounds-outcomes.msgpack`). -A record's payload is exactly one MessagePack value: every runtime refuses a payload with bytes after its first value wherever it reads a payload under the nesting bound, and on write where a write takes bytes the library did not encode itself (a C# type's own formatter, a Rust raw envelope), without walking those bytes, so a tail past the limit is refused as bytes after the value and not as depth; the refusal names the type and the count of bytes after the value, never the bytes (vectors: `tests/vectors/union-parity/bytes-after-outcomes.msgpack`). The CultMesh CDN manifest is a different contract: its wire bytes may carry bytes after the value (`contracts/cultmesh/content-acceptance-vectors.json`), so its reader keeps the nesting bound through `DeserializeBoundedUntyped` and does not refuse them. +A record's payload is exactly one MessagePack value: every runtime refuses a payload with bytes after its first value wherever it reads a payload under the nesting bound, and on write where a write takes bytes the library did not encode itself (a C# type's own formatter, a Rust raw envelope), without walking those bytes, so a tail past the limit is refused as bytes after the value and not as depth; the refusal names the type and the count of bytes after the value, never the bytes (vectors: `tests/vectors/union-parity/bytes-after-outcomes.msgpack`). A CDN manifest is stored and keyed as a record and follows the same rule (`contracts/cultmesh/content-acceptance-vectors.json`: `trailing_nil`, `trailing_reserved_code` and `trailing_truncated_array` are refused). Streams of values (CultNet frames, a store's record array) are different contracts and are not covered. ## Variant Records diff --git a/tests/GameCult.Caching.Tests/BytesAfterVectorTests.cs b/tests/GameCult.Caching.Tests/BytesAfterVectorTests.cs index 1c6e1355..00e54b7a 100644 --- a/tests/GameCult.Caching.Tests/BytesAfterVectorTests.cs +++ b/tests/GameCult.Caching.Tests/BytesAfterVectorTests.cs @@ -99,16 +99,6 @@ namespace GameCult.Caching.Tests } } - [Test] - public void TheBoundedEntryKeepsTheDepthBoundAndToleratesATail() - { - foreach (var (file, _) in Cases()) - Assert.That(CultDocumentMessagePackSerialization.DeserializeBoundedUntyped(typeof(UnionSurface), VectorFiles.Committed(Vector, file), Registry), Is.TypeOf(), file); - - var deep = VectorFiles.Committed(Vector, "depth-past-limit.msgpack"); - Assert.That(Message(() => CultDocumentMessagePackSerialization.DeserializeBoundedUntyped(typeof(UnionSurface), deep, Registry)), Does.Contain($"the limit is {Limit}")); - } - [Test] public void StoreLoadRefuses() { @@ -117,7 +107,7 @@ namespace GameCult.Caching.Tests try { var snapshot = CultDocumentMessagePackSerialization.DeserializeSnapshot(VectorFiles.Committed(Vector, "union-surface.cc")); - snapshot.Records[0].Payload = VectorFiles.Committed(Vector, "bytes-after-nil.msgpack"); + snapshot.Records[0].Payload = Base.Concat(new byte[] { 0xc0 }).ToArray(); var scratch = Path.Combine(work, "tail.cc"); File.WriteAllBytes(scratch, CultDocumentMessagePackSerialization.SerializeSnapshot(snapshot)); var load = Message(() => CultCacheMessagePack.Create(scratch, new CultCacheOpenOptions { Registry = Registry }).Dispose()); @@ -129,6 +119,57 @@ namespace GameCult.Caching.Tests } } + // A directory store names a page by the SHA-256 of its bytes and its manifest commits to that hash: the page holding the + // payload is rewritten with a tail, renamed, and the manifest told its new hash, so the store is well formed and only the payload is wrong. + [Test] + public async System.Threading.Tasks.Task DirectoryLoadPageRefuses() + { + var folder = Path.Combine(Path.GetTempPath(), $"cultlib-bytesafter-dir-{Guid.NewGuid():N}"); + Directory.CreateDirectory(folder); + try + { + var path = Path.Combine(folder, "tail.cc"); + var options = new CultCacheOpenOptions { Registry = Registry, UseDirectoryStore = true, StoreFlushOnDispose = true }; + using (var writer = CultCacheMessagePack.Create(path, options)) + { + var value = CultDocumentMessagePackSerialization.DeserializeUntyped(typeof(UnionSurface), Base, Registry); + await writer.UpsertAsync(typeof(UnionSurface), value, new CultRecordKey("surface")); + await writer.FlushAsync(); + } + + var rehashed = new System.Collections.Generic.List<(byte[] Old, byte[] New)>(); + foreach (var file in Directory.EnumerateFiles(folder, "*.msgpack", SearchOption.AllDirectories).ToArray()) + { + var bytes = File.ReadAllBytes(file); + if (bytes.AsSpan().IndexOf(Base) < 0) continue; + var page = CultDocumentMessagePackSerialization.DeserializePersistedRecord(bytes); + page.Payload = Base.Concat(new byte[] { 0xc0 }).ToArray(); + var forged = CultDocumentMessagePackSerialization.SerializePersistedRecord(page); + File.Delete(file); + File.WriteAllBytes(Path.Combine(Path.GetDirectoryName(file)!, Convert.ToHexString(System.Security.Cryptography.SHA256.HashData(forged)).ToLowerInvariant() + ".msgpack"), forged); + rehashed.Add((System.Security.Cryptography.SHA256.HashData(bytes), System.Security.Cryptography.SHA256.HashData(forged))); + } + + Assert.That(rehashed, Is.Not.Empty, "the record page was found"); + foreach (var file in Directory.EnumerateFiles(folder, "*", SearchOption.AllDirectories).Where(f => !f.EndsWith(".msgpack", StringComparison.Ordinal)).ToArray()) + { + var bytes = File.ReadAllBytes(file); + foreach (var (old, @new) in rehashed) + for (var at = bytes.AsSpan().IndexOf(old); at >= 0; at = bytes.AsSpan().IndexOf(old)) @new.CopyTo(bytes, at); + File.WriteAllBytes(file, bytes); + } + + var thrown = Assert.CatchAsync(async () => { using var cache = await CultCacheMessagePack.OpenAsync(path, new CultCacheOpenOptions { Registry = Registry, UseDirectoryStore = true, ReadOnly = true }); }); + var chain = ""; + for (var e = thrown; e != null; e = e.InnerException) chain += e.Message + "\n"; + Assert.That(chain, Does.Contain($"1 {Clause}")); + } + finally + { + Directory.Delete(folder, recursive: true); + } + } + public sealed class TwoValueFormatter : global::MessagePack.Formatters.IMessagePackFormatter { public void Serialize(ref MessagePackWriter writer, TwoValues value, MessagePackSerializerOptions options) diff --git a/tests/GameCult.Mesh.Tests/BytesAfterPathTests.cs b/tests/GameCult.Mesh.Tests/BytesAfterPathTests.cs new file mode 100644 index 00000000..ed9b904b --- /dev/null +++ b/tests/GameCult.Mesh.Tests/BytesAfterPathTests.cs @@ -0,0 +1,61 @@ +using System; +using System.Linq; +using System.Reflection; +using FluentAssertions; +using GameCult.Caching; +using GameCult.Caching.MessagePack; +using GameCult.Networking; +using NUnit.Framework; + +namespace GameCult.Mesh.Tests; + +// The mesh's typed reads of a raw record decode the payload through the one-value rule: a tail is refused, never ignored. +public class BytesAfterPathTests +{ + private const string Clause = "bytes after its first MessagePack value"; + + private static CultNetRawDocumentRecord WithTail(CultMeshCdnArtifactManifest manifest) + { + var payload = CultDocumentMessagePackSerialization.SerializeUntyped(manifest, typeof(CultMeshCdnArtifactManifest)).Concat(new byte[] { 0xc0 }).ToArray(); + return new CultNetRawDocumentRecord + { + SchemaId = CultDocumentRegistry.Shared.GetRequired().SchemaId, + RecordKey = "k", + StoredAt = "1970-01-01T00:00:00.000Z", + Payload = payload, + }; + } + + private static CultMeshCdnArtifactManifest Manifest() => new() { ArtifactId = "a", Version = "1", ContentHash = "h" }; + + private static string Refusal(Action read) + { + var thrown = Assert.Catch(read)!; + return thrown is TargetInvocationException { InnerException: { } inner } ? inner.ToString() : thrown.ToString(); + } + + [Test] + public void ASnapshotDecodeThroughABindingRefusesATail() + { + var registry = CultMeshCdn.CreateDocumentRegistry(); + var snapshot = new CultNetSnapshotResponseRawMessage { Documents = new[] { WithTail(Manifest()) } }; + Refusal(() => CultMesh.DecodeSnapshotDocuments(snapshot, registry)).Should().Contain(Clause); + } + + [Test] + public void ASnapshotDecodeWithNoBindingRefusesATail() + { + var snapshot = new CultNetSnapshotResponseRawMessage { Documents = new[] { WithTail(Manifest()) } }; + Refusal(() => CultMesh.DecodeSnapshotDocuments(snapshot, new CultNetDocumentRegistry())).Should().Contain(Clause); + } + + [Test] + public void ATypedReadOfOneSnapshotRecordRefusesATail() + { + var record = WithTail(Manifest()); + var snapshot = new CultNetSnapshotResponseRawMessage { MessageId = "m", Documents = new[] { record } }; + var read = typeof(CultMesh).GetMethod("ReadDocumentFromSnapshotResponse", BindingFlags.NonPublic | BindingFlags.Static)! + .MakeGenericMethod(typeof(CultMeshCdnArtifactManifest)); + Refusal(() => read.Invoke(null, new object[] { snapshot, record.SchemaId, record.RecordKey })).Should().Contain(Clause); + } +} diff --git a/tests/GameCult.Mesh.Tests/CultMeshContentVectorTests.cs b/tests/GameCult.Mesh.Tests/CultMeshContentVectorTests.cs index 77407520..0f627cfb 100644 --- a/tests/GameCult.Mesh.Tests/CultMeshContentVectorTests.cs +++ b/tests/GameCult.Mesh.Tests/CultMeshContentVectorTests.cs @@ -462,12 +462,12 @@ public sealed class CultMeshContentVectorTests private static byte[] Reencode(CultMeshContentChunkResponseMessage message) => CultNetSchemaMessageSerialization.Serialize(message); - /// The one reader of manifest bytes: the bounded document reader (CultPayloadDepth 256 included): the CDN manifest contract tolerates bytes after the value, a record payload does not. + /// The one reader of manifest bytes: the production CultNet binding of the manifest (CultPayloadDepth 256 and the one-value rule included). private static CultMeshCdnArtifactManifest ReadManifest(byte[] bytes) => - (CultMeshCdnArtifactManifest)CultDocumentMessagePackSerialization.DeserializeBoundedUntyped(typeof(CultMeshCdnArtifactManifest), bytes, CultDocumentRegistry.Shared); + (CultMeshCdnArtifactManifest)CultMeshCdn.CreateDocumentRegistry().GetByDocumentType(typeof(CultMeshCdnArtifactManifest))!.PayloadDeserializer(bytes); private static byte[] Reencode(CultMeshCdnArtifactManifest manifest) => - CultDocumentMessagePackSerialization.SerializeUntyped(manifest, typeof(CultMeshCdnArtifactManifest)); + CultMeshCdn.CreateDocumentRegistry().GetByDocumentType(typeof(CultMeshCdnArtifactManifest))!.PayloadSerializer(manifest); private static CultMeshContentChunkRequestMessage Request(string id, string hash, string key, int size) => new() { diff --git a/tests/GameCult.Networking.Tests/BytesAfterPathTests.cs b/tests/GameCult.Networking.Tests/BytesAfterPathTests.cs new file mode 100644 index 00000000..6a56ceb1 --- /dev/null +++ b/tests/GameCult.Networking.Tests/BytesAfterPathTests.cs @@ -0,0 +1,62 @@ +using System; +using System.Linq; +using GameCult.Caching; +using GameCult.Caching.MessagePack; +using MessagePack; +using NUnit.Framework; + +namespace GameCult.Networking.Tests +{ + // A record's payload is one MessagePack value: each CultNet path that decodes a payload refuses a tail, by the one rule in + // CultPayloadDepth.Check. One test per path family, so that pointing a path at a tolerant reader turns it red. + public class BytesAfterPathTests + { + private const string Clause = "1 bytes after its first MessagePack value"; + + [CultDocument("tests.bytes_after_net", "tests.bytes_after_net.v1")] + [MessagePackObject] + public sealed class TailDoc + { + [Key(0)] [CultName] public string Id = "a"; + [Key(1)] public int N = 3; + } + + private static readonly CultDocumentRegistry Documents = CultDocumentRegistry.ForTypes(new[] { typeof(TailDoc) }); + + private static byte[] WithTail() => + CultDocumentMessagePackSerialization.SerializeUntyped(new TailDoc(), typeof(TailDoc), Documents).Concat(new byte[] { 0xc0 }).ToArray(); + + private static CultNetRawDocumentRecord Raw(CultNetDocumentRegistry registry, byte[] payload) => new() + { + SchemaId = Documents.GetRequired(typeof(TailDoc)).SchemaId, + RecordKey = "a", + StoredAt = "1970-01-01T00:00:00.000Z", + Payload = payload, + }; + + private static void Refuses(TestDelegate read) => + Assert.That(Assert.Catch(read)!.ToString(), Does.Contain(Clause)); + + [Test] + public void TheGenericBindingRefusesATail() => + Refuses(() => CultNetDocumentBinding.ForDocument(Documents).PayloadDeserializer(WithTail())); + + [Test] + public void TheRuntimeTypeBindingRefusesATail() => + Refuses(() => CultNetDocumentBinding.ForDocument(typeof(TailDoc), Documents).PayloadDeserializer(WithTail())); + + [Test] + public void ARawEnvelopeThroughARegisteredBindingRefusesATail() + { + var registry = new CultNetDocumentRegistry(Documents).Register(CultNetDocumentBinding.ForDocument(Documents)); + Refuses(() => registry.DeserializeRawDocument(Raw(registry, WithTail()))); + } + + [Test] + public void ARawEnvelopeWithNoBindingRefusesATail() + { + var registry = new CultNetDocumentRegistry(Documents); + Refuses(() => registry.DeserializeRawDocument(Raw(registry, WithTail()))); + } + } +} diff --git a/tests/vectors/union-parity/README.md b/tests/vectors/union-parity/README.md index 9468d64d..525f5642 100644 --- a/tests/vectors/union-parity/README.md +++ b/tests/vectors/union-parity/README.md @@ -188,4 +188,4 @@ Must: refuse on read, naming the type and the count of bytes after the first val What a runtime walks: only the first value of the payload, to find where it ends (a leaf counts as complete at its own length, a container when its last child completes). It compares that end with the payload length; a payload that ends early is not bytes after the value and stays the decoder's to refuse. It never walks the tail, so the tail's own shape (a stray `c1`, 300 levels) is never judged. -The CultMesh CDN manifest is not covered by these vectors: its contract (`contracts/cultmesh/content-acceptance-vectors.json`) accepts bytes after the value, and its reader keeps the depth bound through `DeserializeBoundedUntyped`. +A CDN manifest read as a record follows the same rule; its vectors are `contracts/cultmesh/content-acceptance-vectors.json` (`trailing_*` are refusals). diff --git a/unity/org.gamecult.cultlib/CHANGELOG.md b/unity/org.gamecult.cultlib/CHANGELOG.md index ff74007f..5f522621 100644 --- a/unity/org.gamecult.cultlib/CHANGELOG.md +++ b/unity/org.gamecult.cultlib/CHANGELOG.md @@ -6,7 +6,7 @@ All notable changes to this package are documented in this file. ### Breaking -- A document payload with bytes after its first MessagePack value is refused on read and on write (`MessagePackSerializationException` naming the type and the count of bytes after the value, never the bytes); the tail is not walked, so a tail nested past the limit is refused as bytes after the value, not as depth. Before, the tail was ignored on read and dropped on the next write. A payload that ends early is still the deserializer's to refuse. The CultMesh CDN manifest contract still accepts bytes after the value; `CultDocumentMessagePackSerialization.DeserializeBoundedUntyped` (new) reads under the nesting bound alone for it. +- A document payload with bytes after its first MessagePack value is refused on read and on write (`MessagePackSerializationException` naming the type and the count of bytes after the value, never the bytes); the tail is not walked, so a tail nested past the limit is refused as bytes after the value, not as depth. Before, the tail was ignored on read and dropped on the next write. A payload that ends early is still the deserializer's to refuse. A CDN manifest is a record payload and follows the same rule: the acceptance vectors `trailing_nil`, `trailing_reserved_code` and `trailing_truncated_array` in `contracts/cultmesh/content-acceptance-vectors.json` are now refusals. - A single-file store holding two records under one key is unreadable: the reader refuses it with `CultStoreUnreadableException` naming the key (keys compare by ordinal code units, no case folding or normalisation), so a cache, `ReadStoreSnapshot` and a flush refuse the file and leave it as it is. Before, the later record silently replaced the earlier. A directory store reads its manifest through the same reader and refuses a duplicate key the same way. Repair the duplicate with the runtime that wrote it before reading the store here. - A record whose payload carries a slot past the slots its reading type declares, at any depth (the record, a nested object, a list element, a union arm or a unit arm), is loaded read-only: it is served, and every staged write or removal of its key (`Upsert`, `UpsertVariant`, `Remove`, a batch, and `Flatten` of a variant) is refused with `CultSchemaConflictException` before anything is staged, and the store keeps the record byte for byte. Before, the extra slots were skipped on read and dropped on the next write. Declaring an id compatible consents only to depth-0 slots the schema comparison reports ignored under that id; any other extra slot keeps the record read-only whatever is declared, including under the reader's own id. A variant overriding a slot its type does not have is read-only too (it used to be shed on flush). Write such a record from the build that declares those slots. Missing trailing slots still default and stay writable. - `Flatten` (and `FlattenAsync`) of a variant whose base chain holds a record its store does not own (a record held read-only for extra slots, or undeclared drift that drops slots) is refused with `CultSchemaConflictException` naming that base record, and the store is left as it was. Before, the flatten wrote the base's decoded view into a plain record and the base's unread data was lost for that variant. Writing and removing the variant itself are still accepted: they persist only its delta. -- 2.47.3 From eb9012600287a87e301047bdadedcf7d565fe002 Mon Sep 17 00:00:00 2001 From: Libby Date: Thu, 12 Nov 2026 03:15:34 +0000 Subject: [PATCH 5/7] Bytes after: codec DecodeDocument tail pinned; provider command document refuses a tail; dead fallback binding deleted Co-Authored-By: Claude Sonnet 5.5 --- .../CultMeshProviderSessionWire.cs | 5 +++- .../CultNetDocumentRegistry.cs | 12 ++++------ .../BytesAfterVectorTests.cs | 11 +++++++++ .../CultMeshProviderSessionWireTests.cs | 24 +++++++++++++++++++ 4 files changed, 43 insertions(+), 9 deletions(-) diff --git a/src/GameCult.Mesh/CultMeshProviderSessionWire.cs b/src/GameCult.Mesh/CultMeshProviderSessionWire.cs index 03f3a70c..ad9ae66e 100644 --- a/src/GameCult.Mesh/CultMeshProviderSessionWire.cs +++ b/src/GameCult.Mesh/CultMeshProviderSessionWire.cs @@ -284,7 +284,10 @@ namespace GameCult.Mesh throw new ArgumentException($"Expected provider command schema '{CultMeshProviderSessionWireContract.CommandSchema}'.", nameof(document)); if (!string.Equals(document.PayloadEncoding, "messagepack", StringComparison.Ordinal)) throw new ArgumentException("Provider command document must use MessagePack.", nameof(document)); - var command = MessagePackSerializer.Deserialize(document.Payload, Options); + // A raw record's payload is one MessagePack value (ruling trailing-bytes-refuse): a tail is refused by count, never walked. + var command = MessagePackSerializer.Deserialize(new ReadOnlyMemory(document.Payload), Options, out var read); + if (read < document.Payload.Length) + throw new MessagePackSerializationException($"Provider command document: {document.Payload.Length - read} bytes after its first MessagePack value (refused on read)."); if (command == null) throw new MessagePackSerializationException("Provider command decoded to null."); Validate(command); return command; diff --git a/src/GameCult.Networking/CultNetDocumentRegistry.cs b/src/GameCult.Networking/CultNetDocumentRegistry.cs index 19f17c8c..bf17a91f 100644 --- a/src/GameCult.Networking/CultNetDocumentRegistry.cs +++ b/src/GameCult.Networking/CultNetDocumentRegistry.cs @@ -641,23 +641,19 @@ namespace GameCult.Networking string storedAt, CultNetDocumentMessageOptions? options = null) { - var binding = GetByDocumentType(descriptor.DocumentType) ?? - new CultNetDocumentBinding( - descriptor.DocumentType, - descriptor.SchemaId, - value => CultDocumentMessagePackSerialization.SerializeUntyped(value, value.GetType(), _documents), - payload => CultDocumentMessagePackSerialization.DeserializeUntyped(descriptor.DocumentType, payload, _documents)); + var binding = GetByDocumentType(descriptor.DocumentType); return new CultNetRawDocumentRecord { - SchemaId = binding.SchemaId, + SchemaId = binding?.SchemaId ?? descriptor.SchemaId, SchemaName = descriptor.SchemaName, SchemaVersion = descriptor.SchemaVersion, SchemaContentHash = descriptor.ContentHash, RecordKey = key.Value, StoredAt = storedAt, PayloadEncoding = "messagepack", - Payload = binding.PayloadSerializer(document), + Payload = binding?.PayloadSerializer(document) ?? + CultDocumentMessagePackSerialization.SerializeUntyped(document, document.GetType(), _documents), SourceRuntimeId = options?.SourceRuntimeId, SourceAgentId = options?.SourceAgentId, SourceRole = options?.SourceRole, diff --git a/tests/GameCult.Caching.Tests/BytesAfterVectorTests.cs b/tests/GameCult.Caching.Tests/BytesAfterVectorTests.cs index 00e54b7a..86d677e6 100644 --- a/tests/GameCult.Caching.Tests/BytesAfterVectorTests.cs +++ b/tests/GameCult.Caching.Tests/BytesAfterVectorTests.cs @@ -99,6 +99,17 @@ namespace GameCult.Caching.Tests } } + // The codec the store and the inspector read through is a path of its own: DecodeDocument deserializes by the codec's + // delegate, so a tail it did not refuse would be read as a document. + [Test] + public void CodecDecodeDocumentRefusesATail() + { + var codec = CultCacheMessagePack.CreateCodec(Registry); + Assert.That(codec.DecodeDocument(new CultRecordKey("surface"), Base), Is.Not.Null); + var message = Message(() => codec.DecodeDocument(new CultRecordKey("surface"), Base.Concat(new byte[] { 0xc0 }).ToArray())); + Assert.That(message, Does.Contain(typeof(UnionSurface).FullName).And.Contain($"1 {Clause}").And.Contain("refused on read")); + } + [Test] public void StoreLoadRefuses() { diff --git a/tests/GameCult.Mesh.Tests/CultMeshProviderSessionWireTests.cs b/tests/GameCult.Mesh.Tests/CultMeshProviderSessionWireTests.cs index 75125148..41339a85 100644 --- a/tests/GameCult.Mesh.Tests/CultMeshProviderSessionWireTests.cs +++ b/tests/GameCult.Mesh.Tests/CultMeshProviderSessionWireTests.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Linq; using GameCult.Networking; using MessagePack; using NUnit.Framework; @@ -168,6 +169,29 @@ public sealed class CultMeshProviderSessionWireTests }); } + [Test] + public void DecodeCommandDocumentRefusesBytesAfterItsFirstValue() + { + var command = new CultMeshProviderCommandWire + { + CommandId = "cmd-7", CommandKind = "pilot.thrust", ProviderId = "aetheria", ServiceInstanceId = "aetheria-42", + Payload = new Dictionary { ["thrust"] = 1 } + }; + var payload = Convert.FromBase64String(CultMeshProviderSessionWire.EncodePayload(command)); + CultNetRawDocumentRecord Record(byte[] bytes) => new() + { + SchemaId = CultMeshProviderSessionWireContract.CommandSchema, + RecordKey = "provider-command:aetheria:aetheria-42:cmd-7", + PayloadEncoding = "messagepack", + Payload = bytes + }; + + Assert.That(CultMeshProviderSessionWire.DecodeCommandDocument(Record(payload)).CommandId, Is.EqualTo("cmd-7")); + var thrown = Assert.Throws(() => + CultMeshProviderSessionWire.DecodeCommandDocument(Record(payload.Concat(new byte[] { 0xc0, 0xc0 }).ToArray())))!; + Assert.That(thrown.Message, Does.Contain("2 bytes after its first MessagePack value")); + } + [Test] public void LeaseResponse_RoundTrips_WithApplicationAcceptance() { -- 2.47.3 From 96551866d627603b267e8958319f3f81bc91a934 Mon Sep 17 00:00:00 2001 From: Libby Date: Thu, 12 Nov 2026 03:15:39 +0000 Subject: [PATCH 6/7] Changelog: provider command document tail Co-Authored-By: Claude Sonnet 5.5 --- unity/org.gamecult.cultlib/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/unity/org.gamecult.cultlib/CHANGELOG.md b/unity/org.gamecult.cultlib/CHANGELOG.md index 5f522621..a4a2bc11 100644 --- a/unity/org.gamecult.cultlib/CHANGELOG.md +++ b/unity/org.gamecult.cultlib/CHANGELOG.md @@ -6,7 +6,7 @@ All notable changes to this package are documented in this file. ### Breaking -- A document payload with bytes after its first MessagePack value is refused on read and on write (`MessagePackSerializationException` naming the type and the count of bytes after the value, never the bytes); the tail is not walked, so a tail nested past the limit is refused as bytes after the value, not as depth. Before, the tail was ignored on read and dropped on the next write. A payload that ends early is still the deserializer's to refuse. A CDN manifest is a record payload and follows the same rule: the acceptance vectors `trailing_nil`, `trailing_reserved_code` and `trailing_truncated_array` in `contracts/cultmesh/content-acceptance-vectors.json` are now refusals. +- A document payload with bytes after its first MessagePack value is refused on read and on write (`MessagePackSerializationException` naming the type and the count of bytes after the value, never the bytes); the tail is not walked, so a tail nested past the limit is refused as bytes after the value, not as depth. Before, the tail was ignored on read and dropped on the next write. A payload that ends early is still the deserializer's to refuse. A CDN manifest is a record payload and follows the same rule: the acceptance vectors `trailing_nil`, `trailing_reserved_code` and `trailing_truncated_array` in `contracts/cultmesh/content-acceptance-vectors.json` are now refusals. `CultMeshProviderSessionWire.DecodeCommandDocument` refuses a command record whose payload has bytes after its first value (`MessagePackSerializationException` with the count); before, the tail was ignored. Semver: breaking for C# consumers, a major bump. - A single-file store holding two records under one key is unreadable: the reader refuses it with `CultStoreUnreadableException` naming the key (keys compare by ordinal code units, no case folding or normalisation), so a cache, `ReadStoreSnapshot` and a flush refuse the file and leave it as it is. Before, the later record silently replaced the earlier. A directory store reads its manifest through the same reader and refuses a duplicate key the same way. Repair the duplicate with the runtime that wrote it before reading the store here. - A record whose payload carries a slot past the slots its reading type declares, at any depth (the record, a nested object, a list element, a union arm or a unit arm), is loaded read-only: it is served, and every staged write or removal of its key (`Upsert`, `UpsertVariant`, `Remove`, a batch, and `Flatten` of a variant) is refused with `CultSchemaConflictException` before anything is staged, and the store keeps the record byte for byte. Before, the extra slots were skipped on read and dropped on the next write. Declaring an id compatible consents only to depth-0 slots the schema comparison reports ignored under that id; any other extra slot keeps the record read-only whatever is declared, including under the reader's own id. A variant overriding a slot its type does not have is read-only too (it used to be shed on flush). Write such a record from the build that declares those slots. Missing trailing slots still default and stay writable. - `Flatten` (and `FlattenAsync`) of a variant whose base chain holds a record its store does not own (a record held read-only for extra slots, or undeclared drift that drops slots) is refused with `CultSchemaConflictException` naming that base record, and the store is left as it was. Before, the flatten wrote the base's decoded view into a plain record and the base's unread data was lost for that variant. Writing and removing the variant itself are still accepted: they persist only its delta. -- 2.47.3 From 02344c654617b4d5c39082aa9738a416cb459412 Mon Sep 17 00:00:00 2001 From: Libby Date: Thu, 12 Nov 2026 03:23:33 +0000 Subject: [PATCH 7/7] Pin the one-byte tail of a provider command document Co-Authored-By: Claude Sonnet 5.5 --- tests/GameCult.Mesh.Tests/CultMeshProviderSessionWireTests.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/GameCult.Mesh.Tests/CultMeshProviderSessionWireTests.cs b/tests/GameCult.Mesh.Tests/CultMeshProviderSessionWireTests.cs index 41339a85..ca4346aa 100644 --- a/tests/GameCult.Mesh.Tests/CultMeshProviderSessionWireTests.cs +++ b/tests/GameCult.Mesh.Tests/CultMeshProviderSessionWireTests.cs @@ -190,6 +190,9 @@ public sealed class CultMeshProviderSessionWireTests var thrown = Assert.Throws(() => CultMeshProviderSessionWire.DecodeCommandDocument(Record(payload.Concat(new byte[] { 0xc0, 0xc0 }).ToArray())))!; Assert.That(thrown.Message, Does.Contain("2 bytes after its first MessagePack value")); + var one = Assert.Throws(() => + CultMeshProviderSessionWire.DecodeCommandDocument(Record(payload.Concat(new byte[] { 0xc0 }).ToArray())))!; + Assert.That(one.Message, Does.Contain("1 bytes after its first MessagePack value")); } [Test] -- 2.47.3