Repository: moscajs/aedes
Pull request #511 (2020-06-23 07:41:01)

fix: possible leak in handleConnect negate

coveralls: ## Pull Request Test Coverage Report for [Build 144584156](https://coveralls.io/builds/31624911)

* **0** of **2**   **(100.0%)**  changed or added relevant lines in **1** file are covered.
* No unchanged relevant lines lost coverage.
* Overall coverage remained the same at ?**%**

---



|  Totals | [![Coverage Status](https://coveralls.io/builds/31624911/badge)](https://coveralls.io/builds/31624911) |
| :-- | --: |
| Change from base [Build 141084182](https://coveralls.io/builds/31565597): |  0% |
| Covered Lines: |  |
| Relevant Lines: | 0 |

---
##### 💛  - [Coveralls](https://coveralls.io)


Patch:
diff --git a/aedes.d.ts b/aedes.d.ts
index e73241f..3f06ac9 100644
--- a/aedes.d.ts
+++ b/aedes.d.ts
@@ -58,6 +58,7 @@ declare namespace aedes {
 
   interface AedesOptions {
     mq?: any
+    id?: string
     persistence?: any
     concurrency?: number
     heartbeatInterval?: number
diff --git a/docs/Aedes.md b/docs/Aedes.md
index 4adfb87..9a169d3 100644
--- a/docs/Aedes.md
+++ b/docs/Aedes.md
@@ -41,6 +41,7 @@
   - `queueLimit` `<number>` maximum number of queued messages before client session is established. If number of queued items exceeds, `connectionError` throws an error `Client queue limit reached`. __Default__: `42`
   - `maxClientsIdLength` option to override MQTT 3.1.0 clients Id length limit. __Default__: `23`
   - `heartbeatInterval` `<number>` an interval in millisconds at which server beats its health signal in `$SYS/<aedes.id>/heartbeat` topic. __Default__: `60000`
+  - `id` `<string>` aedes broker unique identifier. __Default__: `uuidv4()`
   - `connectTimeout` `<number>` maximum waiting time in milliseconds waiting for a [`CONNECT`][CONNECT] packet. __Default__: `30000`
 - Returns `<Aedes>`
 
diff --git a/lib/client.js b/lib/client.js
index 534fdd6..f0be5a2 100644
--- a/lib/client.js
+++ b/lib/client.js
@@ -288,6 +288,7 @@ Client.prototype.close = function (done) {
     finish)
 
   function finish () {
+    // _disconnected is set only if client is disconnected with a valid disconnect packet
     if (!that._disconnected && that.will) {
       const will = Object.assign({}, that.will)
       that.broker.authorizePublish(that, will, function (err) {
diff --git a/lib/handlers/connect.js b/lib/handlers/connect.js
index a48a312..2d5dd1b 100644
--- a/lib/handlers/connect.js
+++ b/lib/handlers/connect.js
@@ -48,13 +48,10 @@ function handleConnect (client, packet, done) {
   function negate (err, successful) {
     if (!err && successful === true) {
       setImmediate(init, client, packet, done)
-      return
-    }
-    client.connecting = false
-    if (err) {
-      client.broker.emit('connectionError', client, err)
+    } else {
+      client.connecting = false
+      done(err)
     }
-    client.conn.destroy()
   }
 }
 
diff --git a/lib/handlers/index.js b/lib/handlers/index.js
index 1caabfe..311dd69 100644
--- a/lib/handlers/index.js
+++ b/lib/handlers/index.js
@@ -54,9 +54,11 @@ function handle (client, packet, done) {
       client._disconnected = true
       // [MQTT-3.14.4-1] [MQTT-3.14.4-2]
       client.conn.destroy()
+      done()
       return
     default:
       client.conn.destroy()
+      done()
       return
   }
 


Test patch:
diff --git a/test/connect.js b/test/connect.js
index 563d5d3..b5dba35 100644
--- a/test/connect.js
+++ b/test/connect.js
@@ -433,6 +433,46 @@ test('second CONNECT Packet sent from a Client as a protocol violation and disco
   })
 })
 
+test('connect handler calls done when preConnect throws error', function (t) {
+  t.plan(1)
+
+  const broker = aedes({
+    preConnect: function (client, done) {
+      done(Error('error in preconnect'))
+    }
+  })
+
+  t.tearDown(broker.close.bind(broker))
+
+  const s = setup(broker)
+
+  var handleConnect = require('../lib/handlers/connect')
+
+  handleConnect(s.client, {}, function done (err) {
+    t.equal(err.message, 'error in preconnect', 'calls done with error')
+  })
+})
+
+test('handler calls done when disconnect or unknown packet cmd is received', function (t) {
+  t.plan(2)
+
+  const broker = aedes()
+
+  t.tearDown(broker.close.bind(broker))
+
+  const s = setup(broker)
+
+  var handle = require('../lib/handlers/index')
+
+  handle(s.client, { cmd: 'disconnect' }, function done () {
+    t.pass('calls done when disconnect cmd is received')
+  })
+
+  handle(s.client, { cmd: 'fsfadgragae' }, function done () {
+    t.pass('calls done when unknown cmd is received')
+  })
+})
+
 test('reject second CONNECT Packet sent while first CONNECT still in preConnect stage', function (t) {
   t.plan(2)
 
diff --git a/test/types/index.ts b/test/types/index.ts
index 86548f1..21db2bf 100644
--- a/test/types/index.ts
+++ b/test/types/index.ts
@@ -8,6 +8,7 @@ const broker = Server({
   concurrency: 100,
   heartbeatInterval: 60000,
   connectTimeout: 30000,
+  id: 'aedes',
   preConnect: (client: Client, callback) => {
     if (client.req) {
       callback(new Error('not websocket stream'), false)