From 7d52f867a0267692c221d978fbb086317cc24742 Mon Sep 17 00:00:00 2001 From: Martin Cizek Date: Wed, 16 Sep 2020 14:06:26 +0200 Subject: [PATCH 01/12] Client support multiple servers --- docs/client.md | 4 ++-- lib/client/client.js | 29 ++++++++++++++++++++--------- 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/docs/client.md b/docs/client.md index 3aee755..f953e59 100644 --- a/docs/client.md +++ b/docs/client.md @@ -14,7 +14,7 @@ The code to create a new client looks like: var ldap = require('ldapjs'); var client = ldap.createClient({ - url: 'ldap://127.0.0.1:1389' + url: 'ldap://127.0.0.1:1389,127.0.0.2:1389' }); You can use `ldap://` or `ldaps://`; the latter would connect over SSL (note @@ -24,7 +24,7 @@ client is: |Attribute |Description | |---------------|-----------------------------------------------------------| -|url |A valid LDAP URL (proto/host/port only) | +|url |A valid LDAP URL(s) (proto/host(s)/port(s) | |socketPath |Socket path if using AF\_UNIX sockets | |log |A compatible logger instance (Default: no-op logger) | |timeout |Milliseconds client should let operations live for before timing out (Default: Infinity)| diff --git a/lib/client/client.js b/lib/client/client.js index b254d50..45a9bee 100644 --- a/lib/client/client.js +++ b/lib/client/client.js @@ -110,12 +110,17 @@ function Client (options) { EventEmitter.call(this, options) var self = this - var _url - if (options.url) { _url = url.parse(options.url) } - this.host = _url ? _url.hostname : undefined - this.port = _url ? _url.port : false - this.secure = _url ? _url.secure : false - this.url = _url + this.servers = [] + if (options.url) { + var [, protocol, hosts, path] = options.url.match(/^([a-zA-Z]+:\/\/)([^/]*)(.*)/) + + hosts.split(',').forEach((host) => { + this.servers.push(url.parse(`${protocol}${host}${path}`)) + }) + } + + // select random server at the start + this.nextServer = Math.floor(Math.random() * this.servers.length) this.tlsOptions = options.tlsOptions this.socketPath = options.socketPath || false @@ -792,6 +797,8 @@ Client.prototype.connect = function connect () { // Establish basic socket connection function connectSocket (cb) { + self.nextServer = (self.nextServer + 1) % self.servers.length + self.url = self.servers[self.nextServer] cb = once(cb) function onResult (err, res) { @@ -820,12 +827,13 @@ Client.prototype.connect = function connect () { setupClient(cb) } - var port = (self.port || self.socketPath) + var port = (self.servers.length && self.servers[self.nextServer].port) || self.socketPath + var hostname = (self.servers.length && self.servers[self.nextServer].hostname) || undefined if (self.secure) { - socket = tls.connect(port, self.host, self.tlsOptions) + socket = tls.connect(port, hostname, self.tlsOptions) socket.once('secureConnect', onConnect) } else { - socket = net.connect(port, self.host) + socket = net.connect(port, hostname) socket.once('connect', onConnect) } socket.once('error', onResult) @@ -975,6 +983,9 @@ Client.prototype.connect = function connect () { maxDelay: this.reconnect.maxDelay }) failAfter = this.reconnect.failAfter + if (this.servers.length > 1 && failAfter) { + failAfter *= this.servers.length + } } else { retry = backoff.exponential({ initialDelay: 1, From 3ca2c265da553eb3b323dd16a696e1179d7b5a1c Mon Sep 17 00:00:00 2001 From: Martin Cizek Date: Thu, 17 Sep 2020 06:28:03 +0200 Subject: [PATCH 02/12] Support for arrays in url parameter --- docs/client.md | 2 +- lib/client/client.js | 14 +++++++++++--- lib/client/index.js | 2 +- test/client.test.js | 3 +-- 4 files changed, 14 insertions(+), 7 deletions(-) diff --git a/docs/client.md b/docs/client.md index f953e59..c7e2508 100644 --- a/docs/client.md +++ b/docs/client.md @@ -14,7 +14,7 @@ The code to create a new client looks like: var ldap = require('ldapjs'); var client = ldap.createClient({ - url: 'ldap://127.0.0.1:1389,127.0.0.2:1389' + url: ['ldap://127.0.0.1:1389', 'ldap://127.0.0.2:1389'] }); You can use `ldap://` or `ldaps://`; the latter would connect over SSL (note diff --git a/lib/client/client.js b/lib/client/client.js index 45a9bee..addc89a 100644 --- a/lib/client/client.js +++ b/lib/client/client.js @@ -112,10 +112,18 @@ function Client (options) { var self = this this.servers = [] if (options.url) { - var [, protocol, hosts, path] = options.url.match(/^([a-zA-Z]+:\/\/)([^/]*)(.*)/) + var urls = options.url - hosts.split(',').forEach((host) => { - this.servers.push(url.parse(`${protocol}${host}${path}`)) + if (typeof options.url === 'string') { + var [, protocol, hosts, path] = options.url.match(/^([a-zA-Z]+:\/\/)([^/]*)(.*)/) + + urls = hosts.split(',').map((host) => { + return `${protocol}${host}${path}` + }) + } + + this.servers = urls.map((host) => { + return url.parse(host) }) } diff --git a/lib/client/index.js b/lib/client/index.js index 0c23052..fb9db3a 100644 --- a/lib/client/index.js +++ b/lib/client/index.js @@ -7,7 +7,7 @@ module.exports = { Client: Client, createClient: function createClient (options) { if (isObject(options) === false) throw TypeError('options (object) required') - if (options.url && typeof options.url !== 'string') throw TypeError('options.url (string) required') + if (options.url && typeof options.url !== 'string' && !Array.isArray(options.url)) throw TypeError('options.url (string|array) required') if (options.socketPath && typeof options.socketPath !== 'string') throw TypeError('options.socketPath must be a string') if ((options.url && options.socketPath) || !(options.url || options.socketPath)) throw TypeError('options.url ^ options.socketPath (String) required') if (!options.log) options.log = logger diff --git a/test/client.test.js b/test/client.test.js index bd9b9a2..51cf6af 100644 --- a/test/client.test.js +++ b/test/client.test.js @@ -341,9 +341,8 @@ tap.test('createClient', t => { }) t.test('url must be a string', async t => { - const match = /options\.url \(string\) required/ + const match = /options\.url \(string\|array\) required/ t.throws(() => ldap.createClient({ url: {} }), match) - t.throws(() => ldap.createClient({ url: [] }), match) t.throws(() => ldap.createClient({ url: 42 }), match) }) From f9ae5c12d82cc8f7eb0adecef5851e96f9178459 Mon Sep 17 00:00:00 2001 From: Martin Cizek Date: Thu, 17 Sep 2020 07:05:21 +0200 Subject: [PATCH 03/12] Add test --- lib/client/client.js | 4 +--- test/client.test.js | 25 ++++++++++++++++++++++++- 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/lib/client/client.js b/lib/client/client.js index addc89a..720cec0 100644 --- a/lib/client/client.js +++ b/lib/client/client.js @@ -122,9 +122,7 @@ function Client (options) { }) } - this.servers = urls.map((host) => { - return url.parse(host) - }) + this.servers = urls.map(url.parse) } // select random server at the start diff --git a/test/client.test.js b/test/client.test.js index 51cf6af..2bfd228 100644 --- a/test/client.test.js +++ b/test/client.test.js @@ -340,7 +340,7 @@ tap.test('createClient', t => { t.throws(() => ldap.createClient(42), match) }) - t.test('url must be a string', async t => { + t.test('url must be a string or array', async t => { const match = /options\.url \(string\|array\) required/ t.throws(() => ldap.createClient({ url: {} }), match) t.throws(() => ldap.createClient({ url: 42 }), match) @@ -378,6 +378,29 @@ tap.test('createClient', t => { } }) + t.test('url array is correctly assigned', async t => { + getPort().then(function (unusedPortNumber) { + const client = ldap.createClient({ + url: [ + `ldap://0.0.0.0:${unusedPortNumber}`, + `ldap://0.0.0.1:${unusedPortNumber}` + ] + }) + + t.equal(client.servers.length, 2) + }) + }) + + t.test('url string is correctly parsed and assigned', async t => { + getPort().then(function (unusedPortNumber) { + const client = ldap.createClient({ + url: `ldap://0.0.0.0:${unusedPortNumber},0.0.0.1:${unusedPortNumber}` + }) + + t.equal(client.servers.length, 2) + }) + }) + // TODO: this test is really flaky. It would be better if we could validate // the options _withouth_ having to connect to a server. // t.test('attaches a child function to logger', async t => { From 543cb976f305b00f73a4c25e32046114c8aef5c9 Mon Sep 17 00:00:00 2001 From: Martin Cizek Date: Thu, 17 Sep 2020 07:35:53 +0200 Subject: [PATCH 04/12] Add connectTimeout to test --- test/client.test.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/client.test.js b/test/client.test.js index 2bfd228..9694ef0 100644 --- a/test/client.test.js +++ b/test/client.test.js @@ -384,7 +384,8 @@ tap.test('createClient', t => { url: [ `ldap://0.0.0.0:${unusedPortNumber}`, `ldap://0.0.0.1:${unusedPortNumber}` - ] + ], + connectTimeout: 1 }) t.equal(client.servers.length, 2) @@ -394,7 +395,8 @@ tap.test('createClient', t => { t.test('url string is correctly parsed and assigned', async t => { getPort().then(function (unusedPortNumber) { const client = ldap.createClient({ - url: `ldap://0.0.0.0:${unusedPortNumber},0.0.0.1:${unusedPortNumber}` + url: `ldap://0.0.0.0:${unusedPortNumber},0.0.0.1:${unusedPortNumber}`, + connectTimeout: 1 }) t.equal(client.servers.length, 2) From 23e44b2959f18d91ca2bc683a19735ec4878367a Mon Sep 17 00:00:00 2001 From: Martin Cizek Date: Thu, 17 Sep 2020 16:46:34 +0200 Subject: [PATCH 05/12] cr --- docs/client.md | 2 +- lib/client/client.js | 48 +++++++++++++++++++++----------------------- test/client.test.js | 13 +----------- 3 files changed, 25 insertions(+), 38 deletions(-) diff --git a/docs/client.md b/docs/client.md index c7e2508..024a02b 100644 --- a/docs/client.md +++ b/docs/client.md @@ -24,7 +24,7 @@ client is: |Attribute |Description | |---------------|-----------------------------------------------------------| -|url |A valid LDAP URL(s) (proto/host(s)/port(s) | +|url |A string or array of valid LDAP URL(s) (proto/host/port) | |socketPath |Socket path if using AF\_UNIX sockets | |log |A compatible logger instance (Default: no-op logger) | |timeout |Milliseconds client should let operations live for before timing out (Default: Infinity)| diff --git a/lib/client/client.js b/lib/client/client.js index 720cec0..689f7f1 100644 --- a/lib/client/client.js +++ b/lib/client/client.js @@ -110,23 +110,14 @@ function Client (options) { EventEmitter.call(this, options) var self = this - this.servers = [] - if (options.url) { - var urls = options.url - - if (typeof options.url === 'string') { - var [, protocol, hosts, path] = options.url.match(/^([a-zA-Z]+:\/\/)([^/]*)(.*)/) - - urls = hosts.split(',').map((host) => { - return `${protocol}${host}${path}` - }) - } - - this.servers = urls.map(url.parse) - } - + this.urls = options.url ? [].concat(options.url).map(url.parse) : [] // select random server at the start - this.nextServer = Math.floor(Math.random() * this.servers.length) + this._nextServer = Math.floor(Math.random() * this.urls.length) + // updated in connectSocket() after each connect + this.host = undefined + this.port = false + this.secure = false + this.url = undefined this.tlsOptions = options.tlsOptions this.socketPath = options.socketPath || false @@ -803,8 +794,15 @@ Client.prototype.connect = function connect () { // Establish basic socket connection function connectSocket (cb) { - self.nextServer = (self.nextServer + 1) % self.servers.length - self.url = self.servers[self.nextServer] + self._nextServer = (self._nextServer + 1) % self.urls.length + self.url = self.urls[self._nextServer] + + if (self.url) { + self.host = url.hostname + self.port = url.port + self.secure = url.secure + } + cb = once(cb) function onResult (err, res) { @@ -833,8 +831,8 @@ Client.prototype.connect = function connect () { setupClient(cb) } - var port = (self.servers.length && self.servers[self.nextServer].port) || self.socketPath - var hostname = (self.servers.length && self.servers[self.nextServer].hostname) || undefined + var port = (self.urls.length && self.urls[self._nextServer].port) || self.socketPath + var hostname = (self.urls.length && self.urls[self._nextServer].hostname) || undefined if (self.secure) { socket = tls.connect(port, hostname, self.tlsOptions) socket.once('secureConnect', onConnect) @@ -843,7 +841,7 @@ Client.prototype.connect = function connect () { socket.once('connect', onConnect) } socket.once('error', onResult) - initSocket() + initSocket(self.url) // Setup connection timeout handling, if desired if (self.connectTimeout) { @@ -858,9 +856,9 @@ Client.prototype.connect = function connect () { } // Initialize socket events and LDAP parser. - function initSocket () { + function initSocket (url) { tracker = messageTrackerFactory({ - id: self.url ? self.url.href : self.socketPath, + id: url ? url.href : self.socketPath, parser: new Parser({ log: log }) }) @@ -989,8 +987,8 @@ Client.prototype.connect = function connect () { maxDelay: this.reconnect.maxDelay }) failAfter = this.reconnect.failAfter - if (this.servers.length > 1 && failAfter) { - failAfter *= this.servers.length + if (this.urls.length > 1 && failAfter) { + failAfter *= this.urls.length } } else { retry = backoff.exponential({ diff --git a/test/client.test.js b/test/client.test.js index 9694ef0..57a271b 100644 --- a/test/client.test.js +++ b/test/client.test.js @@ -388,18 +388,7 @@ tap.test('createClient', t => { connectTimeout: 1 }) - t.equal(client.servers.length, 2) - }) - }) - - t.test('url string is correctly parsed and assigned', async t => { - getPort().then(function (unusedPortNumber) { - const client = ldap.createClient({ - url: `ldap://0.0.0.0:${unusedPortNumber},0.0.0.1:${unusedPortNumber}`, - connectTimeout: 1 - }) - - t.equal(client.servers.length, 2) + t.equal(client.urls.length, 2) }) }) From f0c864ca85092974249f0d1b87827abeb838c6e7 Mon Sep 17 00:00:00 2001 From: Martin Cizek Date: Thu, 17 Sep 2020 16:58:16 +0200 Subject: [PATCH 06/12] Revert redundant changes --- lib/client/client.js | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/lib/client/client.js b/lib/client/client.js index 689f7f1..dffafe4 100644 --- a/lib/client/client.js +++ b/lib/client/client.js @@ -798,9 +798,9 @@ Client.prototype.connect = function connect () { self.url = self.urls[self._nextServer] if (self.url) { - self.host = url.hostname - self.port = url.port - self.secure = url.secure + self.host = self.url.hostname + self.port = self.url.port + self.secure = self.url.secure } cb = once(cb) @@ -831,13 +831,12 @@ Client.prototype.connect = function connect () { setupClient(cb) } - var port = (self.urls.length && self.urls[self._nextServer].port) || self.socketPath - var hostname = (self.urls.length && self.urls[self._nextServer].hostname) || undefined + var port = (self.port || self.socketPath) if (self.secure) { - socket = tls.connect(port, hostname, self.tlsOptions) + socket = tls.connect(port, self.host, self.tlsOptions) socket.once('secureConnect', onConnect) } else { - socket = net.connect(port, hostname) + socket = net.connect(port, self.host) socket.once('connect', onConnect) } socket.once('error', onResult) From d32a765ee970b2db8153f8db5e3494dc3ffa1738 Mon Sep 17 00:00:00 2001 From: Martin Cizek Date: Thu, 17 Sep 2020 17:49:58 +0200 Subject: [PATCH 07/12] cr --- lib/client/client.js | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/lib/client/client.js b/lib/client/client.js index dffafe4..2862d76 100644 --- a/lib/client/client.js +++ b/lib/client/client.js @@ -795,13 +795,7 @@ Client.prototype.connect = function connect () { // Establish basic socket connection function connectSocket (cb) { self._nextServer = (self._nextServer + 1) % self.urls.length - self.url = self.urls[self._nextServer] - - if (self.url) { - self.host = self.url.hostname - self.port = self.url.port - self.secure = self.url.secure - } + var server = self.urls[self._nextServer] cb = once(cb) @@ -831,16 +825,17 @@ Client.prototype.connect = function connect () { setupClient(cb) } - var port = (self.port || self.socketPath) + var port = (server && server.port) || self.socketPath + var host = server && server.hostname if (self.secure) { - socket = tls.connect(port, self.host, self.tlsOptions) + socket = tls.connect(port, host, self.tlsOptions) socket.once('secureConnect', onConnect) } else { - socket = net.connect(port, self.host) + socket = net.connect(port, host) socket.once('connect', onConnect) } socket.once('error', onResult) - initSocket(self.url) + initSocket(server) // Setup connection timeout handling, if desired if (self.connectTimeout) { @@ -976,6 +971,13 @@ Client.prototype.connect = function connect () { self.emit('socketTimeout') socket.end() }) + + var server = self.urls[self._nextServer] + if (server) { + self.host = server.hostname + self.port = server.port + self.secure = server.secure + } } var retry From f3804bbca6a501649819d1b28e195c8d1a06d6a4 Mon Sep 17 00:00:00 2001 From: Martin Cizek Date: Fri, 18 Sep 2020 06:15:25 +0200 Subject: [PATCH 08/12] cr --- lib/client/client.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/client/client.js b/lib/client/client.js index 2862d76..2bb429c 100644 --- a/lib/client/client.js +++ b/lib/client/client.js @@ -115,8 +115,8 @@ function Client (options) { this._nextServer = Math.floor(Math.random() * this.urls.length) // updated in connectSocket() after each connect this.host = undefined - this.port = false - this.secure = false + this.port = undefined + this.secure = undefined this.url = undefined this.tlsOptions = options.tlsOptions this.socketPath = options.socketPath || false @@ -827,7 +827,7 @@ Client.prototype.connect = function connect () { var port = (server && server.port) || self.socketPath var host = server && server.hostname - if (self.secure) { + if (server && server.secure) { socket = tls.connect(port, host, self.tlsOptions) socket.once('secureConnect', onConnect) } else { @@ -996,7 +996,7 @@ Client.prototype.connect = function connect () { initialDelay: 1, maxDelay: 2 }) - failAfter = 1 + failAfter = this.urls.length || 1 } retry.failAfter(failAfter) From 85824f8980667226290014aa328838c16a2f1387 Mon Sep 17 00:00:00 2001 From: Martin Cizek Date: Fri, 18 Sep 2020 07:48:12 +0200 Subject: [PATCH 09/12] Change test ip address. Co-authored-by: Tony Brix --- test/client.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/client.test.js b/test/client.test.js index 57a271b..910adf0 100644 --- a/test/client.test.js +++ b/test/client.test.js @@ -382,8 +382,8 @@ tap.test('createClient', t => { getPort().then(function (unusedPortNumber) { const client = ldap.createClient({ url: [ - `ldap://0.0.0.0:${unusedPortNumber}`, - `ldap://0.0.0.1:${unusedPortNumber}` + `ldap://127.0.0.1:${unusedPortNumber}`, + `ldap://127.0.0.2:${unusedPortNumber}` ], connectTimeout: 1 }) From c861135f808e2aaa02eff69cf9921d3d97fce3d8 Mon Sep 17 00:00:00 2001 From: Martin Cizek Date: Fri, 18 Sep 2020 15:31:58 +0200 Subject: [PATCH 10/12] Don't randomize first server --- lib/client/client.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/client/client.js b/lib/client/client.js index 2bb429c..e03d5d2 100644 --- a/lib/client/client.js +++ b/lib/client/client.js @@ -111,8 +111,7 @@ function Client (options) { var self = this this.urls = options.url ? [].concat(options.url).map(url.parse) : [] - // select random server at the start - this._nextServer = Math.floor(Math.random() * this.urls.length) + this._nextServer = 0 // updated in connectSocket() after each connect this.host = undefined this.port = undefined From cf29972990651d202615900fb97c1df125f37b5c Mon Sep 17 00:00:00 2001 From: Martin Cizek Date: Fri, 18 Sep 2020 15:36:12 +0200 Subject: [PATCH 11/12] fix --- lib/client/client.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/client/client.js b/lib/client/client.js index e03d5d2..daf9314 100644 --- a/lib/client/client.js +++ b/lib/client/client.js @@ -793,8 +793,8 @@ Client.prototype.connect = function connect () { // Establish basic socket connection function connectSocket (cb) { - self._nextServer = (self._nextServer + 1) % self.urls.length var server = self.urls[self._nextServer] + self._nextServer = (self._nextServer + 1) % self.urls.length cb = once(cb) From ed15ecf28118b920406f23c0dfcbd339d7d619d4 Mon Sep 17 00:00:00 2001 From: Martin Cizek Date: Fri, 18 Sep 2020 16:21:55 +0200 Subject: [PATCH 12/12] Add readme information about how the client picks among multiple servers --- docs/client.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/client.md b/docs/client.md index 024a02b..a12fcb8 100644 --- a/docs/client.md +++ b/docs/client.md @@ -34,6 +34,13 @@ client is: |strictDN |Force strict DN parsing for client methods (Default is true)| |reconnect |Try to reconnect when the connection gets lost (Default is false)| +### url +This parameter takes a single connection string or an array of connection strings +as an input. In case an array is provided, the client tries to connect to the +servers in given order. To achieve random server strategy (e.g. to distribute +the load among the servers), please shuffle the array before passing it as an +argument. + ### Note On Logger A passed in logger is expected to conform to the [Bunyan](https://www.npmjs.com/package/bunyan)