From 2df8b57e5b030d9162cdf36eee7fc2df13dfdc43 Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Tue, 12 Apr 2022 14:57:11 +0200 Subject: [PATCH 1/4] fix(NODE-3928): dont error in response constructor --- src/cmap/commands.ts | 45 +++++++++++++++++---------------- test/unit/cmap/commands.test.js | 24 ++++++++++++++++++ 2 files changed, 47 insertions(+), 22 deletions(-) create mode 100644 test/unit/cmap/commands.test.js diff --git a/src/cmap/commands.ts b/src/cmap/commands.ts index 78e4445f25e..a49d1843198 100644 --- a/src/cmap/commands.ts +++ b/src/cmap/commands.ts @@ -484,15 +484,15 @@ export class Response { responseTo: number; opCode: number; fromCompressed?: boolean; - responseFlags: number; - cursorId: Long; - startingFrom: number; - numberReturned: number; - documents: (Document | Buffer)[]; - cursorNotFound: boolean; - queryFailure: boolean; - shardConfigStale: boolean; - awaitCapable: boolean; + responseFlags?: number; + cursorId?: Long; + startingFrom?: number; + numberReturned?: number; + documents: (Document | Buffer)[] = new Array(0); + cursorNotFound?: boolean; + queryFailure?: boolean; + shardConfigStale?: boolean; + awaitCapable?: boolean; promoteLongs: boolean; promoteValues: boolean; promoteBuffers: boolean; @@ -522,20 +522,7 @@ export class Response { this.opCode = msgHeader.opCode; this.fromCompressed = msgHeader.fromCompressed; - // Read the message body - this.responseFlags = msgBody.readInt32LE(0); - this.cursorId = new BSON.Long(msgBody.readInt32LE(4), msgBody.readInt32LE(8)); - this.startingFrom = msgBody.readInt32LE(12); - this.numberReturned = msgBody.readInt32LE(16); - - // Preallocate document array - this.documents = new Array(this.numberReturned); - // Flag values - this.cursorNotFound = (this.responseFlags & CURSOR_NOT_FOUND) !== 0; - this.queryFailure = (this.responseFlags & QUERY_FAILURE) !== 0; - this.shardConfigStale = (this.responseFlags & SHARD_CONFIG_STALE) !== 0; - this.awaitCapable = (this.responseFlags & AWAIT_CAPABLE) !== 0; this.promoteLongs = typeof this.opts.promoteLongs === 'boolean' ? this.opts.promoteLongs : true; this.promoteValues = typeof this.opts.promoteValues === 'boolean' ? this.opts.promoteValues : true; @@ -574,6 +561,20 @@ export class Response { // (See https://docs.mongodb.com/manual/reference/mongodb-wire-protocol/#wire-op-reply) this.index = 20; + // Read the message body + this.responseFlags = this.data.readInt32LE(0); + this.cursorId = new BSON.Long(this.data.readInt32LE(4), this.data.readInt32LE(8)); + this.startingFrom = this.data.readInt32LE(12); + this.numberReturned = this.data.readInt32LE(16); + + // Preallocate document array + this.documents = new Array(this.numberReturned); + + this.cursorNotFound = (this.responseFlags & CURSOR_NOT_FOUND) !== 0; + this.queryFailure = (this.responseFlags & QUERY_FAILURE) !== 0; + this.shardConfigStale = (this.responseFlags & SHARD_CONFIG_STALE) !== 0; + this.awaitCapable = (this.responseFlags & AWAIT_CAPABLE) !== 0; + // Parse Body for (let i = 0; i < this.numberReturned; i++) { bsonSize = diff --git a/test/unit/cmap/commands.test.js b/test/unit/cmap/commands.test.js new file mode 100644 index 00000000000..c88bdc6b038 --- /dev/null +++ b/test/unit/cmap/commands.test.js @@ -0,0 +1,24 @@ +const { expect } = require('chai'); +const { Response } = require('../../../src/cmap/commands'); + +describe('commands', function () { + describe('Response', function () { + describe('#constructor', function () { + context('when the message body is invalid', function () { + const message = Buffer.from([]); + const header = { + length: 0, + requestId: 0, + responseTo: 0, + opCode: 0 + }; + const body = Buffer.from([]); + + it('does not throw an exception', function () { + const response = new Response(message, header, body); + expect(response.numberReturned).to.be.undefined; + }); + }); + }); + }); +}); From c439f972784cfe71b5e2da9e54c6963c65a8862b Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Fri, 15 Apr 2022 12:55:47 +0200 Subject: [PATCH 2/4] test(NODE-3928): add additional tests --- test/unit/cmap/commands.test.js | 69 +++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/test/unit/cmap/commands.test.js b/test/unit/cmap/commands.test.js index c88bdc6b038..bc6e05ca517 100644 --- a/test/unit/cmap/commands.test.js +++ b/test/unit/cmap/commands.test.js @@ -3,6 +3,24 @@ const { Response } = require('../../../src/cmap/commands'); describe('commands', function () { describe('Response', function () { + describe('#parse', function () { + context('when the message body is invalid', function () { + const message = Buffer.from([]); + const header = { + length: 0, + requestId: 0, + responseTo: 0, + opCode: 0 + }; + const body = Buffer.from([]); + + it('throws an exception', function () { + const response = new Response(message, header, body); + expect(() => response.parse()).to.throw(); + }); + }); + }); + describe('#constructor', function () { context('when the message body is invalid', function () { const message = Buffer.from([]); @@ -15,9 +33,60 @@ describe('commands', function () { const body = Buffer.from([]); it('does not throw an exception', function () { + let error; + try { + const response = new Response(message, header, body); + } + catch(err) { + error = err; + } + expect(error).to.be.undefined; + }); + + it('initializes the documents to an empty array', function () { + const response = new Response(message, header, body); + expect(response.documents).to.be.empty; + }); + + it('does not set the responseFlags', function () { + const response = new Response(message, header, body); + expect(response.responseFlags).to.be.undefined; + }); + + it('does not set the cursorNotFound flag', function () { + const response = new Response(message, header, body); + expect(response.cursorNotFound).to.be.undefined; + }); + + it('does not set the cursorId', function () { + const response = new Response(message, header, body); + expect(response.cursorId).to.be.undefined; + }); + + it('does not set startingFrom', function () { + const response = new Response(message, header, body); + expect(response.startingFrom).to.be.undefined; + }); + + it('does not set numberReturned', function () { const response = new Response(message, header, body); expect(response.numberReturned).to.be.undefined; }); + + it('does not set queryFailure', function () { + const response = new Response(message, header, body); + expect(response.queryFailure).to.be.undefined; + }); + + it('does not set shardConfigStale', function () { + const response = new Response(message, header, body); + expect(response.shardConfigStale).to.be.undefined; + }); + + it('does not set awaitCapable', function () { + const response = new Response(message, header, body); + expect(response.awaitCapable).to.be.undefined; + }); }); }); }); From e60377652b878265d1ae31d66d9e7caf43d6f0fa Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Fri, 15 Apr 2022 13:46:10 +0200 Subject: [PATCH 3/4] fix(NODE-3928): fix lint errors --- test/unit/cmap/commands.test.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/unit/cmap/commands.test.js b/test/unit/cmap/commands.test.js index bc6e05ca517..0a1ef4ccd28 100644 --- a/test/unit/cmap/commands.test.js +++ b/test/unit/cmap/commands.test.js @@ -35,9 +35,8 @@ describe('commands', function () { it('does not throw an exception', function () { let error; try { - const response = new Response(message, header, body); - } - catch(err) { + new Response(message, header, body); + } catch (err) { error = err; } expect(error).to.be.undefined; From 70d5dfcaa1d6f5afb615cb3b1e127abb06c9deba Mon Sep 17 00:00:00 2001 From: Durran Jordan Date: Wed, 20 Apr 2022 13:44:02 +0200 Subject: [PATCH 4/4] test(NODE-3928): add additional test cases --- test/unit/cmap/commands.test.js | 41 ++++++++++++++++++++++++--------- 1 file changed, 30 insertions(+), 11 deletions(-) diff --git a/test/unit/cmap/commands.test.js b/test/unit/cmap/commands.test.js index 0a1ef4ccd28..65993f3c336 100644 --- a/test/unit/cmap/commands.test.js +++ b/test/unit/cmap/commands.test.js @@ -5,18 +5,37 @@ describe('commands', function () { describe('Response', function () { describe('#parse', function () { context('when the message body is invalid', function () { - const message = Buffer.from([]); - const header = { - length: 0, - requestId: 0, - responseTo: 0, - opCode: 0 - }; - const body = Buffer.from([]); + context('when the buffer is empty', function () { + const message = Buffer.from([]); + const header = { + length: 0, + requestId: 0, + responseTo: 0, + opCode: 0 + }; + const body = Buffer.from([]); - it('throws an exception', function () { - const response = new Response(message, header, body); - expect(() => response.parse()).to.throw(); + it('throws an exception', function () { + const response = new Response(message, header, body); + expect(() => response.parse()).to.throw(RangeError, /outside buffer bounds/); + }); + }); + + context('when numReturned is invalid', function () { + const message = Buffer.from([]); + const header = { + length: 0, + requestId: 0, + responseTo: 0, + opCode: 0 + }; + const body = Buffer.alloc(5 * 4); + body.writeInt32LE(-1, 16); + + it('throws an exception', function () { + const response = new Response(message, header, body); + expect(() => response.parse()).to.throw(RangeError, /Invalid array length/); + }); }); }); });