Differentiate CONNECT packets properties and CONNACK packets properties #1200
Conversation
lib/client.js
Outdated
| } | ||
| if (packet.properties.serverKeepAlive && options.keepalive) { | ||
| options.keepalive = packet.properties.serverKeepAlive | ||
| this._shiftPingInterval() | ||
| } | ||
| if (packet.properties.maximumPacketSize) { | ||
| if (!options.properties) { options.properties = {} } | ||
| options.properties.maximumPacketSize = packet.properties.maximumPacketSize | ||
| options.properties.serverMaximumPacketSize = packet.properties.maximumPacketSize |
There was a problem hiding this comment.
you've changed this but where is options.properties.serverMaximumPacketSize referenced?
There was a problem hiding this comment.
It's not use anywhere, It should be use when client sending message, but I don't know the code enough to do it properly, may be this will better translate it meaning?
| options.properties.serverMaximumPacketSize = packet.properties.maximumPacketSize | |
| options.serverProperties.maximumPacketSize = packet.properties.maximumPacketSize |
lib/client.js
Outdated
| @@ -503,11 +503,11 @@ MqttClient.prototype.publish = function (topic, message, opts, callback) { | |||
| if (options.protocolVersion === 5) { | |||
| packet.properties = opts.properties | |||
| if ((!options.properties && packet.properties && packet.properties.topicAlias) || ((opts.properties && options.properties) && | |||
| ((opts.properties.topicAlias && options.properties.topicAliasMaximum && opts.properties.topicAlias > options.properties.topicAliasMaximum) || | |||
| (!options.properties.topicAliasMaximum && opts.properties.topicAlias)))) { | |||
| ((opts.properties.topicAlias && options.properties.serverTopicAliasMaximum && opts.properties.topicAlias > options.properties.serverTopicAliasMaximum) || | |||
There was a problem hiding this comment.
do you know that this change honors the MQTTv5 Spec definition for topicAliasMaximum:
34 (0x22) Byte, Identifier of the Topic Alias Maximum.
811 Followed by the Two Byte Integer representing the Topic Alias Maximum value. It is a Protocol Error to
812 include the Topic Alias Maximum value more than once. If the Topic Alias Maximum property is absent,
813 the default value is 0.
814
815 This value indicates the highest value that the Client will accept as a Topic Alias sent by the Server. The
816 Client uses this value to limit the number of Topic Aliases that it is willing to hold on this Connection. The
817 Server MUST NOT send a Topic Alias in a PUBLISH packet to the Client greater than Topic Alias
818 Maximum [MQTT-3.1.2-26]. A value of 0 indicates that the Client does not accept any Topic Aliases on
819 this connection. If Topic Alias Maximum is absent or zero, the Server MUST NOT send any Topic Aliases
820 to the Client [MQTT-3.1.2-27]
There was a problem hiding this comment.
It should honor topicAliasMaximum property in CONNACK packet, not CONNECT packets property sent by the client, as shown below: https://docs.oasis-open.org/mqtt/mqtt/v5.0/os/mqtt-v5.0-os.html#_Toc3901088
|
the option What you're proposing, The |
|
@heunghingwan I'll check more on this in a bit. I have a bit on my plate rn. Thank you for doing this though! Your contribution is appreciated. |
|
After reading the specification, I found that it clearly shows that the server and client have independent Those appendices summarize them well, glad I haven't miss them😁 |
Addressing bug in #1183