From c4d10a371125b09f505bc7d83221c267a39e1625 Mon Sep 17 00:00:00 2001 From: mustard-mh Date: Fri, 15 Oct 2021 06:47:44 +0000 Subject: [PATCH 1/2] Auth before select database --- lib/client/index.spec.ts | 12 ++++++++++++ lib/client/index.ts | 8 ++++---- lib/test-utils.ts | 7 ++++--- 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/lib/client/index.spec.ts b/lib/client/index.spec.ts index e98814d0582..8ca35e79c4d 100644 --- a/lib/client/index.spec.ts +++ b/lib/client/index.spec.ts @@ -121,6 +121,18 @@ describe('Client', () => { }); }); + describe('authWithDatabaseSelect', () => { + const database = 2 + itWithClient(TestRedisServers.PASSWORD, 'Client should auth success and select index 2', async client => { + assert.equal( + await client.ping(), + 'PONG' + ); + let info = await client.clientInfo() + assert.equal(info.db, database) + }, undefined, { database }); + }) + describe('legacyMode', () => { const client = RedisClient.create({ ...TEST_REDIS_SERVERS[TestRedisServers.OPEN], diff --git a/lib/client/index.ts b/lib/client/index.ts index 7c094e154f3..bd3ba396ced 100644 --- a/lib/client/index.ts +++ b/lib/client/index.ts @@ -180,6 +180,10 @@ export default class RedisClient const v4Commands = this.#options?.legacyMode ? this.#v4 : this, promises = []; + if (this.#options?.username || this.#options?.password) { + await v4Commands.auth(RedisClient.commandOptions({ asap: true }), this.#options); + } + if (this.#selectedDB !== 0) { promises.push(v4Commands.select(RedisClient.commandOptions({ asap: true }), this.#selectedDB)); } @@ -188,10 +192,6 @@ export default class RedisClient promises.push(v4Commands.readonly(RedisClient.commandOptions({ asap: true }))); } - if (this.#options?.username || this.#options?.password) { - promises.push(v4Commands.auth(RedisClient.commandOptions({ asap: true }), this.#options)); - } - const resubscribePromise = this.#queue.resubscribe(); if (resubscribePromise) { promises.push(resubscribePromise); diff --git a/lib/test-utils.ts b/lib/test-utils.ts index 978940ff93d..b0932172277 100644 --- a/lib/test-utils.ts +++ b/lib/test-utils.ts @@ -288,12 +288,13 @@ export function itWithClient( type: TestRedisServers, title: string, fn: (client: RedisClientType) => Promise, - options?: RedisTestOptions + options?: RedisTestOptions, + clientOptions?: RedisClientOptions<{}, {}> ): void { it(title, async function () { if (handleMinimumRedisVersion(this, options?.minimumRedisVersion)) return; - - const client = RedisClient.create(TEST_REDIS_SERVERS[type]); + + const client = RedisClient.create(Object.assign({}, TEST_REDIS_SERVERS[type], clientOptions)); await client.connect(); From 47799f5a0fdf0b604b53d55761b0152efc1391c5 Mon Sep 17 00:00:00 2001 From: leibale Date: Mon, 18 Oct 2021 17:52:51 -0400 Subject: [PATCH 2/2] fix #1681 --- lib/client/index.spec.ts | 20 ++++++++++---------- lib/client/index.ts | 38 +++++++++++++++++++++++++++++--------- lib/client/socket.ts | 17 +++++++++++++++-- lib/test-utils.ts | 14 ++++++++++---- 4 files changed, 64 insertions(+), 25 deletions(-) diff --git a/lib/client/index.spec.ts b/lib/client/index.spec.ts index 8ca35e79c4d..4d30e9be60b 100644 --- a/lib/client/index.spec.ts +++ b/lib/client/index.spec.ts @@ -119,19 +119,19 @@ describe('Client', () => { assert.equal(client.isOpen, false); }); - }); - describe('authWithDatabaseSelect', () => { - const database = 2 - itWithClient(TestRedisServers.PASSWORD, 'Client should auth success and select index 2', async client => { + itWithClient(TestRedisServers.PASSWORD, 'should execute AUTH before SELECT', async client => { assert.equal( - await client.ping(), - 'PONG' + (await client.clientInfo()).db, + 2 ); - let info = await client.clientInfo() - assert.equal(info.db, database) - }, undefined, { database }); - }) + }, { + minimumRedisVersion: [6, 2], + clientOptions: { + database: 2 + } + }); + }); describe('legacyMode', () => { const client = RedisClient.create({ diff --git a/lib/client/index.ts b/lib/client/index.ts index bd3ba396ced..8850574e716 100644 --- a/lib/client/index.ts +++ b/lib/client/index.ts @@ -177,24 +177,44 @@ export default class RedisClient #initiateSocket(): RedisSocket { const socketInitiator = async (): Promise => { - const v4Commands = this.#options?.legacyMode ? this.#v4 : this, - promises = []; - - if (this.#options?.username || this.#options?.password) { - await v4Commands.auth(RedisClient.commandOptions({ asap: true }), this.#options); - } + const promises = []; if (this.#selectedDB !== 0) { - promises.push(v4Commands.select(RedisClient.commandOptions({ asap: true }), this.#selectedDB)); + promises.push( + this.#queue.addCommand( + ['SELECT', this.#selectedDB.toString()], + { asap: true } + ) + ); } if (this.#options?.readonly) { - promises.push(v4Commands.readonly(RedisClient.commandOptions({ asap: true }))); + promises.push( + this.#queue.addCommand( + COMMANDS.READONLY.transformArguments(), + { asap: true } + ) + ); + } + + if (this.#options?.username || this.#options?.password) { + promises.push( + this.#queue.addCommand( + COMMANDS.AUTH.transformArguments({ + username: this.#options.username, + password: this.#options.password ?? '' + }), + { asap: true } + ) + ); } const resubscribePromise = this.#queue.resubscribe(); if (resubscribePromise) { promises.push(resubscribePromise); + } + + if (promises.length) { this.#tick(); } @@ -410,7 +430,7 @@ export default class RedisClient quit = this.QUIT; #tick(): void { - if (!this.#socket.isSocketExists) { + if (!this.#socket.isSocketExists || this.#socket.writableNeedDrain) { return; } diff --git a/lib/client/socket.ts b/lib/client/socket.ts index ca48ad4d542..88ae03003aa 100644 --- a/lib/client/socket.ts +++ b/lib/client/socket.ts @@ -76,6 +76,14 @@ export default class RedisSocket extends EventEmitter { return !!this.#socket; } + // `writable.writableNeedDrain` was added in v15.2.0 and therefore can't be used + // https://nodejs.org/api/stream.html#stream_writable_writableneeddrain + #writableNeedDrain = false; + + get writableNeedDrain(): boolean { + return this.#writableNeedDrain; + } + constructor(initiator?: RedisSocketInitiator, options?: RedisSocketOptions) { super(); @@ -163,7 +171,10 @@ export default class RedisSocket extends EventEmitter { this.#onSocketError(new Error('Socket closed unexpectedly')); } }) - .on('drain', () => this.emit('drain')) + .on('drain', () => { + this.#writableNeedDrain = false; + this.emit('drain'); + }) .on('data', (data: Buffer) => this.emit('data', data)); resolve(socket); @@ -198,7 +209,9 @@ export default class RedisSocket extends EventEmitter { throw new ClientClosedError(); } - return this.#socket.write(toWrite); + const wasFullyWritten = this.#socket.write(toWrite); + this.#writableNeedDrain = !wasFullyWritten; + return wasFullyWritten; } async disconnect(ignoreIsOpen = false): Promise { diff --git a/lib/test-utils.ts b/lib/test-utils.ts index b0932172277..3b823ac6eed 100644 --- a/lib/test-utils.ts +++ b/lib/test-utils.ts @@ -284,17 +284,23 @@ export function describeHandleMinimumRedisVersion(minimumVersion: PartialRedisVe }); } +interface RedisClientTestOptions extends RedisTestOptions { + clientOptions?: RedisClientOptions<{}, {}>; +} + export function itWithClient( type: TestRedisServers, title: string, fn: (client: RedisClientType) => Promise, - options?: RedisTestOptions, - clientOptions?: RedisClientOptions<{}, {}> + options?: RedisClientTestOptions ): void { it(title, async function () { if (handleMinimumRedisVersion(this, options?.minimumRedisVersion)) return; - - const client = RedisClient.create(Object.assign({}, TEST_REDIS_SERVERS[type], clientOptions)); + + const client = RedisClient.create({ + ...TEST_REDIS_SERVERS[type], + ...options?.clientOptions + }); await client.connect();