From e2d516f6bb95f60de103e5c6fb812b8522f09134 Mon Sep 17 00:00:00 2001 From: James Sumners Date: Fri, 30 Jun 2023 08:13:54 -0400 Subject: [PATCH] Address crash for unmatched server responses --- lib/client/client.js | 8 +++- test/issue-890.test.js | 103 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 110 insertions(+), 1 deletion(-) create mode 100644 test/issue-890.test.js diff --git a/lib/client/client.js b/lib/client/client.js index ff81b01..d24f169 100644 --- a/lib/client/client.js +++ b/lib/client/client.js @@ -881,7 +881,13 @@ Client.prototype.connect = function connect () { // object. tracker.parser.on('message', function onMessage (message) { message.connection = self._socket - const { message: trackedMessage, callback } = tracker.fetch(message.messageId) + const trackedObject = tracker.fetch(message.messageId) + if (!trackedObject) { + log.error({ message: message.pojo }, 'unmatched server message received') + return false + } + + const { message: trackedMessage, callback } = trackedObject if (!callback) { log.error({ message: message.pojo }, 'unsolicited message') diff --git a/test/issue-890.test.js b/test/issue-890.test.js new file mode 100644 index 0000000..e0f8ba1 --- /dev/null +++ b/test/issue-890.test.js @@ -0,0 +1,103 @@ +'use strict' + +// This test is complicated. It must simulate a server sending an unsolicited, +// or a mismatched, message in order to force the client's internal message +// tracker to try and find a corresponding sent message that does not exist. +// In order to do that, we need to set a high test timeout and wait for the +// error message to be logged. + +const tap = require('tap') +const ldapjs = require('../') +const { SearchResultEntry } = require('@ldapjs/messages') +const server = ldapjs.createServer() +const SUFFIX = '' + +tap.timeout = 10000 + +server.bind(SUFFIX, (res, done) => { + res.end() + return done() +}) + +server.search(SUFFIX, (req, res, done) => { + const result = new SearchResultEntry({ + objectName: `dc=${req.scopeName}` + }) + + // Respond to the search request with a matched response. + res.send(result) + res.end() + + // After a short delay, send ANOTHER response to the client that will not + // be matched by the client's internal tracker. + setTimeout( + () => { + res.send(result) + res.end() + done() + }, + 100 + ) +}) + +tap.beforeEach(t => { + return new Promise((resolve, reject) => { + server.listen(0, '127.0.0.1', (err) => { + if (err) return reject(err) + + t.context.logMessages = [] + t.context.logger = { + child () { return this }, + debug () {}, + error (...args) { + t.context.logMessages.push(args) + }, + trace () {} + } + + t.context.url = server.url + t.context.client = ldapjs.createClient({ + url: [server.url], + timeout: 5, + log: t.context.logger + }) + + resolve() + }) + }) +}) + +tap.afterEach(t => { + return new Promise((resolve, reject) => { + t.context.client.destroy() + server.close((err) => { + if (err) return reject(err) + resolve() + }) + }) +}) + +tap.test('handle null messages', t => { + const { client, logMessages } = t.context + + // There's no way to get an error from the client when it has received an + // unmatched response from the server. So we need to poll our logger instance + // and detect when the corresponding error message has been logged. + const timer = setInterval( + () => { + if (logMessages.length > 0) { + t.equal( + logMessages.some(msg => msg[1] === 'unmatched server message received'), + true + ) + clearInterval(timer) + t.end() + } + }, + 100 + ) + + client.search('dc=test', (error) => { + t.error(error) + }) +})