diff --git a/crates/bindings-typescript/src/lib/binary_reader.ts b/crates/bindings-typescript/src/lib/binary_reader.ts index 5d68506426d..7e819e20a64 100644 --- a/crates/bindings-typescript/src/lib/binary_reader.ts +++ b/crates/bindings-typescript/src/lib/binary_reader.ts @@ -49,7 +49,13 @@ export default class BinaryReader { readUInt8Array(): Uint8Array { const length = this.readU32(); this.#ensure(length); - return this.readBytes(length); + // Return an owned copy, not a view over the reader's buffer. Decoded column + // values are handed to user code and may be retained, but the runtime reuses + // this buffer across table scans, so a view would be silently invalidated by + // the next iter()/filter(). Callers that only consume the bytes transiently + // (e.g. readString) can use readBytes directly to avoid this copy. + // https://github.com/clockworklabs/SpacetimeDB/issues/5490 + return this.readBytes(length).slice(); } readBool(): boolean { @@ -65,7 +71,10 @@ export default class BinaryReader { } readBytes(length: number): Uint8Array { - // Create a Uint8Array view over the DataView's buffer at the current offset + // Returns a Uint8Array *view* over the reader's underlying buffer (no copy). + // The view is only valid until the runtime reuses that buffer (e.g. the next + // table scan). Consume it immediately or copy it (see readUInt8Array). + // // The #view.buffer is the whole ArrayBuffer, so we need to account for the // #view's starting position in that buffer (#view.byteOffset) and the current #offset const array = new Uint8Array( @@ -182,7 +191,11 @@ export default class BinaryReader { } readString(): string { - const uint8Array = this.readUInt8Array(); - return new TextDecoder('utf-8').decode(uint8Array); + const length = this.readU32(); + this.#ensure(length); + // A view is safe here: TextDecoder copies the bytes synchronously, so nothing + // retains a reference to the reader's buffer. Avoids readUInt8Array's copy. + const bytes = this.readBytes(length); + return new TextDecoder('utf-8').decode(bytes); } } diff --git a/crates/bindings-typescript/tests/binary_read_write.test.ts b/crates/bindings-typescript/tests/binary_read_write.test.ts index 0cd47e971e7..fd4933d56f1 100644 --- a/crates/bindings-typescript/tests/binary_read_write.test.ts +++ b/crates/bindings-typescript/tests/binary_read_write.test.ts @@ -156,3 +156,33 @@ describe('BinaryReader/Writer', () => { expect(deserializedTransactionUpdate).toEqual(transactionUpdate); }); }); + +describe('readUInt8Array buffer ownership', () => { + // BSATN layout for `array`: u32 length (LE) followed by that many bytes. + const encode = (bytes: number[]): Uint8Array => + new Uint8Array([bytes.length, 0, 0, 0, ...bytes]); + + test('returns an owned copy that survives later mutation of the reader buffer', () => { + const buffer = encode([1, 2, 3]); + const value = new BinaryReader(buffer).readUInt8Array(); + expect([...value]).toEqual([1, 2, 3]); + + // Simulate the runtime reusing the scan buffer for a subsequent table scan: + // it overwrites the bytes the value was read from. A view would change here; + // an owned copy must not. + buffer.fill(0xff); + expect([...value]).toEqual([1, 2, 3]); + }); + + test('does not share the reader backing ArrayBuffer', () => { + const buffer = encode([1, 2, 3]); + const value = new BinaryReader(buffer).readUInt8Array(); + expect(value.buffer).not.toBe(buffer.buffer); + }); + + test('readString still decodes correctly', () => { + const bytes = [...new TextEncoder().encode('héllo ☃')]; + const value = new BinaryReader(encode(bytes)).readString(); + expect(value).toBe('héllo ☃'); + }); +});