chore: enhance coverage test (#609)

lib/client.js

@@ -288,23 +288,18 @@ Client.prototype.close = function (done) {
     finish)
 
   function finish () {
+    const will = that.will
     // _disconnected is set only if client is disconnected with a valid disconnect packet
-    if (!that._disconnected && that.will) {
-      const will = Object.assign({}, that.will)
+    if (!that._disconnected && will) {
       that.broker.authorizePublish(that, will, function (err) {
-        if (!err) {
-          that.broker.publish(will, that, done)
-        } else {
-          done()
-        }
-
-        function done (err) {
-          if (!err) {
-            that.broker.persistence.delWill({
-              id: that.id,
-              brokerId: that.broker.id
-            }, noop)
-          }
+        if (err) { return done() }
+        that.broker.publish(will, that, done)
+
+        function done () {
+          that.broker.persistence.delWill({
+            id: that.id,
+            brokerId: that.broker.id
+          }, noop)
         }
       })
     }

lib/handlers/publish.js

@@ -40,20 +40,14 @@ function enqueuePublish (packet, done) {
   switch (packet.qos) {
     case 2:
       client.broker.persistence.incomingStorePacket(client, packet, function (err) {
-        if (err) {
-          done(err)
-        } else {
-          write(client, new PubRec(packet), done)
-        }
+        if (err) { return done(err) }
+        write(client, new PubRec(packet), done)
       })
       break
     case 1:
       write(client, new PubAck(packet), function (err) {
-        if (err) {
-          done(err)
-        } else {
-          client.broker.publish(packet, client, done)
-        }
+        if (err) { return done(err) }
+        client.broker.publish(packet, client, done)
       })
       break
     case 0:

test/auth.js

@@ -586,6 +586,54 @@ test('do not authorize publish', function (t) {
   })
 })
 
+test('modify qos out of range in authorize publish ', function (t) {
+  t.plan(2)
+
+  const s = connect(setup())
+  t.tearDown(s.broker.close.bind(s.broker))
+
+  const expected = {
+    cmd: 'publish',
+    topic: 'foo',
+    payload: Buffer.from('bar'),
+    qos: 0,
+    retain: false,
+    length: 12,
+    dup: false
+  }
+
+  s.broker.authorizePublish = function (client, packet, cb) {
+    if (packet.topic === 'hello') { packet.qos = 10 }
+    cb()
+  }
+
+  s.outStream.on('data', function (packet) {
+    t.fail('should no data sent')
+  })
+  s.broker.mq.on('hello', function (packet, cb) {
+    t.fail('should not publish')
+  })
+  s.broker.mq.on('foo', function (packet, cb) {
+    t.notOk(Object.prototype.hasOwnProperty.call(packet, 'messageId'), 'should not contain messageId in QoS 0')
+    expected.brokerId = s.broker.id
+    expected.brokerCounter = s.broker.counter
+    delete expected.length
+    t.deepEqual(packet, expected, 'packet matches')
+    cb()
+  })
+
+  s.inStream.write({
+    cmd: 'publish',
+    topic: 'hello',
+    payload: 'world'
+  })
+  s.inStream.write({
+    cmd: 'publish',
+    topic: 'foo',
+    payload: 'bar'
+  })
+})
+
 test('authorize subscribe', function (t) {
   t.plan(5)
 

test/qos2.js

@@ -652,3 +652,33 @@ test('packet is written to stream after being stored', function (t) {
     t.end()
   })
 })
+
+test('not send pubrec when persistence fails to store packet', function (t) {
+  t.plan(2)
+
+  const s = connect(setup())
+  const broker = s.broker
+
+  t.tearDown(broker.close.bind(s.broker))
+
+  s.broker.persistence.incomingStorePacket = function (client, packet, done) {
+    t.pass('packet stored')
+    done(new Error('store error'))
+  }
+  s.broker.on('clientError', function (client, err) {
+    t.equal(err.message, 'store error')
+  })
+
+  const packet = {
+    cmd: 'publish',
+    topic: 'hello',
+    payload: 'world',
+    qos: 2,
+    messageId: 42
+  }
+
+  s.inStream.write(packet)
+  s.outStream.once('data', function (packet) {
+    t.fail('should not have pubrec')
+  })
+})