Azure / azure-iot-middleware-freertos

Azure IoT Middleware for FreeRTOS
https://azure.github.io/azure-iot-middleware-freertos/
MIT License
80 stars 24 forks source link

Reported properties are getting client-side when reading the direct method #298

Closed Noushadalik closed 1 year ago

Noushadalik commented 1 year ago

Hi, Reported properties are getting client-side when reading the direct method. I wrote the following portion of code to read the direct method on client side

uint32_t ulHandleCommand( AzureIoTHubClientCommandRequest_t * pxMessage,
                          uint32_t * pulResponseStatus,
                          uint8_t * pucCommandResponsePayloadBuffer,
                          uint32_t ulCommandResponsePayloadBufferSize )
{
     AzureIoTResult_t xResult;
    AzureIoTJSONReader_t xReader;
    AzureIoTJSONWriter_t xWriter;
    int32_t lCommandNameLength;
    int32_t ulCommandResponsePayloadLength;
    LogInfo( ( "Command payload : %.*s \r\n",
               pxMessage->ulPayloadLength,
               ( const char * ) pxMessage->pvMessagePayload ) );
   ESP_LOGI("AZ IOT","Name > %s\r\n",
                   pxMessage->pucCommandName);

I closed relay 5 in the direct method and then get the command and reported properties on the client side. What is the reason behind this?

I (438071) MQTT: Packet received. ReceivedBytes=45.
I (438071) MQTT: De-serialized incoming PUBLISH packet: DeserializerResult=MQTTSuccess.
I (438071) MQTT: State record updated. New state=MQTTPublishDone.
I (438081) AZ IOT: $iothub/methods/POST/RLY5-Close/?$rid=1
I (438081) AzureIoT: prvHandleCommand called for DirectMethod
I (438101) AzureIoT: Command payload : null
I (438101) AZ IOT: Name > RLY5-Close/?$rid=1null$rid=3":1},"reported":{"GPS":{"Lat":"0.00000000","Long":"0.00000000"},"FirmwareDetails":{"Version":"1.2","BuildDate":"2022-04-10T06:04:00Z"},"$version":21689}}vices%2FED-02&sig=wsIpKocAATyM6Wvyoj6CS914Wjh8VwhFgAuEhz%2FMM3A%3D&se=1669808360
rtheil-growlink commented 1 year ago

I'm not familiar with the way you're doing things here. The way I do it in our project is, after successful AzureIoTHubClient_Connect(), execute AzureIoTHubClient_SubscribeCommand() to add a callback method. Within that callback method, you process the incoming message, then respond to the command call with AzureIoTHubClient_SendCommandResponse()

Bare minimum example:

void connect()
{
    AzureIoTResult_t xResult = AzureIoTHubClient_Connect(&xAzureIoTHubClient, true, &xSessionPresent, CONNACK_RECV_TIMEOUT_MS);
    xResult = AzureIoTHubClient_SubscribeProperties(&xAzureIoTHubClient, propertiesCallback, &xAzureIoTHubClient, SUBSCRIBE_TIMEOUT);
}

Callback:

void commandCallback(AzureIoTHubClientCommandRequest_t *pxMessage, void *pvContext)
{
    //process pxMessage

    //respond
    AzureIoTResult_t xResult = AzureIoTHubClient_SendCommandResponse(xHandle, pxMessage, result, (const uint8_t *)deviceMethodResponse, strlen(deviceMethodResponse));
}
danewalton commented 1 year ago

Thanks for helping out here @rtheil-growlink

Hey @Noushadalik

I think the issue is that you're doing a %s format on a value that isn't guaranteed to be NULL terminated. Below are the fields in the struct, one of which is a command name length:

https://github.com/Azure/azure-iot-middleware-freertos/blob/54fa7819c5e3ca76dc4c6b0f9ef0d233ed5b273a/source/include/azure_iot_hub_client.h#L128-L129

If you previously received a device twin in the same buffer or a buffer that is placed near the one which has the topic, you will have a buffer overrun issue.

You could try something like the following to get only the characters for the command name:

ESP_LOGI("AZ IOT","Name > %.*s\r\n",
                   pxMessage->usCommandNameLength, pxMessage->pucCommandName);

Note the .* placed before the s which allows you to pass a length.

danewalton commented 1 year ago

Closing this issue for now due to inactivity. If you would like it reopened, please let us know. For any other questions, please open another Github issue.