Update MQTTSNPacket.c

[davegish]

Looks like a bug in MQTTSNPacket_len(). If the input parameter is 255, it will return 256, but the actual packet size will be 258.

[martinkirsche]

There is already a pull request containing this fix at #187 but unfortunately the maintainer reverted it within #189

[ty4tw]

Hi,

Could you explain why the actual length is 258?
According to the spec,

Messages with lengths smaller than 256 octets may use the shorter 1-octet format.

[martinkirsche]

Could you explain why the actual length is 258?

The length will become 258 because ...

the output of the function MQTTSNPacket_len is fet into MQTTSNPacket_encode. So if I put 255 as the length of the packet without the length field into MQTTSNPacket_len it will currently return 256. But MQTTSNPacket_encode will use a 3-octed length field for such a message. The result is a MQTT-SN message that is longer (258 bytes) than its length field specifies (256 bytes).

See #187 for a demonstration of the effect.

[ty4tw]

Hi Martin,
It was a bug of GatewayTester and fixed.
MQTTSNPacket_encode() is correct.

/**
 * Encodes the MQTT-SN message length
 * @param buf the buffer into which the encoded data is written
 * @param length the length to be encoded
 * @return the number of bytes written to the buffer
 */
int MQTTSNPacket_encode(unsigned char* buf, int length)
{
	int rc = 0;

	FUNC_ENTRY;
	if (length > 255)
	{
		writeChar(&buf, 0x01);
		writeInt(&buf, length);
		rc += 3;
	}
	else
		buf[rc++] = length;

	FUNC_EXIT_RC(rc);
	return rc;
}

The specification defines the message length as follows:

5.2.1 Length
The Length field is either 1- or 3-octet long and specifies the total number of octets contained in the message (including the Length field itself).
If the first octet of the Length field is coded “0x01” then the Length field is 3-octet long; in this case, the two following octets specify the total number of octets of the message (most-significant octet first). Otherwise, the Length field is only 1-octet long and specifies itself the total number of octets contained in the message.
The 3-octet format allows the encoding of message lengths up to 65535 octets. Messages with lengths smaller than 256 octets may use the shorter 1-octet format.
Note that because MQTT-SN does not support message fragmentation and reassembly, the maximum message length that could be used in a network is governed by the maximum packet size that is supported by that network, and not by the maximum length that could be encoded by MQTT-SN.

[davegish]

Hi Tomoaki, As Martin said, the bug is in MQTTSNPacket_len(), not MQTTSNPacket_encode(). /** * Calculates the full packet length including length field * @param length the length of the MQTT-SN packet without the length field * @return the total length of the MQTT-SN packet including the length field */ int MQTTSNPacket_len(int length) { return (length > 255) ? length + 3 : length + 1; } If the input length parameter value is 255, MQTTSNPacket_len() will return 256. In this case, the length field in the MQTTSN packet will be set to 256 but MQTTSNPacket_encode() will encode 258 bytes. So this is the bug. To fix it, you could either use: return (length >*=* 255) ? length + 3 : length + 1; or return (length > 25*4*) ? length + 3 : length + 1; Hope this helps, Dave

…

-- Dave Gish Principal Software Architect The Nielsen Company Skype: dave.gish 973-831-8208

On Sun, Apr 12, 2020 at 8:34 PM Tomoaki Yamaguchi ***@***.***> wrote: Hi Martin, It was a bug of GatewayTester and fixed. MQTTSNPacket_encode() is correct. /** * Encodes the MQTT-SN message length * @param buf the buffer into which the encoded data is written * @param length the length to be encoded * @return the number of bytes written to the buffer */ int MQTTSNPacket_encode(unsigned char* buf, int length) { int rc = 0; FUNC_ENTRY; if (length > 255) { writeChar(&buf, 0x01); writeInt(&buf, length); rc += 3; } else buf[rc++] = length; FUNC_EXIT_RC(rc); return rc; } The specification defines the message length as follows: 5.2.1 Length The Length field is either 1- or 3-octet long and specifies the total number of octets contained in the message (including the Length field itself). If the first octet of the Length field is coded “0x01” then the Length field is 3-octet long; in this case, the two following octets specify the total number of octets of the message (most-significant octet first). Otherwise, the Length field is only 1-octet long and specifies itself the total number of octets contained in the message. The 3-octet format allows the encoding of message lengths up to 65535 octets. Messages with lengths smaller than 256 octets may use the shorter 1-octet format. Note that because MQTT-SN does not support message fragmentation and reassembly, the maximum message length that could be used in a network is governed by the maximum packet size that is supported by that network, and not by the maximum length that could be encoded by MQTT-SN. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#192 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/APEPUBU7XU2OHK44BK54U43RMJMXZANCNFSM4MF7W5TQ> .

