Skip to content

Commit

Permalink
fix: PUBACK packet not compatible with RabbitMQ (#142)
Browse files Browse the repository at this point in the history
* Hack to solve #92.

* Adapted tests - please note this is a breaking change on the wire - also it shouldn't be spec-wise.

* More comments and improved puback handling.

* Improved comments.

---------

Co-authored-by: Andreas Ländle <969523+alaendle@users.noreply.github.com>
  • Loading branch information
alaendle and alaendle committed Jul 7, 2023
1 parent 865e7b3 commit bb3af28
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 7 deletions.
36 changes: 33 additions & 3 deletions test.js
Original file line number Diff line number Diff line change
Expand Up @@ -719,6 +719,22 @@ testParseOnly('Version 5 PUBACK test 1', {
)

testParseAndGenerate('Version 5 PUBACK test 2', {
cmd: 'puback',
messageId: 42,
retain: false,
qos: 0,
dup: false,
length: 2,
topic: null,
payload: null,
reasonCode: 0
}, Buffer.from([
64, 2, // Fixed Header (PUBACK, Remaining Length)
0, 42 // Variable Header (2 Bytes: Packet Identifier 42, Implied reason code: 0 Success, Implied no properties)
]), { protocolVersion: 5 }
)

testParseOnly('Version 5 PUBACK test 2.1', {
cmd: 'puback',
messageId: 42,
retain: false,
Expand Down Expand Up @@ -1868,18 +1884,32 @@ testParseError('Invalid header flag bits, must be 0x0 for puback packet', Buffer
0, 2 // Message ID
]))

testParseGenerate('puback without reason and no MQTT 5 properties', {
cmd: 'puback',
retain: false,
qos: 0,
dup: false,
length: 2,
messageId: 2,
reasonCode: 0
}, Buffer.from([
64, 2, // Header
0, 2 // Message ID
]), { protocolVersion: 5 })

testParseGenerate('puback with reason and no MQTT 5 properties', {
cmd: 'puback',
retain: false,
qos: 0,
dup: false,
length: 3,
length: 4,
messageId: 2,
reasonCode: 16
}, Buffer.from([
64, 3, // Header
64, 4, // Header
0, 2, // Message ID
16 // reason code
16, // reason code
0 // no user properties
]), { protocolVersion: 5 })

testParseGenerate('puback MQTT 5 properties', {
Expand Down
14 changes: 10 additions & 4 deletions writeToStream.js
Original file line number Diff line number Diff line change
Expand Up @@ -409,20 +409,26 @@ function confirmation (packet, stream, opts) {
// Header
stream.write(protocol.ACKS[type][qos][dup][0])

// Length
// Length === 3 is only true of version === 5 and no properties; therefore if reasonCode === 0 we are allowed to skip both bytes - but if we write the reason code we also have to write property length [MQTT-3.4.2-1].
if (length === 3) length += reasonCode !== 0 ? 1 : -1
writeVarByteInt(stream, length)

// Message ID
writeNumber(stream, id)

// reason code in header
if (version === 5) {
// reason code in header - but only if it couldn't be omitted - indicated by length !== 2.
if (version === 5 && length !== 2) {
stream.write(Buffer.from([reasonCode]))
}

// properies mqtt 5
// properties mqtt 5
if (propertiesData !== null) {
propertiesData.write()
} else {
if (length === 4) {
// we have no properties but have written a reason code - so we need to indicate empty properties by filling in a zero.
stream.write(Buffer.from([0]))
}
}
return true
}
Expand Down

0 comments on commit bb3af28

Please sign in to comment.