From 88b2d3cc5396cd8ea2c1377153f048b901bd5151 Mon Sep 17 00:00:00 2001 From: Neal Beeken Date: Wed, 26 Jul 2023 15:18:51 -0400 Subject: [PATCH] feat(NODE-4769)!: remove ISO-8859-1 string support from Binary --- src/binary.ts | 65 ++++++---------------- test/node/bson_node_only_tests.js | 12 ++-- test/node/bson_types_construction_tests.js | 2 +- test/node/extended_json.test.ts | 4 +- test/node/test_full_bson.js | 7 +-- 5 files changed, 27 insertions(+), 63 deletions(-) diff --git a/src/binary.ts b/src/binary.ts index 6c215f23..53c62a83 100644 --- a/src/binary.ts +++ b/src/binary.ts @@ -1,4 +1,4 @@ -import { isUint8Array } from './parser/utils'; +import { isAnyArrayBuffer, isUint8Array } from './parser/utils'; import type { EJSONOptions } from './extended_json'; import { BSONError } from './error'; import { BSON_BINARY_SUBTYPE_UUID_NEW } from './constants'; @@ -65,27 +65,19 @@ export class Binary extends BSONValue { /** * Create a new Binary instance. - * - * This constructor can accept a string as its first argument. In this case, - * this string will be encoded using ISO-8859-1, **not** using UTF-8. - * This is almost certainly not what you want. Use `new Binary(Buffer.from(string))` - * instead to convert the string to a Buffer using UTF-8 first. - * * @param buffer - a buffer object containing the binary data. * @param subType - the option binary type. */ - constructor(buffer?: string | BinarySequence, subType?: number) { + constructor(buffer?: BinarySequence, subType?: number) { super(); if ( !(buffer == null) && - !(typeof buffer === 'string') && + typeof buffer === 'string' && !ArrayBuffer.isView(buffer) && - !(buffer instanceof ArrayBuffer) && + !isAnyArrayBuffer(buffer) && !Array.isArray(buffer) ) { - throw new BSONError( - 'Binary can only be constructed from string, Buffer, TypedArray, or Array' - ); + throw new BSONError('Binary can only be constructed from Uint8Array or number[]'); } this.sub_type = subType ?? Binary.BSON_BINARY_SUBTYPE_DEFAULT; @@ -95,17 +87,9 @@ export class Binary extends BSONValue { this.buffer = ByteUtils.allocate(Binary.BUFFER_SIZE); this.position = 0; } else { - if (typeof buffer === 'string') { - // string - this.buffer = ByteUtils.fromISO88591(buffer); - } else if (Array.isArray(buffer)) { - // number[] - this.buffer = ByteUtils.fromNumberArray(buffer); - } else { - // Buffer | TypedArray | ArrayBuffer - this.buffer = ByteUtils.toLocalBufferType(buffer); - } - + this.buffer = Array.isArray(buffer) + ? ByteUtils.fromNumberArray(buffer) + : ByteUtils.toLocalBufferType(buffer); this.position = this.buffer.byteLength; } } @@ -147,12 +131,12 @@ export class Binary extends BSONValue { } /** - * Writes a buffer or string to the binary. + * Writes a buffer to the binary. * * @param sequence - a string or buffer to be written to the Binary BSON object. * @param offset - specify the binary of where to write the content. */ - write(sequence: string | BinarySequence, offset: number): void { + write(sequence: BinarySequence, offset: number): void { offset = typeof offset === 'number' ? offset : this.position; // If the buffer is to small let's extend the buffer @@ -169,10 +153,7 @@ export class Binary extends BSONValue { this.position = offset + sequence.byteLength > this.position ? offset + sequence.length : this.position; } else if (typeof sequence === 'string') { - const bytes = ByteUtils.fromISO88591(sequence); - this.buffer.set(bytes, offset); - this.position = - offset + sequence.length > this.position ? offset + sequence.length : this.position; + throw new BSONError('input cannot be string'); } } @@ -189,26 +170,12 @@ export class Binary extends BSONValue { return this.buffer.slice(position, position + length); } - /** - * Returns the value of this binary as a string. - * @param asRaw - Will skip converting to a string - * @remarks - * This is handy when calling this function conditionally for some key value pairs and not others - */ - value(asRaw?: boolean): string | BinarySequence { - asRaw = !!asRaw; - + /** returns a view of the binary value as a Uint8Array */ + value(): Uint8Array { // Optimize to serialize for the situation where the data == size of buffer - if (asRaw && this.buffer.length === this.position) { - return this.buffer; - } - - // If it's a node.js buffer object - if (asRaw) { - return this.buffer.slice(0, this.position); - } - // TODO(NODE-4361): remove binary string support, value(true) should be the default / only option here. - return ByteUtils.toISO88591(this.buffer.subarray(0, this.position)); + return this.buffer.length === this.position + ? this.buffer + : this.buffer.subarray(0, this.position); } /** the length of the binary sequence */ diff --git a/test/node/bson_node_only_tests.js b/test/node/bson_node_only_tests.js index 01a2500c..1d2ab795 100644 --- a/test/node/bson_node_only_tests.js +++ b/test/node/bson_node_only_tests.js @@ -9,7 +9,7 @@ const Buffer = require('buffer').Buffer; describe('BSON - Node only', function () { it('Should Correctly Serialize and Deserialize a big Binary object', function (done) { - var data = fs.readFileSync(path.resolve(__dirname, './data/test_gs_weird_bug.png'), 'binary'); + var data = fs.readFileSync(path.resolve(__dirname, './data/test_gs_weird_bug.png')); var bin = new Binary(); bin.write(data); var doc = { doc: bin }; @@ -26,18 +26,17 @@ describe('BSON - Node only', function () { }); describe('Full BSON - Node only', function () { - it('Should Correctly Serialize and Deserialize a big Binary object', function (done) { - var data = fs.readFileSync(path.resolve(__dirname, './data/test_gs_weird_bug.png'), 'binary'); + it('Should Correctly Serialize and Deserialize a big Binary object', function () { + var data = fs.readFileSync(path.resolve(__dirname, './data/test_gs_weird_bug.png')); var bin = new Binary(); bin.write(data); var doc = { doc: bin }; var serialized_data = BSON.serialize(doc); var deserialized_data = BSON.deserialize(serialized_data); - expect(doc.doc.value()).to.equal(deserialized_data.doc.value()); - done(); + expect(doc.doc.value()).to.deep.equal(deserialized_data.doc.value()); }); - it('Should Correctly Deserialize bson file from mongodump', function (done) { + it('Should Correctly Deserialize bson file from mongodump', function () { var data = fs.readFileSync(path.resolve(__dirname, './data/test.bson'), { encoding: null }); data = Buffer.from(data); var docs = []; @@ -46,6 +45,5 @@ describe('Full BSON - Node only', function () { bsonIndex = BSON.deserializeStream(data, bsonIndex, 1, docs, docs.length, { isArray: true }); expect(docs.length).to.equal(1); - done(); }); }); diff --git a/test/node/bson_types_construction_tests.js b/test/node/bson_types_construction_tests.js index 1a1d26e0..a98b1cee 100644 --- a/test/node/bson_types_construction_tests.js +++ b/test/node/bson_types_construction_tests.js @@ -6,7 +6,7 @@ describe('Constructing BSON types', function () { expect(() => new BSON.ObjectId()).to.not.throw(); expect(() => new BSON.BSONRegExp('aaa')).to.not.throw(); expect(() => new BSON.BSONSymbol('aaa')).to.not.throw(); - expect(() => new BSON.Binary('aaa')).to.not.throw(); + expect(() => new BSON.Binary(new Uint8Array())).to.not.throw(); expect(() => new BSON.Code(function () {})).to.not.throw(); expect(() => new BSON.Decimal128('123')).to.not.throw(); expect(() => new BSON.Double(2.3)).to.not.throw(); diff --git a/test/node/extended_json.test.ts b/test/node/extended_json.test.ts index f452da09..bbeea163 100644 --- a/test/node/extended_json.test.ts +++ b/test/node/extended_json.test.ts @@ -184,7 +184,7 @@ describe('Extended JSON', function () { it('should serialize from BSON object to EJSON object', function () { const doc = { - binary: new Binary(''), + binary: new Binary(new Uint8Array([0, 0, 0]), 0xef), code: new Code('function() {}'), dbRef: new DBRef('tests', new Int32(1), 'test'), decimal128: new Decimal128('128'), @@ -203,7 +203,7 @@ describe('Extended JSON', function () { const result = EJSON.serialize(doc, { relaxed: false }); expect(result).to.deep.equal({ - binary: { $binary: { base64: '', subType: '00' } }, + binary: { $binary: { base64: 'AAAA', subType: 'ef' } }, code: { $code: 'function() {}' }, dbRef: { $ref: 'tests', $id: { $numberInt: '1' }, $db: 'test' }, decimal128: { $numberDecimal: '128' }, diff --git a/test/node/test_full_bson.js b/test/node/test_full_bson.js index 0c9f66e0..5b5803bb 100644 --- a/test/node/test_full_bson.js +++ b/test/node/test_full_bson.js @@ -260,17 +260,16 @@ describe('Full BSON', function () { /** * @ignore */ - it('Should Correctly Serialize and Deserialize a Binary object', function (done) { + it('Should Correctly Serialize and Deserialize a Binary object', function () { var bin = new Binary(); var string = 'binstring'; for (var index = 0; index < string.length; index++) { - bin.put(string.charAt(index)); + bin.put(string[index]); } var doc = { doc: bin }; var serialized_data = BSON.serialize(doc); var deserialized_data = BSON.deserialize(serialized_data); - expect(doc.doc.value()).to.equal(deserialized_data.doc.value()); - done(); + expect(doc.doc.value()).to.deep.equal(deserialized_data.doc.value()); }); it('Should Correctly Serialize and Deserialize a ArrayBuffer object', function () {