[ty4tw]

Hi davegish,
you are misunderstanding the length parameter.
@param length the length of the MQTT-SN packet without the length field
int MQTTSNPacket_len(int length);
int MQTTSNPacket_encode(unsigned char* buf, int length)
when the length is 255 MQTTSNPacket_len returns 256 and MQTTSNPacket_encode returns 1.
Nothing is wrong. see the definition of the message length in the specification.

return (length >*=* 255) ? length + 3 : length + 1;     

This returns 258 when the length is 255.

[davegish]

Hi Tomoaki, I don't see how I'm misunderstanding it. According to the spec: 5.2.1 Length The Length field is either 1- or 3-octet long and specifies the total number of octets contained in the message (*including the Length field itself*). The number 256 is impossible to carry as 1 byte, so the length field expands to 3 bytes. In other words, according to the spec, it doesn't seem possible to have a packet size of 256 or 257 bytes. The input parameter to MQTTSNPacket_len() is: @param <https://github.com/param> length the length of the MQTT-SN packet *without *the length field The return value of MQTTSNPacket_len() is: @return the total length of the MQTT-SN packet *including *the length field If the MQTT-SN packet *without* the length field is 255 bytes, then the total length of the MQTT-SN packet *including* the length field will be 258 bytes.

…

-- Dave Gish Principal Software Architect The Nielsen Company Skype: dave.gish 973-831-8208

On Mon, Apr 13, 2020 at 4:47 AM Tomoaki Yamaguchi ***@***.***> wrote: Hi davegish, you are misunderstanding the length parameter. @param <https://github.com/param> length the length of the MQTT-SN packet without the length field int MQTTSNPacket_len(int length); int MQTTSNPacket_encode(unsigned char* buf, int length) when the length is 255 functions return 256. Nothing is wrong. see the definition of the message length of the specification. return (length >*=* 255) ? length + 3 : length + 1; ``` This will return 258 when the length is 255. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#192 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/APEPUBWARRDN6HGKJKCO6MTRMLGT5ANCNFSM4MF7W5TQ> .

[davegish]

Tomoaki: Does this mean a message with length 255 octet may use the shorter 1 octet format ... Dave: Yes. Tomoaki: ... and the total length is 255 + 1 = 256 ? Dave: No. According to the spec, the length value in the header is the *total length* of the packet, *including the length field itself*. 5.2.1 Length The Length field is either 1- or 3-octet long and specifies the *total number of octets* contained in the message (*including the Length field itself*). Examples: 1) if the payload length before adding the length field is 254 octets, the total length is 255, which may use the shorter 1 octet length field. In this case, the value of the length field in the header is 255. 2) if the payload length before adding the length field is 255 octets, the total length is over 255, which requires the 3-octet length field. In this case, the value of the length field in the header is 258.

…

-- Dave Gish Principal Software Architect The Nielsen Company Skype: dave.gish 973-831-8208

On Mon, Apr 13, 2020 at 9:20 AM Tomoaki Yamaguchi ***@***.***> wrote: Messages with lengths smaller than 256 octets may use the shorter 1-octet format. Dose this means message with length 255 octet may use the shorter 1 octet format and total length is 255 + 1 = 256 ? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#192 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/APEPUBXIXVSP6W7IPJESJLLRMMGR5ANCNFSM4MF7W5TQ> .

[ty4tw]

Hi,
Ok I understood.
I will fix the bugs.
Thank you for your cooperation.

[saumilsdk]

hi @ty4tw is that bug fixed in latest release? There are multiple PRs getting rejected and those fixes we are missing in our implementation. Can you please also apply those fixes and release a new version?