Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bug for messages of length 0x100 #187

Merged

Conversation

martinkirsche
Copy link
Contributor

If the parameter length is 255 bytes the result of the previous MQTTSNPacket_len implementation was 256 bytes. But such a message actually needs a 3-octet long Length field, so it should return 258 instead.

Signed-off-by: Martin Kirsche martin.kirsche@gmail.com

@martinkirsche martinkirsche deleted the feature/full-packet-length-fix-1 branch March 19, 2020 21:26
@martinkirsche martinkirsche restored the feature/full-packet-length-fix-1 branch March 19, 2020 21:27
@martinkirsche martinkirsche reopened this Mar 19, 2020
@martinkirsche martinkirsche force-pushed the feature/full-packet-length-fix-1 branch from eb41b23 to 887e109 Compare March 19, 2020 21:35
@martinkirsche martinkirsche changed the base branch from master to develop March 19, 2020 21:37
If the parameter `length` is 255 bytes the result of the previous `MQTTSNPacket_len` implementation was 256 bytes. But such a message actually needs a 3-octet long Length field, so it should return 258 instead.

Signed-off-by: Martin Kirsche <martin.kirsche@gmail.com>
@martinkirsche martinkirsche force-pushed the feature/full-packet-length-fix-1 branch from 887e109 to d27bd86 Compare March 19, 2020 21:40
@ty4tw
Copy link
Contributor

ty4tw commented Mar 23, 2020

Hi,
Thank you for the PR.
A current implementation goes with the spec, I think.

image

@martinkirsche
Copy link
Contributor Author

martinkirsche commented Mar 23, 2020

This issue has nothing to do with the spec. The problem is that 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 than its length field specifies.

Here is a test that demonstrates this:

import socket

sock = socket.socket(socket.AF_INET, socket.SOCK_DGRAM)
sock.bind(("127.0.0.1", 12001))


def connect():
    return bytearray([
        0x07, # Length(0)
        0x04, # MsgType(1)
        0x04, # Flags(2)
        0x01, # ProtocolId(3)
        0x00, # Duration(4)
        0x3c, # Duration(5)
        0x47  # ClientId(6)
    ])

def sub():
    return bytearray([
        0x07, # Length(0)
        0x12, # MsgType(1)
        0x02, # Flags(2)
        0x00, # MsgId (3)
        0x01, # MsgId (4)
        0x41, # TopicName (5)
        0x41  # TopicName (6)
    ])

def pub():
    return bytearray([
        0x01, # Length(0)
        0x01,
        0x02,
        0x0C, # MsgType(1)
        0x02, # Flags(2)
        0x41, # TopicName (3)
        0x41, # TopicName (4)
        0x00, # MsgId (5)
        0x02, # MsgId (6)
    ]) + (bytearray((0x41,)) * (0x102-9))

sock.sendto(connect(), ("127.0.0.1", 10000))
print(sock.recvfrom(2))

sock.sendto(sub(), ("127.0.0.1", 10000))
print(sock.recvfrom(8))

sock.sendto(pub(), ("127.0.0.1", 10000))
print(sock.recvfrom(0x102))

Here is the MQTT-SN-Gateway log:

20200323 221723.906   CONNECT           <---  G                                   07 04 04 01 00 3C 47
20200323 221723.983   CONNACK           --->  GatewayTestClient                   03 05 00

20200323 221723.983   SUBSCRIBE   0001  <---  GatewayTestClient                   07 12 02 00 01 41 41
20200323 221723.983   SUBACK      0001  --->  GatewayTestClient                   08 13 00 00 00 00 01 00
20200323 221723.983   PUBLISH     0002  <---  GatewayTestClient                   01 01 02 0C 02 41 41 00 02 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41
20200323 221723.984   PUBLISH           --->  GatewayTestClient                   01 01 00 0C 02 41 41 00 00 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41 41

@ty4tw ty4tw merged commit 31516e2 into eclipse-paho:develop Mar 24, 2020
@ty4tw
Copy link
Contributor

ty4tw commented Mar 24, 2020

Thank you,
I could understand your PR and merged it.

@martinkirsche martinkirsche deleted the feature/full-packet-length-fix-1 branch March 24, 2020 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants