GameCult.Thing.Contract: RecordPayload.Bytes is a copied ReadOnlyMemory<byte>; array arguments and decoded lists pinned
Under thing2:ruling:payload-bytes-read-only the payload holds a copy made at construction (wire bytes unchanged). New tests: a T[] argument (including through Builders.Boxed) is copied; no list of any decoded golden is writable; a caller's buffer written after construction does not change the payload. The byte[] exemption in the mutable-collection scan is removed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
parent
9405c7a595
commit
4f87e5c5bf
3 changed files with 91 additions and 5 deletions
|
|
@ -107,12 +107,12 @@ public sealed class CommandDecl
|
|||
public sealed class RecordPayload
|
||||
{
|
||||
[Key(0)] public string Schema { get; }
|
||||
[Key(1)] public byte[] Bytes { get; }
|
||||
[Key(1)] public ReadOnlyMemory<byte> Bytes { get; }
|
||||
|
||||
public RecordPayload(string schema, byte[] bytes)
|
||||
public RecordPayload(string schema, ReadOnlyMemory<byte> bytes)
|
||||
{
|
||||
Schema = ThingDomain.Present(schema, "Schema");
|
||||
Bytes = ThingDomain.Present(bytes, "Bytes");
|
||||
Bytes = bytes.ToArray();
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -62,6 +62,7 @@ internal static class Doors
|
|||
if (type == typeof(bool)) return true;
|
||||
if (type == typeof(byte)) return (byte)1;
|
||||
if (type == typeof(byte[])) return new byte[] { 1 };
|
||||
if (type == typeof(ReadOnlyMemory<byte>)) return new ReadOnlyMemory<byte>(new byte[] { 1 });
|
||||
if (type.IsGenericType && type.GetGenericTypeDefinition() == typeof(IReadOnlyList<>))
|
||||
return MakeList(type.GetGenericArguments()[0], 0, depth);
|
||||
var arms = type.GetCustomAttributes<UnionAttribute>(false).ToList();
|
||||
|
|
|
|||
|
|
@ -11,8 +11,7 @@ namespace GameCult.Thing.Contract.Tests;
|
|||
public sealed class ReadOnlyListTests
|
||||
{
|
||||
private static bool IsMutableCollection(Type type) =>
|
||||
type != typeof(byte[]) // an opaque payload, not a tree
|
||||
&& (typeof(IList).IsAssignableFrom(type) || type.GetInterfaces().Concat(new[] { type }).Any(i =>
|
||||
(typeof(IList).IsAssignableFrom(type) || type.GetInterfaces().Concat(new[] { type }).Any(i =>
|
||||
i.IsGenericType && (i.GetGenericTypeDefinition() == typeof(ICollection<>) || i.GetGenericTypeDefinition() == typeof(IDictionary<,>))));
|
||||
|
||||
private static IEnumerable<(Doors.Door Door, int Index, Type Element)> ListParameters() =>
|
||||
|
|
@ -78,4 +77,90 @@ public sealed class ReadOnlyListTests
|
|||
Assert.That(list.Bound, Is.EqualTo("present"), door.Name);
|
||||
}
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void AnArrayPassedToAConstructorIsCopiedNotKept()
|
||||
{
|
||||
var child = Builders.Text("t", "x");
|
||||
var other = Builders.Text("u", "y");
|
||||
var array = new[] { child };
|
||||
var box = (BoxNode)Builders.Boxed("b", array);
|
||||
array[0] = other;
|
||||
Assert.That(box.Children[0], Is.SameAs(child), "writing the caller's array changed the value");
|
||||
Assert.That(box.Children, Is.Not.SameAs(array));
|
||||
Assert.That(box.Children is Node[], Is.False, "the held list is the caller's array type");
|
||||
var boxed = (BoxNode)Builders.Boxed("c", child, other);
|
||||
Assert.That(boxed.Children, Has.Count.EqualTo(2));
|
||||
|
||||
foreach (var (door, index, element) in ListParameters())
|
||||
{
|
||||
var args = door.ValidArgs();
|
||||
var passed = Array.CreateInstance(element, 1);
|
||||
passed.SetValue(Doors.Make(element), 0);
|
||||
args[index] = passed;
|
||||
var value = door.Call(args)!;
|
||||
var name = door.Parameters[index].Name!.TrimStart('@');
|
||||
var held = (IList)value.GetType().GetProperty(char.ToUpperInvariant(name[0]) + name[1..])!.GetValue(value)!;
|
||||
var first = held[0];
|
||||
passed.SetValue(Doors.Make(element), 0);
|
||||
Assert.That(held[0], Is.SameAs(first), $"{door.Name}: writing the caller's array changed the value");
|
||||
Assert.That(held, Is.Not.SameAs(passed), door.Name);
|
||||
}
|
||||
}
|
||||
|
||||
private static void WalkLists(object? value, int depth, Action<object> onList)
|
||||
{
|
||||
if (value is null || depth > 200 || value is string || value.GetType().IsPrimitive || value is ReadOnlyMemory<byte>) return;
|
||||
if (value is IList list)
|
||||
{
|
||||
onList(list);
|
||||
foreach (var item in list) WalkLists(item, depth + 1, onList);
|
||||
return;
|
||||
}
|
||||
|
||||
if (value.GetType().Namespace != "GameCult.Thing.Contract") return;
|
||||
foreach (var property in value.GetType().GetProperties(BindingFlags.Public | BindingFlags.Instance))
|
||||
WalkLists(property.GetValue(value), depth + 1, onList);
|
||||
}
|
||||
|
||||
private static readonly (string Golden, Type Type)[] DecodedGoldens =
|
||||
{
|
||||
("surface-all-arms", typeof(SurfaceDocument)), ("composition-all-params", typeof(CompositionDocument)),
|
||||
("invocation-all-inputs", typeof(CommandInvocation)), ("asset-catalog", typeof(AssetCatalog)),
|
||||
("result-pending", typeof(CommandResult)), ("result-completed", typeof(CommandResult)),
|
||||
("result-completed-open-surface", typeof(CommandResult)), ("result-completed-open-external", typeof(CommandResult)),
|
||||
("result-denied", typeof(CommandResult)), ("result-denied-no-node", typeof(CommandResult)), ("result-failed", typeof(CommandResult)),
|
||||
};
|
||||
|
||||
[Test]
|
||||
public void NoListOfADecodedGoldenIsWritable()
|
||||
{
|
||||
var lists = 0;
|
||||
foreach (var (golden, type) in DecodedGoldens)
|
||||
WalkLists(Wire.Read(type, Wire.Golden($"{golden}.bin")), 0, list =>
|
||||
{
|
||||
lists++;
|
||||
var l = (IList)list;
|
||||
Assert.That(l.IsReadOnly && l.IsFixedSize, Is.True, $"{golden}: {l.GetType().Name} is writable");
|
||||
Assert.That(l is Array, Is.False, $"{golden}: {l.GetType().Name} is an array");
|
||||
Assert.Throws<NotSupportedException>(() => l.Add(null), golden);
|
||||
if (l.Count > 0) Assert.Throws<NotSupportedException>(() => l[0] = l[0], golden);
|
||||
});
|
||||
Assert.That(lists, Is.GreaterThan(20));
|
||||
}
|
||||
|
||||
[Test]
|
||||
public void APayloadHoldsACopyOfTheBytesItWasBuiltFrom()
|
||||
{
|
||||
var buffer = new byte[] { 1, 2, 3 };
|
||||
var payload = new RecordPayload("s", buffer);
|
||||
buffer[0] = 9;
|
||||
Assert.That(payload.Bytes.ToArray(), Is.EqualTo(new byte[] { 1, 2, 3 }), "writing the caller's buffer changed the payload");
|
||||
|
||||
var shared = new byte[] { 4, 5, 6, 7 };
|
||||
var slice = new RecordPayload("s", new ReadOnlyMemory<byte>(shared, 1, 2));
|
||||
shared[1] = 0;
|
||||
Assert.That(slice.Bytes.ToArray(), Is.EqualTo(new byte[] { 5, 6 }));
|
||||
Assert.That(System.Runtime.InteropServices.MemoryMarshal.TryGetArray(payload.Bytes, out var segment) && ReferenceEquals(segment.Array, buffer), Is.False);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue