From 0dafaf3a9bd3e99a65969f9b106827a9ff0a3abb Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Tue, 12 Nov 2024 23:42:44 -0500 Subject: [PATCH 1/2] fix(NODE-6536): Binary.read never returns number[] and reads beyond content --- src/binary.ts | 17 +++++------ src/parser/deserializer.ts | 62 +++++++++++--------------------------- test/node/binary.test.ts | 43 +++++++++++++++++++++++++- test/types/bson.test-d.ts | 2 ++ 4 files changed, 69 insertions(+), 55 deletions(-) diff --git a/src/binary.ts b/src/binary.ts index 6a892b00..d60dd2fa 100644 --- a/src/binary.ts +++ b/src/binary.ts @@ -61,9 +61,9 @@ export class Binary extends BSONValue { /** User BSON type */ static readonly SUBTYPE_USER_DEFINED = 128; - buffer!: Uint8Array; - sub_type!: number; - position!: number; + public buffer: Uint8Array; + public sub_type: number; + public position: number; /** * Create a new Binary instance. @@ -160,16 +160,15 @@ export class Binary extends BSONValue { } /** - * Reads **length** bytes starting at **position**. + * Returns a view of **length** bytes starting at **position**. * * @param position - read from the given position in the Binary. * @param length - the number of bytes to read. */ - read(position: number, length: number): BinarySequence { + read(position: number, length: number): Uint8Array { length = length && length > 0 ? length : this.position; - - // Let's return the data based on the type we have - return this.buffer.slice(position, position + length); + const end = position + length; + return this.buffer.subarray(position, end > this.position ? this.position : end); } /** returns a view of the binary value as a Uint8Array */ @@ -219,7 +218,7 @@ export class Binary extends BSONValue { toUUID(): UUID { if (this.sub_type === Binary.SUBTYPE_UUID) { - return new UUID(this.buffer.slice(0, this.position)); + return new UUID(this.buffer.subarray(0, this.position)); } throw new BSONError( diff --git a/src/parser/deserializer.ts b/src/parser/deserializer.ts index 165a529c..97bdd6bf 100644 --- a/src/parser/deserializer.ts +++ b/src/parser/deserializer.ts @@ -296,7 +296,7 @@ function deserializeObject( // We have a raw value if (raw) { - value = buffer.slice(index, index + objectSize); + value = buffer.subarray(index, index + objectSize); } else { let objectOptions = options; if (!globalUTFValidation) { @@ -374,52 +374,24 @@ function deserializeObject( if (binarySize > buffer.byteLength) throw new BSONError('Binary type size larger than document size'); - // Decode as raw Buffer object if options specifies it - if (buffer['slice'] != null) { - // If we have subtype 2 skip the 4 bytes for the size - if (subType === Binary.SUBTYPE_BYTE_ARRAY) { - binarySize = NumberUtils.getInt32LE(buffer, index); - index += 4; - if (binarySize < 0) - throw new BSONError('Negative binary type element size found for subtype 0x02'); - if (binarySize > totalBinarySize - 4) - throw new BSONError('Binary type with subtype 0x02 contains too long binary size'); - if (binarySize < totalBinarySize - 4) - throw new BSONError('Binary type with subtype 0x02 contains too short binary size'); - } + // If we have subtype 2 skip the 4 bytes for the size + if (subType === Binary.SUBTYPE_BYTE_ARRAY) { + binarySize = NumberUtils.getInt32LE(buffer, index); + index += 4; + if (binarySize < 0) + throw new BSONError('Negative binary type element size found for subtype 0x02'); + if (binarySize > totalBinarySize - 4) + throw new BSONError('Binary type with subtype 0x02 contains too long binary size'); + if (binarySize < totalBinarySize - 4) + throw new BSONError('Binary type with subtype 0x02 contains too short binary size'); + } - if (promoteBuffers && promoteValues) { - value = ByteUtils.toLocalBufferType(buffer.slice(index, index + binarySize)); - } else { - value = new Binary(buffer.slice(index, index + binarySize), subType); - if (subType === constants.BSON_BINARY_SUBTYPE_UUID_NEW && UUID.isValid(value)) { - value = value.toUUID(); - } - } + if (promoteBuffers && promoteValues) { + value = ByteUtils.toLocalBufferType(buffer.subarray(index, index + binarySize)); } else { - // If we have subtype 2 skip the 4 bytes for the size - if (subType === Binary.SUBTYPE_BYTE_ARRAY) { - binarySize = NumberUtils.getInt32LE(buffer, index); - index += 4; - if (binarySize < 0) - throw new BSONError('Negative binary type element size found for subtype 0x02'); - if (binarySize > totalBinarySize - 4) - throw new BSONError('Binary type with subtype 0x02 contains too long binary size'); - if (binarySize < totalBinarySize - 4) - throw new BSONError('Binary type with subtype 0x02 contains too short binary size'); - } - - if (promoteBuffers && promoteValues) { - value = ByteUtils.allocateUnsafe(binarySize); - // Copy the data - for (i = 0; i < binarySize; i++) { - value[i] = buffer[index + i]; - } - } else { - value = new Binary(buffer.slice(index, index + binarySize), subType); - if (subType === constants.BSON_BINARY_SUBTYPE_UUID_NEW && UUID.isValid(value)) { - value = value.toUUID(); - } + value = new Binary(buffer.subarray(index, index + binarySize), subType); + if (subType === constants.BSON_BINARY_SUBTYPE_UUID_NEW && UUID.isValid(value)) { + value = value.toUUID(); } } diff --git a/test/node/binary.test.ts b/test/node/binary.test.ts index 2adffc79..1f94a619 100644 --- a/test/node/binary.test.ts +++ b/test/node/binary.test.ts @@ -1,6 +1,6 @@ import { expect } from 'chai'; import * as vm from 'node:vm'; -import { Binary, BSON } from '../register-bson'; +import { __isWeb__, Binary, BSON } from '../register-bson'; import * as util from 'node:util'; describe('class Binary', () => { @@ -81,6 +81,46 @@ describe('class Binary', () => { }); }); + describe('read()', function () { + const LocalBuffer = __isWeb__ ? Uint8Array : Buffer; + + it('reads a single byte from the buffer', function () { + const binary = new Binary(); + binary.put(0x42); + expect(binary.read(0, 1)).to.deep.equal(LocalBuffer.of(0x42)); + }); + + it('does not read beyond binary.position', function () { + const binary = new Binary(); + binary.put(0x42); + expect(binary.buffer.byteLength).to.equal(256); + expect(binary.read(0, 10)).to.deep.equal(LocalBuffer.of(0x42)); + }); + + it('reads a single byte from the buffer at the given position', function () { + const binary = new Binary(); + binary.put(0x42); + binary.put(0x43); + binary.put(0x44); + expect(binary.read(1, 1)).to.deep.equal(LocalBuffer.of(0x43)); + }); + + it('reads nothing if the position is out of bounds', function () { + const binary = new Binary(); + expect(binary.read(1, 0)).to.have.lengthOf(0); + }); + + it('sets length to position if not provided', function () { + const binary = new Binary(); + binary.put(0x42); + binary.put(0x42); + binary.put(0x42); + expect(binary.position).to.equal(3); + // @ts-expect-error: checking behavior TS doesn't support + expect(binary.read(0)).to.have.lengthOf(3); + }); + }); + context('inspect()', () => { it(`when value is default returns "Binary.createFromBase64('', 0)"`, () => { expect(util.inspect(new Binary())).to.equal(`Binary.createFromBase64('', 0)`); @@ -93,6 +133,7 @@ describe('class Binary', () => { }); it(`when value is default with a subtype returns "Binary.createFromBase64('', 35)"`, () => { + // @ts-expect-error: check null is handled the same as undefined expect(util.inspect(new Binary(null, 0x23))).to.equal(`Binary.createFromBase64('', 35)`); }); diff --git a/test/types/bson.test-d.ts b/test/types/bson.test-d.ts index 88e90aef..dbaacecd 100644 --- a/test/types/bson.test-d.ts +++ b/test/types/bson.test-d.ts @@ -85,3 +85,5 @@ expectNotDeprecated(new ObjectId(42 as string | number)); // Timestamp accepts timestamp because constructor allows: {i:number, t:number} new Timestamp(new Timestamp(0n)) + +expectType<(position: number, length: number) => Uint8Array>(Binary.prototype.read); From 7bed6c05bdb8ab3860c96d1ce88a619d71b0dbdb Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Wed, 13 Nov 2024 10:02:08 -0500 Subject: [PATCH 2/2] docs: add documentation to Binary properties --- src/binary.ts | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/src/binary.ts b/src/binary.ts index d60dd2fa..d3b496c3 100644 --- a/src/binary.ts +++ b/src/binary.ts @@ -61,8 +61,47 @@ export class Binary extends BSONValue { /** User BSON type */ static readonly SUBTYPE_USER_DEFINED = 128; + /** + * The bytes of the Binary value. + * + * The format of a Binary value in BSON is defined as: + * ```txt + * binary ::= int32 subtype (byte*) + * ``` + * + * This `buffer` is the "(byte*)" segment. + * + * Unless the value is subtype 2, then deserialize will read the first 4 bytes as an int32 and set this to the remaining bytes. + * + * ```txt + * binary ::= int32 unsigned_byte(2) int32 (byte*) + * ``` + * + * @see https://bsonspec.org/spec.html + */ public buffer: Uint8Array; + /** + * The binary subtype. + * + * Current defined values are: + * + * - `unsigned_byte(0)` Generic binary subtype + * - `unsigned_byte(1)` Function + * - `unsigned_byte(2)` Binary (Deprecated) + * - `unsigned_byte(3)` UUID (Deprecated) + * - `unsigned_byte(4)` UUID + * - `unsigned_byte(5)` MD5 + * - `unsigned_byte(6)` Encrypted BSON value + * - `unsigned_byte(7)` Compressed BSON column + * - `unsigned_byte(8)` Sensitive + * - `unsigned_byte(9)` Vector + * - `unsigned_byte(128)` - `unsigned_byte(255)` User defined + */ public sub_type: number; + /** + * The Binary's `buffer` can be larger than the Binary's content. + * This property is used to determine where the content ends in the buffer. + */ public position: number; /**