amenzhinsky / iothub

Azure IoT Hub SDK for Golang
MIT License
51 stars 57 forks source link

D2C properties encoding #54

Closed Kofemolka closed 2 years ago

Kofemolka commented 2 years ago

Version: v0.7.0 Transport: MQTT

Minimal example to reproduce.

func sendEvent(hub *iotdevice.Client) {
    _ := hub.SendEvent(context.Background(),
        []byte("whatever"),
        iotdevice.WithSendProperties(
            map[string]string{
                "te st": "to st",
            },
        ),
    )
}

Please notice, that custom property contains space (' ') in both key and value. It is not forbidden by IoT Hub messaging contract (https://docs.microsoft.com/en-us/azure/iot-hub/iot-hub-devguide-messages-construct). Attempt to send such message will result in infinite re-connect loop:

2021/10/28 15:50:06 DEBUG connection established
2021/10/28 15:50:06 DEBUG connection lost: EOF
2021/10/28 15:50:06 DEBUG connection established
2021/10/28 15:50:06 DEBUG connection lost: EOF
2021/10/28 15:50:07 DEBUG connection established
2021/10/28 15:50:07 DEBUG connection lost: read tcp 192.168.88.99:38798->40.113.176.167:8883: read: connection reset by peer
2021/10/28 15:50:07 DEBUG connection established
2021/10/28 15:50:07 DEBUG connection lost: EOF
2021/10/28 15:50:07 DEBUG connection established
2021/10/28 15:50:07 DEBUG connection lost: EOF
2021/10/28 15:50:08 DEBUG connection established
2021/10/28 15:50:08 DEBUG connection lost: read tcp 192.168.88.99:38804->40.113.176.167:8883: read: connection reset by peer
2021/10/28 15:50:08 DEBUG connection established
2021/10/28 15:50:08 DEBUG connection lost: EOF
2021/10/28 15:50:09 DEBUG connection established
2021/10/28 15:50:09 DEBUG connection lost: EOF
2021/10/28 15:50:09 DEBUG connection established
2021/10/28 15:50:09 DEBUG connection lost: EOF
2021/10/28 15:50:09 DEBUG connection established
2021/10/28 15:50:09 DEBUG connection lost: EOF
2021/10/28 15:50:09 DEBUG connection established
2021/10/28 15:50:09 DEBUG connection lost: EOF

This happens because in (github.com/amenzhinsky/iothub@v0.7.0/iotdevice/transport/mqtt/mqtt.go), properties are being incorrectly encoded. Spaces are replaced with '+' char, which is a wildcard in MQTT protocol, and leads to immediate connection drop. Since D2C message was not sent, after re-connect library will attempt to push this message once again leading to another connection drop. Cycle repeats forever.

Incorrectly encoded topic looks like this: devices/my_device/messages/events/te+st=to+st While Python and C++ SDKs produce correctly encoded: devices/my_device/messages/events/te%20st=to%20st

Suggested fix is to use encoding library different from 'net/url' or replace '+' with "%20" on top of it.

Kofemolka commented 2 years ago

@amenzhinsky do you accept pull requests? This one is pretty critical to us. I can create PR with fix next week.

Kofemolka commented 2 years ago

Fixes-incorrect-D2C-properties-encoding.patch.gz

@amenzhinsky please find patch attached.

amenzhinsky commented 2 years ago

@Kofemolka sure, can you submit a pr?

Kofemolka commented 2 years ago

55